Skip to content

OCPBUGS-29840: actuator: Fix DummyActuator to respect Manual mode upgrade gate#1015

Open
jstuever wants to merge 1 commit intoopenshift:masterfrom
jstuever:OCPBUGS-29840
Open

OCPBUGS-29840: actuator: Fix DummyActuator to respect Manual mode upgrade gate#1015
jstuever wants to merge 1 commit intoopenshift:masterfrom
jstuever:OCPBUGS-29840

Conversation

@jstuever
Copy link
Copy Markdown
Contributor

@jstuever jstuever commented Apr 22, 2026

On unsupported platforms, the DummyActuator now checks the Manual mode upgrade annotation requirement. Previously, DummyActuator always reported upgradeable=true, bypassing the administrator confirmation annotation that Manual mode requires for upgrades.

This adds a Client field to DummyActuator and updates the Upgradeable method to delegate to UpgradeableCheck in Manual mode, ensuring consistent upgrade gating behavior across all platforms.

Assisted-by: Claude Sonnet 4.6

Summary by CodeRabbit

  • Refactor

    • Improved internal handling of unsupported platform scenarios through enhanced Kubernetes client integration
    • Refined upgrade checking logic for edge case operational modes
  • Tests

    • Added comprehensive unit tests for upgrade behavior validation across different operational scenarios

On unsupported platforms, the DummyActuator now checks the Manual mode
upgrade annotation requirement. Previously, DummyActuator always reported
upgradeable=true, bypassing the administrator confirmation annotation
that Manual mode requires for upgrades.

This adds a Client field to DummyActuator and updates the Upgradeable
method to delegate to UpgradeableCheck in Manual mode, ensuring
consistent upgrade gating behavior across all platforms.

Assisted-by: Claude Sonnet 4.6
@openshift-ci-robot openshift-ci-robot added jira/severity-moderate Referenced Jira bug's severity is moderate for the branch this PR is targeting. jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. jira/invalid-bug Indicates that a referenced Jira bug is invalid for the branch this PR is targeting. labels Apr 22, 2026
@openshift-ci-robot
Copy link
Copy Markdown
Contributor

@jstuever: This pull request references Jira Issue OCPBUGS-29840, which is invalid:

  • expected the bug to target the "5.0.0" version, but no target version was set

Comment /jira refresh to re-evaluate validity if changes to the Jira bug are made, or edit the title of this pull request to link to a different bug.

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

Details

In response to this:

On unsupported platforms, the DummyActuator now checks the Manual mode upgrade annotation requirement. Previously, DummyActuator always reported upgradeable=true, bypassing the administrator confirmation annotation that Manual mode requires for upgrades.

This adds a Client field to DummyActuator and updates the Upgradeable method to delegate to UpgradeableCheck in Manual mode, ensuring consistent upgrade gating behavior across all platforms.

Assisted-by: Claude Sonnet 4.6

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.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 22, 2026

Walkthrough

The DummyActuator struct is updated to hold a Kubernetes client and its Upgradeable() method behavior is modified to conditionally delegate to utils.UpgradeableCheck when operating in Manual mode with a client available, while maintaining prior behavior in other modes. Controller initialization is updated to pass the Kubernetes client to the dummy actuator instance.

Changes

Cohort / File(s) Summary
Controller Initialization
pkg/operator/controller.go
Updated no-op actuator construction for unsupported platforms to include Client: m.GetClient() parameter.
Core Actuator Logic
pkg/operator/credentialsrequest/actuator/actuator.go
Added Client field to DummyActuator struct; modified Upgradeable() method to conditionally delegate to utils.UpgradeableCheck in Manual mode when client is available, otherwise return OperatorUpgradeable condition; changed root secret retrieval to return empty NamespacedName.
Actuator Tests
pkg/operator/credentialsrequest/actuator/actuator_test.go
Introduced comprehensive TestDummyActuatorUpgradeable unit test with table-driven test cases covering Manual and non-Manual modes, client availability scenarios, and condition assertions; added strPtr helper function.
Status Test Documentation
pkg/operator/credentialsrequest/status_test.go
Updated dummyUpgradeableCondition comment to reflect conditional behavior of Upgradeable() based on client availability and operation mode.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

