From 7f01f35afc087de6815899abb795bfc2514f2657 Mon Sep 17 00:00:00 2001 From: Srinivas Atmakuri Date: Tue, 13 Jun 2023 00:35:45 +0530 Subject: [PATCH] Propagate errors of ARO DNSMasqMachineConfigPool controller to ARO Operator --- .../dnsmasq/machineconfigpool_controller.go | 32 ++++++------ .../machineconfigpool_controller_test.go | 49 ++++++++++++++----- 2 files changed, 56 insertions(+), 25 deletions(-) diff --git a/pkg/operator/controllers/dnsmasq/machineconfigpool_controller.go b/pkg/operator/controllers/dnsmasq/machineconfigpool_controller.go index 5be6a108cba..cb255db94f1 100644 --- a/pkg/operator/controllers/dnsmasq/machineconfigpool_controller.go +++ b/pkg/operator/controllers/dnsmasq/machineconfigpool_controller.go @@ -14,7 +14,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" ) @@ -23,52 +23,56 @@ const ( ) type MachineConfigPoolReconciler struct { - log *logrus.Entry + base.AROController dh dynamichelper.Interface - - client client.Client } func NewMachineConfigPoolReconciler(log *logrus.Entry, client client.Client, dh dynamichelper.Interface) *MachineConfigPoolReconciler { return &MachineConfigPoolReconciler{ - log: log, - dh: dh, - client: client, + AROController: base.AROController{ + Log: log, + Client: client, + Name: MachineConfigPoolControllerName, + }, + dh: dh, } } // Reconcile watches MachineConfigPool objects, and if any changes, // reconciles the associated ARO DNS MachineConfig object func (r *MachineConfigPoolReconciler) 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") mcp := &mcv1.MachineConfigPool{} - err = r.client.Get(ctx, types.NamespacedName{Name: request.Name}, mcp) + err = r.Client.Get(ctx, types.NamespacedName{Name: request.Name}, 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 } 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 } diff --git a/pkg/operator/controllers/dnsmasq/machineconfigpool_controller_test.go b/pkg/operator/controllers/dnsmasq/machineconfigpool_controller_test.go index d88efa00b6e..3374b64d6ef 100644 --- a/pkg/operator/controllers/dnsmasq/machineconfigpool_controller_test.go +++ b/pkg/operator/controllers/dnsmasq/machineconfigpool_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,28 @@ 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 TestMachineConfigPoolReconciler(t *testing.T) { + transitionTime := metav1.Time{Time: time.Now()} + defaultAvailable := utilconditions.ControllerDefaultAvailable(MachineConfigPoolControllerName) + defaultProgressing := utilconditions.ControllerDefaultProgressing(MachineConfigPoolControllerName) + defaultDegraded := utilconditions.ControllerDefaultDegraded(MachineConfigPoolControllerName) + 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,7 +55,9 @@ func TestMachineConfigPoolReconciler(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", @@ -62,7 +74,17 @@ func TestMachineConfigPoolReconciler(t *testing.T) { objects: []client.Object{ &arov1alpha1.Cluster{ ObjectMeta: metav1.ObjectMeta{Name: "cluster"}, - Status: arov1alpha1.ClusterStatus{}, + Status: arov1alpha1.ClusterStatus{ + Conditions: []operatorv1.OperatorCondition{ + defaultAvailable, + defaultProgressing, + { + Type: MachineConfigPoolControllerName + "Controller" + operatorv1.OperatorStatusTypeDegraded, + Status: operatorv1.ConditionTrue, + LastTransitionTime: transitionTime, + }, + }, + }, Spec: arov1alpha1.ClusterSpec{ OperatorFlags: arov1alpha1.OperatorFlags{ controllerEnabled: "true", @@ -77,14 +99,17 @@ func TestMachineConfigPoolReconciler(t *testing.T) { Name: "custom", }, }, - wantErrMsg: "", + wantErrMsg: "", + wantConditions: defaultConditions, }, { name: "MachineConfigPool reconciles ARO DNS 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", @@ -109,7 +134,8 @@ func TestMachineConfigPoolReconciler(t *testing.T) { Name: "custom", }, }, - wantErrMsg: "", + wantErrMsg: "", + wantConditions: defaultConditions, }, } @@ -129,10 +155,11 @@ func TestMachineConfigPoolReconciler(t *testing.T) { client, dh, ) - + ctx := context.Background() _, err := r.Reconcile(context.Background(), tt.request) utilerror.AssertErrorMessage(t, err, tt.wantErrMsg) + utilconditions.AssertControllerConditions(t, ctx, client, tt.wantConditions) }) } }