OKD-443: Handle missing OSImageStream CR on OKD SCOS during upgrades - #31592
OKD-443: Handle missing OSImageStream CR on OKD SCOS during upgrades#31592pskrbasu wants to merge 1 commit into
Conversation
|
Pipeline controller notification For optional jobs, comment This repository is configured in: automatic mode |
openshift-ci-robot
added
the
jira/valid-reference
|
@pskrbasu: This pull request references OKD-443 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the bug to target the "5.0.0" version, but no target version was set. DetailsIn 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. |
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository YAML (base), Central YAML (inherited) Review profile: CHILL Plan: Team Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
|
Scheduling required tests: |
35a1961 to
7c0b734
Compare
|
Scheduling required tests: |
| // validateNodeOS checks that all nodes are running the expected OS for the given stream. | ||
| // This is used as a fallback when the OSImageStream CR is not yet available (e.g., during | ||
| // upgrades from a version that did not support OSImageStream). | ||
| func validateNodeOS(coreClient kclientset.Interface, expectedStream string) { |
There was a problem hiding this comment.
this function is relevant only to OKD so lets make it as such and name it as such - we can drop the rhel-9 and rhel10 markers
7c0b734 to
2c1fb97
Compare
|
Scheduling required tests: |
There was a problem hiding this comment.
This is the function where I'd expect the fix to be, not a new function + "if OKD", as the problem this bugfix describes applies to OCP too.
I think what has happened is that I didn't consider that OKD 4.22 was already using CoreOS 10, so I hardcoded 9 for both without considering this point.
The lack of OS Image Streams for < 4.23/5.0 versions is already covered with this function that performs the validation using basically what you are trying now.
Can we do something like this?
func validatePreOSImageStreamsNodeOS(coreClient kclientset.Interface, isOKD bool) {
// In clusters with no OSImageStreams the nodes should be always RHEL 9
nodes, err := coreClient.CoreV1().Nodes().List(context.TODO(), metav1.ListOptions{})
o.Expect(err).NotTo(o.HaveOccurred(), "Error listing nodes")
targetVersion := 9
if isOKD {
// OKD was already using CoreOS 10 in pre-OSImageStreams versions like 4.22
targetVersion = 10
}
for _, node := range nodes.Items {
osImage := node.Status.NodeInfo.OSImage
o.Expect(osImage).To(o.ContainSubstring(fmt.Sprintf("CoreOS %d.", targetVersion)), "Pre OS Image Stream cluster should use RHEL 9 nodes")
}
}
727da1d to
cac8665
Compare
The validatePreOSImageStreamsNodeOS function hardcoded a check for "CoreOS 9." which is correct for OCP but wrong for OKD, which was already using CoreOS 10 in pre-OSImageStreams versions like 4.22. This caused the "prow job name should match os version" test to fail during 4.22→5.0 OKD upgrades: the test correctly identified the cluster as pre-OSImageStreams (< 4.23) but then failed because the nodes were running CentOS Stream CoreOS 10, not CoreOS 9. Make validatePreOSImageStreamsNodeOS OKD-aware by accepting an isOKD parameter and checking CoreOS 10 for OKD clusters. OCP behavior is unchanged.
cac8665 to
2bb7b76
Compare
|
/lgtm |
|
/cc @neisw |
|
Scheduling required tests: |
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: pablintino, petr-muller, pskrbasu 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 |
openshift-ci
Bot
added
the
approved
|
/label backport-risk-assessed |
openshift-ci
Bot
added
the
backport-risk-assessed
openshift-ci
Bot
assigned
gangwgr,
jadhaj,
kasturinarra,
moebasim,
pamoedom,
prabhapa,
prietyc123 and
sayan-biswas
|
@pskrbasu: 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-sigs/prow repository. I understand the commands that are listed here. |
14 participants
Summary
OSImageStreamCR doesn't exist on OKD SCOS during upgrades from pre-OSImageStream versionsNodeInfo.OSImageinstead of requiring the CR[sig-ci] [Early] prow job name should match os versiontest failure blocking OKD SCOS 5.0 promoted releasesDetails
During upgrades from a version that didn't support OSImageStream (e.g., 4.22→5.0 or ec.5→ec.8), the MCO may not create the
OSImageStreamCR during the upgrade. The test expects it to exist on >= 4.23 clusters, causing a hard failure atjob_names.go:317.The fix adds an
isOKDcheck in theIsNotFoundblock (before the existing pre-4.23 version check) that validates node OS directly using a newvalidateNodeOShelper. This maps stream identifiers to expected OS markers (centos-10→"CentOS Stream CoreOS 10.") and checks all nodes match.OCP is completely unaffected — the new code only triggers when both
isOKD=trueANDapierrors.IsNotFound(err). A missing CR on non-OKD >= 4.23 clusters still fails as before.Why release-5.0
The
openshift-testsbinary in 5.0 payloads comes from this branch. The ec.8 promoted release is currently blocked by this test failure. Targetingrelease-5.0gets the fix into the next 5.0 nightly payload. Will cherry-pick tomainafter merge.Test plan
go vetpassesReferences