Skip to content

Commit 895b126

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request #3314 from k8s-infra-cherrypick-robot/cherry-pick-3256-to-release-1.8
[release-1.8] Fix delete for VMSS flex
2 parents c6d83a6 + 3fad254 commit 895b126

File tree

4 files changed

+56
-20
lines changed

4 files changed

+56
-20
lines changed

‎azure/services/scalesetvms/scalesetvms.go‎

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -151,11 +151,7 @@ func (s *Service) deleteVMSSFlexVM(ctx context.Context, resourceID string) error
151151
if err != nil {
152152
return errors.Wrap(err, fmt.Sprintf("failed to parse resource id %q", resourceID))
153153
}
154-
155-
resourceGroup := parsed.ResourceGroup
156-
resourceName := strings.TrimPrefix(s.Scope.ProviderID(), azure.ProviderIDPrefix)
157-
resourceNameSplits := strings.Split(resourceName, "/")
158-
resourceName = resourceNameSplits[len(resourceNameSplits)-3] + "_" + resourceNameSplits[len(resourceNameSplits)-1]
154+
resourceGroup, resourceName := parsed.ResourceGroup, parsed.ResourceName
159155

160156
log.V(4).Info("entering delete")
161157
future := s.Scope.GetLongRunningOperationState(resourceName, serviceName, infrav1.DeleteFuture)

‎azure/services/scalesetvms/scalesetvms_test.go‎

Lines changed: 54 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ package scalesetvms
1818