🚥 Pre-merge checks | ✅ 10 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Test Structure And Quality ⚠️ Warning Test case expects ConditionTrue when Manual mode has no Client, but security review requires ConditionFalse; assertions lack failure messages. Update test to expect ConditionFalse with ErrorDeterminingUpgradeableReason and add meaningful failure messages to all assertions.
✅ Passed checks (10 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title directly and specifically describes the main change: fixing DummyActuator to respect Manual mode upgrade gate, which aligns with the core objective of enforcing the Manual-mode annotation requirement across all platforms.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Stable And Deterministic Test Names ✅ Passed Pull request does not introduce Ginkgo tests with unstable or dynamic test names. New test uses static, descriptive test case names.
Microshift Test Compatibility ✅ Passed The new test added is a unit test using Go's standard testing.T framework, not a Ginkgo e2e test. The custom check applies only to Ginkgo e2e tests.
Single Node Openshift (Sno) Test Compatibility ✅ Passed This PR adds only standard Go unit tests, not Ginkgo e2e tests. The SNO compatibility check applies only to Ginkgo e2e tests (It(), Describe(), Context() patterns), which are not present.
Topology-Aware Scheduling Compatibility ✅ Passed PR introduces no scheduling constraints or topology-aware configurations that would affect SNO, Two-Node, TNA, or HyperShift clusters.
Ote Binary Stdout Contract ✅ Passed No non-JSON stdout writes detected in process-level code. All changes are properly encapsulated within methods and test functions.
Ipv6 And Disconnected Network Test Compatibility ✅ Passed Pull request adds standard Go unit tests using testing.T framework with fake clients, not Ginkgo e2e tests, so custom check does not apply.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

@openshift-ci openshift-ci Bot requested review from 2uasimojo and dlom April 22, 2026 20:19
@openshift-ci
Copy link
Copy Markdown
Contributor

openshift-ci Bot commented Apr 22, 2026

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jstuever

The full list of commands accepted by this bot can be found here.

The pull request process is described 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

@openshift-ci openshift-ci Bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Apr 22, 2026
@jstuever
Copy link
Copy Markdown
Contributor Author

/jira refresh

@openshift-ci-robot openshift-ci-robot added jira/valid-bug Indicates that a referenced Jira bug is valid for the branch this PR is targeting. and removed jira/invalid-bug Indicates that a referenced Jira bug is invalid for the branch this PR is targeting. labels Apr 22, 2026
@openshift-ci-robot
Copy link
Copy Markdown
Contributor

@jstuever: This pull request references Jira Issue OCPBUGS-29840, 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 (5.0.0) matches configured target version for branch (5.0.0)
  • bug is in the state New, which is one of the valid states (NEW, ASSIGNED, POST)

Requesting review from QA contact:
/cc @jianping-shu

Details

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.

@jstuever
Copy link
Copy Markdown
Contributor Author

/test list

@openshift-ci
Copy link
Copy Markdown
Contributor

openshift-ci Bot commented Apr 22, 2026

@openshift-ci-robot: GitHub didn't allow me to request PR reviews from the following users: jianping-shu.

Note that only openshift members and repo collaborators can review this PR, and authors cannot review their own PRs.

Details

In response to this:

@jstuever: This pull request references Jira Issue OCPBUGS-29840, 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 (5.0.0) matches configured target version for branch (5.0.0)
  • bug is in the state New, which is one of the valid states (NEW, ASSIGNED, POST)

Requesting review from QA contact:
/cc @jianping-shu

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.

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-ci-robot
Copy link
Copy Markdown
Contributor

@jstuever: This pull request references Jira Issue OCPBUGS-29840, which is valid.

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

Requesting review from QA contact:
/cc @jianping-shu

Details

In response to this:

On unsupported platforms, the DummyActuator now checks the Manual mode upgrade annotation requirement. Previously, DummyActuator always reported upgradeable=true, bypassing the administrator confirmation annotation that Manual mode requires for upgrades.

This adds a Client field to DummyActuator and updates the Upgradeable method to delegate to UpgradeableCheck in Manual mode, ensuring consistent upgrade gating behavior across all platforms.

Assisted-by: Claude Sonnet 4.6

Summary by CodeRabbit

  • Refactor

  • Improved internal handling of unsupported platform scenarios through enhanced Kubernetes client integration

  • Refined upgrade checking logic for edge case operational modes

  • Tests

  • Added comprehensive unit tests for upgrade behavior validation across different operational scenarios

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
Copy link
Copy Markdown
Contributor

openshift-ci Bot commented Apr 22, 2026

@openshift-ci-robot: GitHub didn't allow me to request PR reviews from the following users: jianping-shu.

Note that only openshift members and repo collaborators can review this PR, and authors cannot review their own PRs.

Details

In response to this:

@jstuever: This pull request references Jira Issue OCPBUGS-29840, which is valid.

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

Requesting review from QA contact:
/cc @jianping-shu

In response to this:

On unsupported platforms, the DummyActuator now checks the Manual mode upgrade annotation requirement. Previously, DummyActuator always reported upgradeable=true, bypassing the administrator confirmation annotation that Manual mode requires for upgrades.

This adds a Client field to DummyActuator and updates the Upgradeable method to delegate to UpgradeableCheck in Manual mode, ensuring consistent upgrade gating behavior across all platforms.

Assisted-by: Claude Sonnet 4.6

Summary by CodeRabbit

  • Refactor

  • Improved internal handling of unsupported platform scenarios through enhanced Kubernetes client integration

  • Refined upgrade checking logic for edge case operational modes

  • Tests

  • Added comprehensive unit tests for upgrade behavior validation across different operational scenarios

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.

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.

@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 22, 2026

Codecov Report

❌ Patch coverage is 50.00000% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 46.89%. Comparing base (374560d) to head (c172839).

Files with missing lines Patch % Lines
pkg/operator/controller.go 0.00% 1 Missing ⚠️
...g/operator/credentialsrequest/actuator/actuator.go 66.66% 1 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1015   +/-   ##
=======================================
  Coverage   46.88%   46.89%           
=======================================
  Files          98       98           
  Lines       12558    12560    +2     
=======================================
+ Hits         5888     5890    +2     
  Misses       6015     6015           
  Partials      655      655           
Files with missing lines Coverage Δ
pkg/operator/controller.go 0.00% <0.00%> (ø)
...g/operator/credentialsrequest/actuator/actuator.go 44.00% <66.66%> (+4.86%) ⬆️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Copy Markdown

@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

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@pkg/operator/credentialsrequest/actuator/actuator.go`:
- Around line 89-91: The current early-return skips the Manual-mode upgrade gate
when a.Client is nil; update the logic in actuator.go so that when mode ==
operatorv1.CloudCredentialsModeManual you do not default to Upgradeable=true on
a nil client — instead return a failing/closed upgradeable result (i.e.,
Upgradeable=false or an error) if a.Client == nil, and only call
utils.UpgradeableCheck(a.Client, mode, types.NamespacedName{}) when a.Client is
present; adjust the branch around a.Client and utils.UpgradeableCheck to
explicitly fail closed for Manual mode when the client is missing.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: c40d840c-c37e-4ad7-87d2-48da0468c761

📥 Commits

Reviewing files that changed from the base of the PR and between 374560d and c172839.

📒 Files selected for processing (4)
  • pkg/operator/controller.go
  • pkg/operator/credentialsrequest/actuator/actuator.go
  • pkg/operator/credentialsrequest/actuator/actuator_test.go
  • pkg/operator/credentialsrequest/status_test.go

Comment thread pkg/operator/credentialsrequest/actuator/actuator.go
@jstuever
Copy link
Copy Markdown
Contributor Author

/override ci/prow/security
Unrelated, covered by other bugs

@openshift-ci
Copy link
Copy Markdown
Contributor

openshift-ci Bot commented Apr 22, 2026

@jstuever: Overrode contexts on behalf of jstuever: ci/prow/security

Details

In response to this:

/override ci/prow/security
Unrelated, covered by other bugs

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.

@jstuever
Copy link
Copy Markdown
Contributor Author

/retest

1 similar comment
@jstuever
Copy link
Copy Markdown
Contributor Author

/retest

@jstuever
Copy link
Copy Markdown
Contributor Author

/test e2e-hypershift

@openshift-ci
Copy link
Copy Markdown
Contributor

openshift-ci Bot commented Apr 27, 2026

@jstuever: all tests passed!

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.

@jstuever
Copy link
Copy Markdown
Contributor Author

/assign @dlom

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-moderate Referenced Jira bug's severity is moderate 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.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants