HIVE-2199: Drop prefix-based ownership checks in MachinePool controller #2829

huangmingxia · 2026-01-13T11:12:34Z

What does this PR do

Simplifies ownership detection in the MachinePool controller: an object (e.g. MachineSet / MachineAutoscaler) is treated as controlled by a MachinePool only if its hive.openshift.io/machine-pool label value equals MachinePool.spec.name. The previous name-prefix / hive.openshift.io/managed fallback is removed.
Removes unused parameters introduced by the simplification (e.g. dropping cd from syncMachineAutoscalers and updating call sites).

openshift-ci-robot · 2026-01-13T11:12:39Z

@huangmingxia: This pull request references HIVE-2199 which is a valid jira issue.

Details

In response to this:

Fix isControlledByMachinePool to use infraID when building the prefix for identifying MachineSets. MachineSet names follow the format {infraID}-{poolName}-{zone}, not {clusterName}-{poolName}-{zone}.

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.

huangmingxia · 2026-01-13T11:12:48Z

/hold

codecov · 2026-01-13T13:18:04Z

Codecov Report

❌ Patch coverage is 80.00000% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 50.46%. Comparing base (b574bb2) to head (bc52047).
⚠️ Report is 26 commits behind head on master.

Files with missing lines	Patch %	Lines
...g/controller/machinepool/machinepool_controller.go	80.00%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #2829      +/-   ##
==========================================
+ Coverage   50.40%   50.46%   +0.06%     
==========================================
  Files         279      279              
  Lines       34194    34246      +52     
==========================================
+ Hits        17235    17283      +48     
- Misses      15595    15602       +7     
+ Partials     1364     1361       -3

Files with missing lines	Coverage Δ
...g/controller/machinepool/machinepool_controller.go	`63.02% <80.00%> (-0.55%)`	⬇️

... and 4 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

pkg/controller/machinepool/machinepool_controller.go

2uasimojo · 2026-01-15T17:13:12Z

This is looking good. Please remember to update the PR title/description and commit message.

huangmingxia · 2026-01-15T17:17:19Z

/retitle HIVE-2199: Drop prefix-based ownership checks in MachinePool controller

openshift-ci-robot · 2026-01-15T17:20:13Z

@huangmingxia: This pull request references HIVE-2199 which is a valid jira issue.

Details

In response to this:

What does this PR do

Simplifies ownership detection in the MachinePool controller: an object (e.g. MachineSet / MachineAutoscaler) is treated as controlled by a MachinePool only if its hive.openshift.io/machine-pool label value equals MachinePool.spec.name. The previous name-prefix / hive.openshift.io/managed fallback is removed.

Removes unused parameters introduced by the simplification (e.g. dropping cd from syncMachineAutoscalers and updating call sites).

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.

huangmingxia · 2026-01-15T17:23:09Z

@2uasimojo Thank you very much for your review :)
I would appreciate it if you could help fix the security job issue.

2uasimojo · 2026-01-15T22:47:41Z

@huangmingxia Almost there. Need to update the commit message as well (which will unfortunately require a fresh push and CI run).

The security issue is being tracked via HIVE-3053. As long as the failure matches, and is limited to, what's in that card, we can /override the job in PRs. You may not have the power to do that, but I can take care of it when I do the final review.

The isControlledByMachinePool function previously used a two-step approach: first checking for the machinePoolNameLabel, then falling back to prefix-based matching with HiveManagedLabel. This fallback logic was error-prone and could cause false positives. This commit simplifies the ownership detection to only check the machinePoolNameLabel, making it explicit and safer. Objects are now considered controlled by a MachinePool only if they have the hive.openshift.io/machine-pool label matching the pool name. Changes: - Remove prefix-based fallback logic from isControlledByMachinePool - Remove unused cd parameter from syncMachineAutoscalers function - Update all call sites to match the simplified function signatures - Update unit tests to reflect the label-only ownership model This ensures that MachineSets and MachineAutoscalers are only managed when they are explicitly labeled, preventing accidental modification or deletion of objects that happen to match naming patterns but aren't actually managed by Hive.

huangmingxia · 2026-01-16T06:43:59Z

@2uasimojo Ah, my bad - I missed that. Thanks for the reminder!

2uasimojo · 2026-01-21T15:27:42Z

/lgtm
/override ci/prow/security

Ready when you are @huangmingxia

openshift-ci · 2026-01-21T15:30:15Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: 2uasimojo, huangmingxia

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:

~~OWNERS~~ [2uasimojo]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

openshift-ci · 2026-01-21T15:30:34Z

@2uasimojo: Overrode contexts on behalf of 2uasimojo: ci/prow/security

Details

In response to this:

/lgtm
/override ci/prow/security

Ready when you are @huangmingxia

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.

huangmingxia · 2026-01-21T21:02:22Z

/unhold

openshift-ci-robot · 2026-01-21T22:01:36Z

/retest-required

Remaining retests: 0 against base HEAD 00de69a and 2 for PR HEAD bc52047 in total

openshift-ci-robot · 2026-01-21T23:01:21Z

/retest-required

Remaining retests: 0 against base HEAD 3736c56 and 1 for PR HEAD bc52047 in total

huangmingxia · 2026-01-22T14:14:40Z

/override ci/prow/security

openshift-ci · 2026-01-22T14:15:22Z

@huangmingxia: huangmingxia unauthorized: /override is restricted to Repo administrators, approvers in top level OWNERS file, and the following github teams:openshift: openshift-release-oversight openshift-staff-engineers openshift-sustaining-engineers.

Details

In response to this:

/override ci/prow/security

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.

huangmingxia · 2026-01-22T14:16:06Z

@2uasimojo When you have a chance, could you please help override ci/prow/security again? Thank you very much.

openshift-ci-robot · 2026-01-22T20:01:14Z

/retest-required

Remaining retests: 0 against base HEAD dafa229 and 0 for PR HEAD bc52047 in total

2uasimojo · 2026-01-22T20:03:43Z

/override ci/prow/security

2uasimojo · 2026-01-22T20:04:04Z

I swear override used to stick as long as no intervening commits...

openshift-ci · 2026-01-22T20:04:09Z

@2uasimojo: Overrode contexts on behalf of 2uasimojo: ci/prow/security

Details

In response to this:

/override ci/prow/security

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.

huangmingxia · 2026-01-22T20:07:18Z

It might be because I unheld the PR, which caused the job to be retriggered. Thanks again for your time.

openshift-ci · 2026-01-23T18:35:32Z

@huangmingxia: 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.

openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Jan 13, 2026

openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jan 13, 2026

openshift-ci bot requested review from 2uasimojo and suhanime January 13, 2026 11:13

2uasimojo reviewed Jan 14, 2026

View reviewed changes

pkg/controller/machinepool/machinepool_controller.go Outdated Show resolved Hide resolved

huangmingxia force-pushed the HIVE-2199 branch 2 times, most recently from 1424218 to f8ab8d0 Compare January 15, 2026 08:44

openshift-ci bot changed the title ~~HIVE-2199: Use infraID for MachineSet identification~~ HIVE-2199: Drop prefix-based ownership checks in MachinePool controller Jan 15, 2026

huangmingxia force-pushed the HIVE-2199 branch from f8ab8d0 to bc52047 Compare January 16, 2026 06:25

openshift-ci bot assigned 2uasimojo Jan 21, 2026

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jan 21, 2026

openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 21, 2026

openshift-ci bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jan 21, 2026

openshift-merge-bot bot merged commit 5b8cc84 into openshift:master Jan 22, 2026
22 checks passed

HIVE-2199: Drop prefix-based ownership checks in MachinePool controller #2829

HIVE-2199: Drop prefix-based ownership checks in MachinePool controller #2829

Uh oh!

Conversation

huangmingxia commented Jan 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What does this PR do

Uh oh!

openshift-ci-robot commented Jan 13, 2026 • edited by openshift-ci bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

huangmingxia commented Jan 13, 2026

Uh oh!

codecov bot commented Jan 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

2uasimojo commented Jan 15, 2026

Uh oh!

huangmingxia commented Jan 15, 2026 • edited by openshift-ci bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

openshift-ci-robot commented Jan 15, 2026 • edited by openshift-ci bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What does this PR do

Uh oh!

huangmingxia commented Jan 15, 2026

Uh oh!

2uasimojo commented Jan 15, 2026

Uh oh!

huangmingxia commented Jan 16, 2026

Uh oh!

2uasimojo commented Jan 21, 2026

Uh oh!

openshift-ci bot commented Jan 21, 2026

Uh oh!

openshift-ci bot commented Jan 21, 2026

Uh oh!

huangmingxia commented Jan 21, 2026

Uh oh!

openshift-ci-robot commented Jan 21, 2026

Uh oh!

openshift-ci-robot commented Jan 21, 2026

Uh oh!

huangmingxia commented Jan 22, 2026

Uh oh!

openshift-ci bot commented Jan 22, 2026

Uh oh!

huangmingxia commented Jan 22, 2026

Uh oh!

openshift-ci-robot commented Jan 22, 2026

Uh oh!

2uasimojo commented Jan 22, 2026

Uh oh!

2uasimojo commented Jan 22, 2026

Uh oh!

openshift-ci bot commented Jan 22, 2026

Uh oh!

huangmingxia commented Jan 22, 2026

Uh oh!

Uh oh!

openshift-ci bot commented Jan 23, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

huangmingxia commented Jan 13, 2026 •

edited

Loading

openshift-ci-robot commented Jan 13, 2026 •

edited by openshift-ci bot

Loading

codecov bot commented Jan 13, 2026 •

edited

Loading

huangmingxia commented Jan 15, 2026 •

edited by openshift-ci bot

Loading

openshift-ci-robot commented Jan 15, 2026 •

edited by openshift-ci bot

Loading