Promoting cloud provider labels to GA #839
Conversation
k8s-ci-robot
added
cncf-cla: yes
k8s-ci-robot
added
approved
|
/hold |
k8s-ci-robot
added
the
do-not-merge/hold
|
|
||
| ## Design Details | ||
|
|
||
| ### Test Plan |
There was a problem hiding this comment.
Will fill this out in more detail once we agree to move forward
|
|
||
| TBD | ||
|
|
||
| ### Graduation Criteria |
There was a problem hiding this comment.
Will fill this out in more detail once we agree to move forward
|
|
||
| **Note:** Generally we also wait at least 2 releases between beta and GA/stable, since there's no opportunity for user feedback, or even bug reports, in back-to-back releases. | ||
|
|
||
| ### Upgrade / Downgrade Strategy |
There was a problem hiding this comment.
There were a lot more edge cases here than I had originally thought. Happy to list them out in more detail once we agree to move forward with this.
| 4) [v1.15] continue to promote usage of GA labels over beta labels. | ||
| 5) [v1.16] continue to promote usage of GA labels over beta labels. | ||
| 6) [v1.17] continue to promote usage of GA labels over beta labels. | ||
| 7) [v1.18] stop applying beta labels to new resources, existing resources will continue to have those labels unless manually removed. |
There was a problem hiding this comment.
Is there a timeframe for when the controllers can stop handling beta labels? If so, do we expect that users will need to manually update existing objects update/add the GA label, or will this be automated? I'm running into a similar issue trying to deprecate and remove the VolumeZone scheduling predicate
There was a problem hiding this comment.
For this KEP I was thinking by v1.18 it would be okay to remove logic for it in the controllers because we can assume that at least the GA label is there. I think it would be safer to not have any automation in place for removing beta labels though as users can be using those labels in their custom tooling/controllers.
There was a problem hiding this comment.
Sorry, I didn't mean automation for removing beta labels. I meant automation for adding GA labels to existing objects with the beta label.
@thockin do these labels apply to the api deprecation policy? It mentions annotations but not labels.
There was a problem hiding this comment.
Ahh I see sorry, yes I think we need to update kubelet/controllers to automatically add the GA labels to existing objects.
|
Do we have any appetite for dropping "failure-domain" ? I sort of hate that we have codified support for region/zone and not arbitrary other topologies, but I don't see that changing at the same time. |
|
/assign @mtaufen |
I had a similar thought. @andrewsykim is there anything besides the volume zone scheduling predicate that uses these zone/region labels? I want to deprecate and replace that predicate with PV.NodeAffinity, but I need to figure out how to backfill the PV.NodeAffinity from the existing zone labels. That way we can move away from codifying first-class support for zones/regions. |
The failure domain region/zone labels are also applied to nodes (with cloud provider enabled) |
Are there any core Kubernetes components (besides scheduler volume zone predicate) that use those labels? |
IIRC I don't think any other core component consumes though labels aside from scheduler. Though worth noting those labels are widely used by resources created by users (node selectors, node affinity rules, etc) |
No objections from me to drop |
|
My question is whether these need to be generic. E.g. if we renamed
them to cloud-specific names and made each provider populate their
own, what would happen? Any user who uses topology for anything would
now need to know which provider they were using. That's pretty ugly.
Zone and region are common enough that just those 2 provide a great
deal of portability. I guess I am OK to keep them.
Neither I nor Brian feel any love for "failure-domain". I don't think
it adds any value. If we want to namespace it further
"topology.kubernetes.io" or similar, perhaps. That would give us room
to codify others if it ever comes up (e.g. rack seems like an obvious
one)
@davidopp
…On Tue, Feb 19, 2019 at 11:42 AM Andrew Sy Kim ***@***.***> wrote:
Do we have any appetite for dropping "failure-domain" ? I sort of hate that we have codified support for region/zone and not arbitrary other topologies, but I don't see that changing at the same time.
No objections from me to drop failure-domain from the label names.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.
|
|
I should also note that the auto-population (via pvl admission or dynamic provisioning) of zone/region labels only happens for in-tree volumes. CSI drivers need to implement topology, which will add cloud-specific labels to the Node object (which should map 1-1 with the general zone labels). PVs created through dynamic provisioning will have the cloud-specific labels added in PV.NodeAffinity, but not as PV labels. If users manually create PVs for CSI volumes, there are no admission controllers that will automatically add the zone/region labels. Maybe the cloud-specific controllers could do this if we wanted to keep the zone labeling. |
|
/remove-sig architecture pm |
k8s-ci-robot
removed
sig/architecture
|
Hi @andrewsykim, what's the status of this PR? I'm starting to take over kubernetes/kubernetes#75274 |
|
I think the only outstanding concern was next steps for volume topology. @saad-ali and @msau42 mentioned moving away from using labels but I'm not sure removing those labels is feasible. One path forward I see is to continue to label PVs but don't consume that for volume scheduling in the scheduler. Can you comment @saad-ali @msau42? Either way, I think we mostly have consensus on this KEP for node labels, I'd like to merge this early in the cycle if possible. Time boxing this until July 17th with lazy consensus. Will merge then if there are no objections and follow-up on for volumes if needed. |
|
/unassign |
|
|
||
| Today, the cloud provider specific labels are: | ||
| * `beta.kubernetes.io/instance-type` | ||
| * `failure-domain.beta.kubernetes.io/zone` |
There was a problem hiding this comment.
Considering there seems to already be "failure-domain.kubernetes.io/" labels defined, how do we reconcile them with the "topology.kubernetes.io" labels being proposed here?
|
|
||
| ## Summary | ||
|
|
||
| When node and volume resources are created in Kubernetes, labels may be applied based on the underlying cloud provider of the Kubernetes cluster. |
There was a problem hiding this comment.
nit. on the may, should, must scale (https://www.ietf.org/rfc/rfc2119.txt) it seems like this is a should.
|
|
||
| Here is a break down of the implementation steps: | ||
|
|
||
| 1) [v1.14] update components to apply both the GA and beta labels to nodes & volumes. |
There was a problem hiding this comment.
I think these minor release numbers need to be updated?
|
/lgtm |
k8s-ci-robot
added
the
lgtm
|
/lgtm |
andrewsykim
force-pushed
the
ga-cloud-node-labels
branch
from
3f536fb
to
6e17a05
Compare
k8s-ci-robot
removed
the
lgtm
|
Minor comments addressed, PTAL again /hold cancel |
k8s-ci-robot
removed
the
do-not-merge/hold
andrewsykim
force-pushed
the
ga-cloud-node-labels
branch
from
6e17a05
to
5638874
Compare
k8s-ci-robot
added
the
lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: andrewsykim, cheftako, hogepodge, jagosan 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 |
Enhancement issue: #837
This is a KEP proposing we promote the following beta labels to GA:
beta.kubernetes.io/instance-type->node.kubernetes.io/instance-typefailure-domain.beta.kubernetes.io/zone->topology.kubernetes.io/zonefailure-domain.beta.kubernetes.io/region->topology.kubernetes.io/regionFrom what I can tell, this has implications for nodes and volumes.
/assign @dims @liggitt @msau42 @saad-ali @thockin