Merge pull request #37 from qiujian16/fix-typos

Followup of pr33

pkg/operators/klusterlet/controllers/klusterletcontroller/klusterlet_controller.go

@@ -253,7 +253,9 @@ func (n *klusterletController) sync(ctx context.Context, controllerContext facto
 	}
 
 	// Deploy work agent
-	n.forceRollOutWorkAgent(ctx, config, &forceRollOut)
+	if !forceRollOut {
+		forceRollOut = n.forceRollOutWorkAgent(ctx, config)
+	}
 	workGeneration, err := helpers.ApplyDeployment(
 		n.kubeClient,
 		klusterlet.Status.Generations,
@@ -288,31 +290,26 @@ func (n *klusterletController) sync(ctx context.Context, controllerContext facto
 
 // Check if cluster name is changed in hub kubeconfig secret by comparing with deployment args. Force update the deployment
 // if the secret is changed.
-func (n *klusterletController) forceRollOutWorkAgent(ctx context.Context, config klusterletConfig, forceRollOut *bool) {
-	if *forceRollOut {
-		return
-	}
-
+func (n *klusterletController) forceRollOutWorkAgent(ctx context.Context, config klusterletConfig) bool {
 	workDeploymentName := fmt.Sprintf("%s-work-agent", config.KlusterletName)
 	workDeployment, err := n.kubeClient.AppsV1().Deployments(config.KlusterletNamespace).Get(ctx, workDeploymentName, metav1.GetOptions{})
 	if err != nil {
-		return
+		// Do not force roll out if fails to get deployment, applyDeployment will handle this anyway
+		return false
 	}
 
 	if len(workDeployment.Spec.Template.Spec.Containers) != 1 {
-		return
+		// Deployment is manipulated, do not force rollout but leverage generation.
+		return false
 	}
 
-	clusterArgFound := false
 	for _, arg := range workDeployment.Spec.Template.Spec.Containers[0].Args {
 		if arg == fmt.Sprintf("--spoke-cluster-name=%s", config.ClusterName) {
-			clusterArgFound = true
+			return false
 		}
 	}
 
-	if !clusterArgFound {
-		*forceRollOut = true
-	}
+	return true
 }
 
 func (n *klusterletController) cleanUp(ctx context.Context, controllerContext factory.SyncContext, config klusterletConfig) error {

pkg/operators/klusterlet/controllers/klusterletcontroller/klusterlet_controller_test.go

@@ -423,31 +423,27 @@ func TestClusterNameChange(t *testing.T) {
 func TestForceRollOutWorkAgent(t *testing.T) {
 	cases := []struct {
 		name            string
-		forceRollout    bool
 		deployment      *appsv1.Deployment
 		clusterName     string
 		expectedRollout bool
 	}{
 		{
 			name:            "cluster name changes",
-			forceRollout:    false,
 			deployment:      newWorkAgentDeployment("klusterlet", "cluster1"),
 			clusterName:     "cluster2",
 			expectedRollout: true,
 		},
 		{
 			name:            "cluster name does not change",
-			forceRollout:    false,
 			deployment:      newWorkAgentDeployment("klusterlet", "cluster1"),
 			clusterName:     "cluster1",
 			expectedRollout: false,
 		},
 		{
-			name:            "force rollout already true",
-			forceRollout:    true,
-			deployment:      newWorkAgentDeployment("klusterlet", "cluster1"),
+			name:            "deployment not found",
+			deployment:      newWorkAgentDeployment("test", "cluster1"),
 			clusterName:     "cluster1",
-			expectedRollout: true,
+			expectedRollout: false,
 		},
 	}
 
@@ -460,9 +456,9 @@ func TestForceRollOutWorkAgent(t *testing.T) {
 				ClusterName:         c.clusterName,
 				KlusterletName:      klusterlet.Name,
 			}
-			controller.controller.forceRollOutWorkAgent(context.TODO(), config, &c.forceRollout)
-			if c.forceRollout != c.expectedRollout {
-				t.Errorf("Expected force rollout to be %v, but got %v", c.expectedRollout, c.forceRollout)
+			forceRollout := controller.controller.forceRollOutWorkAgent(context.TODO(), config)
+			if forceRollout != c.expectedRollout {
+				t.Errorf("Expected force rollout to be %v, but got %v", c.expectedRollout, forceRollout)
 			}
 		})
 	}

test/integration/clustermanager_test.go

@@ -254,12 +254,12 @@ var _ = ginkgo.Describe("ClusterManager", func() {
 				return false
 			}
 
-			registrationoDeployment, err := kubeClient.AppsV1().Deployments(hubNamespace).Get(context.Background(), hubRegistrationDeployment, metav1.GetOptions{})
+			registrationDeployment, err := kubeClient.AppsV1().Deployments(hubNamespace).Get(context.Background(), hubRegistrationDeployment, metav1.GetOptions{})
 			if err != nil {
 				return false
 			}
 
-			deploymentGeneration := helpers.NewGenerationStatus(appsv1.SchemeGroupVersion.WithResource("deployments"), registrationoDeployment)
+			deploymentGeneration := helpers.NewGenerationStatus(appsv1.SchemeGroupVersion.WithResource("deployments"), registrationDeployment)
 			actualGeneration := helpers.FindGenerationStatus(actual.Status.Generations, deploymentGeneration)
 			if deploymentGeneration.LastGeneration != actualGeneration.LastGeneration {
 				return false