Skip to content

refactor: Pass a PassiveClock to operation controllers#5393

Merged
openshift-merge-bot[bot] merged 1 commit into
Azure:mainfrom
mbarnes:1p/operation-controllers-passive-clock
May 27, 2026
Merged

refactor: Pass a PassiveClock to operation controllers#5393
openshift-merge-bot[bot] merged 1 commit into
Azure:mainfrom
mbarnes:1p/operation-controllers-passive-clock

Conversation

@mbarnes
Copy link
Copy Markdown
Collaborator

@mbarnes mbarnes commented May 26, 2026

Supersedes #5062 since we changed our forking policy.

What

Light refactoring for controller consistency.

Why

This allows the RealClock instance in utils.go to be dropped since all the operation controllers now possess a PassiveClock interface to pass to utility functions.

Testing

Existing unit and integration tests should suffice.

PR Checklist

  • PR is scoped to a single task (no mixed concerns)
  • Title follows Conventional Commits format
  • Summary explains the "Why" behind the change
  • Linked to relevant ticket/issue
  • Screenshots included (if graph/UI/metrics changes)
  • Self-reviewed the diff
  • CI/CD checks are passing (ignore Tide)
  • Draft PR used for WIP (if applicable)
  • Commit history is clean (rebased/squashed)
  • Tricky code blocks are commented
  • Specific reviewers tagged
  • All comment threads resolved before merge

Copilot AI review requested due to automatic review settings May 26, 2026 15:02
@openshift-ci openshift-ci Bot requested review from bennerv and miguelsorianod May 26, 2026 15:02
@mbarnes mbarnes requested a review from tmstff May 26, 2026 15:02
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR refactors backend operation controllers and shared operation-controller utilities to accept a k8s.io/utils/clock.PassiveClock from the caller, allowing controller code to avoid relying on a package-level real clock and improving consistency/injectability.

Changes:

  • Updated operation-controller helper functions (UpdateOperationStatus, patchOperation, polling helpers, delete completion helper) to accept a PassiveClock and use it for transition timestamps.
  • Updated operation controllers (cluster/nodepool/externalauth/credential controllers and dispatchers) to store and pass a PassiveClock via their constructors.
  • Updated backend wiring and integration/test helpers to pass a real clock instance where needed.

Reviewed changes

Copilot reviewed 23 out of 23 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
test-integration/utils/integrationutils/utils.go Updates integration helper to call UpdateOperationStatus with a real clock.
backend/pkg/controllers/operationcontrollers/utils.go Removes local clock usage and threads PassiveClock through status/update helpers.
backend/pkg/controllers/operationcontrollers/operation_revoke_credentials.go Adds a PassiveClock field and threads it into patchOperation.
backend/pkg/controllers/operationcontrollers/operation_revoke_credentials_test.go Wires a real clock into the controller under test.
backend/pkg/controllers/operationcontrollers/operation_request_credential.go Adds a PassiveClock field and threads it into patchOperation.
backend/pkg/controllers/operationcontrollers/operation_request_credential_test.go Wires a real clock into the controller under test.
backend/pkg/controllers/operationcontrollers/operation_node_pool_update.go Adds a PassiveClock field and passes it into nodepool polling helper.
backend/pkg/controllers/operationcontrollers/operation_node_pool_update_test.go Wires a real clock into the controller under test.
backend/pkg/controllers/operationcontrollers/operation_node_pool_delete.go Adds a PassiveClock field and threads it into status/delete helpers.
backend/pkg/controllers/operationcontrollers/operation_node_pool_delete_test.go Wires a real clock into the controller under test.
backend/pkg/controllers/operationcontrollers/operation_node_pool_create.go Adds a PassiveClock field and passes it into nodepool polling helper.
backend/pkg/controllers/operationcontrollers/operation_node_pool_create_test.go Wires a real clock into the controller under test.
backend/pkg/controllers/operationcontrollers/operation_external_auth_update.go Adds a PassiveClock field and passes it into externalauth polling helper.
backend/pkg/controllers/operationcontrollers/operation_external_auth_update_test.go Wires a real clock into the controller under test.
backend/pkg/controllers/operationcontrollers/operation_external_auth_delete.go Adds a PassiveClock field and threads it into delete completion helper.
backend/pkg/controllers/operationcontrollers/operation_external_auth_delete_test.go Wires a real clock into the controller under test.
backend/pkg/controllers/operationcontrollers/operation_external_auth_create.go Adds a PassiveClock field and passes it into externalauth polling helper.
backend/pkg/controllers/operationcontrollers/operation_external_auth_create_test.go Wires a real clock into the controller under test.
backend/pkg/controllers/operationcontrollers/operation_cluster_update.go Accepts/injects PassiveClock and uses it when updating operation status.
backend/pkg/controllers/operationcontrollers/operation_cluster_delete.go Accepts/injects PassiveClock and uses it for billing deletion timestamps and op updates.
backend/pkg/controllers/operationcontrollers/operation_cluster_create.go Accepts/injects PassiveClock and uses it when updating operation status.
backend/pkg/controllers/operationcontrollers/operation_cluster_create_test.go Wires a real clock into the controller under test.
backend/pkg/app/backend.go Threads a backend-level PassiveClock into controller construction.

Comment thread backend/pkg/app/backend.go
Comment thread backend/pkg/controllers/operationcontrollers/utils.go Outdated
This allows the RealClock instance in utils.go to be dropped since
all the operation controllers now possess a PassiveClock interface
to pass to utility functions.
@mbarnes mbarnes force-pushed the 1p/operation-controllers-passive-clock branch from a3d57e2 to efee20c Compare May 26, 2026 15:18
@tmstff
Copy link
Copy Markdown
Collaborator

tmstff commented May 27, 2026

/lgtm

@openshift-ci openshift-ci Bot added the lgtm label May 27, 2026
@tmstff tmstff removed their assignment May 27, 2026
@deads2k
Copy link
Copy Markdown
Collaborator

deads2k commented May 27, 2026

This makes some nice test improvement options.

/approve

@openshift-ci
Copy link
Copy Markdown

openshift-ci Bot commented May 27, 2026

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: deads2k, mbarnes, tmstff

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-merge-bot openshift-merge-bot Bot merged commit aa4e1e0 into Azure:main May 27, 2026
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants