Bug 1816812: Allow test images to be in a single mirror #291
Conversation
|
@smarterclayton: This pull request references Bugzilla bug 1816812, which is valid. The bug has been updated to refer to the pull request using the external bug tracker. 3 validation(s) were run on this bug
In response to this:
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-robot
added
bugzilla/severity-high
openshift-ci-robot
added
approved
smarterclayton
force-pushed
the
offline_images
branch
from
f171621
to
0425b95
Compare
openshift-ci-robot
removed
the
needs-rebase
smarterclayton
force-pushed
the
offline_images
branch
from
0425b95
to
4f6d059
Compare
|
@smarterclayton: This pull request references Bugzilla bug 1816812, which is valid. 3 validation(s) were run on this bug
In response to this:
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. |
|
/retest |
|
@sttts david did a pass and had me get the upstream created and update the comments. I would appreciate review and ack that you understand why we will carry this (the upstream may take a while to merge / get consensus, but the change here is a very scoped carry in order to support customers who want offline tests). |
|
/retest |
2 similar comments
|
/retest |
|
/retest |
|
@sttts now that we've branched, can you review and raise any concerns so we can merge the offline images change? |
|
/assign @marun |
|
@smarterclayton: This pull request references Bugzilla bug 1816812, which is valid. 3 validation(s) were run on this bug
In response to this:
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. |
| } | ||
|
|
||
| // GetOriginalImageConfigs returns the configuration before any mapping rules. | ||
| func GetOriginalImageConfigs() map[int]Config { |
There was a problem hiding this comment.
Must be used by the mirroring code in order to get the original state and by tests that have to do dynamic lookups (you have to know where Kube thinks the images are in order to translate some access)
|
|
||
| // GetMappedImageConfigs returns the images if they were mapped to the provided | ||
| // image repository. | ||
| func GetMappedImageConfigs(originalImageConfigs map[int]Config, repo string) map[int]Config { |
There was a problem hiding this comment.
I kept the public repo names consistent with the package style (i agree that is a better name, but everything in this package is Get* and I didn't want to drift too much)
|
some nits. Looks reasonable otherwise. /retest |
|
@smarterclayton: The following tests failed, say
Full PR test history. Your PR dashboard. 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. |
|
/retest |
smarterclayton
force-pushed
the
offline_images
branch
from
4f6d059
to
227fe0f
Compare
|
Updated with review comments |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: smarterclayton, sttts The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
7 similar comments
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest |
|
/override ci/prow/e2e-cmd |
|
@smarterclayton: Overrode contexts on behalf of smarterclayton: ci/prow/e2e-cmd In response to this:
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. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
4 similar comments
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
@smarterclayton: The following test failed, say
Full PR test history. Your PR dashboard. 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. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
1 similar comment
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
@smarterclayton: Some pull requests linked via external trackers have merged: The following pull requests linked via external trackers have not merged:
These pull request must merge or be unlinked from the Bugzilla bug in order for it to move to the next state. Bugzilla bug 1816812 has not been moved to the MODIFIED state. In response to this:
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. |
|
/cherry-pick release-4.6 |
|
/cherrypick release-4.6 |
|
@smarterclayton: new pull request created: #536 In response to this:
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. |
|
@smarterclayton: new pull request could not be created: failed to create pull request against openshift/kubernetes#openshift-cherrypick-robot:cherry-pick-291-to-release-4.6 from release-4.6: status code 422 not one of [201], body: {"message":"Validation Failed","errors":[{"resource":"PullRequest","code":"custom","message":"A pull request already exists for openshift-cherrypick-robot:cherry-pick-291-to-release-4.6."}],"documentation_url":"https://docs.github.com/rest/reference/pulls#create-a-pull-request"} In response to this:
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. |
In downstream contexts, it's extremely useful to be able to combine all the "testable" images in Kubernetes into a single repo so that a user could mirror these offline in one chunk, and audit the set of images for changes. For instance, within OpenShift we would like to have a single place we can place all the images used by all the tests with a single authentication scheme. While some images are not "real" and can't be mirrored (for instance, the images that point to an auth protected registry), that is not the majority.
This code makes it possible to specify an environment variable KUBE_TEST_REPO that maps the static strings of the registry to a single repository by placing the uniqueness in a tag. For instance:
would translate
k8s.gcr.io/prometheus-to-sd:v0.5.0toquay.io/openshift/community-e2e-images:e2e-30-k8s-gcr-io-prometheus-to-sd-v0-5-0-6JI59Yih4oaj3oQOjRfhyQ.The tag is a safe form of the name, plus the index (the constant within manifest.go), plus a hash of the full input. The length of the tag is constrained to the minimum of hash + index + the safe name.
The public method is changed to return two maps - index to original name and index to test repo name. These maps would be the same if the env var is not set.
In order to enable offline test execution we need to be able to collect all images used by Kube e2e into a single repo and then
mirror them. This structure may be useful upstream in the future, but some assessment of the particular pattern in use will be
necessary to determine the best outcome.
The presence of the env var switches all pullable images to use a single repository (which has been prepared by openshift-tests)
except for those images that are explicitly not intended to be pulled (those tests can't run offline anyway). The original map
is returned so the mirror can identify the mapping in use between hardcoded value and external value.
See openshift/origin#24887 for more details.
Upstream PR kubernetes#93510.