Correct testing/time issues in pkg/deploy (#3808)

- Percolate up the time to wait for LB healthcheck probes, test @ 0 sec
- Correct a context timeout test case, test @ 0 sec timeout

cmd/aro/deploy.go

@@ -87,7 +87,7 @@ func deploy(ctx context.Context, log *logrus.Entry) error {
 		return err
 	}
 
-	err = deployer.PreDeploy(ctx)
+	err = deployer.PreDeploy(ctx, 30)
 	if err != nil {
 		return err
 	}

pkg/deploy/deploy.go

@@ -31,7 +31,7 @@ import (
 var _ Deployer = (*deployer)(nil)
 
 type Deployer interface {
-	PreDeploy(context.Context) error
+	PreDeploy(context.Context, int) error
 	DeployRP(context.Context) error
 	DeployGateway(context.Context) error
 	UpgradeRP(context.Context) error

pkg/deploy/predeploy.go

@@ -40,7 +40,7 @@ const (
 
 // PreDeploy deploys managed identity, NSGs and keyvaults, needed for main
 // deployment
-func (d *deployer) PreDeploy(ctx context.Context) error {
+func (d *deployer) PreDeploy(ctx context.Context, lbHealthcheckWaitTimeSec int) error {
 	// deploy global rbac
 	err := d.deployRPGlobalSubscription(ctx)
 	if err != nil {
@@ -154,7 +154,7 @@ func (d *deployer) PreDeploy(ctx context.Context) error {
 		return err
 	}
 
-	return d.configureServiceSecrets(ctx)
+	return d.configureServiceSecrets(ctx, lbHealthcheckWaitTimeSec)
 }
 
 func (d *deployer) deployRPGlobal(ctx context.Context, rpServicePrincipalID, gatewayServicePrincipalID string) error {
@@ -359,7 +359,7 @@ func (d *deployer) deployPreDeploy(ctx context.Context, resourceGroupName, deplo
 	})
 }
 
-func (d *deployer) configureServiceSecrets(ctx context.Context) error {
+func (d *deployer) configureServiceSecrets(ctx context.Context, lbHealthcheckWaitTimeSec int) error {
 	isRotated := false
 	for _, s := range []struct {
 		kv         keyvault.Manager
@@ -400,7 +400,7 @@ func (d *deployer) configureServiceSecrets(ctx context.Context) error {
 	}
 
 	if isRotated {
-		err = d.restartOldScalesets(ctx)
+		err = d.restartOldScalesets(ctx, lbHealthcheckWaitTimeSec)
 		if err != nil {
 			return err
 		}
@@ -485,14 +485,14 @@ func (d *deployer) ensureSecretKey(ctx context.Context, kv keyvault.Manager, sec
 	})
 }
 
-func (d *deployer) restartOldScalesets(ctx context.Context) error {
+func (d *deployer) restartOldScalesets(ctx context.Context, lbHealthcheckWaitTimeSec int) error {
 	scalesets, err := d.vmss.List(ctx, d.config.RPResourceGroupName)
 	if err != nil {
 		return err
 	}
 
 	for _, vmss := range scalesets {
-		err = d.restartOldScaleset(ctx, *vmss.Name)
+		err = d.restartOldScaleset(ctx, *vmss.Name, lbHealthcheckWaitTimeSec)
 		if err != nil {
 			return err
 		}
@@ -501,7 +501,7 @@ func (d *deployer) restartOldScalesets(ctx context.Context) error {
 	return nil
 }
 
-func (d *deployer) restartOldScaleset(ctx context.Context, vmssName string) error {
+func (d *deployer) restartOldScaleset(ctx context.Context, vmssName string, lbHealthcheckWaitTimeSec int) error {
 	if !strings.HasPrefix(vmssName, rpVMSSPrefix) {
 		return &api.CloudError{
 			StatusCode: http.StatusBadRequest,
@@ -531,7 +531,7 @@ func (d *deployer) restartOldScaleset(ctx context.Context, vmssName string) erro
 		}
 
 		// wait for load balancer probe to change the vm health status
-		time.Sleep(30 * time.Second)
+		time.Sleep(time.Duration(lbHealthcheckWaitTimeSec) * time.Second)
 		timeoutCtx, cancel := context.WithTimeout(ctx, time.Hour)
 		defer cancel()
 		err = d.waitForReadiness(timeoutCtx, vmssName, *vm.InstanceID)

pkg/deploy/predeploy_test.go

@@ -494,7 +494,7 @@ func TestPreDeploy(t *testing.T) {
 				m(mockDeployments, mockResourceGroups, mockMSIs, mockKV, mockVMSS, mockVMSSVM, tt.testParams)
 			}
 
-			err := d.PreDeploy(ctx)
+			err := d.PreDeploy(ctx, 0)
 			utilerror.AssertErrorMessage(t, err, tt.wantErr)
 		})
 	}
@@ -1074,7 +1074,7 @@ func TestConfigureServiceSecrets(t *testing.T) {
 				m(mockKV, mockVMSS, mockVMSSVM, tt.testParams)
 			}
 
-			err := d.configureServiceSecrets(ctx)
+			err := d.configureServiceSecrets(ctx, 0)
 			utilerror.AssertErrorMessage(t, err, tt.wantErr)
 		})
 	}
@@ -1470,7 +1470,7 @@ func TestRestartOldScalesets(t *testing.T) {
 				m(mockVMSS, mockVMSSVM, tt.testParams)
 			}
 
-			err := d.restartOldScalesets(ctx)
+			err := d.restartOldScalesets(ctx, 0)
 			utilerror.AssertErrorMessage(t, err, tt.wantErr)
 		})
 	}
@@ -1564,14 +1564,15 @@ func TestRestartOldScaleset(t *testing.T) {
 				m(mockVMSS, tt.testParams)
 			}
 
-			err := d.restartOldScaleset(ctx, tt.testParams.vmssName)
+			err := d.restartOldScaleset(ctx, tt.testParams.vmssName, 0)
 			utilerror.AssertErrorMessage(t, err, tt.wantErr)
 		})
 	}
 }
 
 func TestWaitForReadiness(t *testing.T) {
-	ctxTimeout, cancel := context.WithTimeout(context.Background(), 11*time.Second)
+	ctxRegular, cancelRegular := context.WithTimeout(context.Background(), 11*time.Second)
+	ctxFastTimeout, cancelFastTimeout := context.WithTimeout(context.Background(), 0*time.Second)
 
 	type testParams struct {
 		resourceGroup string
@@ -1599,7 +1600,8 @@ func TestWaitForReadiness(t *testing.T) {
 				resourceGroup: rgName,
 				vmssName:      vmssName,
 				vmInstanceID:  instanceID,
-				ctx:           ctxTimeout,
+				ctx:           ctxFastTimeout,
+				cancel:        cancelFastTimeout,
 			},
 			mocks:   []mock{getInstanceViewMock(unhealthyVMSS)},
 			wantErr: "timed out waiting for the condition",
@@ -1610,8 +1612,8 @@ func TestWaitForReadiness(t *testing.T) {
 				resourceGroup: rgName,
 				vmssName:      vmssName,
 				vmInstanceID:  instanceID,
-				ctx:           ctxTimeout,
-				cancel:        cancel,
+				ctx:           ctxRegular,
+				cancel:        cancelRegular,
 			},
 			mocks: []mock{getInstanceViewMock(healthyVMSS)},
 		},
@@ -1634,7 +1636,7 @@ func TestWaitForReadiness(t *testing.T) {
 				m(mockVMSS, tt.testParams)
 			}
 
-			defer cancel()
+			defer tt.testParams.cancel()
 			err := d.waitForReadiness(tt.testParams.ctx, tt.testParams.vmssName, tt.testParams.vmInstanceID)
 			utilerror.AssertErrorMessage(t, err, tt.wantErr)
 		})