Skip to content

[release-4.22] OCPBUGS-94518: add ACR pull identity to worker cloud.conf and VMSS#8865

Open
twolff-gh wants to merge 3 commits into
openshift:release-4.22from
twolff-gh:cherry-pick-8472-to-release-4.22
Open

[release-4.22] OCPBUGS-94518: add ACR pull identity to worker cloud.conf and VMSS#8865
twolff-gh wants to merge 3 commits into
openshift:release-4.22from
twolff-gh:cherry-pick-8472-to-release-4.22

Conversation

@twolff-gh

@twolff-gh twolff-gh commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

What this PR does / why we need it:

Backports #8472 to release-4.22. Adds ACR pull identity support to worker cloud.conf and VMSS so kubelet's ACR credential provider can authenticate to Azure Container Registry using a managed identity instead of image pull secrets.

Which issue(s) this PR fixes:

Fixes https://redhat.atlassian.net/browse/OCPBUGS-86616
Fixes https://redhat.atlassian.net/browse/OCPBUGS-94518

Special notes for your reviewer:

Not a clean cherry-pick. Two conflicts resolved:

  1. control-plane-operator/.../azure/config_test.go: k8sutil.SerializeResource → util.SerializeResource (the util → k8sutil rename from PR CNTRLPLANE-3343: Extract support/k8sutil package from support/util #8396 was never backported to release-4.22)
  2. api/hypershift/v1beta1/hostedcluster_types.go and hosted_controlplane.go: git silently carried over the AutoNode/KarpenterConfig/KarpenterAWSConfig pointer→struct type changes from main. Reverted to pointers and regenerated deepcopy to match release-4.22.

Two additional commits added per reviewer guidance:

Checklist:

  • Subject and description added to both, commit and PR.
  • Relevant issues have been referenced.
  • This change includes docs.
  • This change includes unit tests.

@openshift-merge-bot

Copy link
Copy Markdown
Contributor

Pipeline controller notification
This repo is configured to use the pipeline controller. Second-stage tests will be triggered either automatically or after lgtm label is added, depending on the repository configuration. The pipeline controller will automatically detect which contexts are required and will utilize /test Prow commands to trigger the second stage.

For optional jobs, comment /test ? to see a list of all defined jobs. To trigger manually all jobs from second stage use /pipeline required command.

This repository is configured in: LGTM mode

@openshift-ci openshift-ci Bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jun 29, 2026
@openshift-ci

openshift-ci Bot commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@coderabbitai

coderabbitai Bot commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository YAML (base), Central YAML (inherited)

Review profile: CHILL

Plan: Pro Plus

Run ID: f0e3f64f-e96d-4f82-8823-1347f5f35b0d

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@openshift-ci openshift-ci Bot added do-not-merge/needs-area area/api Indicates the PR includes changes for the API area/cli Indicates the PR includes changes for CLI area/control-plane-operator Indicates the PR includes changes for the control plane operator - in an OCP release area/documentation Indicates the PR includes changes for documentation area/hypershift-operator Indicates the PR includes changes for the hypershift operator and API - outside an OCP release area/platform/azure PR/issue for Azure (AzurePlatform) platform and removed do-not-merge/needs-area labels Jun 29, 2026
@codecov

codecov Bot commented Jun 29, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 69.23077% with 44 lines in your changes missing coverage. Please review.
✅ Project coverage is 35.98%. Comparing base (fb3b1a3) to head (c180cce).
⚠️ Report is 11 commits behind head on release-4.22.

Files with missing lines Patch % Lines
...ition-server/controllers/local_ignitionprovider.go 0.00% 19 Missing ⚠️
...ition-server/controllers/tokensecret_controller.go 73.33% 5 Missing and 3 partials ⚠️
...erator/controllers/nodepool/nodepool_controller.go 74.07% 6 Missing and 1 partial ⚠️
hypershift-operator/controllers/nodepool/azure.go 80.00% 2 Missing and 1 partial ⚠️
hypershift-operator/controllers/nodepool/config.go 90.00% 2 Missing and 1 partial ⚠️
hypershift-operator/controllers/nodepool/token.go 57.14% 2 Missing and 1 partial ⚠️
ignition-server/cmd/run_local_ignitionprovider.go 0.00% 1 Missing ⚠️
Additional details and impacted files
@@               Coverage Diff                @@
##           release-4.22    #8865      +/-   ##
================================================
+ Coverage         35.89%   35.98%   +0.09%     
================================================
  Files               774      774              
  Lines             94808    94959     +151     
================================================
+ Hits              34027    34174     +147     
+ Misses            57998    57997       -1     
- Partials           2783     2788       +5     
Files with missing lines Coverage Δ
...lplane/v2/cloud_controller_manager/azure/config.go 68.10% <100.00%> (+6.15%) ⬆️
...rator/controllers/hostedcluster/metrics/metrics.go 92.57% <100.00%> (+0.24%) ⬆️
ignition-server/controllers/cache.go 100.00% <ø> (ø)
ignition-server/cmd/run_local_ignitionprovider.go 0.00% <0.00%> (ø)
hypershift-operator/controllers/nodepool/azure.go 89.70% <80.00%> (-0.88%) ⬇️
hypershift-operator/controllers/nodepool/config.go 85.94% <90.00%> (+0.55%) ⬆️
hypershift-operator/controllers/nodepool/token.go 81.98% <57.14%> (-0.56%) ⬇️
...erator/controllers/nodepool/nodepool_controller.go 40.87% <74.07%> (+1.29%) ⬆️
...ition-server/controllers/tokensecret_controller.go 67.70% <73.33%> (+8.68%) ⬆️
...ition-server/controllers/local_ignitionprovider.go 11.51% <0.00%> (-0.31%) ⬇️

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@twolff-gh
twolff-gh force-pushed the cherry-pick-8472-to-release-4.22 branch from e4513ac to 68f3b24 Compare June 30, 2026 14:42
@twolff-gh
twolff-gh marked this pull request as ready for review June 30, 2026 20:15
@openshift-ci openshift-ci Bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jun 30, 2026
@openshift-ci
openshift-ci Bot requested review from devguyio and muraee June 30, 2026 20:16
@celebdor

Copy link
Copy Markdown
Collaborator

/jira cherry-pick OCPBUGS-86616

@openshift-ci-robot

Copy link
Copy Markdown

@celebdor: Jira Issue OCPBUGS-86616 has been cloned as Jira Issue OCPBUGS-94518. Will retitle bug to link to clone.
/retitle OCPBUGS-94518: Cherry pick 8472 to release 4.22

Details

In response to this:

/jira cherry-pick OCPBUGS-86616

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.

@openshift-ci openshift-ci Bot changed the title Cherry pick 8472 to release 4.22 OCPBUGS-94518: Cherry pick 8472 to release 4.22 Jun 30, 2026
@openshift-ci-robot openshift-ci-robot added jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. jira/invalid-bug Indicates that a referenced Jira bug is invalid for the branch this PR is targeting. labels Jun 30, 2026
@openshift-ci-robot

Copy link
Copy Markdown

@twolff-gh: This pull request references Jira Issue OCPBUGS-94518, which is invalid:

  • release note text must be set and not match the template OR release note type must be set to "Release Note Not Required". For more information you can reference the OpenShift Bug Process.

Comment /jira refresh to re-evaluate validity if changes to the Jira bug are made, or edit the title of this pull request to link to a different bug.

The bug has been updated to refer to the pull request using the external bug tracker.

Details

In response to this:

What this PR does / why we need it:

Backports #8472 to release-4.22. Adds ACR pull identity support to worker cloud.conf and VMSS so kubelet's ACR credential provider can authenticate to Azure Container Registry using a managed identity instead of image pull secrets.

Which issue(s) this PR fixes:

Fixes https://redhat.atlassian.net/browse/OCPBUGS-86616

Special notes for your reviewer:

Not a clean cherry-pick. Two conflicts resolved:

  1. control-plane-operator/.../azure/config_test.go: k8sutil.SerializeResource → util.SerializeResource (the util → k8sutil rename from PR CNTRLPLANE-3343: Extract support/k8sutil package from support/util #8396 was never backported to release-4.22)
  2. api/hypershift/v1beta1/hostedcluster_types.go and hosted_controlplane.go: git silently carried over the AutoNode/KarpenterConfig/KarpenterAWSConfig pointer→struct type changes from main. Reverted to pointers and regenerated deepcopy to match release-4.22.

Checklist:

  • Subject and description added to both, commit and PR.
  • Relevant issues have been referenced.
  • This change includes docs.
  • This change includes unit tests.

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.

@celebdor celebdor changed the title OCPBUGS-94518: Cherry pick 8472 to release 4.22 [release-4.22] OCPBUGS-94518: add ACR pull identity to worker cloud.conf and VMSS Jun 30, 2026
@celebdor

Copy link
Copy Markdown
Collaborator

/jira refresh

@openshift-ci-robot openshift-ci-robot added jira/valid-bug Indicates that a referenced Jira bug is valid for the branch this PR is targeting. and removed jira/invalid-bug Indicates that a referenced Jira bug is invalid for the branch this PR is targeting. labels Jun 30, 2026
@openshift-ci-robot

Copy link
Copy Markdown

@celebdor: This pull request references Jira Issue OCPBUGS-94518, which is valid. The bug has been moved to the POST state.

7 validation(s) were run on this bug
  • bug is open, matching expected state (open)
  • bug target version (4.22.0) matches configured target version for branch (4.22.0)
  • bug is in the state ASSIGNED, which is one of the valid states (NEW, ASSIGNED, POST)
  • release note text is set and does not match the template
  • dependent bug Jira Issue OCPBUGS-86616 is in the state Verified, which is one of the valid states (MODIFIED, ON_QA, VERIFIED)
  • dependent Jira Issue OCPBUGS-86616 targets the "5.0.0" version, which is one of the valid target versions: 5.0.0
  • bug has dependents

The bug has been updated to refer to the pull request using the external bug tracker.

Details

In response to this:

/jira refresh

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.

@enxebre

enxebre commented Jul 1, 2026

Copy link
Copy Markdown
Member

Are both things the vm attachment and the cloud config propagated as a file for the payload generation needed for kubelet ACR pull to work?

If the premise above is true, current implementation doesn't guarantee the latter. Changes to the cloud config do not result in new payload generation and propagation to nodes disk. This might be a good candidate for https://redhat.atlassian.net/browse/OCPSTRAT-3299. With current code you'd get what you want for:

  • New nodePools
  • Existing nodePools if the cloud config doesn't change day 2
  • Existing nodepools if the cloud config change day 2 and something triggers a payload token regeneration

You don't get cloud config day2 changes alone propagated to the nodes disk

@twolff-gh

twolff-gh commented Jul 2, 2026

Copy link
Copy Markdown
Contributor Author

Are both things the vm attachment and the cloud config propagated as a file for the payload generation needed for kubelet ACR pull to work?

If the premise above is true, current implementation doesn't guarantee the latter. Changes to the cloud config do not result in new payload generation and propagation to nodes disk. This might be a good candidate for https://redhat.atlassian.net/browse/OCPSTRAT-3299. With current code you'd get what you want for:

* New nodePools

* Existing nodePools if the cloud config doesn't change day 2

* Existing nodepools if the cloud config change day 2 and something triggers a payload token regeneration

You don't get cloud config day2 changes alone propagated to the nodes disk

I see what your saying. Makes sense.
Can it be added to ConfigGenerator.Hash() ?

And later refactored for OCPSTRAT-3299?

@enxebre

@enxebre

enxebre commented Jul 3, 2026

Copy link
Copy Markdown
Member

Can it be added to ConfigGenerator.Hash() ?

Actually changes to ContainerRegistry will always trigger the rollout because of the machine template changes. I think we just need to gate writeCloudProviderConfig on a hash of the cloud config similar to what we do here for others already to mitigate the race

if pullSecretHash != "" && util.HashSimple(pullSecret) != pullSecretHash {
return nil, fmt.Errorf("pull secret does not match hash")
}
additionalTrustBundle, err := p.fetchAdditionalTrustBundle(ctx)
if err != nil {
return nil, err
}
// NOTE: the additionalTrustBundle only contains the bundle in hc.spec.additionalTrustBundle. The hash is generated
// by the nodepool controller only based on that bundle, so we test a match here. However, the bundle that we want
// to pass to the MCO is the aggregate of the hc.spec.additionalTrustBundle and hc.spec.configuration.proxy.trustedCA
// That is contained in the trusted-ca-bundle-managed configmap.
if additionalTrustBundleHash != "" && util.HashSimple(additionalTrustBundle) != additionalTrustBundleHash {

if hcConfigurationHash != "" && mcsConfig.Data["configuration-hash"] != hcConfigurationHash {
return nil, fmt.Errorf("machine-config-server configmap is out of date, waiting for update %s != %s", mcsConfig.Data["configuration-hash"], hcConfigurationHash)

@JoelSpeed

Copy link
Copy Markdown
Contributor

/approve for API

@openshift-ci

openshift-ci Bot commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: JoelSpeed, twolff-gh
Once this PR has been reviewed and has the lgtm label, please assign enxebre 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

@jparrill

Copy link
Copy Markdown
Contributor

Please fix the description to reflect the right bug which is fixing this PR: https://redhat.atlassian.net/browse/OCPBUGS-94518

@jparrill

Copy link
Copy Markdown
Contributor

Agreeing the review from Alberto, please take a look to these comments. Thanks!

@sdminonne

Copy link
Copy Markdown
Contributor

@twolff-gh mind updating the description to say Fixes https://redhat.atlassian.net/browse/OCPBUGS-94518

@openshift-ci-robot

Copy link
Copy Markdown

@twolff-gh: This pull request references Jira Issue OCPBUGS-94518, which is valid.

7 validation(s) were run on this bug
  • bug is open, matching expected state (open)
  • bug target version (4.22.0) matches configured target version for branch (4.22.0)
  • bug is in the state POST, which is one of the valid states (NEW, ASSIGNED, POST)
  • release note text is set and does not match the template
  • dependent bug Jira Issue OCPBUGS-86616 is in the state Verified, which is one of the valid states (MODIFIED, ON_QA, VERIFIED)
  • dependent Jira Issue OCPBUGS-86616 targets the "5.0.0" version, which is one of the valid target versions: 5.0.0
  • bug has dependents
Details

In response to this:

What this PR does / why we need it:

Backports #8472 to release-4.22. Adds ACR pull identity support to worker cloud.conf and VMSS so kubelet's ACR credential provider can authenticate to Azure Container Registry using a managed identity instead of image pull secrets.

Which issue(s) this PR fixes:

Fixes https://redhat.atlassian.net/browse/OCPBUGS-86616
Fixes https://redhat.atlassian.net/browse/OCPBUGS-94518

Special notes for your reviewer:

Not a clean cherry-pick. Two conflicts resolved:

  1. control-plane-operator/.../azure/config_test.go: k8sutil.SerializeResource → util.SerializeResource (the util → k8sutil rename from PR CNTRLPLANE-3343: Extract support/k8sutil package from support/util #8396 was never backported to release-4.22)
  2. api/hypershift/v1beta1/hostedcluster_types.go and hosted_controlplane.go: git silently carried over the AutoNode/KarpenterConfig/KarpenterAWSConfig pointer→struct type changes from main. Reverted to pointers and regenerated deepcopy to match release-4.22.

Checklist:

  • Subject and description added to both, commit and PR.
  • Relevant issues have been referenced.
  • This change includes docs.
  • This change includes unit tests.

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.

@twolff-gh

twolff-gh commented Jul 22, 2026

Copy link
Copy Markdown
Contributor Author

@twolff-gh mind updating the description to say Fixes https://redhat.atlassian.net/browse/OCPBUGS-94518

@sdminonne Done!

@twolff-gh

Copy link
Copy Markdown
Contributor Author

@enxebre All comments have been addressed now with the merge of the fix and the metrics. Ready for review
cc @jparrill

@devguyio

Copy link
Copy Markdown
Contributor

/uncc @devguyio

@openshift-ci
openshift-ci Bot removed the request for review from devguyio July 22, 2026 20:25
@devguyio

devguyio commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

/pipeline required
unblocking 2nd stage to get the e2e signal until you get the tags from @enxebre @JoelSpeed @jparrill or @sdminonne

@openshift-merge-bot

Copy link
Copy Markdown
Contributor

Scheduling tests matching the pipeline_run_if_changed or not excluded by pipeline_skip_if_only_changed parameters:
/test e2e-aks-4-21
/test e2e-aws-4-21
/test e2e-aks
/test e2e-aws
/test e2e-aws-upgrade-hypershift-operator
/test e2e-azure-self-managed
/test e2e-kubevirt-aws-ovn-reduced
/test e2e-v2-aws

1 similar comment
@openshift-merge-bot

Copy link
Copy Markdown
Contributor

Scheduling tests matching the pipeline_run_if_changed or not excluded by pipeline_skip_if_only_changed parameters:
/test e2e-aks-4-21
/test e2e-aws-4-21
/test e2e-aks
/test e2e-aws
/test e2e-aws-upgrade-hypershift-operator
/test e2e-azure-self-managed
/test e2e-kubevirt-aws-ovn-reduced
/test e2e-v2-aws

@twolff-gh

Copy link
Copy Markdown
Contributor Author

/test e2e-kubevirt-aws-ovn-reduced
/test e2e-aks-4-21
/test e2e-aws-4-21
/test e2e-kubevirt-aws-ovn-reduced

@twolff-gh

Copy link
Copy Markdown
Contributor Author

/test e2e-azure-self-managed

@twolff-gh

Copy link
Copy Markdown
Contributor Author

/test e2e-aws-4-21
/test e2e-aws-upgrade-hypershift-operator

@jparrill jparrill left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Dropped a comment. Thanks!

Comment thread api/hypershift/v1beta1/azure.go Outdated
twolff-gh and others added 2 commits July 23, 2026 10:10
Cherry-pick of openshift#8472 to release-4.22.

Resolved cherry-pick conflict: reverted k8sutil back to util (the
k8sutil rename from main was never backported) and regenerated
deepcopy for release-4.22 struct-typed AutoNode/KarpenterConfig
fields.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Backport of be7bd91 from main (PR openshift#8840), adapted for release-4.22
metrics file structure.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@twolff-gh
twolff-gh force-pushed the cherry-pick-8472-to-release-4.22 branch from f67a269 to 00ba333 Compare July 23, 2026 17:18
…o prevent serving stale content

Backport of PR openshift#8946 (946d0f9, 077d180) from main, adapted for
release-4.22. Hashing uses existing HashSimple inlined at call sites
instead of adding HashConfigMapData to support/util. No osStream
parameter (CNTRLPLANE-3553 not on this branch).

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@twolff-gh
twolff-gh force-pushed the cherry-pick-8472-to-release-4.22 branch from 00ba333 to c180cce Compare July 23, 2026 18:18
@twolff-gh

Copy link
Copy Markdown
Contributor Author

/test e2e-aks-4-21
/test e2e-aws-4-21
/test e2e-aks
/test e2e-aws
/test e2e-aws-upgrade-hypershift-operator
/test e2e-azure-self-managed
/test e2e-kubevirt-aws-ovn-reduced
/test e2e-v2-aws

@twolff-gh
twolff-gh requested a review from jparrill July 23, 2026 19:06
@twolff-gh

Copy link
Copy Markdown
Contributor Author

/test e2e-aks-4-21
/test e2e-aws-upgrade-hypershift-operator

@openshift-ci

openshift-ci Bot commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

@twolff-gh: 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/api Indicates the PR includes changes for the API area/cli Indicates the PR includes changes for CLI area/control-plane-operator Indicates the PR includes changes for the control plane operator - in an OCP release area/documentation Indicates the PR includes changes for documentation area/hypershift-operator Indicates the PR includes changes for the hypershift operator and API - outside an OCP release area/platform/azure PR/issue for Azure (AzurePlatform) platform jira/valid-bug Indicates that a referenced Jira bug is valid for the branch this PR is targeting. jira/valid-reference Indicates that this PR references a valid Jira ticket of any type.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants