adds the derived image test scripts for prow#769
Conversation
|
/assign cgwalters |
cgwalters
left a comment
cgwalters
left a comment
There was a problem hiding this comment.
Overall LGTM, some things that can be fixed now or in a followup!
| # Ensure we're in the designated cosa directory so the push-container commands work | ||
| cd "$COSA_DIR" | ||
|
|
||
| # Tags with the cosa build ID / arch - unique to this specific build |
There was a problem hiding this comment.
Will anything prune these tags by default? From a quick look at openshift/release I see image pruners set up on the CI clusters but not the central CI registry?
There was a problem hiding this comment.
I'm not sure to be honest. It would be nice to push to the ephemeral CI registry first so one doesn't have to worry about pruning these tags.
|
|
||
| # Tag with the Prow Build ID because we don't want to overwrite our well-known | ||
| # tags yet, but our test cluster needs the image to be pushed someplace so we | ||
| # can ingest it. We use the BUILD_ID value because its unique to each job so |
There was a problem hiding this comment.
(Doesn't need to be in this PR but I think we should add a TODO here about how we actually do want to push to the "CI namespace" that prow creates that is naturally lifecycle bound to the PR, but we just can't do that right now because Prow wants to own the build process)
There was a problem hiding this comment.
I was thinking about that and only pushing to the external registry on success since that would eliminate the need to prune the build ID tags. However, there are two things I need to figure out:
- How to get those creds within a test since the
KUBECONFIGenv var points to ones ephemeral test cluster, not the ephemeral CI registry? - Is the ephemeral test cluster is created with the perms to pull from the CI namespace registry? I have to assume that it is, but I could be wrong.
| cosa push-container "registry.ci.openshift.org/rhcos-devel/rhel-coreos:$BUILD_ID" | ||
|
|
||
| # Perform the derived OS image build tests | ||
| export BASE_IMAGE_TAG="$BUILD_ID" |
There was a problem hiding this comment.
Hmm, I think we should inject the full pull spec in the future - this would give us more flexibility. (We'll hopefully be pushing the official images outside of rhcos-devel in the near future)
There was a problem hiding this comment.
This is consumed by the derived OS test, but I agree. I'll address that now since the test is in the same repo.
There was a problem hiding this comment.
I updated the Golang test to search for BASE_IMAGE_PULLSPEC and default to registry.ci.openshift.org/rhcos-devel/rhel-coreos:latest if that env var isn't set.
openshift-ci
Bot
added
the
approved
cheesesashimi
force-pushed
the
zzlotnik/add-derived-test-scripts
branch
from
f7438fe to
cc4fad8
Compare
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: cgwalters, cheesesashimi The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
@cheesesashimi: all tests passed! Full PR test history. Your PR dashboard. DetailsInstructions 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. |
3 participants
Instead of having complex Bash scripts embedded in YAML structures, it would be better if the scripts that were written and refined in openshift/release#26912 lived in this repo. This allows additional benefits such as static analysis, reuse, etc.