Support for kubernetes clusters with user assigned msi #3736

kkmsft · 2018-08-24T08:43:25Z

What this PR does / why we need it:

Support for kubernetes clusters with user assigned msi

Which issue this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close that issue when PR gets merged): fixes #

Support for kubernetes clusters with user assigned msi
Adds the go sdk files for msi apis.
Avoid role assignment retry if the its already present.

Special notes for your reviewer:

If applicable:

documentation
unit tests
tested backward compatibility (ie. deploy with previous version, upgrade with this branch)

Release note:

jackfrancis · 2018-08-24T22:29:12Z

examples/kubernetes-msi-userassigned/kube-vma.json

+      "orchestratorRelease": "1.10",
+      "kubernetesConfig": {
+        "useManagedIdentity": true,
+	"userAssignedID": "acsenginetestid"


nit: spaces

jackfrancis · 2018-08-24T22:29:23Z

examples/kubernetes-msi-userassigned/kube-vmss.json

+      "orchestratorRelease": "1.10",
+      "kubernetesConfig": {
+        "useManagedIdentity": true,
+	"userAssignedID": "acsenginetestid"


jackfrancis · 2018-08-24T22:33:08Z

parts/k8s/kubernetesagentresourcesvmas.t

@@ -146,10 +146,14 @@
    },
 {{end}}
  {
+    {{if UserAssignedIDEnabled}}
+     "apiVersion": "[variables('apiVersionUserMSI')]",


Does the API version above work w/ managed disks? The way this is implemented it assumes that the user MSI API version takes precedence over the managed disks API version.

Testing was done with this and the managed disk set as below and it created the cluster fine.

"agentPoolProfiles": [
{
"name": "agentpool1",
"count": 3,
"vmSize": "Standard_D2_v2",
"availabilityProfile": "VirtualMachineScaleSets",
"storageProfile": "ManagedDisks"
}

Is there a way to use one api version only for parts of the specification - like for example to specify the identity in this case ?

jackfrancis · 2018-08-24T22:35:30Z

parts/k8s/kubernetesagentresourcesvmas.t

@@ -255,7 +268,8 @@
      },
      "type": "Microsoft.Compute/virtualMachines"
    },
-    {{if UseManagedIdentity}}
+    {{if UseManagedIdentity }}


nit: remove this add'l space so the changeset is easier to parse

jackfrancis · 2018-08-24T22:38:49Z

parts/k8s/kubernetesconfigs.sh

@@ -149,6 +149,7 @@ function configureK8s() {
    "cloudProviderRateLimitQPS": ${CLOUDPROVIDER_RATELIMIT_QPS},
    "cloudProviderRateLimitBucket": ${CLOUDPROVIDER_RATELIMIT_BUCKET},
    "useManagedIdentityExtension": ${USE_MANAGED_IDENTITY_EXTENSION},
+    "userAssignedIdentityID": "${USE_ASSIGNED_IDENTITY_ID}",


s/{USE_ASSIGNED_IDENTITY_ID}/{USER_ASSIGNED_IDENTITY_ID} (note the add'l "R" in USER)

jackfrancis · 2018-08-24T22:39:17Z

parts/k8s/kubernetesmastervars.t

@@ -134,7 +139,7 @@
    "customSearchDomainsScript": "{{GetKubernetesB64CustomSearchDomainsScript}}",
    "sshdConfig": "{{GetB64sshdConfig}}",
 {{if not IsOpenShift}}
-    "provisionScriptParametersCommon": "[concat('ADMINUSER=',parameters('linuxAdminUsername'),' ETCD_DOWNLOAD_URL=',parameters('etcdDownloadURLBase'),' ETCD_VERSION=',parameters('etcdVersion'),' DOCKER_ENGINE_VERSION=',parameters('dockerEngineVersion'),' DOCKER_REPO=',parameters('dockerEngineDownloadRepo'),' TENANT_ID=',variables('tenantID'),' KUBERNETES_VERSION={{.OrchestratorProfile.OrchestratorVersion}} HYPERKUBE_URL=',parameters('kubernetesHyperkubeSpec'),' APISERVER_PUBLIC_KEY=',parameters('apiserverCertificate'),' SUBSCRIPTION_ID=',variables('subscriptionId'),' RESOURCE_GROUP=',variables('resourceGroup'),' LOCATION=',variables('location'),' VM_TYPE=',variables('vmType'),' SUBNET=',variables('subnetName'),' NETWORK_SECURITY_GROUP=',variables('nsgName'),' VIRTUAL_NETWORK=',variables('virtualNetworkName'),' VIRTUAL_NETWORK_RESOURCE_GROUP=',variables('virtualNetworkResourceGroupName'),' ROUTE_TABLE=',variables('routeTableName'),' PRIMARY_AVAILABILITY_SET=',variables('primaryAvailabilitySetName'),' PRIMARY_SCALE_SET=',variables('primaryScaleSetName'),' SERVICE_PRINCIPAL_CLIENT_ID=',variables('servicePrincipalClientId'),' SERVICE_PRINCIPAL_CLIENT_SECRET=',variables('singleQuote'),variables('servicePrincipalClientSecret'),variables('singleQuote'),' KUBELET_PRIVATE_KEY=',parameters('clientPrivateKey'),' TARGET_ENVIRONMENT=',parameters('targetEnvironment'),' NETWORK_PLUGIN=',parameters('networkPlugin'),' VNET_CNI_PLUGINS_URL=',parameters('vnetCniLinuxPluginsURL'),' CNI_PLUGINS_URL=',parameters('cniPluginsURL'),' CLOUDPROVIDER_BACKOFF=',parameters('cloudproviderConfig').cloudProviderBackoff,' CLOUDPROVIDER_BACKOFF_RETRIES=',parameters('cloudproviderConfig').cloudProviderBackoffRetries,' CLOUDPROVIDER_BACKOFF_EXPONENT=',parameters('cloudproviderConfig').cloudProviderBackoffExponent,' CLOUDPROVIDER_BACKOFF_DURATION=',parameters('cloudproviderConfig').cloudProviderBackoffDuration,' CLOUDPROVIDER_BACKOFF_JITTER=',parameters('cloudproviderConfig').cloudProviderBackoffJitter,' CLOUDPROVIDER_RATELIMIT=',parameters('cloudproviderConfig').cloudProviderRatelimit,' CLOUDPROVIDER_RATELIMIT_QPS=',parameters('cloudproviderConfig').cloudProviderRatelimitQPS,' CLOUDPROVIDER_RATELIMIT_BUCKET=',parameters('cloudproviderConfig').cloudProviderRatelimitBucket,' USE_MANAGED_IDENTITY_EXTENSION=',variables('useManagedIdentityExtension'),' USE_INSTANCE_METADATA=',variables('useInstanceMetadata'),' LOAD_BALANCER_SKU=',variables('loadBalancerSku'),' EXCLUDE_MASTER_FROM_STANDARD_LB=',variables('excludeMasterFromStandardLB'),' CONTAINER_RUNTIME=',parameters('containerRuntime'),' CONTAINERD_DOWNLOAD_URL_BASE=',parameters('containerdDownloadURLBase'),' POD_INFRA_CONTAINER_SPEC=',parameters('kubernetesPodInfraContainerSpec'),' KMS_PROVIDER_VAULT_NAME=',variables('clusterKeyVaultName'))]",
+    "provisionScriptParametersCommon": "[concat('ADMINUSER=',parameters('linuxAdminUsername'),' ETCD_DOWNLOAD_URL=',parameters('etcdDownloadURLBase'),' ETCD_VERSION=',parameters('etcdVersion'),' DOCKER_ENGINE_VERSION=',parameters('dockerEngineVersion'),' DOCKER_REPO=',parameters('dockerEngineDownloadRepo'),' TENANT_ID=',variables('tenantID'),' KUBERNETES_VERSION={{.OrchestratorProfile.OrchestratorVersion}} HYPERKUBE_URL=',parameters('kubernetesHyperkubeSpec'),' APISERVER_PUBLIC_KEY=',parameters('apiserverCertificate'),' SUBSCRIPTION_ID=',variables('subscriptionId'),' RESOURCE_GROUP=',variables('resourceGroup'),' LOCATION=',variables('location'),' VM_TYPE=',variables('vmType'),' SUBNET=',variables('subnetName'),' NETWORK_SECURITY_GROUP=',variables('nsgName'),' VIRTUAL_NETWORK=',variables('virtualNetworkName'),' VIRTUAL_NETWORK_RESOURCE_GROUP=',variables('virtualNetworkResourceGroupName'),' ROUTE_TABLE=',variables('routeTableName'),' PRIMARY_AVAILABILITY_SET=',variables('primaryAvailabilitySetName'),' PRIMARY_SCALE_SET=',variables('primaryScaleSetName'),' SERVICE_PRINCIPAL_CLIENT_ID=',variables('servicePrincipalClientId'),' SERVICE_PRINCIPAL_CLIENT_SECRET=',variables('singleQuote'),variables('servicePrincipalClientSecret'),variables('singleQuote'),' KUBELET_PRIVATE_KEY=',parameters('clientPrivateKey'),' TARGET_ENVIRONMENT=',parameters('targetEnvironment'),' NETWORK_PLUGIN=',parameters('networkPlugin'),' VNET_CNI_PLUGINS_URL=',parameters('vnetCniLinuxPluginsURL'),' CNI_PLUGINS_URL=',parameters('cniPluginsURL'),' CLOUDPROVIDER_BACKOFF=',parameters('cloudproviderConfig').cloudProviderBackoff,' CLOUDPROVIDER_BACKOFF_RETRIES=',parameters('cloudproviderConfig').cloudProviderBackoffRetries,' CLOUDPROVIDER_BACKOFF_EXPONENT=',parameters('cloudproviderConfig').cloudProviderBackoffExponent,' CLOUDPROVIDER_BACKOFF_DURATION=',parameters('cloudproviderConfig').cloudProviderBackoffDuration,' CLOUDPROVIDER_BACKOFF_JITTER=',parameters('cloudproviderConfig').cloudProviderBackoffJitter,' CLOUDPROVIDER_RATELIMIT=',parameters('cloudproviderConfig').cloudProviderRatelimit,' CLOUDPROVIDER_RATELIMIT_QPS=',parameters('cloudproviderConfig').cloudProviderRatelimitQPS,' CLOUDPROVIDER_RATELIMIT_BUCKET=',parameters('cloudproviderConfig').cloudProviderRatelimitBucket,' USE_MANAGED_IDENTITY_EXTENSION=',variables('useManagedIdentityExtension'),' USE_ASSIGNED_IDENTITY_ID=',variables('userAssignedClientID'),' USE_INSTANCE_METADATA=',variables('useInstanceMetadata'),' LOAD_BALANCER_SKU=',variables('loadBalancerSku'),' EXCLUDE_MASTER_FROM_STANDARD_LB=',variables('excludeMasterFromStandardLB'),' CONTAINER_RUNTIME=',parameters('containerRuntime'),' CONTAINERD_DOWNLOAD_URL_BASE=',parameters('containerdDownloadURLBase'),' POD_INFRA_CONTAINER_SPEC=',parameters('kubernetesPodInfraContainerSpec'),' KMS_PROVIDER_VAULT_NAME=',variables('clusterKeyVaultName'))]",


see above comment about changing to USER_ASSIGNED_IDENTITY_ID (not the additional "R")

jackfrancis · 2018-08-24T22:43:02Z

Lint errors will help nudge unit tests:

pkg/armhelpers/msi.go:14:1:warning: exported method AzureClient.IsUserAssignedIDPresent should have comment or be unexported (golint)
pkg/armhelpers/msi.go:33:1:warning: exported method AzureClient.CreateUserAssignedID should have comment or be unexported (golint)

jackfrancis · 2018-08-24T22:43:30Z

Please update Gopkg.toml to reflect the addition of these new dependencies.

jackfrancis · 2018-08-24T22:43:53Z

This is looking well on its way, thanks @kkmsft! See comments.

kkmsft · 2018-08-29T08:20:21Z

@jackfrancis - not able to get the Gopkg.toml updated since this just includes packages (services/preview/msi/mgmt/2015-08-31-preview/msi) from the azure-sdk-for-go project. Will catch you offline and figure out what to do about this..

codecov · 2018-08-29T09:12:20Z

Codecov Report

Merging #3736 into master will decrease coverage by 0.08%.
The diff coverage is 34%.

@@            Coverage Diff             @@
##           master    #3736      +/-   ##
==========================================
- Coverage   55.41%   55.33%   -0.09%     
==========================================
  Files         108      109       +1     
  Lines       16102    16150      +48     
==========================================
+ Hits         8923     8936      +13     
- Misses       6416     6446      +30     
- Partials      763      768       +5

- Update the Gopkg.lock to include used dependant package. - Removing the k8s version from example since its later versions which support the user assigned MSI.

jackfrancis · 2018-08-30T20:07:28Z

/lgtm

acs-bot · 2018-08-30T20:07:32Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jackfrancis, kkmsft

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:

~~OWNERS~~ [jackfrancis]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

jackfrancis · 2018-08-30T20:07:43Z

Thanks @kkmsft!

acs-bot added the size/XXL label Aug 24, 2018

jackfrancis reviewed Aug 24, 2018

View reviewed changes

kkmsft added 4 commits August 29, 2018 17:20

Support Kubernetes clusters with user assigned MSI.

ea8954f

Add dependant go sdk files for using user assigned msi apis.

b1708dc

- Address review comments.

c974338

- Update the Gopkg.lock to include used dependant package. - Removing the k8s version from example since its later versions which support the user assigned MSI.

Fix the mock client

17b12ff

jackfrancis force-pushed the user_assigned_msi branch from 0355908 to 17b12ff Compare August 30, 2018 00:20

ghost assigned jackfrancis Aug 30, 2018

ghost added the in progress label Aug 30, 2018

Reverting the 409 ignore code and putting a TODO comment instead.

9208570

acs-bot added the lgtm label Aug 30, 2018

acs-bot added the approved label Aug 30, 2018

jackfrancis merged commit 93a0267 into Azure:master Aug 30, 2018

ghost removed the in progress label Aug 30, 2018

kkmsft deleted the user_assigned_msi branch October 14, 2018 00:21

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support for kubernetes clusters with user assigned msi #3736

Support for kubernetes clusters with user assigned msi #3736

kkmsft commented Aug 24, 2018

jackfrancis Aug 24, 2018

jackfrancis Aug 24, 2018

jackfrancis Aug 24, 2018

kkmsft Aug 29, 2018

jackfrancis Aug 24, 2018

jackfrancis Aug 24, 2018

jackfrancis Aug 24, 2018

jackfrancis commented Aug 24, 2018

jackfrancis commented Aug 24, 2018

jackfrancis commented Aug 24, 2018

kkmsft commented Aug 29, 2018

codecov bot commented Aug 29, 2018 •

edited

Loading

jackfrancis commented Aug 30, 2018

acs-bot commented Aug 30, 2018

jackfrancis commented Aug 30, 2018

Support for kubernetes clusters with user assigned msi #3736

Support for kubernetes clusters with user assigned msi #3736

Conversation

kkmsft commented Aug 24, 2018

jackfrancis Aug 24, 2018

Choose a reason for hiding this comment

jackfrancis Aug 24, 2018

Choose a reason for hiding this comment

jackfrancis Aug 24, 2018

Choose a reason for hiding this comment

kkmsft Aug 29, 2018

Choose a reason for hiding this comment

jackfrancis Aug 24, 2018

Choose a reason for hiding this comment

jackfrancis Aug 24, 2018

Choose a reason for hiding this comment

jackfrancis Aug 24, 2018

Choose a reason for hiding this comment

jackfrancis commented Aug 24, 2018

jackfrancis commented Aug 24, 2018

jackfrancis commented Aug 24, 2018

kkmsft commented Aug 29, 2018

codecov bot commented Aug 29, 2018 • edited Loading

Codecov Report

jackfrancis commented Aug 30, 2018

acs-bot commented Aug 30, 2018

jackfrancis commented Aug 30, 2018

codecov bot commented Aug 29, 2018 •

edited

Loading