1919
import (
2020
"context"
21+
"fmt"
2122
"net/http"
2223
"testing"
2324
"time"
@@ -35,6 +36,7 @@ import (
3536
"sigs.k8s.io/cluster-api-provider-azure/azure/converters"
3637
"sigs.k8s.io/cluster-api-provider-azure/azure/scope"
3738
"sigs.k8s.io/cluster-api-provider-azure/azure/services/scalesetvms/mock_scalesetvms"
39+
"sigs.k8s.io/cluster-api-provider-azure/azure/services/virtualmachines/mock_virtualmachines"
3840
infrav1exp "sigs.k8s.io/cluster-api-provider-azure/exp/api/v1beta1"
3941
gomock2 "sigs.k8s.io/cluster-api-provider-azure/internal/test/matchers/gomock"
4042
clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
@@ -172,17 +174,18 @@ func TestService_Reconcile(t *testing.T) {
172174
func TestService_Delete(t *testing.T) {
173175
cases := []struct {
174176
Name string
175-
Setup func(s *mock_scalesetvms.MockScaleSetVMScopeMockRecorder, m *mock_scalesetvms.MockclientMockRecorder)
177+
Setup func(s *mock_scalesetvms.MockScaleSetVMScopeMockRecorder, m *mock_scalesetvms.MockclientMockRecorder, v *mock_virtualmachines.MockClientMockRecorder)
176178
Err error
177179
CheckIsErr bool
178180
}{
179181
{
180182
Name: "should start deleting successfully if no long running operation is active",
181-
Setup: func(s *mock_scalesetvms.MockScaleSetVMScopeMockRecorder, m *mock_scalesetvms.MockclientMockRecorder) {
183+
Setup: func(s *mock_scalesetvms.MockScaleSetVMScopeMockRecorder, m *mock_scalesetvms.MockclientMockRecorder, v *mock_virtualmachines.MockClientMockRecorder) {
182184
s.ResourceGroup().Return("rg")
183185
s.InstanceID().Return("0")
184186
s.ProviderID().Return("foo")
185187
s.ScaleSetName().Return("scaleset")
188+
s.OrchestrationMode().Return(infrav1.UniformOrchestrationMode)
186189
s.GetLongRunningOperationState("0", serviceName, infrav1.DeleteFuture).Return(nil)
187190
future := &infrav1.Future{
188191
Type: infrav1.DeleteFuture,
@@ -199,11 +202,12 @@ func TestService_Delete(t *testing.T) {
199202
},
200203
{
201204
Name: "should finish deleting successfully when there's a long running operation that has completed",
202-
Setup: func(s *mock_scalesetvms.MockScaleSetVMScopeMockRecorder, m *mock_scalesetvms.MockclientMockRecorder) {
205+
Setup: func(s *mock_scalesetvms.MockScaleSetVMScopeMockRecorder, m *mock_scalesetvms.MockclientMockRecorder, v *mock_virtualmachines.MockClientMockRecorder) {
203206
s.ResourceGroup().Return("rg")
204207
s.InstanceID().Return("0")
205208
s.ProviderID().Return("foo")
206209
s.ScaleSetName().Return("scaleset")
210+
s.OrchestrationMode().Return(infrav1.UniformOrchestrationMode)
207211
future := &infrav1.Future{
208212
Type: infrav1.DeleteFuture,
209213
}
@@ -215,23 +219,25 @@ func TestService_Delete(t *testing.T) {
215219
},
216220
{
217221
Name: "should not error when deleting, but resource is 404",
218-
Setup: func(s *mock_scalesetvms.MockScaleSetVMScopeMockRecorder, m *mock_scalesetvms.MockclientMockRecorder) {
222+
Setup: func(s *mock_scalesetvms.MockScaleSetVMScopeMockRecorder, m *mock_scalesetvms.MockclientMockRecorder, v *mock_virtualmachines.MockClientMockRecorder) {
219223
s.ResourceGroup().Return("rg")
220224
s.InstanceID().Return("0")
221225
s.ProviderID().Return("foo")
222226
s.ScaleSetName().Return("scaleset")
227+
s.OrchestrationMode().Return(infrav1.UniformOrchestrationMode)
223228
s.GetLongRunningOperationState("0", serviceName, infrav1.DeleteFuture).Return(nil)
224229
m.DeleteAsync(gomock2.AContext(), "rg", "scaleset", "0").Return(nil, autorest404)
225230
m.Get(gomock2.AContext(), "rg", "scaleset", "0").Return(compute.VirtualMachineScaleSetVM{}, nil)
226231
},
227232
},
228233
{
229234
Name: "should error when deleting, but a non-404 error is returned from DELETE call",
230-
Setup: func(s *mock_scalesetvms.MockScaleSetVMScopeMockRecorder, m *mock_scalesetvms.MockclientMockRecorder) {
235+
Setup: func(s *mock_scalesetvms.MockScaleSetVMScopeMockRecorder, m *mock_scalesetvms.MockclientMockRecorder, v *mock_virtualmachines.MockClientMockRecorder) {
231236
s.ResourceGroup().Return("rg")
232237
s.InstanceID().Return("0")
233238
s.ProviderID().Return("foo")
234239
s.ScaleSetName().Return("scaleset")
240+
s.OrchestrationMode().Return(infrav1.UniformOrchestrationMode)
235241
s.GetLongRunningOperationState("0", serviceName, infrav1.DeleteFuture).Return(nil)
236242
m.DeleteAsync(gomock2.AContext(), "rg", "scaleset", "0").Return(nil, errors.New("boom"))
237243
m.Get(gomock2.AContext(), "rg", "scaleset", "0").Return(compute.VirtualMachineScaleSetVM{}, nil)
@@ -240,11 +246,12 @@ func TestService_Delete(t *testing.T) {
240246
},
241247
{
242248
Name: "should return error when a long running operation is active and getting the result returns an error",
243-
Setup: func(s *mock_scalesetvms.MockScaleSetVMScopeMockRecorder, m *mock_scalesetvms.MockclientMockRecorder) {
249+
Setup: func(s *mock_scalesetvms.MockScaleSetVMScopeMockRecorder, m *mock_scalesetvms.MockclientMockRecorder, v *mock_virtualmachines.MockClientMockRecorder) {
244250
s.ResourceGroup().Return("rg")
245251
s.InstanceID().Return("0")
246252
s.ProviderID().Return("foo")
247253
s.ScaleSetName().Return("scaleset")
254+
s.OrchestrationMode().Return(infrav1.UniformOrchestrationMode)
248255
future := &infrav1.Future{
249256
Type: infrav1.DeleteFuture,
250257
}
@@ -254,26 +261,61 @@ func TestService_Delete(t *testing.T) {
254261
},
255262
Err: errors.Wrap(errors.New("boom"), "failed to get result of long running operation"),
256263
},
264+
{
265+
Name: "(flex) should delete successfully if no long-running operation is active",
266+
Setup: func(s *mock_scalesetvms.MockScaleSetVMScopeMockRecorder, m *mock_scalesetvms.MockclientMockRecorder, v *mock_virtualmachines.MockClientMockRecorder) {
267+
s.ResourceGroup().Return("my-cluster")
268+
s.ScaleSetName().Return("scaleset")
269+
s.InstanceID().Return("0")
270+
s.ProviderID().Return("azure:///subscriptions/1234-5678/resourceGroups/my-cluster/providers/Microsoft.Compute/virtualMachines/my-cluster_1234abcd")
271+
s.OrchestrationMode().Return(infrav1.FlexibleOrchestrationMode)
272+
s.GetLongRunningOperationState("my-cluster_1234abcd", serviceName, infrav1.DeleteFuture).Return(nil)
273+
vmGetter := &VMSSFlexVMGetter{
274+
Name: "my-cluster_1234abcd",
275+
ResourceGroup: "my-cluster",
276+
}
277+
future := &infrav1.Future{
278+
Type: infrav1.DeleteFuture,
279+
}
280+
sdkFuture, _ := converters.FutureToSDK(*future)
281+
v.DeleteAsync(gomock2.AContext(), vmGetter).Return(sdkFuture, nil)
282+
s.DeleteLongRunningOperationState("my-cluster_1234abcd", serviceName, infrav1.DeleteFuture)
283+
v.GetByID(gomock2.AContext(), "/subscriptions/1234-5678/resourceGroups/my-cluster/providers/Microsoft.Compute/virtualMachines/my-cluster_1234abcd").Return(compute.VirtualMachine{}, nil)
284+
},
285+
},
286+
{
287+
Name: "(flex) should error if providerID is invalid",
288+
Setup: func(s *mock_scalesetvms.MockScaleSetVMScopeMockRecorder, m *mock_scalesetvms.MockclientMockRecorder, v *mock_virtualmachines.MockClientMockRecorder) {
289+
s.ResourceGroup().Return("my-cluster")
290+
s.ScaleSetName().Return("scaleset")
291+
s.InstanceID().Return("0")
292+
s.ProviderID().Return("foo")
293+
s.OrchestrationMode().Return(infrav1.FlexibleOrchestrationMode)
294+
v.GetByID(gomock2.AContext(), "foo").Return(compute.VirtualMachine{}, nil)
295+
},
296+
Err: errors.Wrap(fmt.Errorf("parsing failed for %s. Invalid resource Id format", "foo"), fmt.Sprintf("failed to parse resource id %q", "foo")),
297+
},
257298
}
258299

259300
for _, c := range cases {
260301
t.Run(c.Name, func(t *testing.T) {
261302
var (
262-
g = NewWithT(t)
263-
mockCtrl = gomock.NewController(t)
264-
scopeMock = mock_scalesetvms.NewMockScaleSetVMScope(mockCtrl)
265-
clientMock = mock_scalesetvms.NewMockclient(mockCtrl)
303+
g = NewWithT(t)
304+
mockCtrl = gomock.NewController(t)
305+
scopeMock = mock_scalesetvms.NewMockScaleSetVMScope(mockCtrl)
306+
clientMock = mock_scalesetvms.NewMockclient(mockCtrl)
307+
vmClientMock = mock_virtualmachines.NewMockClient(mockCtrl)
266308
)
267309
defer mockCtrl.Finish()
268310

269311
scopeMock.EXPECT().SubscriptionID().Return("subID").AnyTimes()
270312
scopeMock.EXPECT().BaseURI().Return("https://localhost/").AnyTimes()
271313
scopeMock.EXPECT().Authorizer().Return(nil).AnyTimes()
272-
scopeMock.EXPECT().OrchestrationMode().Return(infrav1.UniformOrchestrationMode).AnyTimes()
273314

274315
service := NewService(scopeMock)
275316
service.Client = clientMock
276-
c.Setup(scopeMock.EXPECT(), clientMock.EXPECT())
317+
service.VMClient = vmClientMock
318+
c.Setup(scopeMock.EXPECT(), clientMock.EXPECT(), vmClientMock.EXPECT())
277319

278320
if err := service.Delete(context.TODO()); c.Err == nil {
279321
g.Expect(err).To(Succeed())

‎test/e2e/azure_machinepools.go‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,6 @@ func AzureMachinePoolsSpec(ctx context.Context, inputGetter func() AzureMachineP
117117
}
118118
wg.Wait()
119119

120-
/* TODO: uncomment with scale down fix for flexible mode
121120
for _, mp := range machinepools {
122121
goalReplicas := pointer.Int32Deref(mp.Spec.Replicas, 0) - 1
123122
Byf("Scaling machine pool %s in from %d to %d", mp.Name, *mp.Spec.Replicas, goalReplicas)
@@ -135,7 +134,6 @@ func AzureMachinePoolsSpec(ctx context.Context, inputGetter func() AzureMachineP
135134
}(mp)
136135
}
137136
wg.Wait()
138-
*/
139137

140138
By("verifying that workload nodes are schedulable")
141139
clientset := workloadClusterProxy.GetClientSet()

‎test/e2e/azure_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -532,7 +532,7 @@ var _ = Describe("Workload cluster creation", func() {
532532
withControlPlaneInterval(specName, "wait-control-plane"),
533533
), result)
534534

535-
By("Verifying machinepool can scale out", func() {
535+
By("Verifying machinepool can scale out and in", func() {
536536
AzureMachinePoolsSpec(ctx, func() AzureMachinePoolsSpecInput {
537537
return AzureMachinePoolsSpecInput{
538538
Cluster: result.Cluster,

0 commit comments

Comments
 (0)