Skip to content

Conversation

@jiajliu
Copy link
Contributor

@jiajliu jiajliu commented Jan 20, 2026

  1. Revert Revert "OTA-1604: migrate ocp-46922 from otp to cvo repo" #1297 in commit
  2. Fix the issue found in microshift ci test in commit
  • Added handling for NotFound errors when the Infrastructure resource doesn't exist in IsHypershift, to fix the potential issue like microshift failure
  • To be consistent with OTP where cvo cases are not labeled for microshift, added SkipIfMicroshift to skip tests when running on a microshift.

@coderabbitai
Copy link

coderabbitai bot commented Jan 20, 2026

Walkthrough

This change adds a new test for cluster-version-operator that validates correct runlevel and security context configuration. It includes a JSON test payload entry, a new CVO integration test, and utility helper functions for environment detection and Kubernetes client initialization.

Changes

Cohort / File(s) Summary
Test Payload Definition
\.openshift-tests-extension/openshift_payload_cluster-version-operator.json
Added new test payload entry named "[Jira:"Cluster Version Operator"] cluster-version-operator should have correct runlevel and scc" with standard structure (labels, resources, source, lifecycle, environmentSelector).
CVO Integration Tests
test/cvo/cvo.go
Introduced new test descriptor block with BeforeEach setup obtaining Kubernetes REST config and client, plus test validating: (a) namespace openshift-cluster-version has openshift.io/run-level label (empty value); (b) CVO pod has annotation openshift.io/scc with value hostaccess. Added constant cvoNamespace.
Test Utility Helpers
test/util/util.go
Created new utility file with functions: IsHypershift and SkipIfHypershift (detect external topology mode), IsMicroshift and SkipIfMicroshift (poll for microshift-version configmap), GetRestConfig (load Kubernetes config), GetKubeClient and GetConfigClient (construct API clients). Includes polling logic with timeout handling and detailed error propagation.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

✨ Finishing touches
  • 📝 Generate docstrings

Comment @coderabbitai help to get the list of available commands and usage tips.

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jan 20, 2026

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: jiajliu
Once this PR has been reviewed and has the lgtm label, please assign wking for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found 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

@jiajliu jiajliu changed the title TRT-251 TRT-251:Reapply "OTA-1604: migrate ocp-46922 from otp to cvo repo" Jan 20, 2026
@openshift-ci-robot openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Jan 20, 2026
@openshift-ci-robot
Copy link
Contributor

openshift-ci-robot commented Jan 20, 2026

@jiajliu: This pull request references TRT-251 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 story to target the "4.22.0" version, but no target version was set.

Details

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.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In `@test/cvo/cvo.go`:
- Around line 72-82: Update the redundant wording "openshift.io/scc annotation"
to "openshift.io/scc" in the test that inspects the CVO pod: change the
human-readable description string passed to By(...) and the
Expect(...).To(Equal(...)) failure message so they reference "openshift.io/scc"
(use cvoPod.Name and sccAnnotation as already referenced) instead of the
duplicated "annotation" word; ensure the strings around the annotation key are
updated where cvoPod and sccAnnotation are used.

@jiajliu jiajliu changed the title TRT-251:Reapply "OTA-1604: migrate ocp-46922 from otp to cvo repo" TRT-2511:Reapply "OTA-1604: migrate ocp-46922 from otp to cvo repo" Jan 20, 2026
@openshift-ci-robot
Copy link
Contributor

openshift-ci-robot commented Jan 20, 2026

@jiajliu: This pull request references TRT-2511 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 "4.22.0" version, but no target version was set.

Details

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.

@openshift-ci-robot
Copy link
Contributor

openshift-ci-robot commented Jan 20, 2026

@jiajliu: This pull request references TRT-2511 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 "4.22.0" version, but no target version was set.

Details

In response to this:

  1. Revert Revert "OTA-1604: migrate ocp-46922 from otp to cvo repo" #1297 in commit
  2. Fix the issue found in microshift ci test in commit
  • Added handling for NotFound errors when the Infrastructure resource doesn't exist in IsHypershift, to fix the potential issue like microshift failure
  • To be consistent with OTP where cvo cases are not labeled for microshift, added SkipIfMicroshift to skip tests when running on a microshift.

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.

