From dffeee0df7173c40d68432981a37fc157cdee11a 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 | 68 +++++++++++++++---- 2 files changed, 71 insertions(+), 29 deletions(-) diff --git a/pkg/operator/controllers/dnsmasq/cluster_controller.go b/pkg/operator/controllers/dnsmasq/cluster_controller.go index ab1d342e176..e16911789f8 100644 --- a/pkg/operator/controllers/dnsmasq/cluster_controller.go +++ b/pkg/operator/controllers/dnsmasq/cluster_controller.go @@ -10,7 +10,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" @@ -18,6 +17,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" ) @@ -28,49 +28,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..319d44fcb97 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,29 @@ 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 + startConditions []operatorv1.OperatorCondition + wantConditions []operatorv1.OperatorCondition }{ { name: "no cluster", @@ -44,7 +55,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 +65,28 @@ 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: "", + startConditions: defaultConditions, + 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", @@ -74,13 +99,25 @@ func TestClusterReconciler(t *testing.T) { }, request: ctrl.Request{}, wantErrMsg: "", + startConditions: []operatorv1.OperatorCondition{ + defaultAvailable, + defaultProgressing, + { + Type: ClusterControllerName + "Controller" + operatorv1.OperatorStatusTypeDegraded, + Status: operatorv1.ConditionTrue, + LastTransitionTime: transitionTime, + }, + }, + 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 +133,10 @@ 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: "", + startConditions: defaultConditions, + wantConditions: defaultConditions, }, } @@ -118,10 +157,11 @@ func TestClusterReconciler(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) }) } }