read image acquiring status

[enoodle]

Meant to report that image-inspector failed to acquire the image, this will read ImageAcquireSuccess, ImageAcquireError and abort the job if needed with the error reported from image-inspector.

Based on image-inspector change from: openshift/image-inspector#82

Currently (Before these two fixes) when Image-Inspector is failing to acquire the image it will exit and the Job on the ManageIQ side will wait until timeout is reached for the information to be served.

When the image-inspector image is updated with this change but ManageIQ is running without this patch the error message will look similar to this:

cannot analyze image 172.30.6.216:5000/test2/origin-ruby-sample@sha256:050b667cd1c1d5c5781f0c8ee5763ab5ef26e3a15c4d85cf7d143c1513890cec with id 172.30.6.216: detected ids were 

But this patch will change it to:

image acquiring error: Unable to pull docker image: <nil>

BZ: https://bugzilla.redhat.com/show_bug.cgi?id=1491643
BZ: https://bugzilla.redhat.com/show_bug.cgi?id=1484936

[miq-bot]

Some comments on commit enoodle@b135d68

spec/models/manageiq/providers/kubernetes/container_manager/scanning/job_spec.rb

• ⚠️ - 413 - Detected allow_any_instance_of. This RSpec method is highly discouraged, please only use when absolutely necessary.

[miq-bot]

Checked commit enoodle@b135d68 with ruby 2.3.3, rubocop 0.47.1, haml-lint 0.20.0, and yamllint 1.10.0
2 files checked, 0 offenses detected
Everything looks fine. 🍰

[enoodle]

@ilackarms @cben @yaacov Can you review please?

[moolitayer]

When the image-inspector image is updated with this change but ManageIQ is running without this patch the error message will look similar to this:

What happens when we have the patch here but old image inspector? can you add a test that makes sure we do not crash in that scenario

[enoodle]

What happens when we have the patch here but old image inspector? can you add a test that makes sure we do not crash in that scenario

Nothing, Image-Inspector will exit with error and the Job will wait until this Jobs receives timeout and deletes it. The same timeout tests for Jobs apply here.

[moolitayer]

A thought, we have this check in several places:

return queue_signal(:abort_job, "no image found", "error") unless image

I think we should test if the image was archived. we would still need what you did here but would be able to report sooner before communicating with image-inspector in some cases.

BTW since there is work done on pod watch we should know fast when an image originating from a pod was deleted

[enoodle]

@moolitayer We should do this on another PR, this one is only for reading status from Image-Inspector.

[yaacov]

LGTM 👍

[yaacov]

note: the LGTM above pending openshift/image-inspector#82 :-)

[ilackarms]

looks good 👍

[moolitayer]

@moolitayer We should do this on another PR, this one is only for reading status from Image-Inspector.

I'm fine with that, my comment is regarding the bzs mentioned here

[moolitayer]

LGTM 👍

[moolitayer]

@enoodle just making sure again, you are saying we can merge here before the image-inspector change right?
Also I think it's worth doing a PR for not going to image inspector if the image is archived, WDYT?
(changing the image existence check for post image archive)

[enoodle]

@moolitayer

just making sure again, you are saying we can merge here before the image-inspector change right?

Yes, With the happy flow inspector_metadata.ImageAcquireSuccess will be nil which is not false. If there is a problem concerning acquiring the image then the pod will just exit on error and the Job will hang until timeout (which is what is currently happening)

Also I think it's worth doing a PR for not going to image inspector if the image is archived, WDYT?

Yes, I will take care of it

[cben]

please set backport labels according to BZs.

[enoodle]

@miq-bot add_label gaprindashvili/yes