@jiajliu
Copy link
Contributor Author

jiajliu commented Jan 20, 2026

/payload-job periodic-ci-openshift-microshift-release-4.22-periodics-e2e-aws-ovn-ocp-conformance

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jan 20, 2026

@jiajliu: trigger 1 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command

  • periodic-ci-openshift-microshift-release-4.22-periodics-e2e-aws-ovn-ocp-conformance

See details on https://pr-payload-tests.ci.openshift.org/runs/ci/988df7e0-f5d7-11f0-87ee-a25503e630b6-0

@jiajliu
Copy link
Contributor Author

jiajliu commented Jan 21, 2026

Test pass on standard OCP:

$ curl -s https://gcsweb-ci.apps.ci.l2s4.p1.openshiftapps.com/gcs/test-platform-results/pr-logs/pull/openshift_cluster-version-operator/1301/pull-ci-openshift-cluster-version-operator-main-e2e-agnostic-ovn/2013515794691919872/artifacts/e2e-agnostic-ovn/openshift-e2e-test/artifacts/e2e.log|grep "cluster-version-operator should have"
started: 0/32/430 "[Jira:\"Cluster Version Operator\"] cluster-version-operator should have correct runlevel and scc"
passed: (200ms) 2026-01-20T09:35:57 "[Jira:\"Cluster Version Operator\"] cluster-version-operator should have correct runlevel and scc"

Test skip on Microshift:

$ curl -s https://gcsweb-ci.apps.ci.l2s4.p1.openshiftapps.com/gcs/test-platform-results/logs/periodic-ci-openshift-microshift-release-4.22-periodics-e2e-aws-ovn-ocp-conformance/2013783467631841280/artifacts/e2e-aws-ovn-ocp-conformance/openshift-microshift-e2e-origin-conformance/artifacts/e2e.log|grep "cluster-version-operator should have"
started: 0/100/103 "[Jira:\"Cluster Version Operator\"] cluster-version-operator should have correct runlevel and scc"
skipped: (200ms) 2026-01-21T02:37:47 "[Jira:\"Cluster Version Operator\"] cluster-version-operator should have correct runlevel and scc"

/verified by @jiajliu

cc @hongkailiu to review, thanks.

@openshift-merge-robot openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 25, 2026
@openshift-merge-robot openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 26, 2026
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In `@test/cvo/cvo.go`:
- Around line 56-60: The test currently calls SkipIfHypershift(ctx, restCfg) but
misses skipping MicroShift; add a call to SkipIfMicroshift(ctx, restCfg) right
after the SkipIfHypershift invocation, capture its returned error into err2 (or
reuse err) and assert o.Expect(err2).NotTo(o.HaveOccurred(), "Failed to
determine if cluster is MicroShift") so the test exits on MicroShift clusters
before proceeding with CVO checks; ensure you call the same context/restCfg
variables and keep the existing HyperShift skip intact.

infrastructure, err := configClient.ConfigV1().Infrastructures().Get(ctx, "cluster", metav1.GetOptions{})
if err != nil {
if apierrors.IsNotFound(err) {
Copy link
Member

Choose a reason for hiding this comment

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

Is this what happens on MicroShift?
The linked function does not have it. Maybe we should do it there too?

Copy link
Contributor Author

@jiajliu jiajliu Jan 26, 2026

Choose a reason for hiding this comment

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

Yes, it was. There will not the issue in origin because it will return nil whatever err. So it will not break the test.

Copy link
Member

Choose a reason for hiding this comment

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

Maybe add a guard in this function for MicroShift so that we do not need to depend on notFound?

if m,_:=IsMicroshift(ctx, restConfig); m {
  return false, nil
}

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jan 26, 2026

@jiajliu: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/e2e-agnostic-ovn 2369a08 link true /test e2e-agnostic-ovn
ci/prow/e2e-agnostic-ovn-upgrade-into-change 2369a08 link true /test e2e-agnostic-ovn-upgrade-into-change

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-sigs/prow repository. I understand the commands that are listed here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

jira/valid-reference Indicates that this PR references a valid Jira ticket of any type.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants