update the container-service sdk from 2020-02-01 to 2021-03-01 #1416

LochanRn · 2021-06-07T09:41:38Z

What type of PR is this?
/kind feature

What this PR does / why we need it:
Updates the existing containerService sdk to the latest version i.e 2021-03-01.

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 #1418

Special notes for your reviewer:

Please confirm that if this PR changes any image versions, then that's the sole change this PR makes.

TODOs:

squashed commits
includes documentation
adds unit tests

Release note:

container-service api update

k8s-ci-robot · 2021-06-07T09:41:42Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
To complete the pull request process, please assign devigned after the PR has been reviewed.
You can assign the PR to them by writing /assign @devigned in a comment when ready.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

OWNERS

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

k8s-ci-robot · 2021-06-07T09:41:46Z

Hi @LochanRn. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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.

LochanRn · 2021-06-07T09:44:10Z

This is just an initial commit to review changes/modifications done after changing the containerservice sdk.

devigned

The update of the API version looks good. I only question the VMSizeTypes to *string change and what it means about the behavior of the PUT request to Azure.

The change can be seen by comparing: API version 2021-02-01 and API version 2021-03-01.

One thing to consider. Perhaps, this should be it's own PR, rather than combining this with the AAD support as the name of the PR implies, [WIP] aad support for aks.

devigned · 2021-06-07T11:46:53Z

azure/services/agentpools/agentpools.go

@@ -52,7 +52,7 @@ func (s *Service) Reconcile(ctx context.Context, spec interface{}) error {

 	profile := containerservice.AgentPool{
 		ManagedClusterAgentPoolProfileProperties: &containerservice.ManagedClusterAgentPoolProfileProperties{
-			VMSize:              containerservice.VMSizeTypes(agentPoolSpec.SKU),
+			VMSize:              &agentPoolSpec.SKU,


I think the enumeration of VMSizeTypes was too restrictive and the AKS team wanted to allow any string to be specified regardless of the string being in the set of VMSizeTypes.

@CecileRobertMichon what do you think?

You’d be correct :)

Azure/azure-rest-api-specs#13763 (comment)

Thank you for the confirmation, @alexeldeib!

devigned · 2021-06-07T13:18:13Z

/ok-to-test

CecileRobertMichon · 2021-06-07T17:45:27Z

@LochanRn please amend the commit to accurately describe what this is doing (it's not actually adding support for AAD)

LochanRn · 2021-06-07T18:56:57Z

/test pull-cluster-api-provider-azure-e2e-windows

CecileRobertMichon · 2021-06-07T22:34:36Z

I tried creating an aks cluster from this changeset, getting:

[manager] E0607 22:33:26.804946       7 controller.go:302] controller-runtime/manager/controller/azuremanagedcontrolplane "msg"="Reconciler error" "error"="error creating AzureManagedControlPlane default/aks-template: failed to reconcile managed cluster: failed to reconcile managed cluster aks-template: failed to create managed cluster, failed to begin operation: containerservice.ManagedClustersClient#CreateOrUpdate: Failure sending request: StatusCode=0 -- Original Error: Code=\"MustDefineAtLeastOneSystemPool\" Message=\"Must define at least one system pool.\"" "name"="aks-template" "namespace"="default" "reconciler group"="infrastructure.cluster.x-k8s.io" "reconciler kind"="AzureManagedControlPlane"

/hold

CecileRobertMichon · 2021-06-14T17:49:44Z

@alexeldeib can you validate that setting the agent pool to mode System for now is appropriate to mitigate the error until #1376 is implemented?

alexeldeib · 2021-06-14T17:55:57Z

I think we need to implement #1376 in tandem with the SDK update tbh, or else deletion behavior will get pretty messy. @LochanRn reached out on slack and sounds like will work on both #1376 + SDK update. I think the AAD functionality would be good in a follow up PR after that.

k8s-ci-robot · 2021-06-18T22:56:15Z

@LochanRn: PR needs rebase.

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.

…pi-provider-azure into aad-aks-support

k8s-ci-robot · 2021-06-20T19:56:55Z

@LochanRn: The following tests failed, say /retest to rerun all failed tests:

Test name	Commit	Details	Rerun command
pull-cluster-api-provider-azure-verify	`58b57c9`	link	`/test pull-cluster-api-provider-azure-verify`
pull-cluster-api-provider-azure-e2e	`58b57c9`	link	`/test pull-cluster-api-provider-azure-e2e`
pull-cluster-api-provider-azure-test	`58b57c9`	link	`/test pull-cluster-api-provider-azure-test`
pull-cluster-api-provider-azure-build	`58b57c9`	link	`/test pull-cluster-api-provider-azure-build`
pull-cluster-api-provider-azure-apidiff	`58b57c9`	link	`/test pull-cluster-api-provider-azure-apidiff`
pull-cluster-api-provider-azure-e2e-windows	`58b57c9`	link	`/test pull-cluster-api-provider-azure-e2e-windows`
pull-cluster-api-provider-azure-coverage	`58b57c9`	link	`/test pull-cluster-api-provider-azure-coverage`

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/test-infra repository. I understand the commands that are listed here.

LochanRn · 2021-07-08T13:06:03Z

closing this PR as it is fixed in #1475

k8s-ci-robot requested review from devigned and juan-lee June 7, 2021 09:41

k8s-ci-robot added area/provider/azure Issues or PRs related to azure provider sig/cluster-lifecycle Categorizes an issue or PR as relevant to SIG Cluster Lifecycle. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Jun 7, 2021

k8s-ci-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Jun 7, 2021

devigned suggested changes Jun 7, 2021

View reviewed changes

LochanRn changed the title ~~[WIP] aad support for aks~~ update the container-service api from 2020-02-01 to 2021-03-01 Jun 7, 2021

k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jun 7, 2021

LochanRn changed the title ~~update the container-service api from 2020-02-01 to 2021-03-01~~ update the container-service sdk from 2020-02-01 to 2021-03-01 Jun 7, 2021

updated the containerservice sdk

89b8d83

LochanRn force-pushed the aad-aks-support branch from 83ba71f to 89b8d83 Compare June 7, 2021 17:54

k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jun 7, 2021

fixed min one system pool error.

d3ae03e

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 18, 2021

Merge branch 'master' of https://github.com/kubernetes-sigs/cluster-a…

58b57c9

…pi-provider-azure into aad-aks-support

k8s-ci-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jun 20, 2021

LochanRn mentioned this pull request Jun 25, 2021

added support for system node pools in managedClusters #1475

Closed

3 tasks

LochanRn closed this Jul 8, 2021

LochanRn mentioned this pull request Jul 13, 2021

Added support for system node pools in managedClusters #1520

Merged

3 tasks

LochanRn deleted the aad-aks-support branch July 25, 2021 12:13

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

update the container-service sdk from 2020-02-01 to 2021-03-01 #1416

update the container-service sdk from 2020-02-01 to 2021-03-01 #1416

LochanRn commented Jun 7, 2021 •

edited

Loading

k8s-ci-robot commented Jun 7, 2021

k8s-ci-robot commented Jun 7, 2021

LochanRn commented Jun 7, 2021

devigned left a comment

devigned Jun 7, 2021

alexeldeib Jun 7, 2021

devigned Jun 7, 2021

devigned commented Jun 7, 2021

CecileRobertMichon commented Jun 7, 2021

LochanRn commented Jun 7, 2021

CecileRobertMichon commented Jun 7, 2021

CecileRobertMichon commented Jun 14, 2021

alexeldeib commented Jun 14, 2021 •

edited

Loading

k8s-ci-robot commented Jun 18, 2021

k8s-ci-robot commented Jun 20, 2021

LochanRn commented Jul 8, 2021

update the container-service sdk from 2020-02-01 to 2021-03-01 #1416

update the container-service sdk from 2020-02-01 to 2021-03-01 #1416

Conversation

LochanRn commented Jun 7, 2021 • edited Loading

k8s-ci-robot commented Jun 7, 2021

k8s-ci-robot commented Jun 7, 2021

LochanRn commented Jun 7, 2021

devigned left a comment

Choose a reason for hiding this comment

devigned Jun 7, 2021

Choose a reason for hiding this comment

alexeldeib Jun 7, 2021

Choose a reason for hiding this comment

devigned Jun 7, 2021

Choose a reason for hiding this comment

devigned commented Jun 7, 2021

CecileRobertMichon commented Jun 7, 2021

LochanRn commented Jun 7, 2021

CecileRobertMichon commented Jun 7, 2021

CecileRobertMichon commented Jun 14, 2021

alexeldeib commented Jun 14, 2021 • edited Loading

k8s-ci-robot commented Jun 18, 2021

k8s-ci-robot commented Jun 20, 2021

LochanRn commented Jul 8, 2021

LochanRn commented Jun 7, 2021 •

edited

Loading

alexeldeib commented Jun 14, 2021 •

edited

Loading