OSSM-6140 Ensure IstioCNI status is updated on reconcile errors

controllers/istiocni/istiocni_controller.go

@@ -234,35 +234,37 @@ func (r *IstioCNIReconciler) SetupWithManager(mgr ctrl.Manager) error {
 		Complete(r)
 }
 
-func (r *IstioCNIReconciler) updateStatus(ctx context.Context, cni *v1alpha1.IstioCNI, err error) error {
-	log := logf.FromContext(ctx)
-	reconciledCondition := r.determineReconciledCondition(err)
+func (r *IstioCNIReconciler) determineStatus(ctx context.Context, cni *v1alpha1.IstioCNI, reconcileErr error) v1alpha1.IstioCNIStatus {
+	reconciledCondition := r.determineReconciledCondition(reconcileErr)
 	readyCondition := r.determineReadyCondition(ctx, cni)
-	if err != nil {
-		return err
-	}
 
 	status := cni.Status.DeepCopy()
 	status.ObservedGeneration = cni.Generation
 	status.SetCondition(reconciledCondition)
 	status.SetCondition(readyCondition)
 	status.State = deriveState(reconciledCondition, readyCondition)
+	return *status
+}
+
+func (r *IstioCNIReconciler) updateStatus(ctx context.Context, cni *v1alpha1.IstioCNI, reconcileErr error) error {
+	status := r.determineStatus(ctx, cni, reconcileErr)
 
-	if reflect.DeepEqual(cni.Status, *status) {
+	if reflect.DeepEqual(cni.Status, status) {
 		return nil
 	}
 
-	statusErr := r.Client.Status().Patch(ctx, cni, kube.NewStatusPatch(*status))
+	statusErr := r.Client.Status().Patch(ctx, cni, kube.NewStatusPatch(status))
 	if statusErr != nil {
+		log := logf.FromContext(ctx)
 		log.Error(statusErr, "failed to patch status")
 
 		// ensure that we retry the reconcile by returning the status error
 		// (but without overriding the original error)
-		if err == nil {
+		if reconcileErr == nil {
 			return statusErr
 		}
 	}
-	return err
+	return reconcileErr
 }
 
 func deriveState(reconciledCondition, readyCondition v1alpha1.IstioCNICondition) v1alpha1.IstioCNIConditionReason {

controllers/istiocni/istiocni_controller_test.go

@@ -16,12 +16,14 @@ package istiocni
 
 import (
 	"context"
+	"fmt"
 	"testing"
 
 	"github.com/google/go-cmp/cmp"
 	"github.com/istio-ecosystem/sail-operator/api/v1alpha1"
 	"github.com/istio-ecosystem/sail-operator/pkg/common"
 	"github.com/istio-ecosystem/sail-operator/pkg/scheme"
+	. "github.com/onsi/gomega"
 	appsv1 "k8s.io/api/apps/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/util/intstr"
@@ -330,3 +332,52 @@ func TestApplyImageDigests(t *testing.T) {
 		})
 	}
 }
+
+func TestDetermineStatus(t *testing.T) {
+	tests := []struct {
+		name         string
+		reconcileErr error
+	}{
+		{
+			name:         "no error",
+			reconcileErr: nil,
+		},
+		{
+			name:         "reconcile error",
+			reconcileErr: fmt.Errorf("some reconcile error"),
+		},
+	}
+
+	ctx := context.TODO()
+	resourceDir := t.TempDir()
+	cl := fake.NewClientBuilder().WithScheme(scheme.Scheme).Build()
+	r := NewIstioCNIReconciler(cl, scheme.Scheme, nil, resourceDir, nil, nil)
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			g := NewWithT(t)
+
+			cni := &v1alpha1.IstioCNI{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:       "my-cni",
+					Generation: 123,
+				},
+			}
+
+			reconciledCondition := r.determineReconciledCondition(tt.reconcileErr)
+			readyCondition := r.determineReadyCondition(ctx, cni)
+
+			status := r.determineStatus(ctx, cni, tt.reconcileErr)
+
+			g.Expect(status.ObservedGeneration).To(Equal(cni.Generation))
+			g.Expect(status.State).To(Equal(deriveState(reconciledCondition, readyCondition)))
+			g.Expect(normalize(status.GetCondition(v1alpha1.IstioCNIConditionTypeReconciled))).To(Equal(normalize(reconciledCondition)))
+			g.Expect(normalize(status.GetCondition(v1alpha1.IstioCNIConditionTypeReady))).To(Equal(normalize(readyCondition)))
+		})
+	}
+}
+
+func normalize(condition v1alpha1.IstioCNICondition) v1alpha1.IstioCNICondition {
+	condition.LastTransitionTime = metav1.Time{}
+	return condition
+}