Skip to content

Conversation

@bparees
Copy link
Contributor

@bparees bparees commented Mar 16, 2022

This is my understanding after discussion with @derekwaynecarr

@bparees
Copy link
Contributor Author

bparees commented Mar 16, 2022

@smarterclayton @sttts @soltysh ptal

The lack of clarity here caused some consternation recently as we discovered a bunch of build tests that consume redhat product images for exercising app building/deployment.

While it's probably undesirable that they do that, i don't see anyone investing in rewriting them to use an alternate image any time soon, nor, i imagine, do we want to mirror those product images since they aren't ours to publish.

@openshift-ci openshift-ci bot requested review from smarterclayton and sttts March 16, 2022 16:39
@openshift-ci openshift-ci bot added e2e-images-update Related to images used by e2e tests approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Mar 16, 2022
# Images used by e2e tests

We limit the set of images used by e2e to reduce duplication and to allow us to provide offline mirroring of images for customers and restricted test environments. Every image used in e2e must be part of this utility package or referenced by the upstream `k8s.io/kubernetes/test/utils/image` package.
We limit the set of images used by conformance e2e to reduce duplication and to allow us to provide offline mirroring of images for customers and restricted test environments. Every image used in e2e must be part of this utility package or referenced by the upstream `k8s.io/kubernetes/test/utils/image` package.
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think this applies to conformance tests only, the way we have it configured we don't differentiate between conformance and non-conformance images.

Copy link
Contributor Author

@bparees bparees Mar 16, 2022

Choose a reason for hiding this comment

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

pulledInvalidImages allows pulls from registry.redhat.io, among other places (for particular images), it is not enforcing that everything come from quay.io/openshift/community-e2e-images

Copy link
Contributor Author

@bparees bparees Mar 16, 2022

Choose a reason for hiding this comment

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

in fact weirdly, that code does not allow quay.io/openshift/community-e2e-images which raises a whole other set of questions about how the jobs are being run:

allowedPrefixes := sets.NewString(

Copy link
Contributor Author

Choose a reason for hiding this comment

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

(guessing maybe quay.io/openshift/community-e2e-images is being passed in as the arg to pulledInvalidImages so it gets added as a valid prefix?)

regardless, we definitely have tests that run today that pull from other locations and aren't being flagged by this. Possibly because it is builds that are pulling the image, not pods.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

and as another example of the murky waters we are currently in, take this test fixture for example:

- imageChangeParams:
automatic: true
containerNames:
- postgresql
from:
kind: ImageStreamTag
name: postgresql:latest
namespace: openshift

technically this is ok(passed image prefix validation) because it results in a pod that pulls from the internal registry because the imagestream it references is configured with local reference policy(i.e. it uses pullthrough). But the reality is that the image being pulled is coming from registry.redhat.io and unless someone mirrored the content and configured the samples operator to ref the mirror for the imagestreams it manages, it won't actually work in a disconnected environment.

It certainly isn't meeting the currently stated policy of "All images used by e2e are mirrored to quay.io/openshift/community-e2e-images:tag"

@derekwaynecarr
Copy link
Member

/approve
/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Mar 29, 2022
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Mar 29, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: bparees, derekwaynecarr

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-bot
Copy link
Contributor

/retest-required

Please review the full test history for this PR and help us cut down flakes.

1 similar comment
@openshift-bot
Copy link
Contributor

/retest-required

Please review the full test history for this PR and help us cut down flakes.

@bparees
Copy link
Contributor Author

bparees commented Mar 29, 2022

/override ci/prow/e2e-aws-csi
/override ci/prow/e2e-aws-fips
/override ci/prow/e2e-aws-single-node

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Mar 29, 2022

@bparees: Overrode contexts on behalf of bparees: ci/prow/e2e-aws-csi, ci/prow/e2e-aws-fips, ci/prow/e2e-aws-single-node

Details

In response to this:

/override ci/prow/e2e-aws-csi
/override ci/prow/e2e-aws-fips
/override ci/prow/e2e-aws-single-node

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Mar 29, 2022

@bparees: all tests passed!

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@openshift-merge-robot openshift-merge-robot merged commit a7a76d9 into openshift:master Mar 29, 2022
@bparees bparees deleted the imagemirror branch April 14, 2022 13:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. e2e-images-update Related to images used by e2e tests lgtm Indicates that a PR is ready to be merged.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants