Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Bugfix for #5285] Expose flags for storage and untar images #5306

Merged
merged 1 commit into from
Oct 15, 2021

Conversation

tkrishtop
Copy link
Contributor

@tkrishtop tkrishtop commented Oct 12, 2021

Description of the change:

Exposes two tags to provide a possibility to overwrite hardcoded images:

--storage-image busybox 
--untar-image registry.access.redhat.com/ubi8/ubi:8.4

Motivation for the change:

Closes #5285

Operator-sdk scorecard pulls two hardcoded images during the job execution:

Image: "busybox",

scorecardUntarImage = "registry.access.redhat.com/ubi8/ubi:8.4"

In the disconnected environment that pull doesn't work because the images are pinned by tags and not by digests.
As discussed in #5285 with @jmccormick2001 and @estroz , the solution could be to expose two tags to provide a possibility to overwrite these images with their local-registry copies.

Tests in disconnected environment

The changes from this PR were installed as operator-sdk-flag and tested by running in the disconnected environment

operator-sdk-flag scorecard \
    --output json \
    --selector=test=basic-check-spec-test \
    --kubeconfig "{{ kubeconfig_path }}" \
    --namespace scorecard-testing \
    --service-account default \
    --config {{ scorecard_config_path }} \
    --verbose \
    --wait-time 300s \
    --untar-image registry.access.redhat.com/ubi8@sha256:910f6bc0b5ae9b555eb91b88d28d568099b060088616eba2867b07ab6ea457c7 \
    --storage-image docker.io/library/busybox@sha256:c71cb4f7e8ececaffb34037c2637dc86820e4185100e18b4d02d613a9bd772af \
    "{{ scorecard_operator_dir }}"

and

operator-sdk-flag scorecard \
    --output json \
    --selector=suite=olm \
    --kubeconfig "{{ kubeconfig_path }}" \
    --namespace scorecard-testing \
    --service-account default \
    --config {{ scorecard_config_path }} \
    --verbose \
    --wait-time 300s \
    "{{ scorecard_operator_dir }}"

Here are scorecard logs of the above tests

Checklist

If the pull request includes user-facing changes, extra documentation is required:

@openshift-ci openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Oct 12, 2021
@tkrishtop tkrishtop force-pushed the busybox_ubi branch 4 times, most recently from 43cef19 to 0982ab0 Compare October 13, 2021 07:40
@tkrishtop tkrishtop changed the title [WIP] [Bugfix for #5285] Expose flags for storage and untar images [Bugfix for #5285] Expose flags for storage and untar images Oct 13, 2021
@openshift-ci openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Oct 13, 2021
@tkrishtop
Copy link
Contributor Author

Hi @camilamacedo86, thank you for the review! I've addressed all the documentation issues and replied concerning SHA usage.

Copy link
Member

@estroz estroz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Oct 13, 2021
@jmrodri
Copy link
Member

jmrodri commented Oct 14, 2021

The test-sanity target is failing. It doesn't like something with the generated help text.

…ar images

Signed-off-by: Tatiana Krishtop <26544656+tkrishtop@users.noreply.github.com>
@openshift-ci openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Oct 14, 2021
@openshift-ci
Copy link

openshift-ci bot commented Oct 14, 2021

New changes are detected. LGTM label has been removed.

@tkrishtop
Copy link
Contributor Author

The test-sanity target is failing. It doesn't like something with the generated help text.

@jmrodri sorry for the issue, that was an uncommitted change for generated help text in cmd.go, don't know really how it was detected o_O. I've fixed that, and re-tested in local, make test-sanity works fine now.

@tkrishtop
Copy link
Contributor Author

Hi @estroz and @jmccormick2001, could you please approve CI runs? Everything should work now.

@tkrishtop
Copy link
Contributor Author

Hi @jmccormick2001, thank you for re-running the tests!

Yesterday's go/e2e tests were passing (the only problem was with the sanity tests), and today there is a timeout.
Since yesterday I only changed two lines in cmd.go in the text description of the flags. Normally, that should not impact go/e2e tests. Could you please re-run the tests once again?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
6 participants