Azure: support node label keys having underscores #5116
Conversation
Azure VMSS tags allowed character set isn't the exact same as the Kubernetes labels' character set.
In particular "/" aren't supported as VMSS tags, but are allowed for Kubernetes labels.
To support reflecting label values containing "/" to VMSS tags, the cluster-autoscaler evaluates underscores ("_") in VMSS tags keys as slashes ("/") in nodes labels keys.
But due to that substitution, a node label containing "_" (also a valid character for a k8s label) can't be reflected to VMSS tags (as it will be mis-read as a "/").
This PR takes inspiration from rfc6901's JSON escaping (rather hideous, but possibly familiar as it's used by kubectl-patch), by using ("~2") as a replacement for "_".
The tilde (~) character isn't allowed in k8s labels (and taints) keys so there's no possible ambiguities or regressions (a VMSS tag key having a ~ is currently evaluated to an impossible k8s label).
Labels and taints supported character sets:
https://kubernetes.io/docs/concepts/overview/working-with-objects/labels/#syntax-and-character-set
https://kubernetes.io/docs/reference/generated/kubectl/kubectl-commands#taint
k8s-ci-robot
added
cncf-cla: yes
|
The change looks good to me. /cc @gandhipr for the final review/OK as I moved on from Azure. |
|
/lgtm |
|
@gandhipr: 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. |
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: bpineau, gandhipr, marwanad 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 |
k8s-ci-robot
added
lgtm
…score-tags Azure: support node label keys having underscores
Azure VMSS tags allowed character set isn't the exact same as the Kubernetes labels' character set. In particular "/" aren't supported as VMSS tags, but are allowed for Kubernetes labels. To support reflecting label values containing "/" to VMSS tags, the cluster-autoscaler evaluates underscores ("_") in VMSS tags keys as slashes ("/") in nodes labels keys.
But due to that substitution, a node label containing "_" (also a valid character for a k8s label) can't be reflected to VMSS tags (as it will be mis-read as a "/").
This PR takes inspiration from rfc6901's JSON escaping (rather hideous, but possibly familiar as it's used by kubectl-patch), by using ("~2") as a replacement for "_".
The tilde (
~) character isn't allowed in k8s labels (and taints) keys so there's no possible ambiguities or regressions (a VMSS tag key having a "~" is currently evaluated to an impossible k8s label).Labels and taints supported character sets:
https://kubernetes.io/docs/concepts/overview/working-with-objects/labels/#syntax-and-character-set
https://kubernetes.io/docs/reference/generated/kubectl/kubectl-commands#taint
Which component this PR applies to?
What type of PR is this?
What this PR does / why we need it:
Which issue(s) this PR fixes:
Fixes #
Special notes for your reviewer:
Does this PR introduce a user-facing change?
Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.: