Skip to content

backend: Pass a PassiveClock to operation controllers#5062

Closed
mbarnes wants to merge 1 commit into
mainfrom
1p/operation-controllers-passive-clock
Closed

backend: Pass a PassiveClock to operation controllers#5062
mbarnes wants to merge 1 commit into
mainfrom
1p/operation-controllers-passive-clock

Conversation

@mbarnes
Copy link
Copy Markdown
Collaborator

@mbarnes mbarnes commented Apr 28, 2026

What

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.

Why

Light refactoring for controller consistency.

Testing

Existing unit and integration tests should suffice.

@openshift-ci openshift-ci Bot requested review from geoberle and janboll April 28, 2026 21:02
@mbarnes mbarnes force-pushed the 1p/operation-controllers-passive-clock branch from 4a7a048 to 752d59a Compare April 29, 2026 01:56
Copilot AI review requested due to automatic review settings April 29, 2026 01:56
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 to consistently use an injected k8s.io/utils/clock.PassiveClock for timestamps, removing the need for a package-level RealClock in operationcontrollers/utils.go and improving testability/consistency.

Changes:

  • Plumb a PassiveClock through operation controller constructors and store it on controller structs.
  • Update shared operation controller utilities (UpdateOperationStatus, patchOperation, polling helpers, delete completion) to accept/use the injected clock instead of a global clock.
  • Update backend wiring and integration/test callsites to pass a real clock where needed.

Reviewed changes

Copilot reviewed 24 out of 24 changed files in this pull request and generated no comments.

Show a summary per file
File Description
test-integration/utils/integrationutils/utils.go Passes RealClock{} into UpdateOperationStatus after signature change.
backend/pkg/controllers/operationcontrollers/utils.go Removes global clock and threads PassiveClock through status update / patch / poll / delete helpers.
backend/pkg/controllers/operationcontrollers/operation_revoke_credentials_test.go Sets controller clock in unit test fixture.
backend/pkg/controllers/operationcontrollers/operation_revoke_credentials.go Adds clock field + constructor param; uses it when patching operation status.
backend/pkg/controllers/operationcontrollers/operation_request_credential_test.go Sets controller clock in unit test fixture.
backend/pkg/controllers/operationcontrollers/operation_request_credential.go Adds clock field + constructor param; uses it when patching operation status.
backend/pkg/controllers/operationcontrollers/operation_node_pool_update_test.go Sets controller clock in unit test fixture.
backend/pkg/controllers/operationcontrollers/operation_node_pool_update.go Adds clock field + constructor param; passes clock into polling helper.
backend/pkg/controllers/operationcontrollers/operation_node_pool_delete_test.go Sets controller clock in unit test fixture.
backend/pkg/controllers/operationcontrollers/operation_node_pool_delete.go Adds clock field + constructor param; uses it for delete completion + status updates.
backend/pkg/controllers/operationcontrollers/operation_node_pool_create_test.go Sets controller clock in unit test fixture.
backend/pkg/controllers/operationcontrollers/operation_node_pool_create.go Adds clock field + constructor param; passes clock into polling helper.
backend/pkg/controllers/operationcontrollers/operation_external_auth_update_test.go Sets controller clock in unit test fixture.
backend/pkg/controllers/operationcontrollers/operation_external_auth_update.go Adds clock field + constructor param; passes clock into polling helper.
backend/pkg/controllers/operationcontrollers/operation_external_auth_delete_test.go Sets controller clock in unit test fixture.
backend/pkg/controllers/operationcontrollers/operation_external_auth_delete.go Adds clock field + constructor param; uses it for delete completion.
backend/pkg/controllers/operationcontrollers/operation_external_auth_create_test.go Sets controller clock in unit test fixture.
backend/pkg/controllers/operationcontrollers/operation_external_auth_create.go Adds clock field + constructor param; passes clock into polling helper.
backend/pkg/controllers/operationcontrollers/operation_cluster_update_test.go Sets controller clock in unit test fixture.
backend/pkg/controllers/operationcontrollers/operation_cluster_update.go Adds clock field + constructor param; uses it for operation status updates.
backend/pkg/controllers/operationcontrollers/operation_cluster_delete.go Updates constructor to accept clock; uses it for billing deletion timestamp + delete completion + status updates.
backend/pkg/controllers/operationcontrollers/operation_cluster_create_test.go Sets controller clock in unit test fixture.
backend/pkg/controllers/operationcontrollers/operation_cluster_create.go Adds clock field + constructor param; uses it for operation status updates.
backend/pkg/app/backend.go Introduces a backend-level PassiveClock and passes it into controller constructors (instead of instantiating RealClock{} per call).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@mbarnes
Copy link
Copy Markdown
Collaborator Author

mbarnes commented Apr 29, 2026

/retest

@tmstff
Copy link
Copy Markdown
Collaborator

tmstff commented May 4, 2026

/lgtm

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 752d59a to 2ccddc0 Compare May 5, 2026 12:41
@openshift-ci openshift-ci Bot removed the lgtm label May 5, 2026
@tmstff
Copy link
Copy Markdown
Collaborator

tmstff commented May 5, 2026

/lgtm

@openshift-ci openshift-ci Bot added the lgtm label May 5, 2026
@openshift-ci
Copy link
Copy Markdown

openshift-ci Bot commented May 5, 2026

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: mbarnes, tmstff
Once this PR has been reviewed and has the lgtm label, please assign deads2k 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

@openshift-ci
Copy link
Copy Markdown

openshift-ci Bot commented May 6, 2026

PR needs rebase.

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.

@mbarnes
Copy link
Copy Markdown
Collaborator Author

mbarnes commented May 26, 2026

Moving these changes to #5393 since our forking policy changed and there's too many merge conflicts here.

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.

3 participants