Skip to content

OCPBUGS-56114: Fix ImplicitlyEnabledCapabilities #1191

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

Conversation

hongkailiu
Copy link
Member

It is a mistake introduced by https://github.com/openshift/cluster-version-operator/pull/1157/files#diff-db97dc5ae62ef3b4c4dabfdc43cbc3ae0ceae9b58748cadebde679e32b5bec2bL175

As a result, it changes the type name in ClusterVersion which was definitely not the intension of #1157. So we change the constant back with this pull.

The mistake is found by @jiajliu while doing the pre-merge testing for #1133.

It looks like I was not careful enough before confirming IDE (GoLand)'s smart editing. My apologies for that.

Additionally, a couple of cases in unit tests are modified in the pull to avoid regression like this from happening again.

@openshift-ci-robot openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label May 13, 2025
@openshift-ci-robot
Copy link
Contributor

@hongkailiu: This pull request explicitly references no jira issue.

In response to this:

It is a mistake introduced by https://github.com/openshift/cluster-version-operator/pull/1157/files#diff-db97dc5ae62ef3b4c4dabfdc43cbc3ae0ceae9b58748cadebde679e32b5bec2bL175

As a result, it changes the type name in ClusterVersion which was definitely not the intension of #1157. So we change the constant back with this pull.

The mistake is found by @jiajliu while doing the pre-merge testing for #1133.

It looks like I was not careful enough before confirming IDE (GoLand)'s smart editing. My apologies for that.

Additionally, a couple of cases in unit tests are modified in the pull to avoid regression like this from happening again.

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.

@hongkailiu
Copy link
Member Author

/cc @petr-muller @jiajliu

@openshift-ci openshift-ci bot requested review from jiajliu and petr-muller May 13, 2025 13:28
@petr-muller
Copy link
Member

Good find. Will we need to backport this to 4.19? If so we'll need an OCPBUGS card.

@hongkailiu
Copy link
Member Author

hongkailiu commented May 13, 2025

/hold

Let me create a card because of the backporting.

Checked the dates:
Openshift 4.19 Development Branch Opens | Nov 22, 2024

and #1157 got in on Feb 25, 2025.

So backport only to 4.19.

@openshift-ci openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label May 13, 2025
@hongkailiu hongkailiu changed the title NO-JIRA: Fix ImplicitlyEnabledCapabilities OCPBUGS-56114: Fix ImplicitlyEnabledCapabilities May 13, 2025
@openshift-ci-robot openshift-ci-robot added jira/severity-important Referenced Jira bug's severity is important for the branch this PR is targeting. jira/valid-bug Indicates that a referenced Jira bug is valid for the branch this PR is targeting. labels May 13, 2025
@openshift-ci-robot
Copy link
Contributor

@hongkailiu: This pull request references Jira Issue OCPBUGS-56114, which is valid. The bug has been moved to the POST state.

3 validation(s) were run on this bug
  • bug is open, matching expected state (open)
  • bug target version (4.20.0) matches configured target version for branch (4.20.0)
  • bug is in the state New, which is one of the valid states (NEW, ASSIGNED, POST)

Requesting review from QA contact:
/cc @jiajliu

The bug has been updated to refer to the pull request using the external bug tracker.

In response to this:

It is a mistake introduced by https://github.com/openshift/cluster-version-operator/pull/1157/files#diff-db97dc5ae62ef3b4c4dabfdc43cbc3ae0ceae9b58748cadebde679e32b5bec2bL175

As a result, it changes the type name in ClusterVersion which was definitely not the intension of #1157. So we change the constant back with this pull.

The mistake is found by @jiajliu while doing the pre-merge testing for #1133.

It looks like I was not careful enough before confirming IDE (GoLand)'s smart editing. My apologies for that.

Additionally, a couple of cases in unit tests are modified in the pull to avoid regression like this from happening again.

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.

@hongkailiu
Copy link
Member Author

/jira refresh

@openshift-ci-robot
Copy link
Contributor

@hongkailiu: This pull request references Jira Issue OCPBUGS-56114, which is valid.

