Skip to content

Comments

backend: Add missing operation controllers#4189

Open
mbarnes wants to merge 1 commit intomainfrom
1p/more-operation-controllers
Open

backend: Add missing operation controllers#4189
mbarnes wants to merge 1 commit intomainfrom
1p/more-operation-controllers

Conversation

@mbarnes
Copy link
Collaborator

@mbarnes mbarnes commented Feb 24, 2026

Related to ARO-24384 - Move Cluster Service CRUD calls to ARO-HCP backend

What

This adds operation controllers to monitor asynchronous create, update, and delete operations for node pool and external auth resource types and removes this logic from OperationScanner.

Although it's dead code now, I left operations_scanner.go in place because there's a good deal of metrics code there that we need to decide whether to adapt to the operation controllers or discard.

There was some debate about whether the OperationScanner metrics were actually useful, since they really just tracked CS polling success rate rather than async operation success rate.

Why

This is a preliminary step toward getting Cluster Service calls moved to a new backend controller.

Special notes for your reviewer

@openshift-ci openshift-ci bot requested review from geoberle and janboll February 24, 2026 10:32
@openshift-ci
Copy link

openshift-ci bot commented Feb 24, 2026

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: mbarnes

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

This adds operation controllers to monitor asynchronous create,
update, and delete operations for node pool and external auth
resource types and removes this logic from OperationsScanner.

Although it's dead code now, I left operations_scanner.go in place
because there's a good deal of metrics code there that we need to
decide whether to adapt to the operation controlers or discard.

There was some debate about whether the current backend metrics are
actually useful; tracking CS polling success rate rather than async
operation success rate.
@mbarnes mbarnes force-pushed the 1p/more-operation-controllers branch from 7944a87 to 2330255 Compare February 24, 2026 13:36
Comment on lines +464 to +465
newOperationStatus := arm.ProvisioningStateSucceeded
logger.Info("new status", "newStatus", newOperationStatus)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This looked odd to me, but it's what the old operation scanner code had. At the time it was initially written, CS must not have offered a status field for ExternalAuth. In working on this I realized that's since been added but I wasn't made aware.

This PR is a straight move of the old code. I'll defer actually checking ExternalAuth status to a follow-up PR.

@mbarnes
Copy link
Collaborator Author

mbarnes commented Feb 24, 2026

/test e2e-parallel

@openshift-ci
Copy link

openshift-ci bot commented Feb 24, 2026

@mbarnes: 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/e2e-parallel 2330255 link true /test e2e-parallel

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant