Restrict containsInvalidImage to a specific namespace (#1670)

Signed-off-by: Jonathan West <jonwest@redhat.com>

Author: jgwest

controllers/argocd/statefulset.go

@@ -787,8 +787,10 @@ func (r *ReconcileArgoCD) reconcileApplicationControllerStatefulSet(cr *argoproj
 		podSpec.Volumes = getArgoImportVolumes(export)
 	}
 
-	invalidImagePod := containsInvalidImage(cr, r)
-	if invalidImagePod {
+	invalidImagePod, err := containsInvalidImage(*cr, *r)
+	if err != nil {
+		return err
+	} else if invalidImagePod {
 		argoutil.LogResourceDeletion(log, ss, "one or more pods has an invalid image")
 		if err := r.Client.Delete(context.TODO(), ss); err != nil {
 			return err
@@ -1002,22 +1004,52 @@ func updateNodePlacementStateful(existing *appsv1.StatefulSet, ss *appsv1.Statef
 
 // Returns true if a StatefulSet has pods in ErrImagePull or ImagePullBackoff state.
 // These pods cannot be restarted automatially due to known kubernetes issue https://github.com/kubernetes/kubernetes/issues/67250
-func containsInvalidImage(cr *argoproj.ArgoCD, r *ReconcileArgoCD) bool {
-
-	brokenPod := false
+func containsInvalidImage(cr argoproj.ArgoCD, r ReconcileArgoCD) (bool, error) {
 
 	podList := &corev1.PodList{}
-	listOption := client.MatchingLabels{common.ArgoCDKeyName: fmt.Sprintf("%s-%s", cr.Name, "application-controller")}
+	applicationControllerListOption := client.MatchingLabels{common.ArgoCDKeyName: fmt.Sprintf("%s-%s", cr.Name, "application-controller")}
 
-	if err := r.Client.List(context.TODO(), podList, listOption); err != nil {
+	if err := r.Client.List(context.TODO(), podList, applicationControllerListOption, client.InNamespace(cr.Namespace)); err != nil {
 		log.Error(err, "Failed to list Pods")
+		return false, err
+	}
+
+	if len(podList.Items) == 0 {
+		// No pods, no work to do
+		return false, nil
+	}
+
+	if len(podList.Items) != 1 {
+		// There should only be 0 or 1. If this message is printed, it suggests a problem.
+		log.Info("Unexpected number of pods in 'containsInvalidImage' pod list", "podListItems", fmt.Sprintf("%d", len(podList.Items)), "namespace", cr.Namespace)
+		return false, nil
+	}
+
+	appControllerPod := podList.Items[0]
+
+	if len(appControllerPod.Status.ContainerStatuses) == 0 {
+		// No container statuses for application-controller, no work to do.
+		return false, nil
 	}
-	if len(podList.Items) > 0 {
-		if len(podList.Items[0].Status.ContainerStatuses) > 0 {
-			if podList.Items[0].Status.ContainerStatuses[0].State.Waiting != nil && (podList.Items[0].Status.ContainerStatuses[0].State.Waiting.Reason == "ImagePullBackOff" || podList.Items[0].Status.ContainerStatuses[0].State.Waiting.Reason == "ErrImagePull") {
-				brokenPod = true
+
+	brokenPod := false
+
+	waitingState := appControllerPod.Status.ContainerStatuses[0].State.Waiting
+	if waitingState != nil {
+
+		waitingReason := waitingState.Reason
+		if waitingReason == "ImagePullBackOff" || waitingReason == "ErrImagePull" {
+
+			var containerImage string
+			if len(appControllerPod.Spec.Containers) > 0 {
+				containerImage = appControllerPod.Spec.Containers[0].Image
 			}
+
+			log.Info("A broken pod was detected", "waitingReason", waitingReason, "containerImage", containerImage)
+			brokenPod = true
 		}
+
 	}
-	return brokenPod
+
+	return brokenPod, nil
 }

controllers/argocd/statefulset_test.go

@@ -638,11 +638,13 @@ func Test_UpdateNodePlacementStateful(t *testing.T) {
 	}
 }
 
-func Test_ContainsValidImage(t *testing.T) {
+func Test_ContainsInvalidImage(t *testing.T) {
 
 	a := makeTestArgoCD()
 	po := &corev1.Pod{
 		ObjectMeta: metav1.ObjectMeta{
+			Name:      "argo-cd-application-controller",
+			Namespace: a.Namespace,
 			Labels: map[string]string{
 				common.ArgoCDKeyName: fmt.Sprintf("%s-%s", a.Name, "application-controller"),
 			},
@@ -658,10 +660,23 @@ func Test_ContainsValidImage(t *testing.T) {
 	cl := makeTestReconcilerClient(sch, objs, objs, runtimeObjs)
 	r := makeTestReconciler(cl, sch)
 
-	if containsInvalidImage(a, r) {
+	// Test that containsInvalidImage returns false if there is nothing wrong with the Pod
+	containsInvalidImageRes, err := containsInvalidImage(*a, *r)
+	assert.NoError(t, err)
+	if containsInvalidImageRes {
 		t.Fatalf("containsInvalidImage failed, got true, expected false")
 	}
 
+	// Test that containsInvalidImage returns true if the Pod is in ErrImagePull
+	po.Status.ContainerStatuses = []corev1.ContainerStatus{
+		{State: corev1.ContainerState{Waiting: &corev1.ContainerStateWaiting{Reason: "ErrImagePull"}}}}
+	err = cl.Status().Update(context.Background(), po)
+	assert.NoError(t, err)
+
+	containsInvalidImageRes, err = containsInvalidImage(*a, *r)
+	assert.NoError(t, err)
+	assert.True(t, containsInvalidImageRes)
+
 }
 
 func TestReconcileArgoCD_reconcileApplicationController_withDynamicSharding(t *testing.T) {