Propagate errors of ARO DNSMasqMachineConfig controller

pkg/operator/controllers/dnsmasq/machineconfig_controller.go

@@ -15,7 +15,7 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 
-	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"
 )
 
@@ -24,51 +24,53 @@ const (
 )
 
 type MachineConfigReconciler struct {
-	log *logrus.Entry
+	base.AROController
 
 	dh dynamichelper.Interface
-
-	client client.Client
 }
 
 var rxARODNS = regexp.MustCompile("^99-(.*)-aro-dns$")
 
 func NewMachineConfigReconciler(log *logrus.Entry, client client.Client, dh dynamichelper.Interface) *MachineConfigReconciler {
 	return &MachineConfigReconciler{
-		log:    log,
-		dh:     dh,
-		client: client,
+		AROController: base.AROController{
+			Log:    log,
+			Client: client,
+			Name:   MachineConfigControllerName,
+		},
+		dh: dh,
 	}
 }
 
 // Reconcile watches ARO DNS MachineConfig objects, and if any changes,
 // reconciles it
 func (r *MachineConfigReconciler) Reconcile(ctx context.Context, request ctrl.Request) (ctrl.Result, error) {
-	instance := &arov1alpha1.Cluster{}
-	err := r.client.Get(ctx, types.NamespacedName{Name: arov1alpha1.SingletonClusterName}, instance)
+	instance, err := r.GetCluster(ctx)
 	if err != nil {
 		return reconcile.Result{}, err
 	}
 
 	if !instance.Spec.OperatorFlags.GetSimpleBoolean(controllerEnabled) {
-		r.log.Debug("controller is disabled")
+		r.Log.Debug("controller is disabled")
 		return reconcile.Result{}, nil
 	}
 
-	r.log.Debug("running")
+	r.Log.Debug("running")
 	m := rxARODNS.FindStringSubmatch(request.Name)
 	if m == nil {
 		return reconcile.Result{}, nil
 	}
 	role := m[1]
 
 	mcp := &mcv1.MachineConfigPool{}
-	err = r.client.Get(ctx, types.NamespacedName{Name: role}, mcp)
+	err = r.Client.Get(ctx, types.NamespacedName{Name: role}, mcp)
 	if kerrors.IsNotFound(err) {
+		r.ClearDegraded(ctx)
 		return reconcile.Result{}, nil
 	}
 	if err != nil {
-		r.log.Error(err)
+		r.Log.Error(err)
+		r.SetDegraded(ctx, err)
 		return reconcile.Result{}, err
 	}
 	if mcp.GetDeletionTimestamp() != nil {
@@ -77,10 +79,12 @@ func (r *MachineConfigReconciler) Reconcile(ctx context.Context, request ctrl.Re
 
 	err = reconcileMachineConfigs(ctx, instance, r.dh, *mcp)
 	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/dnsmasq/machineconfig_controller_test.go

@@ -6,8 +6,10 @@ package dnsmasq
 import (
 	"context"
 	"testing"
+	"time"
 
 	"github.com/golang/mock/gomock"
+	operatorv1 "github.com/openshift/api/operator/v1"
 	mcv1 "github.com/openshift/machine-config-operator/pkg/apis/machineconfiguration.openshift.io/v1"
 	"github.com/sirupsen/logrus"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -18,20 +20,27 @@ import (
 
 	arov1alpha1 "github.com/Azure/ARO-RP/pkg/operator/apis/aro.openshift.io/v1alpha1"
 	mock_dynamichelper "github.com/Azure/ARO-RP/pkg/util/mocks/dynamichelper"
+	utilconditions "github.com/Azure/ARO-RP/test/util/conditions"
 	utilerror "github.com/Azure/ARO-RP/test/util/error"
 )
 
 func TestMachineConfigReconciler(t *testing.T) {
+	transitionTime := metav1.Time{Time: time.Now()}
+	defaultAvailable := utilconditions.ControllerDefaultAvailable(MachineConfigControllerName)
+	defaultProgressing := utilconditions.ControllerDefaultProgressing(MachineConfigControllerName)
+	defaultDegraded := utilconditions.ControllerDefaultDegraded(MachineConfigControllerName)
+	defaultConditions := []operatorv1.OperatorCondition{defaultAvailable, defaultProgressing, defaultDegraded}
 	fakeDh := func(controller *gomock.Controller) *mock_dynamichelper.MockInterface {
 		return mock_dynamichelper.NewMockInterface(controller)
 	}
 
 	tests := []struct {
-		name       string
-		objects    []client.Object
-		mocks      func(mdh *mock_dynamichelper.MockInterface)
-		request    ctrl.Request
-		wantErrMsg string
+		name           string
+		objects        []client.Object
+		mocks          func(mdh *mock_dynamichelper.MockInterface)
+		request        ctrl.Request
+		wantErrMsg     string
+		wantConditions []operatorv1.OperatorCondition
 	}{
 		{
 			name:       "no cluster",
@@ -45,24 +54,37 @@ func TestMachineConfigReconciler(t *testing.T) {
 			objects: []client.Object{
 				&arov1alpha1.Cluster{
 					ObjectMeta: metav1.ObjectMeta{Name: "cluster"},
-					Status:     arov1alpha1.ClusterStatus{},
+					Status: arov1alpha1.ClusterStatus{
+						Conditions: defaultConditions,
+					},
 					Spec: arov1alpha1.ClusterSpec{
 						OperatorFlags: arov1alpha1.OperatorFlags{
 							controllerEnabled: "false",
 						},
 					},
 				},
 			},
-			mocks:      func(mdh *mock_dynamichelper.MockInterface) {},
-			request:    ctrl.Request{},
-			wantErrMsg: "",
+			mocks:          func(mdh *mock_dynamichelper.MockInterface) {},
+			request:        ctrl.Request{},
+			wantErrMsg:     "",
+			wantConditions: defaultConditions,
 		},
 		{
 			name: "no MachineConfigPool for MachineConfig does nothing",
 			objects: []client.Object{
 				&arov1alpha1.Cluster{
 					ObjectMeta: metav1.ObjectMeta{Name: "cluster"},
-					Status:     arov1alpha1.ClusterStatus{},
+					Status: arov1alpha1.ClusterStatus{
+						Conditions: []operatorv1.OperatorCondition{
+							defaultAvailable,
+							defaultProgressing,
+							{
+								Type:               MachineConfigControllerName + "Controller" + operatorv1.OperatorStatusTypeDegraded,
+								Status:             operatorv1.ConditionTrue,
+								LastTransitionTime: transitionTime,
+							},
+						},
+					},
 					Spec: arov1alpha1.ClusterSpec{
 						OperatorFlags: arov1alpha1.OperatorFlags{
 							controllerEnabled: "true",
@@ -77,14 +99,17 @@ func TestMachineConfigReconciler(t *testing.T) {
 					Name:      "99-custom-aro-dns",
 				},
 			},
-			wantErrMsg: "",
+			wantErrMsg:     "",
+			wantConditions: defaultConditions,
 		},
 		{
 			name: "valid MachineConfigPool for MachineConfig reconciles MachineConfig",
 			objects: []client.Object{
 				&arov1alpha1.Cluster{
 					ObjectMeta: metav1.ObjectMeta{Name: "cluster"},
-					Status:     arov1alpha1.ClusterStatus{},
+					Status: arov1alpha1.ClusterStatus{
+						Conditions: defaultConditions,
+					},
 					Spec: arov1alpha1.ClusterSpec{
 						OperatorFlags: arov1alpha1.OperatorFlags{
 							controllerEnabled: "true",
@@ -106,7 +131,8 @@ func TestMachineConfigReconciler(t *testing.T) {
 					Name:      "99-custom-aro-dns",
 				},
 			},
-			wantErrMsg: "",
+			wantErrMsg:     "",
+			wantConditions: defaultConditions,
 		},
 	}
 
@@ -126,10 +152,11 @@ func TestMachineConfigReconciler(t *testing.T) {
 				client,
 				dh,
 			)
-
-			_, err := r.Reconcile(context.Background(), tt.request)
+			ctx := context.Background()
+			_, err := r.Reconcile(ctx, tt.request)
 
 			utilerror.AssertErrorMessage(t, err, tt.wantErrMsg)
+			utilconditions.AssertControllerConditions(t, ctx, client, tt.wantConditions)
 		})
 	}
 }