fix: report deployment apply failures on the cluster status

pkg/apis/authzed/v1alpha1/conditions.go

@@ -25,6 +25,7 @@ const (
 	ConditionTypeRolloutError        = "RolloutError"
 
 	ConditionReasonMissingSecret = "MissingSecret"
+	ConditionReasonApplyFailed   = "ApplyFailed"
 )
 
 func NewValidatingConfigCondition(secretHash string) metav1.Condition {
@@ -106,3 +107,13 @@ func NewPodErrorCondition(message string) metav1.Condition {
 		Message:            message,
 	}
 }
+
+func NewApplyFailedCondition(err error) metav1.Condition {
+	return metav1.Condition{
+		Type:               ConditionTypeRolloutError,
+		Status:             metav1.ConditionTrue,
+		Reason:             ConditionReasonApplyFailed,
+		LastTransitionTime: metav1.NewTime(time.Now()),
+		Message:            fmt.Sprintf("Error applying deployment: %s", err),
+	}
+}

pkg/controller/ensure_deployment.go

@@ -88,9 +88,26 @@ func (m *DeploymentHandler) Handle(ctx context.Context) {
 			),
 		)
 		if err != nil {
+			// surface the failure on the status so it's visible without
+			// reading operator logs, e.g. when a user-provided patch results
+			// in a deployment the API server rejects
+			currentStatus.SetStatusCondition(v1alpha1.NewApplyFailedCondition(err))
+			if patchErr := m.patchStatus(ctx, currentStatus); patchErr != nil {
+				QueueOps.RequeueAPIErr(ctx, patchErr)
+				return
+			}
 			QueueOps.RequeueAPIErr(ctx, err)
 			return
 		}
+
+		// clear any error from a previous failed apply
+		if cond := currentStatus.FindStatusCondition(v1alpha1.ConditionTypeRolloutError); cond != nil && cond.Reason == v1alpha1.ConditionReasonApplyFailed {
+			currentStatus.RemoveStatusCondition(v1alpha1.ConditionTypeRolloutError)
+			if err := m.patchStatus(ctx, currentStatus); err != nil {
+				QueueOps.RequeueAPIErr(ctx, err)
+				return
+			}
+		}
 		ctx = CtxCurrentSpiceDeployment.WithValue(ctx, deployment)
 	}
 

pkg/controller/ensure_deployment_test.go

@@ -24,6 +24,7 @@ import (
 func TestEnsureDeploymentHandler(t *testing.T) {
 	now := metav1.Now()
 	var nextKey handler.Key = "next"
+	applyErr := fmt.Errorf("apply error")
 	tests := []struct {
 		name string
 
@@ -34,14 +35,16 @@ func TestEnsureDeploymentHandler(t *testing.T) {
 		pods                []*corev1.Pod
 		currentStatus       *v1alpha1.SpiceDBCluster
 		replicas            int32
+		applyErr            error
 
-		expectNext         handler.Key
-		expectStatus       *v1alpha1.SpiceDBCluster
-		expectRequeueErr   error
-		expectRequeueAfter bool
-		expectApply        bool
-		expectDelete       bool
-		expectPatchStatus  bool
+		expectNext          handler.Key
+		expectStatus        *v1alpha1.SpiceDBCluster
+		expectRequeueErr    error
+		expectRequeueAPIErr error
+		expectRequeueAfter  bool
+		expectApply         bool
+		expectDelete        bool
+		expectPatchStatus   bool
 	}{
 		{
 			name:               "creates if no deployments",
@@ -50,6 +53,40 @@ func TestEnsureDeploymentHandler(t *testing.T) {
 			expectApply:        true,
 			expectRequeueAfter: true,
 		},
+		{
+			name:              "reports apply failure on status",
+			migrationHash:     "testtesttesttest",
+			secretHash:        "secret",
+			applyErr:          applyErr,
+			expectApply:       true,
+			expectPatchStatus: true,
+			expectStatus: &v1alpha1.SpiceDBCluster{Status: v1alpha1.ClusterStatus{Conditions: []metav1.Condition{{
+				Type:               v1alpha1.ConditionTypeRolloutError,
+				Status:             metav1.ConditionTrue,
+				LastTransitionTime: now,
+				Reason:             v1alpha1.ConditionReasonApplyFailed,
+				Message:            "Error applying deployment: apply error",
+			}}}},
+			expectRequeueAPIErr: applyErr,
+		},
+		{
+			name: "clears apply failure once apply succeeds",
+			currentStatus: &v1alpha1.SpiceDBCluster{Status: v1alpha1.ClusterStatus{Conditions: []metav1.Condition{{
+				Type:               v1alpha1.ConditionTypeRolloutError,
+				Status:             metav1.ConditionTrue,
+				LastTransitionTime: now,
+				Reason:             v1alpha1.ConditionReasonApplyFailed,
+				Message:            "Error applying deployment: apply error",
+			}}}},
+			migrationHash:     "testtesttesttest",
+			secretHash:        "secret",
+			expectApply:       true,
+			expectPatchStatus: true,
+			expectStatus: &v1alpha1.SpiceDBCluster{Status: v1alpha1.ClusterStatus{
+				Conditions: []metav1.Condition{},
+			}},
+			expectRequeueAfter: true,
+		},
 		{
 			// Regression test for https://github.com/authzed/spicedb-operator/issues/415
 			// When all credential refs are configured with `skip: true`, ConfigChangedHandler
@@ -407,7 +444,7 @@ func TestEnsureDeploymentHandler(t *testing.T) {
 			h := &DeploymentHandler{
 				applyDeployment: func(_ context.Context, _ *applyappsv1.DeploymentApplyConfiguration) (*appsv1.Deployment, error) {
 					applyCalled = true
-					return nil, nil
+					return nil, tt.applyErr
 				},
 				deleteDeployment: func(_ context.Context, _ types.NamespacedName) error {
 					deleteCalled = true
@@ -439,6 +476,10 @@ func TestEnsureDeploymentHandler(t *testing.T) {
 				require.Equal(t, 1, ctrls.RequeueErrCallCount())
 				require.Equal(t, tt.expectRequeueErr, ctrls.RequeueErrArgsForCall(0))
 			}
+			if tt.expectRequeueAPIErr != nil {
+				require.Equal(t, 1, ctrls.RequeueAPIErrCallCount())
+				require.Equal(t, tt.expectRequeueAPIErr, ctrls.RequeueAPIErrArgsForCall(0))
+			}
 			require.Equal(t, tt.expectRequeueAfter, ctrls.RequeueAfterCallCount() == 1)
 		})
 	}