Closed alexeldeib closed 4 years ago
Hi @alexeldeib I would like to take this one assuming that's cool :)
Got a couple questions for ya-- what action should be taken if the cluster is in a non-terminal mode? Any advice on testing this one? Should I create unit tests for the parent functions inside the agentpools and managedclusters packages?
Go for it! 👍
We don't want to send any additional CreateOrUpdate calls if the cluster is already in a non-terminal state.
For testing, we could create some mocks and then test the behavior of the logic around the actual service calls -- e.g. if we do a get, we can mock returning a cluster in "Provisioning" state, and we should expect no calls to CreateorUpdate.
Picking disks as a random example for mocks:
/remove-help
@alexeldeib Hi Alex, hope you're having a good weekend! I'm facing an interesting issue, hoping I can take your advice on how I should approach this one :)
I am building my unit tests, but am getting an assignment error on a mock function response. In the agentPools unit test, I am mocking the Get()
call to respond with an AgentPool struct. However, mock (I am using v1.4.3 latest) is throwing the following error:
agentpools_test.go:61: wrong type of argument 0 to Return for *mock_agentpools.MockClient.Get: containerservice.AgentPool is not assignable to containerservice.AgentPool
I have taken a couple approaches so far:
Get()
mock to the 'groups' unit test, specifically TestDeleteGroups where line 311 is returning resources.Group
struct in their mockThanks again for the help, and let me know if there is another approach to debugging this that I am missing :)
The error is probably caused by one import being different, but having the same type name. This is an error that is often found when using a different api version for an Azure SDK for Go type.
Nice progress! David beat me to it, but:
And
Need to match.
(Btw, I usually go by Ace :) )
Oops, sorry Ace! Thanks for the help guys, that solved my issue :sweat_smile:
Hi guys, any advice on which issue I should tackle next?
@jroden, love the enthusiasm!! Come over and join us in https://slack.k8s.io #cluster-api-azure. It's easier having these kind of conversations in chat.
If that doesn't work out for you, then see if any of these are interesting. Thank you!
/kind bug
Describe the solution you'd like https://github.com/kubernetes-sigs/cluster-api-provider-azure/blob/4b049939e18f0ba735735c41ac3fa9f828cd5236/cloud/services/managedclusters/managedclusters.go#L181
This code unconditionally sends the cluster to Azure for updates. We need to do a Get() first on the cluster and ensure the provisioning state is in a terminal mode (succeeded, failed, etc -- NOT updating, deleting, or otherwise in progress).
The field is ProvisioningState in this struct from the response: https://godoc.org/github.com/Azure/azure-sdk-for-go/services/containerservice/mgmt/2019-10-01/containerservice#ManagedClusterProperties
agentpools client does the get(), but doesn't check provisioning state: https://github.com/kubernetes-sigs/cluster-api-provider-azure/blob/4b049939e18f0ba735735c41ac3fa9f828cd5236/cloud/services/agentpools/agentpools.go#L67-L70
cc @serbrech
/help-wanted /good-first-issue