Fix reconciliation of Installation with 0 replicas (#296)

mattermost · Jul 26, 2022 · 99dadb1 · 99dadb1
1 parent 4ab9324
commit 99dadb1
Show file tree

Hide file tree

Showing 3 changed files with 31 additions and 4 deletions.
diff --git a/controllers/mattermost/mattermost/controller.go b/controllers/mattermost/mattermost/controller.go
@@ -210,6 +210,7 @@ func (r *MattermostReconciler) startNonReconcilingMMProcessing(ctx context.Conte
 		return false, errors.Wrap(err, "failed to list Mattermosts")
 	}
 
+	reqLogger.Info("Non reconciling being processed", "processing", r.reconcilingRateLimiter.nonReconcilingBeingProcessed)
 	// Check if limit of Mattermosts reconciling at the same time is reached.
 	if countReconciling(mmListInstallations.Items)+r.reconcilingRateLimiter.nonReconcilingBeingProcessed >= r.MaxReconciling {
 		reqLogger.Info(fmt.Sprintf("Reached limit of reconciling or processing installations, requeuing in %s", r.RequeueOnLimitDelay.String()))

diff --git a/controllers/mattermost/mattermost/controller_test.go b/controllers/mattermost/mattermost/controller_test.go
@@ -183,6 +183,30 @@ func TestReconcile(t *testing.T) {
 		err = c.Update(context.TODO(), replicaSet)
 		require.NoError(t, err)
 
+		t.Run("succeed if 0 replicas expected", func(t *testing.T) {
+			orgReplicas := mm.Spec.Replicas
+			replicasZero := int32(0)
+			mm.Spec.Replicas = &replicasZero
+
+			err = c.Update(context.TODO(), mm)
+			require.NoError(t, err)
+			// Revert changes for purpose of other tests
+			defer func() {
+				mm.Spec.Replicas = orgReplicas
+				mm.Status = mmv1beta.MattermostStatus{}
+				err = c.Update(context.TODO(), mm)
+				require.NoError(t, err)
+			}()
+
+			res, err = r.Reconcile(context.Background(), req)
+			require.NoError(t, err)
+			assert.Empty(t, res)
+
+			err = c.Get(context.TODO(), mmKey, mm)
+			require.NoError(t, err)
+			assert.Equal(t, mmv1beta.Stable, mm.Status.State)
+		})
+
 		// Create expected mattermost pods.
 		podTemplate := corev1.Pod{
 			ObjectMeta: metav1.ObjectMeta{

diff --git a/controllers/mattermost/mattermost/health_check.go b/controllers/mattermost/mattermost/health_check.go
@@ -53,7 +53,7 @@ func (r *MattermostReconciler) checkMattermostHealth(mattermost *mmv1beta.Matter
 		replicas = *mattermost.Spec.Replicas
 	}
 
-	if podsStatus.UpdatedReplicas == 0 {
+	if replicas > 0 && podsStatus.UpdatedReplicas == 0 {
 		return status, fmt.Errorf("mattermost pods not yet updated")
 	}
 
@@ -79,9 +79,11 @@ func (r *MattermostReconciler) checkMattermostHealth(mattermost *mmv1beta.Matter
 		status.Endpoint = endpoint
 	}
 
-	// At least one pod is updated and LB/Ingress is ready therefore we are at
-	// least ready to server traffic.
-	status.State = mmv1beta.Ready
+	if replicas > 0 {
+		// At least one pod is updated and LB/Ingress is ready therefore we are at
+		// least ready to server traffic.
+		status.State = mmv1beta.Ready
+	}
 
 	if podsStatus.UpdatedReplicas != replicas {
 		return status, fmt.Errorf("found %d updated replicas, but wanted %d", podsStatus.UpdatedReplicas, replicas)