3 validation(s) were run on this bug
  • bug is open, matching expected state (open)
  • bug target version (4.20.0) matches configured target version for branch (4.20.0)
  • bug is in the state POST, which is one of the valid states (NEW, ASSIGNED, POST)

Requesting review from QA contact:
/cc @jiajliu

In response to this:

/jira refresh

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.

@hongkailiu
Copy link
Member Author

/hold cancel

@openshift-ci openshift-ci bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label May 13, 2025
@petr-muller
Copy link
Member

and #1157 got in on Feb 25, 2025.

Huh we apparently have some test gaps if we can change the value in the API and no tests fail 🤨

@openshift-ci openshift-ci bot added lgtm Indicates that a PR is ready to be merged. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels May 13, 2025
@hongkailiu
Copy link
Member Author

Huh we apparently have some test gaps if we can change the value in the API and no tests fail 🤨

After this pull, the gaps should be closed, e.g., by https://github.com/openshift/cluster-version-operator/pull/1191/files#diff-95a39e68d5f2a7a2f3e7236db558c3154a9967e84f74810888386b13485d5e2fL118

IIRC, I was told (by our Steve 😄 ) that Unit tests should be using direct inputs (instead of constants from the core code).
Now the lesson is more impressive to me.

This commit addresses the issue found in the last round of pre-merge
test [1].

This commit could be reverted after maximal 2 releases.
(It is alright to keep it too)
Assume this PR gets merged to main which is ATM dev branch: 4.20.
When the main branches points to 4.22, all the clusters affetecd
by the bug (originally 4.19 and 4.20) should be fixed when they
are upgraded to 4.20/4.21. Thus any 4.22 cluster should not be
affected.

[1]. https://issues.redhat.com/browse/OCPBUGS-56114?focusedId=27251461&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-27251461
@openshift-ci openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label May 27, 2025
Copy link
Member

@wking wking left a comment

Choose a reason for hiding this comment

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

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label May 27, 2025
Copy link
Contributor

openshift-ci bot commented May 27, 2025

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: hongkailiu, petr-muller, wking

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Copy link
Contributor

openshift-ci bot commented May 27, 2025

@hongkailiu: The following test 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/okd-scos-e2e-aws-ovn 6db9ef4 link false /test okd-scos-e2e-aws-ovn

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.

@jiajliu
Copy link

jiajliu commented May 28, 2025

/label qe-approved

@openshift-ci openshift-ci bot added the qe-approved Signifies that QE has signed off on this PR label May 28, 2025
@openshift-merge-bot openshift-merge-bot bot merged commit b6ad9e5 into openshift:main May 28, 2025
15 of 16 checks passed
@openshift-ci-robot
Copy link
Contributor

@hongkailiu: Jira Issue OCPBUGS-56114: All pull requests linked via external trackers have merged:

Jira Issue OCPBUGS-56114 has been moved to the MODIFIED state.

In response to this:

It is a mistake introduced by https://github.com/openshift/cluster-version-operator/pull/1157/files#diff-db97dc5ae62ef3b4c4dabfdc43cbc3ae0ceae9b58748cadebde679e32b5bec2bL175

As a result, it changes the type name in ClusterVersion which was definitely not the intension of #1157. So we change the constant back with this pull.

The mistake is found by @jiajliu while doing the pre-merge testing for #1133.

It looks like I was not careful enough before confirming IDE (GoLand)'s smart editing. My apologies for that.

Additionally, a couple of cases in unit tests are modified in the pull to avoid regression like this from happening again.

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

jiajliu commented May 28, 2025

/cherry-pick release-4.19

@openshift-cherrypick-robot

@jiajliu: new pull request created: #1199

In response to this:

/cherry-pick release-4.19

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.

@openshift-bot
Copy link
Contributor

[ART PR BUILD NOTIFIER]

Distgit: cluster-version-operator
This PR has been included in build cluster-version-operator-container-v4.20.0-202505280547.p0.gb6ad9e5.assembly.stream.el9.
All builds following this will include this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. jira/severity-important Referenced Jira bug's severity is important for the branch this PR is targeting. jira/valid-bug Indicates that a referenced Jira bug is valid for the branch this PR is targeting. jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. lgtm Indicates that a PR is ready to be merged. qe-approved Signifies that QE has signed off on this PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants