Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

While updating agentPools need to check for only fields which are mutable. #1536

Closed
LochanRn opened this issue Jul 17, 2021 · 3 comments
Closed
Assignees
Labels
area/managedclusters Issues related to managed AKS clusters created through the CAPZ ManagedCluster Type kind/bug Categorizes issue or PR as related to a bug.

Comments

@LochanRn
Copy link
Member

LochanRn commented Jul 17, 2021

/kind bug
/area managedclusters
What steps did you take and what happened:
[A clear and concise description of what the bug is.]
When an update is triggered for agentpools the agentpool service compares the exiting agentPool and the updated agentPool
before it actually makes a CreateOrUpdate api call.

existingProfile := containerservice.AgentPool{
ManagedClusterAgentPoolProfileProperties: &containerservice.ManagedClusterAgentPoolProfileProperties{
VMSize: existingPool.ManagedClusterAgentPoolProfileProperties.VMSize,
OsType: containerservice.Linux,
OsDiskSizeGB: existingPool.ManagedClusterAgentPoolProfileProperties.OsDiskSizeGB,
Count: existingPool.ManagedClusterAgentPoolProfileProperties.Count,
Type: containerservice.VirtualMachineScaleSets,
OrchestratorVersion: existingPool.ManagedClusterAgentPoolProfileProperties.OrchestratorVersion,
VnetSubnetID: existingPool.ManagedClusterAgentPoolProfileProperties.VnetSubnetID,
},
}
// Diff and check if we require an update
diff := cmp.Diff(profile, existingProfile)

The current logic checks for the all the fields while comparison, most of the fields are immutable and should be avoided.

The fields for which AKS supports mutating are

  • k8sVersion
  • count
  • mode

What did you expect to happen:
The right way to implement it would be:

existingProfile := containerservice.AgentPool{
	ManagedClusterAgentPoolProfileProperties: &containerservice.ManagedClusterAgentPoolProfileProperties{
		Count:               existingPool.Count,
		OrchestratorVersion: existingPool.OrchestratorVersion,
		Mode:                existingPool.Mode,
	},
}

normalizedProfile := containerservice.AgentPool{
	ManagedClusterAgentPoolProfileProperties: &containerservice.ManagedClusterAgentPoolProfileProperties{
		Count:               profile.Count,
		OrchestratorVersion: profile.OrchestratorVersion,
		Mode:                profile.Mode,
	},
}

// Diff and check if we require an update
diff := cmp.Diff(existingProfile, normalizedProfile)

Anything else you would like to add:
[Miscellaneous information that will assist in solving the issue.]

Environment:

  • cluster-api-provider-azure version:
  • Kubernetes version: (use kubectl version):
  • OS (e.g. from /etc/os-release):
@k8s-ci-robot k8s-ci-robot added the kind/bug Categorizes issue or PR as related to a bug. label Jul 17, 2021
@CecileRobertMichon
Copy link
Contributor

@LochanRn are you taking this one or should we mark it as good-first-issue?

/area managedclusters

@k8s-ci-robot k8s-ci-robot added the area/managedclusters Issues related to managed AKS clusters created through the CAPZ ManagedCluster Type label Jul 19, 2021
@LochanRn
Copy link
Member Author

LochanRn commented Jul 19, 2021

I have added a fix for this in the system-node-pool PR.

@CecileRobertMichon
Copy link
Contributor

/assign @LochanRn

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/managedclusters Issues related to managed AKS clusters created through the CAPZ ManagedCluster Type kind/bug Categorizes issue or PR as related to a bug.
Projects
None yet
Development

No branches or pull requests

3 participants