Propagate errors of GenevaLogging controller

pkg/operator/controllers/genevalogging/genevalogging.go

@@ -24,7 +24,7 @@ import (
 
 func (r *Reconciler) securityContextConstraints(ctx context.Context, name, serviceAccountName string) (*securityv1.SecurityContextConstraints, error) {
 	scc := &securityv1.SecurityContextConstraints{}
-	err := r.client.Get(ctx, types.NamespacedName{Name: "privileged"}, scc)
+	err := r.Client.Get(ctx, types.NamespacedName{Name: "privileged"}, scc)
 	if err != nil {
 		return nil, err
 	}

pkg/operator/controllers/genevalogging/genevalogging_controller.go

@@ -19,6 +19,7 @@ import (
 
 	"github.com/Azure/ARO-RP/pkg/operator"
 	arov1alpha1 "github.com/Azure/ARO-RP/pkg/operator/apis/aro.openshift.io/v1alpha1"
+	"github.com/Azure/ARO-RP/pkg/operator/controllers/base"
 	"github.com/Azure/ARO-RP/pkg/util/dynamichelper"
 )
 
@@ -34,65 +35,78 @@ const (
 
 // Reconciler reconciles a Cluster object
 type Reconciler struct {
-	log *logrus.Entry
+	base.AROController
 
-	dh     dynamichelper.Interface
-	client client.Client
+	dh dynamichelper.Interface
 }
 
 func NewReconciler(log *logrus.Entry, client client.Client, dh dynamichelper.Interface) *Reconciler {
 	return &Reconciler{
-		log: log,
-
-		dh:     dh,
-		client: client,
+		AROController: base.AROController{
+			Log:    log,
+			Client: client,
+			Name:   ControllerName,
+		},
+		dh: dh,
 	}
 }
 
-// Reconcile the genevalogging deployment.
-func (r *Reconciler) Reconcile(ctx context.Context, request ctrl.Request) (ctrl.Result, error) {
-	instance := &arov1alpha1.Cluster{}
-	err := r.client.Get(ctx, types.NamespacedName{Name: arov1alpha1.SingletonClusterName}, instance)
+func (r *Reconciler) ensureResources(ctx context.Context, instance *arov1alpha1.Cluster) error {
+	operatorSecret := &corev1.Secret{}
+	operatorSecretName := types.NamespacedName{
+		Namespace: operator.Namespace,
+		Name:      operator.SecretName,
+	}
+	err := r.Client.Get(ctx, operatorSecretName, operatorSecret)
 	if err != nil {
-		return reconcile.Result{}, err
+		return err
 	}
 
-	if !instance.Spec.OperatorFlags.GetSimpleBoolean(controllerEnabled) {
-		r.log.Debug("controller is disabled")
-		return reconcile.Result{}, nil
+	resources, err := r.resources(ctx, instance, operatorSecret.Data[GenevaCertName], operatorSecret.Data[GenevaKeyName])
+	if err != nil {
+		return err
 	}
 
-	r.log.Debug("running")
-	operatorSecret := &corev1.Secret{}
-	err = r.client.Get(ctx, types.NamespacedName{Namespace: operator.Namespace, Name: operator.SecretName}, operatorSecret)
+	err = dynamichelper.SetControllerReferences(resources, instance)
 	if err != nil {
-		return reconcile.Result{}, err
+		return err
 	}
 
-	resources, err := r.resources(ctx, instance, operatorSecret.Data[GenevaCertName], operatorSecret.Data[GenevaKeyName])
+	err = dynamichelper.Prepare(resources)
 	if err != nil {
-		r.log.Error(err)
-		return reconcile.Result{}, err
+		return err
 	}
 
-	err = dynamichelper.SetControllerReferences(resources, instance)
+	err = r.dh.Ensure(ctx, resources...)
 	if err != nil {
-		r.log.Error(err)
-		return reconcile.Result{}, err
+		return err
 	}
 
-	err = dynamichelper.Prepare(resources)
+	return nil
+}
+
+// Reconcile the genevalogging deployment.
+func (r *Reconciler) Reconcile(ctx context.Context, request ctrl.Request) (ctrl.Result, error) {
+	instance, err := r.GetCluster(ctx)
 	if err != nil {
-		r.log.Error(err)
+		r.Log.Error(err)
 		return reconcile.Result{}, err
 	}
 
-	err = r.dh.Ensure(ctx, resources...)
+	if !instance.Spec.OperatorFlags.GetSimpleBoolean(controllerEnabled) {
+		r.Log.Debug("controller is disabled")
+		return reconcile.Result{}, nil
+	}
+
+	r.Log.Debug("running")
+	err = r.ensureResources(ctx, instance)
 	if err != nil {
-		r.log.Error(err)
+		r.Log.Error(err)
+		r.SetDegraded(ctx, err)
 		return reconcile.Result{}, err
 	}
 
+	r.ClearConditions(ctx)
 	return reconcile.Result{}, nil
 }
 

pkg/operator/controllers/genevalogging/genevalogging_test.go

@@ -4,23 +4,32 @@ package genevalogging
 // Licensed under the Apache License 2.0.
 
 import (
+	"context"
 	"errors"
 	"fmt"
 	"testing"
 
 	"github.com/go-test/deep"
+	"github.com/golang/mock/gomock"
 	operatorv1 "github.com/openshift/api/operator/v1"
+	securityv1 "github.com/openshift/api/security/v1"
 	"github.com/sirupsen/logrus"
 	appsv1 "k8s.io/api/apps/v1"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	ctrl "sigs.k8s.io/controller-runtime"
+	"sigs.k8s.io/controller-runtime/pkg/client"
 	ctrlfake "sigs.k8s.io/controller-runtime/pkg/client/fake"
 
+	"github.com/Azure/ARO-RP/pkg/operator"
 	arov1alpha1 "github.com/Azure/ARO-RP/pkg/operator/apis/aro.openshift.io/v1alpha1"
+	"github.com/Azure/ARO-RP/pkg/operator/controllers/base"
+	mock_dynamichelper "github.com/Azure/ARO-RP/pkg/util/mocks/dynamichelper"
 	_ "github.com/Azure/ARO-RP/pkg/util/scheme"
 	"github.com/Azure/ARO-RP/pkg/util/version"
 	testdatabase "github.com/Azure/ARO-RP/test/database"
+	utilconditions "github.com/Azure/ARO-RP/test/util/conditions"
+	utilerror "github.com/Azure/ARO-RP/test/util/error"
 )
 
 func getContainer(d *appsv1.DaemonSet, containerName string) (corev1.Container, error) {
@@ -33,11 +42,32 @@ func getContainer(d *appsv1.DaemonSet, containerName string) (corev1.Container,
 }
 
 func TestGenevaLoggingDaemonset(t *testing.T) {
+	nominalMocks := func(mockDh *mock_dynamichelper.MockInterface) {
+		mockDh.EXPECT().Ensure(
+			gomock.Any(),
+			gomock.AssignableToTypeOf(&securityv1.SecurityContextConstraints{}),
+			gomock.AssignableToTypeOf(&corev1.Namespace{}),
+			gomock.AssignableToTypeOf(&corev1.ConfigMap{}),
+			gomock.AssignableToTypeOf(&corev1.Secret{}),
+			gomock.AssignableToTypeOf(&corev1.ServiceAccount{}),
+			gomock.AssignableToTypeOf(&appsv1.DaemonSet{}),
+		).Times(1)
+	}
+
+	defaultConditions := []operatorv1.OperatorCondition{
+		utilconditions.ControllerDefaultAvailable(ControllerName),
+		utilconditions.ControllerDefaultProgressing(ControllerName),
+		utilconditions.ControllerDefaultDegraded(ControllerName),
+	}
+
 	tests := []struct {
 		name              string
 		request           ctrl.Request
 		operatorFlags     arov1alpha1.OperatorFlags
 		validateDaemonset func(*appsv1.DaemonSet) []error
+		mocks             func(mockDh *mock_dynamichelper.MockInterface)
+		wantErrMsg        string
+		wantConditions    []operatorv1.OperatorCondition
 	}{
 		{
 			name: "no flags given",
@@ -71,6 +101,9 @@ func TestGenevaLoggingDaemonset(t *testing.T) {
 
 				return
 			},
+			mocks:          nominalMocks,
+			wantErrMsg:     "",
+			wantConditions: defaultConditions,
 		},
 		{
 			name: "fluentbit changed",
@@ -105,6 +138,9 @@ func TestGenevaLoggingDaemonset(t *testing.T) {
 
 				return
 			},
+			mocks:          nominalMocks,
+			wantErrMsg:     "",
+			wantConditions: defaultConditions,
 		},
 		{
 			name: "mdsd changed",
@@ -139,23 +175,58 @@ func TestGenevaLoggingDaemonset(t *testing.T) {
 
 				return
 			},
+			mocks:      nominalMocks,
+			wantErrMsg: "",
+			wantConditions: []operatorv1.OperatorCondition{
+				utilconditions.ControllerDefaultAvailable(ControllerName),
+				utilconditions.ControllerDefaultProgressing(ControllerName),
+				utilconditions.ControllerDefaultDegraded(ControllerName),
+			},
 		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
+			controller := gomock.NewController(t)
+			defer controller.Finish()
+
 			instance := &arov1alpha1.Cluster{
 				ObjectMeta: metav1.ObjectMeta{Name: "cluster"},
-				Status:     arov1alpha1.ClusterStatus{Conditions: []operatorv1.OperatorCondition{}},
+				Status:     arov1alpha1.ClusterStatus{Conditions: defaultConditions},
 				Spec: arov1alpha1.ClusterSpec{
 					ResourceID:    testdatabase.GetResourcePath("00000000-0000-0000-0000-000000000000", "testcluster"),
 					OperatorFlags: tt.operatorFlags,
 					ACRDomain:     "acrDomain",
 				},
 			}
 
+			resources := []client.Object{
+				instance,
+				&corev1.Secret{
+					ObjectMeta: metav1.ObjectMeta{
+						Namespace: operator.Namespace,
+						Name:      operator.SecretName,
+					},
+					Data: map[string][]byte{
+						GenevaCertName: {},
+						GenevaKeyName:  {},
+					},
+				},
+				&securityv1.SecurityContextConstraints{
+					ObjectMeta: metav1.ObjectMeta{
+						Name: "privileged",
+					},
+				},
+			}
+
+			mockDh := mock_dynamichelper.NewMockInterface(controller)
+
 			r := &Reconciler{
-				log:    logrus.NewEntry(logrus.StandardLogger()),
-				client: ctrlfake.NewClientBuilder().WithObjects(instance).Build(),
+				AROController: base.AROController{
+					Log:    logrus.NewEntry(logrus.StandardLogger()),
+					Client: ctrlfake.NewClientBuilder().WithObjects(resources...).Build(),
+					Name:   ControllerName,
+				},
+				dh: mockDh,
 			}
 
 			daemonset, err := r.daemonset(instance)
@@ -167,6 +238,13 @@ func TestGenevaLoggingDaemonset(t *testing.T) {
 			for _, err := range errs {
 				t.Error(err)
 			}
+
+			tt.mocks(mockDh)
+			ctx := context.Background()
+			_, err = r.Reconcile(ctx, tt.request)
+
+			utilerror.AssertErrorMessage(t, err, tt.wantErrMsg)
+			utilconditions.AssertControllerConditions(t, ctx, r.Client, tt.wantConditions)
 		})
 	}
 }