make unit test to work

Signed-off-by: Yingrong Zhao <[email protected]>

controllers/agentconfig_controller.go

@@ -159,7 +159,7 @@ func (r *AgentConfigReconciler) Reconcile(ctx context.Context, req ctrl.Request)
 			if updated {
 				return ctrl.Result{}, nil
 			}
-			err := r.DeleteTemporaryPVC(ctx, log, tempPVC, agentCfg)
+			err = r.DeleteTemporaryPVC(ctx, log, tempPVC, agentCfg)
 			if err != nil {
 				return ctrl.Result{}, err
 			}
@@ -582,9 +582,10 @@ func (r *AgentConfigReconciler) bindPVWithHashPVC(ctx context.Context, log logr.
 			return false, err
 		}
 
+		return true, nil
 	}
 
-	return true, nil
+	return false, nil
 }
 
 func (r *AgentConfigReconciler) DeleteTemporaryPVC(ctx context.Context, log logr.Logger, tempPVC *corev1.PersistentVolumeClaim, agentCfg *porterv1.AgentConfig) error {
@@ -599,8 +600,6 @@ func (r *AgentConfigReconciler) DeleteTemporaryPVC(ctx context.Context, log logr
 
 		return nil
 	}
-	// release old pvc from agentCfg
-	//update the pv with the new pvc
 
 	log.V(Log4Debug).Info("Deleting temporary persistent volume claim.", "persistentvolumeclaim", tempPVC.Name, "namespace", tempPVC.Namespace)
 	if err := r.Delete(ctx, tempPVC); err != nil {

controllers/agentconfig_controller_test.go

@@ -122,6 +122,8 @@ func TestAgentConfigReconciler_Reconcile(t *testing.T) {
 	require.NoError(t, controller.Create(ctx, pv))
 	pvc.Spec.VolumeName = pv.Name
 	pvc.Status.Phase = corev1.ClaimBound
+	// the pvc controller should have updated the pvc with the pvc-protection finalizer
+	pvc.Finalizers = append(pvc.Finalizers, "kubernetes.io/pvc-protection")
 	require.NoError(t, controller.Update(ctx, pvc))
 
 	triggerReconcile()
@@ -135,22 +137,54 @@ func TestAgentConfigReconciler_Reconcile(t *testing.T) {
 
 	require.NotEmpty(t, actionList.Spec.Volumes)
 
+	// verify that the pv that has plugins installed has been updated with the expected lables and claim reference
+	pluginsPV := &corev1.PersistentVolume{}
+	require.NoError(t, controller.Get(ctx, client.ObjectKey{Namespace: agentCfg.Namespace, Name: pv.Name}, pluginsPV))
+	pluginLabels, exists := pluginsPV.Labels[porterv1.LablePlugins]
+	require.True(t, exists)
+	require.Equal(t, agentCfg.Spec.GetPluginsLabels()[porterv1.LablePlugins], pluginLabels)
+	rn, exists := pluginsPV.Labels[porterv1.LabelResourceName]
+	require.True(t, exists)
+	require.Equal(t, agentCfg.Name, rn)
+	require.Equal(t, agentCfg.GetPVCName(), pluginsPV.Spec.ClaimRef.Name)
+
 	triggerReconcile()
 
-	// verify that the tmp pvc is deleted
+	// verify that the tmp pvc's finalizer is deleted
 	tmpPVC := &corev1.PersistentVolumeClaim{}
+	require.NoError(t, controller.Get(ctx, client.ObjectKey{Namespace: agentCfg.Namespace, Name: pvc.Name}, tmpPVC))
+	require.Empty(t, tmpPVC.GetFinalizers())
+
+	triggerReconcile()
+
+	tmpPVC = &corev1.PersistentVolumeClaim{}
 	require.True(t, apierrors.IsNotFound(controller.Get(ctx, client.ObjectKey{Namespace: agentCfg.Namespace, Name: pvc.Name}, tmpPVC)))
 
 	triggerReconcile()
 
-	// the renamed pvc should be bounded to the existing pv once it's ready
+	// the renamed pvc should be created with label selector set and correct access mode
 	renamedPVC := &corev1.PersistentVolumeClaim{}
 	require.NoError(t, controller.Get(ctx, client.ObjectKey{Namespace: agentCfg.Namespace, Name: agentCfg.GetPVCName()}, renamedPVC))
-	renamedPVC.Status.Phase = corev1.ClaimBound
-	require.NoError(t, controller.Update(ctx, renamedPVC))
+	readonlyMany := []corev1.PersistentVolumeAccessMode{corev1.ReadOnlyMany}
+	require.Equal(t, readonlyMany, renamedPVC.Spec.AccessModes)
+	matchLables := agentCfg.Spec.GetPluginsLabels()
+	matchLables[porterv1.LabelResourceName] = agentCfg.Name
+	require.Equal(t, matchLables, renamedPVC.Spec.Selector.MatchLabels)
+
+	triggerReconcile()
+
+	// Fail the action
+	action.Status.Phase = porterv1.PhaseFailed
+	action.Status.Conditions = []metav1.Condition{{Type: string(porterv1.ConditionFailed), Status: metav1.ConditionTrue}}
+	require.NoError(t, controller.Status().Update(ctx, &action))
 
 	triggerReconcile()
 
+	// Verify that the agent config status shows the action is failed
+	require.NotNil(t, agentCfg.Status.Action, "expected Action to still be set")
+	assert.Equal(t, porterv1.PhaseFailed, agentCfg.Status.Phase, "incorrect Phase")
+	assert.True(t, apimeta.IsStatusConditionTrue(agentCfg.Status.Conditions, string(porterv1.ConditionFailed)))
+
 	// Edit the agent config spec
 	agentCfg.Generation = 2
 	agentCfg.Spec.Plugins[0].Name = "test-plugin-2"
@@ -165,18 +199,6 @@ func TestAgentConfigReconciler_Reconcile(t *testing.T) {
 
 	triggerReconcile()
 
-	// Fail the action
-	action.Status.Phase = porterv1.PhaseFailed
-	action.Status.Conditions = []metav1.Condition{{Type: string(porterv1.ConditionFailed), Status: metav1.ConditionTrue}}
-	require.NoError(t, controller.Status().Update(ctx, &action))
-
-	triggerReconcile()
-
-	// Verify that the agent config status shows the action is failed
-	require.NotNil(t, agentCfg.Status.Action, "expected Action to still be set")
-	assert.Equal(t, porterv1.PhaseFailed, agentCfg.Status.Phase, "incorrect Phase")
-	assert.True(t, apimeta.IsStatusConditionTrue(agentCfg.Status.Conditions, string(porterv1.ConditionFailed)))
-
 	// Retry the last action
 	lastAction := agentCfg.Status.Action.Name
 	agentCfg.Annotations = map[string]string{porterv1.AnnotationRetry: "retry-1"}