-
Notifications
You must be signed in to change notification settings - Fork 122
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
NO-JIRA: rework manifests and drop mentions of 9.4 #1742
NO-JIRA: rework manifests and drop mentions of 9.4 #1742
Conversation
@dustymabe: This pull request explicitly references no jira issue. 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 openshift-eng/jira-lifecycle-plugin repository. |
cosa_build | ||
kola_test_qemu | ||
;; | ||
"rhcos-9-build-test-metal") |
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.
Let's keep the "9" for 9.6 and update openshift/release as well?
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.
ehh. I kind of prefer the namespaced to subversion model.. here's an example of why:
today in #1735 (comment) I was looking to see why the CI on this PR was failing and had to actually dig into the tests to see that ci/prow/rhcos-9-build-test-qemu
is RHEL 9.4 based.
I'm OK with changing it back to the way it was if there are good reasons, but do prefer for RHEL to have major/minor in there.
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.
That sounds good to me. Though we do still need to update openshift/release to drop those tests.
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.
hmm.. I guess this is why we don't do this:
[dustymabe@media release (upstreammaster =)]$ pwd
/var/b/shared/code/github.com/openshift/release
[dustymabe@media release (upstreammaster =)]$ grep -r 'rhcos-9-build-test-qemu' | wc -l
96
because we'd have to update it each time we change RHEL minor versions?
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.
Some of those may be generated through make update
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.
Yeah, it looks a lot worse than it is. The only file you need to manually change is https://github.com/openshift/release/blob/efd41d3fee52d517cb521a59343871cd440b01dc/ci-operator/config/openshift/os/openshift-os-master.yaml.
The awkward thing is that wiring in a new minor is a three step process:
- add the entrypoints here; they just do
exit 0
- add the tests in openshift/release
- change
exit 0
to actually test and iterate here
which was the rationale for not including a minor version here.
It's true that it gets confusing when we set up a new minor though. One thing we could do is to have rhcos-9-build-test-qemu
and rhcos-9-next-build-test-qemu
instead. So in this case, the first would've been 9.4 and the second 9.6. But then when we purge 9.4, we make rhcos-9-build-test-qemu
become 9.6, and neuter -next
to just exit 0
again.
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.
ok I did a new upload here and also opened openshift/release#61527
I went with rhcos-9next
as the prefix. I wanted to combine the 9
with next
to force my brain not to separate them.
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.
Can you also update the README?
cosa_build | ||
kola_test_qemu | ||
;; | ||
"rhcos-9-build-test-metal") |
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.
That sounds good to me. Though we do still need to update openshift/release to drop those tests.
This makes a manifest to be shared amongst rhel-9.4 rhel-9.6 and c9s and moves some initial configuration into it, including the disabling of composefs and the dropping of the zram-generator configuration.
We've moved on to 9.6 for OCP 4.19+ so there is no need to keep the definitions around in our repo here.
a3b15c7
to
cb74377
Compare
done |
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.
Some comments but looks good overall. Thanks for cleaning this up!
cosa_init "ocp-rhel-9.6" | ||
cosa_build | ||
kola_test_qemu | ||
"rhcos-9next-build-test-qemu") |
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.
The addition of these entrypoints could be a separate commit with its own rationale.
/label acknowledge-critical-fixes-only |
/approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: dustymabe, jlebon 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 |
The extensions container CI failure is being worked on. /override ci/prow/images |
@jlebon: Overrode contexts on behalf of jlebon: ci/prow/e2e-aws, ci/prow/images 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-sigs/prow repository. |
@dustymabe: 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-sigs/prow repository. I understand the commands that are listed here. |
We've moved on to 9.6 for OCP 4.19+ so there is no need to keep the definitions around in our repo here.