From 71e23071dff568651ebd8466c89764714a5aba2d Mon Sep 17 00:00:00 2001 From: Srinivas Atmakuri Date: Tue, 13 Jun 2023 00:15:28 +0530 Subject: [PATCH] Propagate errors of ARO DNSMasqCluster controller to ARO Operator --- .../controllers/dnsmasq/cluster_controller.go | 32 +++++----- .../dnsmasq/cluster_controller_test.go | 62 ++++++++++++++----- 2 files changed, 62 insertions(+), 32 deletions(-) diff --git a/pkg/operator/controllers/dnsmasq/cluster_controller.go b/pkg/operator/controllers/dnsmasq/cluster_controller.go index b316b3e5709..d898e4558f3 100644 --- a/pkg/operator/controllers/dnsmasq/cluster_controller.go +++ b/pkg/operator/controllers/dnsmasq/cluster_controller.go @@ -9,7 +9,6 @@ import ( mcv1 "github.com/openshift/machine-config-operator/pkg/apis/machineconfiguration.openshift.io/v1" "github.com/sirupsen/logrus" kruntime "k8s.io/apimachinery/pkg/runtime" - "k8s.io/apimachinery/pkg/types" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/builder" "sigs.k8s.io/controller-runtime/pkg/client" @@ -17,6 +16,7 @@ import ( "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" ) @@ -27,49 +27,51 @@ const ( ) type ClusterReconciler struct { - log *logrus.Entry - + base.AROController dh dynamichelper.Interface - - client client.Client } func NewClusterReconciler(log *logrus.Entry, client client.Client, dh dynamichelper.Interface) *ClusterReconciler { return &ClusterReconciler{ - log: log, - dh: dh, - client: client, + AROController: base.AROController{ + Log: log, + Client: client, + Name: ClusterControllerName, + }, + dh: dh, } } // Reconcile watches the ARO object, and if it changes, reconciles all the // 99-%s-aro-dns machineconfigs func (r *ClusterReconciler) 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") mcps := &mcv1.MachineConfigPoolList{} - err = r.client.List(ctx, mcps) + err = r.Client.List(ctx, mcps) 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, mcps.Items...) 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/cluster_controller_test.go b/pkg/operator/controllers/dnsmasq/cluster_controller_test.go index 55e39429b23..f2830ac5cb3 100644 --- a/pkg/operator/controllers/dnsmasq/cluster_controller_test.go +++ b/pkg/operator/controllers/dnsmasq/cluster_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" @@ -17,20 +19,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 TestClusterReconciler(t *testing.T) { + transitionTime := metav1.Time{Time: time.Now()} + defaultAvailable := utilconditions.ControllerDefaultAvailable(ClusterControllerName) + defaultProgressing := utilconditions.ControllerDefaultProgressing(ClusterControllerName) + defaultDegraded := utilconditions.ControllerDefaultDegraded(ClusterControllerName) + 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", @@ -44,7 +54,9 @@ func TestClusterReconciler(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", @@ -52,16 +64,27 @@ func TestClusterReconciler(t *testing.T) { }, }, }, - mocks: func(mdh *mock_dynamichelper.MockInterface) {}, - request: ctrl.Request{}, - wantErrMsg: "", + mocks: func(mdh *mock_dynamichelper.MockInterface) {}, + request: ctrl.Request{}, + wantErrMsg: "", + wantConditions: defaultConditions, }, { name: "no MachineConfigPools does nothing", objects: []client.Object{ &arov1alpha1.Cluster{ ObjectMeta: metav1.ObjectMeta{Name: "cluster"}, - Status: arov1alpha1.ClusterStatus{}, + Status: arov1alpha1.ClusterStatus{ + Conditions: []operatorv1.OperatorCondition{ + defaultAvailable, + defaultProgressing, + { + Type: ClusterControllerName + "Controller" + operatorv1.OperatorStatusTypeDegraded, + Status: operatorv1.ConditionTrue, + LastTransitionTime: transitionTime, + }, + }, + }, Spec: arov1alpha1.ClusterSpec{ OperatorFlags: arov1alpha1.OperatorFlags{ controllerEnabled: "true", @@ -72,15 +95,18 @@ func TestClusterReconciler(t *testing.T) { mocks: func(mdh *mock_dynamichelper.MockInterface) { mdh.EXPECT().Ensure(gomock.Any()).Times(1) }, - request: ctrl.Request{}, - wantErrMsg: "", + request: ctrl.Request{}, + wantErrMsg: "", + wantConditions: defaultConditions, }, { name: "valid MachineConfigPool creates 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", @@ -96,8 +122,9 @@ func TestClusterReconciler(t *testing.T) { mocks: func(mdh *mock_dynamichelper.MockInterface) { mdh.EXPECT().Ensure(gomock.Any(), gomock.AssignableToTypeOf(&mcv1.MachineConfig{})).Times(1) }, - request: ctrl.Request{}, - wantErrMsg: "", + request: ctrl.Request{}, + wantErrMsg: "", + wantConditions: defaultConditions, }, } @@ -118,10 +145,11 @@ func TestClusterReconciler(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) }) } }