Repository: Azure/ARO-RP
Pull request #2788 (2023-03-21 17:13:41)

fixes wrong return value for retry in vmss cleaner in deployer
### Which issue this PR addresses:
fixes the wrong return value in the cleaner which prevented the deployer to redeploy the vmss
<!--
Please include a link to the ADO work item as well as any GitHub issues.

Usage: `Fixes #<GitHub issue number>`, or `Fixes (paste link of issue)`.
-->
Fixes

### What this PR does / why we need it:

<!--
Include a brief summary of what the PR is intended to accomplish and how the PR
does it. (2-3 sentences)
-->

### Test plan for issue:

<!--
How did you test that this PR works?

- Are there unit tests?
- Are there integration/e2e tests?
- If it is not possible to write automated tests, explain why and document how
  to manually test and verify the feature.
-->

### Is there any documentation that needs to be updated for this PR?

<!--
- If yes and the docs are in GitHub, include doc updates in the PR.
- If yes and the docs are not in GitHub (i.e. ADO wiki), include a link to the
  docs.
- If no, explain why (e.g. "tech debt cleanup, N/A").
-->



Patch:
diff --git a/pkg/deploy/vmsscleaner/clean.go b/pkg/deploy/vmsscleaner/clean.go
index dc9c76434..fc8a47e00 100644
--- a/pkg/deploy/vmsscleaner/clean.go
+++ b/pkg/deploy/vmsscleaner/clean.go
@@ -85,9 +85,8 @@ func (c *cleaner) UpdateVMSSProbes(ctx context.Context, rgName string) (retry bo
 		err = c.vmss.CreateOrUpdateAndWait(ctx, rgName, name, vmss)
 		if err != nil {
 			c.log.Warn(err)
-			return false // If update failed, gateway vmss still exists. Don't retry.
+			return false // If update failed, gateway vmss still has a reference to the probe
 		}
 	}
-	// no scaleset matched, so we should not retry and return the error
-	return false
+	return true
 }


Test patch:
diff --git a/pkg/deploy/vmsscleaner/clean_test.go b/pkg/deploy/vmsscleaner/clean_test.go
index 4c92348fe..a131aaaf4 100644
--- a/pkg/deploy/vmsscleaner/clean_test.go
+++ b/pkg/deploy/vmsscleaner/clean_test.go
@@ -6,6 +6,7 @@ package vmsscleaner
 import (
 	"context"
 	"errors"
+	"io"
 	"testing"
 
 	mgmtcompute "github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2020-06-01/compute"
@@ -128,3 +129,81 @@ func TestRemoveFailedScaleset(t *testing.T) {
 		})
 	}
 }
+
+func TestUpdateProbe(t *testing.T) {
+	var tests = []struct {
+		name              string
+		expected          bool
+		listErr           error
+		createOrUpdateErr error
+		listReturn        []mgmtcompute.VirtualMachineScaleSet
+	}{
+		{
+			name:     "list error",
+			expected: false,
+			listErr:  errors.New("error"),
+		},
+		{
+			name:              "update error",
+			createOrUpdateErr: errors.New("error"),
+			expected:          false,
+			listReturn: []mgmtcompute.VirtualMachineScaleSet{
+				{
+					Name: to.StringPtr("gateway-vmss-redhat"),
+					VirtualMachineScaleSetProperties: &mgmtcompute.VirtualMachineScaleSetProperties{
+						VirtualMachineProfile: &mgmtcompute.VirtualMachineScaleSetVMProfile{
+							NetworkProfile: &mgmtcompute.VirtualMachineScaleSetNetworkProfile{},
+						},
+					},
+				},
+			},
+		},
+		{
+			name:     "success",
+			expected: true,
+			listReturn: []mgmtcompute.VirtualMachineScaleSet{
+				{
+					Name: to.StringPtr("gateway-vmss-redhat"),
+					VirtualMachineScaleSetProperties: &mgmtcompute.VirtualMachineScaleSetProperties{
+						VirtualMachineProfile: &mgmtcompute.VirtualMachineScaleSetVMProfile{
+							NetworkProfile: &mgmtcompute.VirtualMachineScaleSetNetworkProfile{},
+						},
+					},
+				},
+			},
+		},
+		{
+			name:     "not gateway",
+			expected: true,
+			listReturn: []mgmtcompute.VirtualMachineScaleSet{
+				{
+					Name: to.StringPtr("spencer's-vmss"),
+				},
+			},
+		},
+	}
+	for _, tt := range tests {
+		tt := tt
+		t.Run(tt.name, func(t *testing.T) {
+			controller := gomock.NewController(t)
+			defer controller.Finish()
+
+			mockVMSS := mock_compute.NewMockVirtualMachineScaleSetsClient(controller)
+			mockVMSS.EXPECT().List(gomock.Any(), gomock.Any()).AnyTimes().Return(tt.listReturn, tt.listErr)
+			mockVMSS.EXPECT().CreateOrUpdateAndWait(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).AnyTimes().Return(tt.createOrUpdateErr)
+
+			logger := logrus.Logger{}
+			logger.Out = io.Discard
+			c := cleaner{
+				log:  logrus.NewEntry(&logger),
+				vmss: mockVMSS,
+			}
+			ctx := context.Background()
+			rg := "someid"
+			retry := c.UpdateVMSSProbes(ctx, rg)
+			if retry != tt.expected {
+				t.Error(retry)
+			}
+		})
+	}
+}