1.29: Fix PersistentVolumeLabel admission plugin on Azure #124528
Conversation
k8s-ci-robot
added
release-note
|
This cherry pick PR is for a release branch and has not yet been approved by Release Managers. To merge this cherry pick, it must first be approved ( If you didn't cherry-pick this change to all supported release branches, please leave a comment describing why other cherry-picks are not needed to speed up the review process. If you're not sure is it required to cherry-pick this change to all supported release branches, please consult the cherry-pick guidelines document. AFTER it has been approved by code owners, please leave the following comment on a line by itself, with no leading whitespace: /cc kubernetes/release-managers (This command will request a cherry pick review from Release Managers and should work for all GitHub users, whether they are members of the Kubernetes GitHub organization or not.) For details on the patch release process and schedule, see the Patch Releases page. 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/test-infra repository. |
k8s-ci-robot
added
cncf-cla: yes
Do not provide any topology labels for PVs when the Azure cloud provider is not initialized. Providing `topology.kubernetes.io/region: ""` is wrong, because such region does not exist. This affects only in-tree PVs. We have CSI migration in place that makes sure newly provisioned PVs already have correct nodeAffinity, so topology labes are not necessary. And for manually provisioned PVs, no topology label is better that a wrong one, because users can add the label later, but they can't update PV spec.nodeAffinity that would be created from PV labels during PersistentVolumeLabel admission. Note that the whole Azure cloud provider is removed in v1.30, so I'm using just a minimal patch.
jsafrane
force-pushed
the
fix-azure-topology
branch
from
fc73095
to
e5561a4
Compare
k8s-ci-robot
added
size/M
k8s-ci-robot
added
the
lgtm
|
LGTM label has been added. Git tree hash: ea211e15833b2759cc5c94df440870e5e6a20b7b
|
jsafrane
changed the title
|
/approve |
k8s-ci-robot
added
the
approved
|
/lgtm |
|
@cubxxw: changing LGTM is restricted to collaborators In 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/test-infra repository. |
|
@jsafrane: The following test failed, say
Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR. 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. |
|
/retest-required |
|
/priority important-soon |
k8s-ci-robot
added
priority/important-soon
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: andyzhangx, dims, jsafrane The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/test pull-kubernetes-e2e-kind-cloud-provider-loadbalancer |
|
@Verolop: The specified target(s) for
The following commands are available to trigger optional jobs:
Use
In 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. |
…4528-upstream-release-1.27 Automated cherry pick of #124528: Fix PersistentVolumeLabel admission plugin on Azure
…4528-upstream-release-1.28 Automated cherry pick of #124528: Fix PersistentVolumeLabel admission plugin on Azure
|
@Verolop all required tests passed, it really misses just cherry pick approval |
|
/retest-required |
|
/test pull-kubernetes-e2e-kind-cloud-provider-loadbalancer |
|
@bridgetkromhout: The specified target(s) for
The following commands are available to trigger optional jobs:
Use
In 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. |
|
/retest |
|
/test all |
|
/cc kubernetes/release-managers |
This is a manual patch for v1.29. The whole Azure cloud provider was removed in v1.30 and master, so I cannot fix it there and cherry pick it to older branches. I will backport it to 1.28 and older (if they're still supported).
What type of PR is this?
/kind bug
What this PR does / why we need it:
Do not provide any topology labels for PVs when the Azure cloud provider is not initialized. Providing
topology.kubernetes.io/region: ""is wrong, because such region does not exist and PVs with this label would not be usable by any node.This affects only in-tree PVs. We have CSI migration in place that makes sure newly provisioned PVs already have correct nodeAffinity, so topology labels are not necessary. And for manually provisioned PVs, no topology label is better that a wrong one, because users can add the label later, but they can't update PV spec.nodeAffinity that would be created from wrong PV label during PersistentVolumeLabel admission.
Note that the whole Azure cloud provider is removed in v1.30, so I'm using just a minimal patch.
Which issue(s) this PR fixes:
Fixes #124525
Does this PR introduce a user-facing change?
Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.: