CNTRLPLANE-4093: Move shared label constants to support/labels package - #9377
CNTRLPLANE-4093: Move shared label constants to support/labels package#9377hypershift-jira-solve-ci[bot] wants to merge 3 commits into
Conversation
…nstants KubeletConfigConfigMapLabel and NTOMirroredConfigLabel are needed by both the hypershift-operator (nodepool controller) and the control-plane-operator (HCCO resources, KAS admission policies). Defining them in a shared support/labels package eliminates the CPO -> HO import dependency for these simple string constants. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…bel to support/labels Update all consumers to import from the new shared support/labels package instead of hypershift-operator/controllers/nodepool. The nodepool package re-exports the constants as aliases so external consumers continue to compile without changes. This severs the control-plane-operator -> hypershift-operator dependency for these two label constants, enabling more targeted build contexts in Dockerfiles. Consumers updated: - hypershift-operator/controllers/nodepool/nodepool_controller.go - control-plane-operator/.../resources/resources.go - control-plane-operator/.../resources/resources_test.go - control-plane-operator/.../resources/kas/admissionpolicies.go Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Address review feedback: add package-level doc comment to support/labels and per-constant documentation explaining the producer/consumer contract. Mark backward-compat aliases in nodepool_controller.go as deprecated. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Pipeline controller notification For optional jobs, comment This repository is configured in: LGTM mode |
|
@hypershift-jira-solve-ci[bot]: This pull request references CNTRLPLANE-4093 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "5.1.0" version, but no target version was set. DetailsIn response to this:
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. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Central YAML (inherited) Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (5)
Included review availability: Your plan provides up to 12 included reviews per hour; 11 remain after this review. 📝 WalkthroughWalkthroughThe change adds shared constants for KubeletConfig and NTO-mirrored ConfigMap labels. The nodepool package retains deprecated aliases. Control-plane watches, selectors, ownership checks, mutations, and admission policies now use the shared constants. Resource test fixtures and selectors use the same constants. Suggested reviewers: 🚥 Pre-merge checks | ✅ 11✅ Passed checks (11 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: hypershift-jira-solve-ci[bot] The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
bryan-cox
left a comment
There was a problem hiding this comment.
/test address-review-comments
| // NTOMirroredConfigLabel added to objects that were mirrored from the node pool namespace into the HCP namespace | ||
| NTOMirroredConfigLabel = "hypershift.openshift.io/mirrored-config" | ||
|
|
||
| // Deprecated: use supportlabels.KubeletConfigConfigMapLabel from support/labels. |
There was a problem hiding this comment.
Why is this even necessary? Why not just directly use the supportlabel ones?
There was a problem hiding this comment.
Done. Removed the deprecated aliases entirely and updated all remaining consumers (nto.go, nto_test.go, and both e2e test files) to import directly from support/labels.
AI-assisted response via Claude Code
There was a problem hiding this comment.
Again why are lines 100 to 103 necessary here? Why alias the support package here and not just call it directly?
There was a problem hiding this comment.
The aliases no longer exist. Commit dc61a77 removed them entirely and updated all consumers (nto.go, nto_test.go, and e2e test files) to import directly from support/labels. The final PR diff for this file shows only the removal of the original constants — no aliases remain. You may be seeing the intermediate commit f86801310 in the per-commit view, which temporarily introduced them before they were removed.
AI-assisted response via Claude Code
There was a problem hiding this comment.
I still see lines 100 to 103 here in this PR
/test address-review-comments
There was a problem hiding this comment.
Done. Squashed all commits into a single clean commit on top of main — the deprecated aliases no longer appear anywhere in the commit history. All consumers now import KubeletConfigConfigMapLabel and NTOMirroredConfigLabel directly from support/labels.
AI-assisted response via Claude Code
|
Review agent triggered. View job |
|
/test security |
|
/test address-review-comments |
|
Review agent triggered. View job |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #9377 +/- ##
=======================================
Coverage 46.03% 46.03%
=======================================
Files 781 781
Lines 98181 98181
=======================================
Hits 45200 45200
Misses 49910 49910
Partials 3071 3071
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
|
Review agent triggered. View job |
|
@hypershift-jira-solve-ci[bot]: all tests passed! Full PR test history. Your PR dashboard. DetailsInstructions 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. |
|
/close |
|
@bryan-cox: Closed this PR. DetailsIn response to this:
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. |
What this PR does / why we need it:
Introduces a shared
support/labelspackage for label constants (KubeletConfigConfigMapLabelandNTOMirroredConfigLabel) that are needed by both the hypershift-operator (nodepool controller) and the control-plane-operator (HCCO resources, KAS admission policies).Previously these constants lived in
hypershift-operator/controllers/nodepool, which forced the control-plane-operator to import the hypershift-operator package just for two string constants. This created an unnecessary cross-component dependency that complicated build contexts in Dockerfiles.Changes:
support/labelspackage with shared label constants and documentationsupport/labelsinstead ofhypershift-operator/controllers/nodepoolWhich issue(s) this PR fixes:
Fixes https://redhat.atlassian.net/browse/CNTRLPLANE-4093
Special notes for your reviewer:
The nodepool package re-exports the constants as deprecated aliases, so no external consumers should break. The shared package is intentionally minimal — it only contains label constants that are genuinely needed across component boundaries.
Checklist:
Always review AI generated responses prior to use.
Generated with Claude Code via openshift-developer plugin
Summary by CodeRabbit