Add proposal of cross resource group nodes for Azure #2479
Conversation
k8s-ci-robot
added
area/provider/azure
k8s-ci-robot
added
cncf-cla: yes
| - The name of resource group, which is used for Azure nodes. | ||
| - `on-prem`, which is used for on-prem nodes. | ||
|
|
||
| When initializing nodes, these two labels should be set for kubelet by provisioning tools, e.g. |
There was a problem hiding this comment.
are these looking to add new labels that kubelets set on their own Node objects? xref #911 for reasons why this is not desirable.
There was a problem hiding this comment.
Also yep, it's for kubelet add labels for itself. We can add those labels to the whitelist after #911.
Note that azure cloud provider would validate resource group by calling Azure API. If the value is not consistent with Azure, then errors would be reported (InstanceNotFound).
|
|
||
| ## Design | ||
|
|
||
| Instance metadata is a general way to fetch node information for Azure, but it doesn't work if cloud-controller-manager is used (`kubelet --cloud-provider=external`). So it won't be used in this proposal. Instead, following two labels are proposed for providing required information: |
There was a problem hiding this comment.
use of the external cloud provider typically involves a controller that pulls information from metadata and sets it on Node objects. Using a source of truth outside the individual kubelets is preferable for setting topology information on Node objects, since it doesn't require trusting all nodes to categorize themselves appropriately.
There was a problem hiding this comment.
Azure cloud provider would validate the resource group from Azure API. The label provides an easy way to find which resource group should be used when checking whether the node exists or not on Azure.
There's also an alternative way (see last part) of querying node information directly from Azure API. But it is not feasible because that's time-consuming and easy to hit rate limits.
| - `alpha.service-controller.kubernetes.io/exclude-balancer=true`, which is used to exclude the node from load balancer | ||
| - `kubernetes.azure.com/resource-group=<rg-name>`, which provides external RG and is used to get node information. Candidate values are | ||
| - The name of resource group, which is used for Azure nodes. | ||
| - `on-prem`, which is used for on-prem nodes. |
There was a problem hiding this comment.
is this a reserved name (is an invalid resource group name)?
There was a problem hiding this comment.
yep, it's a valid resource group name. Actually, all label values are valid resource group name. So it's a reserved name here.
|
|
||
| - `alpha.service-controller.kubernetes.io/exclude-balancer=true`, which is used to exclude the node from load balancer | ||
| - `kubernetes.azure.com/resource-group=<rg-name>`, which provides external RG and is used to get node information. Candidate values are | ||
| - The name of resource group, which is used for Azure nodes. |
There was a problem hiding this comment.
are all resource group names valid label values?
There was a problem hiding this comment.
Thanks for reminding this.
No, resource group name is in length 1-90, while label value only accepts 63. I should add a limitation part for this.
|
@feiskyer -- this should be submitted as a KEP. Additionally, if we're planning work for this in the 1.12 Release cycle you'll need to:
/hold |
k8s-ci-robot
added
the
do-not-merge/hold
|
@justaugustus it's already tracked by feature kubernetes/enhancements#604. Let me convert this to KEP tomorrow. |
k8s-ci-robot
added
the
sig/architecture
|
@justaugustus Moved to |
|
@feiskyer -- thanks for moving this over (and for already having the feature issue opened)! :) |
|
/hold cancel |
k8s-ci-robot
removed
the
do-not-merge/hold
|
ping @justaugustus @khenidak Any new comments? |
|
|
||
| ### Assumptions | ||
|
|
||
| While news nodes (either from different RGs or on-prem) would be supported in this proposal, not all features would be supported for them. For example, AzureDisk will not work for on-prem nodes. |
|
/lgtm |
|
@kkmsft: changing LGTM is restricted to assignees, and only kubernetes/community repo collaborators may be assigned issues. 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 |
k8s-ci-robot
added
the
lgtm
There was a problem hiding this comment.
@feiskyer -- some doc nits to address.
Additionally, going back to @liggitt's point, we've got a problem with the RG naming. Kubernetes label values and RG names seem to have similar enough restrictions, which means there doesn't appear to be a simple way to differentiate between nodes that should and shouldn't be managed within the context of the Azure cloud provider.
Are we saying that resource groups named on-prem are marked as reserved by ARM? No Azure documentation suggestions that, so we'd need clarification there. I also don't like that that encodes a Kubernetes implementation detail to the cloud provider and hides it from the cluster administrator.
As an idea, could we instead do something like:
- set
kubernetes.azure.com/resource-group= - set an additional new label e.g.,
alpha.service-controller.kubernetes.io/cloud-provider-managed=false
So instances might hold the following labels:
Cross RG Azure instances
kubernetes.azure.com/resource-group=fooalpha.service-controller.kubernetes.io/cloud-provider-managed=true
On-prem / non-Azure managed instances
kubernetes.azure.com/resource-group=alpha.service-controller.kubernetes.io/cloud-provider-managed=false
Thoughts?
/hold
| - sig-azure | ||
| reviewers: | ||
| - name: "@khenidak" | ||
| - name: "@colemickens" |
There was a problem hiding this comment.
"@justaugustus" **
Cole is no longer a Technical Lead for SIG Azure.
|
|
||
| ## Summary | ||
|
|
||
| This KEP aims to add cross resource group (RG) nodes and on-prem nodes support to Azure cloud provider. |
There was a problem hiding this comment.
add support for cross resource group (RG) and on-prem nodes to the Azure cloud provider.**
|
|
||
| ## Motivation | ||
|
|
||
| Today, Azure cloud provider only supports nodes from specified RG (which is set in cloud provider configure file). For nodes in different RG, Azure cloud provider reports `InstanceNotFound` error and thus they would be removed by controller manager. So as on-prem nodes. |
There was a problem hiding this comment.
in the** cloud provider configuration** file
Maybe we should also link out to a configuration file example?
There was a problem hiding this comment.
s/So as on-prem nodes./The same holds true for on-prem nodes.**
|
|
||
| Today, Azure cloud provider only supports nodes from specified RG (which is set in cloud provider configure file). For nodes in different RG, Azure cloud provider reports `InstanceNotFound` error and thus they would be removed by controller manager. So as on-prem nodes. | ||
|
|
||
| With managed cluster (limited access to nodes) users may need nodes where they can customize in ways that are not possible in a managed service. So this document proposes to support joining arbitrary nodes to the cluster and make required changes for both Azure cloud provider and provision setups, which includes |
There was a problem hiding this comment.
With managed clusters, like AKS, there is limited access to configure nodes. There are instances where users may need to customize nodes in ways that are not possible in a managed service.**
There was a problem hiding this comment.
This document proposes support for joining arbitrary nodes to a cluster and the required changes to make in both the Azure cloud provider and provisioned setups, which include:**
|
|
||
| With managed cluster (limited access to nodes) users may need nodes where they can customize in ways that are not possible in a managed service. So this document proposes to support joining arbitrary nodes to the cluster and make required changes for both Azure cloud provider and provision setups, which includes | ||
|
|
||
| - Provision tools should setup kubelet with required labels (e.g. via `--node-labels`) |
|
|
||
| - Nodes are in same region and set with required labels (as clarified in the following design part) | ||
| - Nodes will not be part of the load balancer managed by cloud provider | ||
| - Both node and container networking have setup properly |
There was a problem hiding this comment.
s/have setup properly/are properly configured
| - Both node and container networking have setup properly | ||
| - AzureDisk is supported for Azure cross-RG nodes, but not for on-prem nodes | ||
|
|
||
| Besides, feature gate [ServiceNodeExclusion](https://github.com/kubernetes/kubernetes/blob/master/pkg/features/kube_features.go#L174) should also be enabled for Kubernetes cluster. |
|
|
||
| ### Non-Goals | ||
|
|
||
| Note that provisioning Kubernetes cluster, setting up networking and provisioning new nodes are out of this proposal scope. Those could be done by external provisioning tools (e.g. acs-engine). |
k8s-ci-robot
added
the
do-not-merge/hold
k8s-ci-robot
removed
the
lgtm
|
Offline chat with @justaugustus, we have agreed to use label @justaugustus Addressed comments, PTAL |
|
Perfect. Thanks for addressing all of my nits, @feiskyer! |
k8s-ci-robot
added
the
lgtm
|
/assign @calebamiles @idvoretskyi @jdumars |
|
/hold cancel |
k8s-ci-robot
removed
the
do-not-merge/hold
|
@justaugustus you should be able to. /uncc |
|
/approve |
|
/approve |
k8s-ci-robot
added
the
approved
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: idvoretskyi, justaugustus 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 |
|
@idvoretskyi @justaugustus Thank you very much. |
Automatic merge from submit-queue (batch tested with PRs 66980, 67604, 67741, 67715). If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>. Add support of Azure cross resource group nodes **What this PR does / why we need it**: Part of feature [Cross resource group nodes](kubernetes/enhancements#604). This PR adds support of Azure cross resource group nodes that are labeled with `kubernetes.azure.com/resource-group=<rg-name>` and `alpha.service-controller.kubernetes.io/exclude-balancer=true` **Which issue(s) this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close the issue(s) when PR gets merged)*: Fixes # **Special notes for your reviewer**: See designs [here](kubernetes/community#2479). **Release note**: ```release-note Azure cloud provider now supports cross resource group nodes that are labeled with `kubernetes.azure.com/resource-group=<rg-name>` and `alpha.service-controller.kubernetes.io/exclude-balancer=true` ``` /sig azure /kind feature
Add proposal of cross resource group nodes for Azure.
/sig azure
/assign @brendandburns @khenidak @colemickens
/cc @justaugustus @andyzhangx @kubernetes/sig-azure-misc