identify openshift images through command variable

[enoodle]

Factor out Openshift image identification and use the command attribute as we are also parsing digest for images originating only from pods.

[enoodle]

cc @moolitayer
@miq-bot add_label providers/containers enhancement

[miq-bot]

@enoodle Cannot apply the following label because they are not recognized: providers/containers enhancement

[simon3z]

@enoodle why command being present would give an indication whether it's an openshift_image or not?

[enoodle]

We are only collecting this field from Openshift images and I expect all of them to have it. In case not all images have command and we can't find another field that answers these criteria then we will need to add something to mark it.

BTW this is currently will cause an error message when ManageIQ attempts to annotate an image that doesn't exists but this won't result in a negative user experience so maybe we also want to leave it as is.

[simon3z]

@enoodle I think the idea is good but the current implementation is poor. Maybe we should do something at refresh time (e.g. use a type column and have different classes for the different types of images).

[moolitayer]

We will need to avoid some confusion. We have

1. images that come from a kubernetes cluster (only through running pods)
2. images that come from an openshift cluster through get images
3. images that come from an openshift cluster through running pods

we usually use Inheritance for 1. vs the others, this case we would need distinguish between 2 and others.

[simon3z]

images that come from a kubernetes cluster (only through running pods)
images that come from an openshift cluster through running pods

@moolitayer what's the difference between these two?

[moolitayer]

@moolitayer what's the difference between these two?

For this pr they are the same. I would expect 1 and 2 to be represented by app/models/container_image
and 3 to be under app/models/manageiq/providers/openshift/container_manager. Maybe someone else has a better idea

[simon3z]

For this pr they are the same. I would expect 1 and 2 to be represented by app/models/container_image
and 3 to be under app/models/manageiq/providers/openshift/container_manager. Maybe someone else has a better idea

@moolitayer with reference to your comment, I don't think there's any difference between 1 and 3, they are both ContainerImage (there's nothing specific of OpenShift).
Instead 2 is a different type: OpenShiftImage and yes it will probably need to live in app/models/manageiq/providers/openshift/container_manager.

[chessbyte]

@simon3z @moolitayer bump

[simon3z]

@enoodle as mentioned in the past I would like to see this at refresh-time. Feel free to update this PR or close it to handle the issue in another one.

[enoodle]

@moolitayer @simon3z I have added ems_ref field to ContainerImage to identify if the image has an entity in the EMS too. This will be accompanied with a one line PR to manageiq-providers-openshift if you will say that this direction is OK.

[simon3z]

@moolitayer @simon3z I have added ems_ref field to ContainerImage to identify if the image has an entity in the EMS too. This will be accompanied with a one line PR to manageiq-providers-openshift if you will say that this direction is OK.

@enoodle we need a unique identifier for the image but again I think we need also two different types for the images.

[miq-bot]

Checked commit enoodle@934d7ce with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
3 files checked, 5 offenses detected

app/models/miq_action.rb

• ❗ - Line 745, Col 5 - Style/NegatedIf - Favor if over unless for negative conditions.

spec/models/miq_action_spec.rb

• ❗ - Line 229, Col 61 - Style/AlignHash - Align the elements of a hash literal if they span more than one line.
• ❗ - Line 230, Col 61 - Style/AlignHash - Align the elements of a hash literal if they span more than one line.
• ❗ - Line 239, Col 61 - Style/AlignHash - Align the elements of a hash literal if they span more than one line.
• ❗ - Line 240, Col 61 - Style/AlignHash - Align the elements of a hash literal if they span more than one line.

[Fryguy]

unless + ! + blank? is a recipe for confusion... This is equivalent to if rec.ems_ref.blank?

[Fryguy]

@simon3z I'll wait on your before I review further. I think my opinion matches yours in #15022 (comment)

[enoodle]

Different classes for container images: #15386

[miq-bot]

This pull request is not mergeable. Please rebase and repush.

[Fryguy]

Database migrations have now been moved to the https://github.com/ManageIQ/manageiq-schema repo. Please see http://talk.manageiq.org/t/new-split-repo-manageiq-schema/2478 for instructions on how to transfer your database migrations. If this PR contains only migrations, I will leave it open for a short time during the transition, after which I will close this if it has not been moved over.