-
Notifications
You must be signed in to change notification settings - Fork 4.8k
test: Update images README and add OWNERS #25769
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
Merged
openshift-merge-robot
merged 1 commit into
openshift:master
from
smarterclayton:image_owners
Dec 15, 2020
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,9 @@ | ||
| # See the OWNERS docs at https://go.k8s.io/owners | ||
| reviewers: | ||
| - smarterclayton | ||
| - soltysh | ||
| - sttts | ||
| approvers: | ||
| - smarterclayton | ||
| - soltysh | ||
| - sttts |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -17,7 +17,11 @@ for easy mirroring by the `openshift-tests images` command. | |||||
| * Describe your use case in a PR to this file and have it approved | ||||||
| * Define a standard CI build and publish the image to quay.io in the openshift namespace | ||||||
| * Add the new reference to the `init()` method in this package | ||||||
| * Have the automation promote the image to the quay mirror location. | ||||||
| * Reference your image inside your tests tests using `github.com/openshift/origin/test/extended/util/image.LocationFor("my.source/image/location:versioned_tag")` | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
| * Regenerate the verify output with `make update` and compare the diff to see your source image located | ||||||
| * Ensure your tests fail with "image cannot be pulled" errors. | ||||||
| * Have the reviewer promote the image to the quay mirror location. | ||||||
| * The reviewer should approve the PR and merge. | ||||||
|
|
||||||
| When adding a new image, first make the code changes and compile the `openshift-tests` binary. Then run `hack/update-generated-bindata.sh` to update `test/extended/util/image/zz_generated.txt`. Contact one of the OWNERS of this directory and have them review the image for inclusion into our suite (usually granted in the process above). Before merge and after review they will run the following command to mirror the content to quay: | ||||||
|
|
||||||
|
|
@@ -50,4 +54,42 @@ When a new version of Kubernetes is introduced new images will likely need to be | |||||
| 4. Retest the PR, which should pass or identify new failures | ||||||
| 5. If an upstream image is removed that OpenShift tests depend on, those tests should be refactored to use the appropriate equivalent. | ||||||
|
|
||||||
| Step 3 only has to be run once per new image version introduced in a test. | ||||||
| Step 3 only has to be run once per new image version introduced in a test. | ||||||
|
|
||||||
|
|
||||||
| ## When reviewing | ||||||
|
|
||||||
| We control images so that we are confident that if a user ran the tests binary in a controlled and protected offline environment that we are not introducing excessive risk for the user by running the tests (which run privileged). That means: | ||||||
|
|
||||||
| * Using images that are reproducible - can be updated if a security vulnerability is found | ||||||
| * Using images that are published to a secured location - a malicious third party shouldn't be able to trivially take over the location the image is published to to inject an invalid tag | ||||||
| * Using images that are versioned - `latest` or rolling tags where the API of the image can be broken MUST NOT be allowed, because then a future mirror might regress old tests in old versions | ||||||
|
|
||||||
| Kubernetes has a working process that we consider acceptable for upstream images documented at https://github.com/kubernetes/kubernetes/blob/master/test/images/README.md - images maintained by other communities likely do not satisfy these criteria and must be reviewed with these criteria in mind. | ||||||
|
|
||||||
| OpenShift test images must be built via CI and published to quay in a versioned fashion (no regressions). | ||||||
|
|
||||||
| New images should be added when: | ||||||
|
|
||||||
| 1. An upstream component refactors to use a different image | ||||||
| 1. Ask whether the upstream image is a better image (i.e. is it better managed, more generic, well built, kept up to date by some process) | ||||||
| 2. A new test is added and needs an image AND none of the existing images are sufficient AND none of the existing images can be extended to solve it | ||||||
| 1. I.e. agnhost is a generic tool for simulating clients inside a pod, and so it is better to use that function OR extend it than adding a separate test simulation | ||||||
| 2. The shell image is the ultimate catch all - ANY bash code that isn't wierd should use that. If the bash code needs a novel new command we should add it to the `tools` image (which shell image points to) if it matches the criteria for tools (small Linux utilities that are useful for debugging an openshift cluster / node that are likely to be useful in a wide range of areas) | ||||||
| 3. Don't introduce new versions of an existing image unless there is no choice - i.e. if you need `redis` and are not testing a specific version of redis, just use the existing image | ||||||
|
|
||||||
| ### Mirroring images for approved changes before the PR is merged | ||||||
|
|
||||||
| In order to merge the PR, the tests have to pass, which means the new image has to be mirrored prior to merge. | ||||||
|
|
||||||
| When mirroring from a PR (granting access), you should check out the PR in question and build locally. You should probably rebase the local PR to ensure you don't stomp changes in master (checking out a PR doesn't exactly match what is tested). | ||||||
|
|
||||||
| Then run | ||||||
|
|
||||||
| openshift-tests images --upstream --to-repository quay.io/openshift/community-e2e-images | ||||||
|
|
||||||
| to verify that all things check out. If everything looks good, run | ||||||
|
|
||||||
| openshift-tests images --upstream --to-repository quay.io/openshift/community-e2e-images | oc image mirror -f - --filter-by-os=.* | ||||||
|
|
||||||
| You must be logged in (to docker, using `oc registry login --registry=quay.io` or `skopeo login` or `docker login`) to a quay account that has write permission to `quay.io/openshift/community-e2e-images` which every OWNER should have. | ||||||
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To the best of my understanding CI builds don't push to quay.io nor to the openshift namespace, so i'm confused by this.
CI builds push to registry.svc.ci.openshift.org and generally in the ocp namespace (though certainly they can push to other namespaces)