-
Notifications
You must be signed in to change notification settings - Fork 416
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
OCPBUGS-50849: add RHEL variant to IsEL() check #4856
OCPBUGS-50849: add RHEL variant to IsEL() check #4856
Conversation
@djoshy: This pull request references Jira Issue OCPBUGS-50849, which is valid. The bug has been moved to the POST state. 3 validation(s) were run on this bug
Requesting review from QA contact: The bug has been updated to refer to the pull request using the external bug tracker. 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. |
a15f161
to
4061297
Compare
/test all |
/periodic-job ? |
/payload-job ? |
/label acknowledge-critical-fixes-only This could potentially be a blocking issue for 4.19 as it breaks ipsec extension installation |
/testwith openshift/origin/master/e2e-aws-ovn-ipsec-serial |
/test openshift/origin/master/e2e-aws-ovn-ipsec-serial |
@djoshy: The specified target(s) for
The following commands are available to trigger optional jobs:
Use
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. |
/lgtm /hold |
pkg/daemon/update.go
Outdated
@@ -1784,7 +1784,7 @@ func (dn *CoreOSDaemon) applyExtensions(oldConfig, newConfig *mcfgv1.MachineConf | |||
// Right now it supports default (traditional), realtime kernel and 64k pages kernel | |||
func (dn *CoreOSDaemon) switchKernel(oldConfig, newConfig *mcfgv1.MachineConfig) error { | |||
// We support Kernel update only on RHCOS and SCOS nodes | |||
if !dn.os.IsEL() { | |||
if !dn.os.IsCoreOSVariant() { |
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.
I think at least for some of these (based on the comments) we want to exclude FCOS from running these (although I suppose you really shouldn't be trying to switch kernel on FCOS so it's probably fine to relax the checks on our end).
Thinking through the now possible cases based on openshift/enhancements#1755:
- RHEL 9.6 (now)
- RHCOS 9.x (old)
- RHCOS 8.x (bootimage only)
- SCOS
- FCOS
- RHEL workers (I suppose we're still doing RHEL 9 workers?)
Then we'd want this to work for 1-4. So basically:
- if os.variantID == coreos, this can be 1/2/4/5, so we'd want to filter out 5
- if os.id == rhcos || os.id == scos, this can be 2/3/4, so we're missing 1
- if os.id == rhel, this can be 1/6 so we'd want to filter out 6
With that in mind maybe the easiest way is to add a conditional check for 3, i.e. os.id==rhcos || (os.id==rhel && os.variantID == coreos)
as a new check?
Please correct me if I'm overcomplicating, just thinking out loud
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.
Updated as per discussion on call 👍
4061297
to
0636873
Compare
@djoshy: This pull request references Jira Issue OCPBUGS-50849, which is valid. 3 validation(s) were run on this bug
Requesting review from QA contact: 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. |
Verifying the fix using these jobs
|
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.
/lgtm
/hold
Holding since the referenced QE jobs are failing, although it doesn't seem MCO related. Will let @sergiordlr unhold when ready
@djoshy The CI testing is still failing with this PR https://github.com/openshift/cluster-network-operator/pull/2649/checks?check_run_id=37539542021, PTAL. |
Took a look at the logs there and it seems like 2 of the nodes made it, but 1 didn't. Strange. I'm going to kick off another run with some more debug logs. |
b94cb8e
to
40131c0
Compare
Last couple runs were green on installs, but I think we just got lucky since all worker nodes went through 2 MC changes. Rerunning with the a slight change to the log message so it'll always write to the journal |
40131c0
to
206ae22
Compare
206ae22
to
1169e57
Compare
I might be losing my mind here, but it seems like |
/unhold |
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: djoshy, isabella-janssen, yuqi-zhang 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 |
I'm still unable to see the debug log on the |
@djoshy: 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. |
1b3974b
into
openshift:master
@djoshy: Jira Issue OCPBUGS-50849: All pull requests linked via external trackers have merged: Jira Issue OCPBUGS-50849 has 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 openshift-eng/jira-lifecycle-plugin repository. |
[ART PR BUILD NOTIFIER] Distgit: ose-machine-config-operator |
- What I did
Added an additional case for the isEL() check to account for the changes in
/etc/os-release
from RHCOS layers EP. Some more context here: openshift/enhancements#1755- How to verify it
The test listed in the bug,
periodic-ci-openshift-openshift-tests-private-release-4.19-multi-nightly-gcp-ipi-ovn-ipsec-arm-mixarch-f14
should succeed. I don't think there is a way to directly run it from this PR, so @sergiordlr will be doing this test manually via the QE private deck.