Skip to content

Commit edb9142

Browse files
authored
Merge pull request #689 from jroden/master
[managedcluster] should check provisioning status before attempting changes
2 parents a7b849c + 40acdc3 commit edb9142

File tree

8 files changed

+551
-5
lines changed

8 files changed

+551
-5
lines changed

cloud/services/agentpools/agentpools.go

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -68,10 +68,6 @@ func (s *Service) Reconcile(ctx context.Context, spec interface{}) error {
6868
if err != nil && !azure.ResourceNotFound(err) {
6969
return errors.Wrapf(err, "failed to get existing agent pool")
7070
}
71-
existingPool, ok := existingSpec.(containerservice.AgentPool)
72-
if !ok {
73-
return errors.New("expected agent pool specification")
74-
}
7571

7672
// For updates, we want to pass whatever we find in the existing
7773
// cluster, normalized to reflect the input we originally provided.
@@ -84,6 +80,16 @@ func (s *Service) Reconcile(ctx context.Context, spec interface{}) error {
8480
return fmt.Errorf("failed to create or update agent pool, %#+v", err)
8581
}
8682
} else {
83+
existingPool, ok := existingSpec.(containerservice.AgentPool)
84+
if !ok {
85+
return errors.New("expected agent pool specification")
86+
}
87+
ps := *existingPool.ManagedClusterAgentPoolProfileProperties.ProvisioningState
88+
if ps != "Canceled" && ps != "Failed" && ps != "Succeeded" {
89+
klog.V(2).Infof("Unable to update existing agent pool in non terminal state. Agent pool must be in one of the following provisioning states: canceled, failed, or succeeded")
90+
return nil
91+
}
92+
8793
// Normalize individual agent pools to diff in case we need to update
8894
existingProfile := containerservice.AgentPool{
8995
ManagedClusterAgentPoolProfileProperties: &containerservice.ManagedClusterAgentPoolProfileProperties{
Lines changed: 142 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,142 @@
1+
/*
2+
Copyright 2020 The Kubernetes Authors.
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
package agentpools
18+
19+
import (
20+
"context"
21+
"net/http"
22+
"testing"
23+
24+
"github.com/Azure/azure-sdk-for-go/services/containerservice/mgmt/2020-02-01/containerservice"
25+
"github.com/Azure/go-autorest/autorest"
26+
"github.com/golang/mock/gomock"
27+
. "github.com/onsi/gomega"
28+
"sigs.k8s.io/cluster-api-provider-azure/cloud/services/agentpools/mock_agentpools"
29+
)
30+
31+
func TestReconcile(t *testing.T) {
32+
g := NewWithT(t)
33+
34+
provisioningstatetestcases := []struct {
35+
name string
36+
agentpoolspec Spec
37+
provisioningStatesToTest []string
38+
expectedError string
39+
expect func(m *mock_agentpools.MockClientMockRecorder, provisioningstate string)
40+
}{
41+
{
42+
name: "agentpool in terminal provisioning state",
43+
agentpoolspec: Spec{
44+
ResourceGroup: "my-rg",
45+
Cluster: "my-cluster",
46+
Name: "my-agentpool",
47+
},
48+
provisioningStatesToTest: []string{"Canceled", "Succeeded", "Failed"},
49+
expectedError: "",
50+
expect: func(m *mock_agentpools.MockClientMockRecorder, provisioningstate string) {
51+
m.CreateOrUpdate(context.TODO(), "my-rg", "my-cluster", "my-agentpool", gomock.Any()).Return(nil)
52+
m.Get(context.TODO(), "my-rg", "my-cluster", "my-agentpool").Return(containerservice.AgentPool{ManagedClusterAgentPoolProfileProperties: &containerservice.ManagedClusterAgentPoolProfileProperties{
53+
ProvisioningState: &provisioningstate,
54+
}}, nil)
55+
},
56+
},
57+
{
58+
name: "agentpool in nonterminal provisioning state",
59+
agentpoolspec: Spec{
60+
ResourceGroup: "my-rg",
61+
Cluster: "my-cluster",
62+
Name: "my-agentpool",
63+
},
64+
provisioningStatesToTest: []string{"Deleting", "InProgress", "randomStringHere"},
65+
expectedError: "",
66+
expect: func(m *mock_agentpools.MockClientMockRecorder, provisioningstate string) {
67+
m.Get(context.TODO(), "my-rg", "my-cluster", "my-agentpool").Return(containerservice.AgentPool{ManagedClusterAgentPoolProfileProperties: &containerservice.ManagedClusterAgentPoolProfileProperties{
68+
ProvisioningState: &provisioningstate,
69+
}}, nil)
70+
},
71+
},
72+
}
73+
74+
for _, tc := range provisioningstatetestcases {
75+
for _, provisioningstate := range tc.provisioningStatesToTest {
76+
t.Logf("Testing agentpool provision state: " + provisioningstate)
77+
t.Run(tc.name, func(t *testing.T) {
78+
mockCtrl := gomock.NewController(t)
79+
agentpoolsMock := mock_agentpools.NewMockClient(mockCtrl)
80+
81+
tc.expect(agentpoolsMock.EXPECT(), provisioningstate)
82+
83+
s := &Service{
84+
Client: agentpoolsMock,
85+
}
86+
87+
err := s.Reconcile(context.TODO(), &tc.agentpoolspec)
88+
if tc.expectedError != "" {
89+
g.Expect(err).To(HaveOccurred())
90+
g.Expect(err).To(MatchError(tc.expectedError))
91+
} else {
92+
g.Expect(err).NotTo(HaveOccurred())
93+
mockCtrl.Finish()
94+
}
95+
})
96+
}
97+
}
98+
99+
testcases := []struct {
100+
name string
101+
agentpoolspec Spec
102+
expectedError string
103+
expect func(m *mock_agentpools.MockClientMockRecorder)
104+
}{
105+
{
106+
name: "no agentpool exists",
107+
agentpoolspec: Spec{
108+
ResourceGroup: "my-rg",
109+
Cluster: "my-cluster",
110+
Name: "my-agentpool",
111+
},
112+
expectedError: "",
113+
expect: func(m *mock_agentpools.MockClientMockRecorder) {
114+
m.CreateOrUpdate(context.TODO(), "my-rg", "my-cluster", "my-agentpool", gomock.Any()).Return(nil)
115+
m.Get(context.TODO(), "my-rg", "my-cluster", "my-agentpool").Return(containerservice.AgentPool{}, autorest.NewErrorWithResponse("", "", &http.Response{StatusCode: 404}, "Not Found"))
116+
},
117+
},
118+
}
119+
120+
for _, tc := range testcases {
121+
t.Logf("Testing " + tc.name)
122+
t.Run(tc.name, func(t *testing.T) {
123+
mockCtrl := gomock.NewController(t)
124+
agentpoolsMock := mock_agentpools.NewMockClient(mockCtrl)
125+
126+
tc.expect(agentpoolsMock.EXPECT())
127+
128+
s := &Service{
129+
Client: agentpoolsMock,
130+
}
131+
132+
err := s.Reconcile(context.TODO(), &tc.agentpoolspec)
133+
if tc.expectedError != "" {
134+
g.Expect(err).To(HaveOccurred())
135+
g.Expect(err).To(MatchError(tc.expectedError))
136+
} else {
137+
g.Expect(err).NotTo(HaveOccurred())
138+
mockCtrl.Finish()
139+
}
140+
})
141+
}
142+
}

cloud/services/agentpools/mock_agentpools/agentpools_mock.go

Lines changed: 94 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.
Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
/*
2+
Copyright 2019 The Kubernetes Authors.
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
// Run go generate to regenerate this mock.
18+
//go:generate ../../../../hack/tools/bin/mockgen -destination agentpools_mock.go -package mock_agentpools -source ../client.go Client
19+
//go:generate /usr/bin/env bash -c "cat ../../../../hack/boilerplate/boilerplate.generatego.txt agentpools_mock.go > _agentpools_mock.go && mv _agentpools_mock.go agentpools_mock.go"
20+
package mock_agentpools //nolint

cloud/services/managedclusters/managedclusters.go

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -178,7 +178,22 @@ func (s *Service) Reconcile(ctx context.Context, spec interface{}) error {
178178
*properties.AgentPoolProfiles = append(*properties.AgentPoolProfiles, profile)
179179
}
180180

181-
err := s.Client.CreateOrUpdate(ctx, managedClusterSpec.ResourceGroup, managedClusterSpec.Name, properties)
181+
existingSpec, err := s.Get(ctx, spec)
182+
if err != nil && !azure.ResourceNotFound(err) {
183+
return errors.Wrapf(err, "failed to get existing managed cluster")
184+
} else if !azure.ResourceNotFound(err) {
185+
existingPool, ok := existingSpec.(containerservice.ManagedCluster)
186+
if !ok {
187+
return errors.New("expected managed cluster")
188+
}
189+
ps := *existingPool.ManagedClusterProperties.ProvisioningState
190+
if ps != "Canceled" && ps != "Failed" && ps != "Succeeded" {
191+
klog.V(2).Infof("Unable to update existing managed cluster in non terminal state. Managed cluster must be in one of the following provisioning states: canceled, failed, or succeeded")
192+
return nil
193+
}
194+
}
195+
196+
err = s.Client.CreateOrUpdate(ctx, managedClusterSpec.ResourceGroup, managedClusterSpec.Name, properties)
182197
if err != nil {
183198
return fmt.Errorf("failed to create or update managed cluster, %#+v", err)
184199
}

0 commit comments

Comments
 (0)