Skip to content

Conversation

@smarterclayton
Copy link
Contributor

Document the review criteria as part of accepting rebase.

@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Dec 15, 2020
@smarterclayton smarterclayton force-pushed the image_owners branch 2 times, most recently from 1637b76 to 9236dc8 Compare December 15, 2020 15:12
@bparees
Copy link
Contributor

bparees commented Dec 15, 2020

Can we get some documentation that explains what this process actually does?

Things i'm confused about:

  1. if i add a new test that uses a new image (assume i have a good reason), does that image get listed in images.go's init function as an exception? I wouldn't think so, but the current docs seem to tell you to do that. I would have assumed that the update tooling would just "find" my image reference in my test and add it to the zz_generated.txt

  2. does something being listed in zz_generated.txt result in it being mirrored? What is doing that mirroring and when?

  3. What actually happens at runtime? My impression is that the idea behind this is that all the image strings in test code get substituted w/ the mirror (quay.io or whatever is specified on the command line) location prior to test execution? Is that accurate?

I also put some questions in https://github.com/openshift/origin/pull/24887/files#r538841935 that could use clarification.

Copy link
Contributor

@soltysh soltysh 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-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Dec 15, 2020
Document the review criteria as part of accepting rebase.
Copy link
Contributor

@soltysh soltysh 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-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Dec 15, 2020
@openshift-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: smarterclayton, soltysh

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

* 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")`
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
* Reference your image inside your tests tests using `github.com/openshift/origin/test/extended/util/image.LocationFor("my.source/image/location:versioned_tag")`
* Reference your image inside your tests using `github.com/openshift/origin/test/extended/util/image.LocationFor("my.source/image/location:versioned_tag")`

* An upstream image that is provided by `test/utils/image`.
2. If your use case is novel, you must:
* 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
Copy link
Contributor

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)

@openshift-merge-robot
Copy link
Contributor

@smarterclayton: The following test failed, say /retest to rerun all failed tests:

Test name Commit Details Rerun command
ci/prow/e2e-agnostic-cmd 20dc4ec link /test e2e-agnostic-cmd

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 a79bbb3 into openshift:master Dec 15, 2020
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. 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