From 2059f0f3fd4353a61e48e5bb9fd804995b6552de Mon Sep 17 00:00:00 2001 From: safwank97 Date: Fri, 15 Sep 2023 10:57:20 -0400 Subject: [PATCH] Propagate errors of ARO MachineHealthCheckController to ARO Operator --- cmd/aro/operator.go | 6 +- .../machinehealthcheck_controller.go | 64 ++++++++++++------- .../machinehealthcheck_controller_test.go | 36 ++++++----- 3 files changed, 64 insertions(+), 42 deletions(-) diff --git a/cmd/aro/operator.go b/cmd/aro/operator.go index 6ed38bca917..ba3de7fcb1e 100644 --- a/cmd/aro/operator.go +++ b/cmd/aro/operator.go @@ -191,10 +191,10 @@ func operator(ctx context.Context, log *logrus.Entry) error { client)).SetupWithManager(mgr); err != nil { return fmt.Errorf("unable to create controller %s: %v", autosizednodes.ControllerName, err) } - if err = (machinehealthcheck.NewReconciler( - log.WithField("controller", machinehealthcheck.ControllerName), + if err = (machinehealthcheck.NewMachineHealthCheckReconciler( + log.WithField("controller", machinehealthcheck.MHCControllerName), client, dh)).SetupWithManager(mgr); err != nil { - return fmt.Errorf("unable to create controller %s: %v", machinehealthcheck.ControllerName, err) + return fmt.Errorf("unable to create controller %s: %v", machinehealthcheck.MHCControllerName, err) } if err = (ingress.NewReconciler( log.WithField("controller", ingress.ControllerName), diff --git a/pkg/operator/controllers/machinehealthcheck/machinehealthcheck_controller.go b/pkg/operator/controllers/machinehealthcheck/machinehealthcheck_controller.go index 809aeb0b9a7..825684dafcd 100644 --- a/pkg/operator/controllers/machinehealthcheck/machinehealthcheck_controller.go +++ b/pkg/operator/controllers/machinehealthcheck/machinehealthcheck_controller.go @@ -13,7 +13,6 @@ import ( monitoringv1 "github.com/prometheus-operator/prometheus-operator/pkg/apis/monitoring/v1" "github.com/sirupsen/logrus" kruntime "k8s.io/apimachinery/pkg/runtime" - "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/kubernetes/scheme" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/builder" @@ -22,6 +21,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" ) @@ -32,50 +32,63 @@ var machinehealthcheckYaml []byte var mhcremediationalertYaml []byte const ( - ControllerName string = "MachineHealthCheck" - managed string = "aro.machinehealthcheck.managed" - enabled string = "aro.machinehealthcheck.enabled" + MHCControllerName string = "MachineHealthCheck" + MHCManaged string = "aro.machinehealthcheck.managed" + MHCEnabled string = "aro.machinehealthcheck.enabled" ) -type Reconciler struct { - log *logrus.Entry +type MachineHealthCheckReconciler struct { + base.AROController dh dynamichelper.Interface - - client client.Client } -func NewReconciler(log *logrus.Entry, client client.Client, dh dynamichelper.Interface) *Reconciler { - return &Reconciler{ - log: log, - dh: dh, - client: client, +func NewMachineHealthCheckReconciler(log *logrus.Entry, client client.Client, dh dynamichelper.Interface) *MachineHealthCheckReconciler { + return &MachineHealthCheckReconciler{ + AROController: base.AROController{ + Log: log, + Client: client, + Name: MHCControllerName, + }, + dh: dh, } } -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) +// Reconcile watches MachineHealthCheck objects, and if any changes, +// reconciles the associated ARO MachineHealthCheck object + +func (r *MachineHealthCheckReconciler) Reconcile(ctx context.Context, request ctrl.Request) (ctrl.Result, error) { + instance, err := r.GetCluster(ctx) + if err != nil { return reconcile.Result{}, err } - if !instance.Spec.OperatorFlags.GetSimpleBoolean(enabled) { - r.log.Debug("controller is disabled") + if !instance.Spec.OperatorFlags.GetSimpleBoolean(MHCEnabled) { + r.Log.Debug("controller is disabled") return reconcile.Result{}, nil } - r.log.Debug("running") - if !instance.Spec.OperatorFlags.GetSimpleBoolean(managed) { + r.Log.Debug("running") + if !instance.Spec.OperatorFlags.GetSimpleBoolean(MHCManaged) { + r.SetProgressing(ctx, "We are disabling the MHCHealthCheck because this cluster at current moment is NOT MHC Managed.") + err := r.dh.EnsureDeleted(ctx, "MachineHealthCheck", "openshift-machine-api", "aro-machinehealthcheck") if err != nil { + r.Log.Error(err) + r.SetDegraded(ctx, err) + return reconcile.Result{RequeueAfter: time.Hour}, err } err = r.dh.EnsureDeleted(ctx, "PrometheusRule", "openshift-machine-api", "mhc-remediation-alert") if err != nil { + r.Log.Error(err) + r.SetDegraded(ctx, err) + return reconcile.Result{RequeueAfter: time.Hour}, err } + return reconcile.Result{}, nil } @@ -84,6 +97,9 @@ func (r *Reconciler) Reconcile(ctx context.Context, request ctrl.Request) (ctrl. for _, asset := range [][]byte{machinehealthcheckYaml, mhcremediationalertYaml} { resource, _, err := scheme.Codecs.UniversalDeserializer().Decode(asset, nil, nil) if err != nil { + r.Log.Error(err) + r.SetDegraded(ctx, err) + return reconcile.Result{}, err } @@ -105,21 +121,25 @@ func (r *Reconciler) Reconcile(ctx context.Context, request ctrl.Request) (ctrl. // create/update the MHC CR err = r.dh.Ensure(ctx, resources...) if err != nil { + r.Log.Error(err) + r.SetDegraded(ctx, err) + return reconcile.Result{}, err } + r.ClearConditions(ctx) return reconcile.Result{}, nil } // SetupWithManager will manage only our MHC resource with our specific controller name -func (r *Reconciler) SetupWithManager(mgr ctrl.Manager) error { +func (r *MachineHealthCheckReconciler) SetupWithManager(mgr ctrl.Manager) error { aroClusterPredicate := predicate.NewPredicateFuncs(func(o client.Object) bool { return strings.EqualFold(arov1alpha1.SingletonClusterName, o.GetName()) }) return ctrl.NewControllerManagedBy(mgr). For(&arov1alpha1.Cluster{}, builder.WithPredicates(aroClusterPredicate)). - Named(ControllerName). + Named(MHCControllerName). Owns(&machinev1beta1.MachineHealthCheck{}). Owns(&monitoringv1.PrometheusRule{}). Complete(r) diff --git a/pkg/operator/controllers/machinehealthcheck/machinehealthcheck_controller_test.go b/pkg/operator/controllers/machinehealthcheck/machinehealthcheck_controller_test.go index d25953cd03a..0c21580b36a 100644 --- a/pkg/operator/controllers/machinehealthcheck/machinehealthcheck_controller_test.go +++ b/pkg/operator/controllers/machinehealthcheck/machinehealthcheck_controller_test.go @@ -23,7 +23,7 @@ import ( ) // Test reconcile function -func TestReconciler(t *testing.T) { +func TestMachineHealthCheckReconciler(t *testing.T) { type test struct { name string instance *arov1alpha1.Cluster @@ -46,7 +46,7 @@ func TestReconciler(t *testing.T) { }, Spec: arov1alpha1.ClusterSpec{ OperatorFlags: arov1alpha1.OperatorFlags{ - enabled: strconv.FormatBool(false), + MHCEnabled: strconv.FormatBool(false), }, }, }, @@ -64,8 +64,8 @@ func TestReconciler(t *testing.T) { }, Spec: arov1alpha1.ClusterSpec{ OperatorFlags: arov1alpha1.OperatorFlags{ - enabled: strconv.FormatBool(true), - managed: strconv.FormatBool(false), + MHCEnabled: strconv.FormatBool(true), + MHCManaged: strconv.FormatBool(false), }, }, }, @@ -83,8 +83,8 @@ func TestReconciler(t *testing.T) { }, Spec: arov1alpha1.ClusterSpec{ OperatorFlags: arov1alpha1.OperatorFlags{ - enabled: strconv.FormatBool(true), - managed: strconv.FormatBool(false), + MHCEnabled: strconv.FormatBool(true), + MHCManaged: strconv.FormatBool(false), }, }, }, @@ -102,8 +102,8 @@ func TestReconciler(t *testing.T) { }, Spec: arov1alpha1.ClusterSpec{ OperatorFlags: arov1alpha1.OperatorFlags{ - enabled: strconv.FormatBool(true), - managed: strconv.FormatBool(false), + MHCEnabled: strconv.FormatBool(true), + MHCManaged: strconv.FormatBool(false), }, }, }, @@ -122,8 +122,8 @@ func TestReconciler(t *testing.T) { }, Spec: arov1alpha1.ClusterSpec{ OperatorFlags: arov1alpha1.OperatorFlags{ - enabled: strconv.FormatBool(true), - managed: strconv.FormatBool(true), + MHCEnabled: strconv.FormatBool(true), + MHCManaged: strconv.FormatBool(true), }, }, }, @@ -140,8 +140,8 @@ func TestReconciler(t *testing.T) { }, Spec: arov1alpha1.ClusterSpec{ OperatorFlags: arov1alpha1.OperatorFlags{ - enabled: strconv.FormatBool(true), - managed: strconv.FormatBool(true), + MHCEnabled: strconv.FormatBool(true), + MHCManaged: strconv.FormatBool(true), }, }, }, @@ -165,11 +165,13 @@ func TestReconciler(t *testing.T) { } ctx := context.Background() - r := &Reconciler{ - log: logrus.NewEntry(logrus.StandardLogger()), - dh: mdh, - client: clientBuilder.Build(), - } + + r := NewMachineHealthCheckReconciler( + logrus.NewEntry(logrus.StandardLogger()), + clientBuilder.Build(), + mdh, + ) + request := ctrl.Request{} request.Name = "cluster"