diff --git a/pkg/operator/controllers/base/aro_controller.go b/pkg/operator/controllers/base/aro_controller.go new file mode 100644 index 000000000..048528810 --- /dev/null +++ b/pkg/operator/controllers/base/aro_controller.go @@ -0,0 +1,105 @@ +package base + +// Copyright (c) Microsoft Corporation. +// Licensed under the Apache License 2.0. + +import ( + "context" + + operatorv1 "github.com/openshift/api/operator/v1" + "github.com/openshift/library-go/pkg/operator/v1helpers" + "github.com/sirupsen/logrus" + "k8s.io/apimachinery/pkg/api/equality" + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client" + + arov1alpha1 "github.com/Azure/ARO-RP/pkg/operator/apis/aro.openshift.io/v1alpha1" +) + +type AROController struct { + Log *logrus.Entry + Client client.Client + Name string +} + +func (c *AROController) SetConditions(ctx context.Context, cnds ...*operatorv1.OperatorCondition) { + cluster, err := c.GetCluster(ctx) + if err != nil { + c.Log.Warn("Failed to retrieve ARO cluster resource") + return + } + + newConditions := cluster.Status.DeepCopy().Conditions + for _, cnd := range cnds { + v1helpers.SetOperatorCondition(&newConditions, *cnd) + } + + if equality.Semantic.DeepEqual(cluster.Status.Conditions, newConditions) { + return + } + + cluster.Status.Conditions = newConditions + if err := c.Client.Status().Update(ctx, cluster); err != nil { + c.Log.Error("error updating controller conditions", err) + } +} + +func (c *AROController) SetProgressing(ctx context.Context, message string) { + cnd := c.defaultProgressing() + cnd.Status = operatorv1.ConditionTrue + cnd.Message = message + + c.SetConditions(ctx, cnd) +} + +func (c *AROController) ClearProgressing(ctx context.Context) { + c.SetConditions(ctx, c.defaultProgressing()) +} + +func (c *AROController) SetDegraded(ctx context.Context, err error) { + cnd := c.defaultDegraded() + cnd.Status = operatorv1.ConditionTrue + cnd.Message = err.Error() + + c.SetConditions(ctx, cnd) +} + +func (c *AROController) ClearDegraded(ctx context.Context) { + c.SetConditions(ctx, c.defaultDegraded()) +} + +func (c *AROController) ClearConditions(ctx context.Context) { + c.SetConditions(ctx, c.defaultAvailable(), c.defaultProgressing(), c.defaultDegraded()) +} + +func (c *AROController) GetCluster(ctx context.Context) (*arov1alpha1.Cluster, error) { + cluster := &arov1alpha1.Cluster{} + err := c.Client.Get(ctx, types.NamespacedName{Name: arov1alpha1.SingletonClusterName}, cluster) + + return cluster, err +} + +func (c *AROController) defaultAvailable() *operatorv1.OperatorCondition { + return &operatorv1.OperatorCondition{ + Type: c.conditionName(operatorv1.OperatorStatusTypeAvailable), + Status: operatorv1.ConditionTrue, + } +} + +func (c *AROController) defaultProgressing() *operatorv1.OperatorCondition { + return &operatorv1.OperatorCondition{ + Type: c.conditionName(operatorv1.OperatorStatusTypeProgressing), + Status: operatorv1.ConditionFalse, + } +} + +func (c *AROController) defaultDegraded() *operatorv1.OperatorCondition { + return &operatorv1.OperatorCondition{ + Type: c.conditionName(operatorv1.OperatorStatusTypeDegraded), + Status: operatorv1.ConditionFalse, + } +} + +func (c *AROController) conditionName(conditionType string) string { + return c.Name + "Controller" + conditionType +} diff --git a/pkg/operator/controllers/base/aro_controller_test.go b/pkg/operator/controllers/base/aro_controller_test.go new file mode 100644 index 000000000..d6c58e0d0 --- /dev/null +++ b/pkg/operator/controllers/base/aro_controller_test.go @@ -0,0 +1,143 @@ +package base + +// Copyright (c) Microsoft Corporation. +// Licensed under the Apache License 2.0. + +import ( + "context" + "errors" + "testing" + "time" + + operatorv1 "github.com/openshift/api/operator/v1" + "github.com/sirupsen/logrus" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + ctrlfake "sigs.k8s.io/controller-runtime/pkg/client/fake" + + arov1alpha1 "github.com/Azure/ARO-RP/pkg/operator/apis/aro.openshift.io/v1alpha1" + _ "github.com/Azure/ARO-RP/pkg/util/scheme" + utilconditions "github.com/Azure/ARO-RP/test/util/conditions" +) + +func TestConditions(t *testing.T) { + ctx := context.Background() + + controllerName := "Fake" + + now := metav1.NewTime(time.Now()) + past := metav1.NewTime(now.Add(-1 * time.Hour)) + + internetReachable := operatorv1.OperatorCondition{ + Type: arov1alpha1.InternetReachableFromMaster, + Status: operatorv1.ConditionFalse, + LastTransitionTime: now, + } + + defaultAvailable := utilconditions.ControllerDefaultAvailable(controllerName) + defaultProgressing := utilconditions.ControllerDefaultProgressing(controllerName) + defaultDegraded := utilconditions.ControllerDefaultDegraded(controllerName) + + defaultAvailableInPast := *defaultAvailable.DeepCopy() + defaultAvailableInPast.LastTransitionTime = past + + unavailable := *defaultAvailable.DeepCopy() + unavailable.Status = operatorv1.ConditionFalse + unavailable.Message = "Something bad happened" + + isProgressing := *defaultProgressing.DeepCopy() + isProgressing.Status = operatorv1.ConditionTrue + isProgressing.Message = "Controller is performing task" + + isDegraded := *defaultDegraded.DeepCopy() + isDegraded.Status = operatorv1.ConditionTrue + isDegraded.Message = "Controller failed to perform task" + + for _, tt := range []struct { + name string + start []operatorv1.OperatorCondition + action func(c AROController) + want []operatorv1.OperatorCondition + }{ + { + name: "SetConditions - sets all provided conditions", + start: []operatorv1.OperatorCondition{internetReachable}, + action: func(c AROController) { + c.SetConditions(ctx, &defaultAvailable, &defaultProgressing, &defaultDegraded) + }, + want: []operatorv1.OperatorCondition{internetReachable, defaultAvailable, defaultProgressing, defaultDegraded}, + }, + { + name: "SetConditions - if condition exists and status matches, does not update", + start: []operatorv1.OperatorCondition{internetReachable, defaultAvailableInPast}, + action: func(c AROController) { + c.SetConditions(ctx, &defaultAvailable, &defaultProgressing, &defaultDegraded) + }, + want: []operatorv1.OperatorCondition{internetReachable, defaultAvailableInPast, defaultProgressing, defaultDegraded}, + }, + { + name: "SetConditions - if condition exists and status does not match, updates", + start: []operatorv1.OperatorCondition{internetReachable, defaultAvailableInPast}, + action: func(c AROController) { + c.SetConditions(ctx, &unavailable, &defaultProgressing, &defaultDegraded) + }, + want: []operatorv1.OperatorCondition{internetReachable, unavailable, defaultProgressing, defaultDegraded}, + }, + { + name: "SetProgressing - sets Progressing to true with message", + start: []operatorv1.OperatorCondition{defaultAvailable, defaultProgressing, defaultDegraded}, + action: func(c AROController) { + c.SetProgressing(ctx, isProgressing.Message) + }, + want: []operatorv1.OperatorCondition{defaultAvailable, isProgressing, defaultDegraded}, + }, + { + name: "ClearProgressing - sets Progressing to false and clears message", + start: []operatorv1.OperatorCondition{defaultAvailable, isProgressing, defaultDegraded}, + action: func(c AROController) { + c.ClearProgressing(ctx) + }, + want: []operatorv1.OperatorCondition{defaultAvailable, defaultProgressing, defaultDegraded}, + }, + { + name: "SetDegraded - sets Degraded to true with message", + start: []operatorv1.OperatorCondition{defaultAvailable, defaultProgressing, defaultDegraded}, + action: func(c AROController) { + err := errors.New(isDegraded.Message) + c.SetDegraded(ctx, err) + }, + want: []operatorv1.OperatorCondition{defaultAvailable, defaultProgressing, isDegraded}, + }, + { + name: "ClearDegraded - sets Degraded to false and clears message", + start: []operatorv1.OperatorCondition{defaultAvailable, defaultProgressing, isDegraded}, + action: func(c AROController) { + c.ClearDegraded(ctx) + }, + want: []operatorv1.OperatorCondition{defaultAvailable, defaultProgressing, defaultDegraded}, + }, + } { + t.Run(tt.name, func(t *testing.T) { + client := ctrlfake.NewClientBuilder(). + WithObjects( + &arov1alpha1.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: arov1alpha1.SingletonClusterName, + }, + Status: arov1alpha1.ClusterStatus{ + Conditions: tt.start, + OperatorVersion: "unknown", + }, + }, + ).Build() + + controller := AROController{ + Log: logrus.NewEntry(logrus.StandardLogger()), + Client: client, + Name: controllerName, + } + + tt.action(controller) + utilconditions.AssertControllerConditions(t, ctx, client, tt.want) + }) + } +} diff --git a/pkg/operator/controllers/machineset/machineset_controller.go b/pkg/operator/controllers/machineset/machineset_controller.go index d155097e7..3aba44a5e 100644 --- a/pkg/operator/controllers/machineset/machineset_controller.go +++ b/pkg/operator/controllers/machineset/machineset_controller.go @@ -18,7 +18,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/predicate" "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" ) const ( @@ -28,45 +28,52 @@ const ( ) type Reconciler struct { - log *logrus.Entry - - client client.Client + base.AROController } // MachineSet reconciler watches MachineSet objects for changes, evaluates total worker replica count, and reverts changes if needed. func NewReconciler(log *logrus.Entry, client client.Client) *Reconciler { return &Reconciler{ - log: log, - client: client, + AROController: base.AROController{ + Log: log, + Client: client, + Name: ControllerName, + }, } } 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) + 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") + modifiedMachineset := &machinev1beta1.MachineSet{} - err = r.client.Get(ctx, types.NamespacedName{Name: request.Name, Namespace: machineSetsNamespace}, modifiedMachineset) + err = r.Client.Get(ctx, types.NamespacedName{Name: request.Name, Namespace: machineSetsNamespace}, modifiedMachineset) if err != nil { + r.Log.Error(err) + r.SetDegraded(ctx, err) + return reconcile.Result{}, err } machinesets := &machinev1beta1.MachineSetList{} selector, _ := labels.Parse("machine.openshift.io/cluster-api-machine-role=worker") - err = r.client.List(ctx, machinesets, &client.ListOptions{ + err = r.Client.List(ctx, machinesets, &client.ListOptions{ Namespace: machineSetsNamespace, LabelSelector: selector, }) if err != nil { + r.Log.Error(err) + r.SetDegraded(ctx, err) + return reconcile.Result{}, err } @@ -75,6 +82,8 @@ func (r *Reconciler) Reconcile(ctx context.Context, request ctrl.Request) (ctrl. for _, machineset := range machinesets.Items { // If there are any custom machinesets in the list, bail and don't requeue if !strings.Contains(machineset.Name, instance.Spec.InfraID) { + r.ClearDegraded(ctx) + return reconcile.Result{}, nil } if machineset.Spec.Replicas != nil { @@ -83,15 +92,19 @@ func (r *Reconciler) Reconcile(ctx context.Context, request ctrl.Request) (ctrl. } if replicaCount < minSupportedReplicas { - r.log.Infof("Found less than %v worker replicas. The MachineSet controller will attempt scaling.", minSupportedReplicas) + r.Log.Infof("Found less than %v worker replicas. The MachineSet controller will attempt scaling.", minSupportedReplicas) // Add replicas to the object, and call Update modifiedMachineset.Spec.Replicas = to.Int32Ptr(int32(minSupportedReplicas-replicaCount) + *modifiedMachineset.Spec.Replicas) - err := r.client.Update(ctx, modifiedMachineset) + err := r.Client.Update(ctx, modifiedMachineset) if err != nil { + 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/machineset/machineset_controller_test.go b/pkg/operator/controllers/machineset/machineset_controller_test.go index 00fd721bd..f58e49797 100644 --- a/pkg/operator/controllers/machineset/machineset_controller_test.go +++ b/pkg/operator/controllers/machineset/machineset_controller_test.go @@ -7,9 +7,11 @@ import ( "context" "strconv" "testing" + "time" "github.com/Azure/go-autorest/autorest/to" machinev1beta1 "github.com/openshift/api/machine/v1beta1" + operatorv1 "github.com/openshift/api/operator/v1" "github.com/sirupsen/logrus" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" @@ -20,10 +22,17 @@ import ( arov1alpha1 "github.com/Azure/ARO-RP/pkg/operator/apis/aro.openshift.io/v1alpha1" "github.com/Azure/ARO-RP/pkg/util/cmp" _ "github.com/Azure/ARO-RP/pkg/util/scheme" + utilconditions "github.com/Azure/ARO-RP/test/util/conditions" utilerror "github.com/Azure/ARO-RP/test/util/error" ) func TestReconciler(t *testing.T) { + transitionTime := metav1.Time{Time: time.Now()} + defaultAvailable := utilconditions.ControllerDefaultAvailable(ControllerName) + defaultProgressing := utilconditions.ControllerDefaultProgressing(ControllerName) + defaultDegraded := utilconditions.ControllerDefaultDegraded(ControllerName) + defaultConditions := []operatorv1.OperatorCondition{defaultAvailable, defaultProgressing, defaultDegraded} + fakeMachineSets := func(replicas0 int32, replicas1 int32, replicas2 int32) []client.Object { workerMachineSet0 := &machinev1beta1.MachineSet{ ObjectMeta: metav1.ObjectMeta{ @@ -65,31 +74,37 @@ func TestReconciler(t *testing.T) { } tests := []struct { - name string - objectName string - machinesets []client.Object - wantReplicas int32 - featureFlag bool - assertReplicas bool - wantErr string + name string + objectName string + machinesets []client.Object + wantReplicas int32 + featureFlag bool + assertReplicas bool + wantErr string + startConditions []operatorv1.OperatorCondition + wantConditions []operatorv1.OperatorCondition }{ { - name: "no worker replicas, machineset-0 modified", - objectName: "aro-fake-machineset-0", - machinesets: fakeMachineSets(0, 0, 0), - wantReplicas: 2, - featureFlag: true, - assertReplicas: true, - wantErr: "", + name: "no worker replicas, machineset-0 modified", + objectName: "aro-fake-machineset-0", + machinesets: fakeMachineSets(0, 0, 0), + wantReplicas: 2, + featureFlag: true, + assertReplicas: true, + wantErr: "", + startConditions: defaultConditions, + wantConditions: defaultConditions, }, { - name: "no worker replicas, feature flag is false", - objectName: "aro-fake-machineset-0", - machinesets: fakeMachineSets(0, 0, 0), - wantReplicas: 0, - featureFlag: false, - assertReplicas: true, - wantErr: "", + name: "no worker replicas, feature flag is false", + objectName: "aro-fake-machineset-0", + machinesets: fakeMachineSets(0, 0, 0), + wantReplicas: 0, + featureFlag: false, + assertReplicas: true, + wantErr: "", + startConditions: defaultConditions, + wantConditions: defaultConditions, }, { name: "no worker replicas, custom machineset is present", @@ -111,10 +126,12 @@ func TestReconciler(t *testing.T) { }, ) }(), - wantReplicas: 0, - featureFlag: true, - assertReplicas: true, - wantErr: "", + wantReplicas: 0, + featureFlag: true, + assertReplicas: true, + wantErr: "", + startConditions: defaultConditions, + wantConditions: defaultConditions, }, { name: "one worker replica, machineset-0 modified", @@ -124,31 +141,57 @@ func TestReconciler(t *testing.T) { featureFlag: true, assertReplicas: true, wantErr: "", + startConditions: []operatorv1.OperatorCondition{ + defaultAvailable, + defaultProgressing, + { + Type: ControllerName + "Controller" + operatorv1.OperatorStatusTypeDegraded, + Status: operatorv1.ConditionTrue, + LastTransitionTime: transitionTime, + Message: `machinesets.machine.openshift.io "aro-fake-machineset-0" not found`, + }, + }, + wantConditions: defaultConditions, }, { - name: "two worker replicas, machineset-0 modified", - objectName: "aro-fake-machineset-0", - machinesets: fakeMachineSets(1, 1, 0), - wantReplicas: 1, - featureFlag: true, - assertReplicas: true, - wantErr: "", + name: "two worker replicas, machineset-0 modified", + objectName: "aro-fake-machineset-0", + machinesets: fakeMachineSets(1, 1, 0), + wantReplicas: 1, + featureFlag: true, + assertReplicas: true, + wantErr: "", + startConditions: defaultConditions, + wantConditions: defaultConditions, }, { - name: "two worker replicas in machineset-1, machineset-0 modified", - objectName: "aro-fake-machineset-0", - machinesets: fakeMachineSets(0, 2, 0), - wantReplicas: 0, - featureFlag: true, - assertReplicas: true, - wantErr: "", + name: "two worker replicas in machineset-1, machineset-0 modified", + objectName: "aro-fake-machineset-0", + machinesets: fakeMachineSets(0, 2, 0), + wantReplicas: 0, + featureFlag: true, + assertReplicas: true, + wantErr: "", + startConditions: defaultConditions, + wantConditions: defaultConditions, }, { - name: "machineset-0 not found", - objectName: "aro-fake-machineset-0", - featureFlag: true, - assertReplicas: false, - wantErr: `machinesets.machine.openshift.io "aro-fake-machineset-0" not found`, + name: "machineset-0 not found", + objectName: "aro-fake-machineset-0", + featureFlag: true, + assertReplicas: false, + wantErr: `machinesets.machine.openshift.io "aro-fake-machineset-0" not found`, + startConditions: defaultConditions, + wantConditions: []operatorv1.OperatorCondition{ + defaultAvailable, + defaultProgressing, + { + Type: ControllerName + "Controller" + operatorv1.OperatorStatusTypeDegraded, + Status: operatorv1.ConditionTrue, + LastTransitionTime: transitionTime, + Message: `machinesets.machine.openshift.io "aro-fake-machineset-0" not found`, + }, + }, }, } @@ -162,14 +205,17 @@ func TestReconciler(t *testing.T) { ControllerEnabled: strconv.FormatBool(tt.featureFlag), }, }, + Status: arov1alpha1.ClusterStatus{ + Conditions: tt.startConditions, + }, } - clientFake := ctrlfake.NewClientBuilder().WithObjects(instance).WithObjects(tt.machinesets...).Build() + clientFake := ctrlfake.NewClientBuilder(). + WithObjects(instance). + WithObjects(tt.machinesets...). + Build() - r := &Reconciler{ - log: logrus.NewEntry(logrus.StandardLogger()), - client: clientFake, - } + r := NewReconciler(logrus.NewEntry(logrus.StandardLogger()), clientFake) request := ctrl.Request{} request.Name = tt.objectName @@ -178,10 +224,11 @@ func TestReconciler(t *testing.T) { _, err := r.Reconcile(ctx, request) utilerror.AssertErrorMessage(t, err, tt.wantErr) + utilconditions.AssertControllerConditions(t, ctx, clientFake, tt.wantConditions) if tt.assertReplicas { modifiedMachineset := &machinev1beta1.MachineSet{} - err = r.client.Get(ctx, types.NamespacedName{Name: request.Name, Namespace: machineSetsNamespace}, modifiedMachineset) + err = r.Client.Get(ctx, types.NamespacedName{Name: request.Name, Namespace: machineSetsNamespace}, modifiedMachineset) if err != nil { t.Error(err) } diff --git a/pkg/operator/controllers/node/node_controller.go b/pkg/operator/controllers/node/node_controller.go index 1366fba99..1c0948314 100644 --- a/pkg/operator/controllers/node/node_controller.go +++ b/pkg/operator/controllers/node/node_controller.go @@ -9,7 +9,6 @@ import ( "strings" "time" - operatorv1 "github.com/openshift/api/operator/v1" "github.com/sirupsen/logrus" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -20,8 +19,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/util/conditions" + "github.com/Azure/ARO-RP/pkg/operator/controllers/base" "github.com/Azure/ARO-RP/pkg/util/ready" ) @@ -34,48 +32,41 @@ const ( // Reconciler spots nodes that look like they're stuck upgrading. When this // happens, it tries to drain them disabling eviction (so PDBs don't count). type Reconciler struct { - log *logrus.Entry + base.AROController kubernetescli kubernetes.Interface - - client client.Client } func NewReconciler(log *logrus.Entry, client client.Client, kubernetescli kubernetes.Interface) *Reconciler { return &Reconciler{ - log: log, + AROController: base.AROController{ + Log: log, + Client: client, + Name: ControllerName, + }, + kubernetescli: kubernetescli, - client: client, } } 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) + 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") - cnds, err := conditions.GetControllerConditions(ctx, r.client, ControllerName) - if err != nil { - return reconcile.Result{}, err - } node := &corev1.Node{} - err = r.client.Get(ctx, types.NamespacedName{Name: request.Name}, node) + err = r.Client.Get(ctx, types.NamespacedName{Name: request.Name}, node) if err != nil { - r.log.Error(err) - - cnds.Degraded.Status = operatorv1.ConditionTrue - cnds.Degraded.Message = err.Error() - - conditions.SetControllerConditions(ctx, r.client, cnds) + r.Log.Error(err) + r.SetDegraded(ctx, err) return reconcile.Result{}, err } @@ -83,7 +74,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, request ctrl.Request) (ctrl. // don't interfere with masters, don't want to trample etcd-quorum-guard. if node.Labels != nil { if _, ok := node.Labels["node-role.kubernetes.io/master"]; ok { - conditions.SetControllerConditions(ctx, r.client, cnds) + r.ClearConditions(ctx) return reconcile.Result{}, nil } } @@ -91,20 +82,18 @@ func (r *Reconciler) Reconcile(ctx context.Context, request ctrl.Request) (ctrl. if !isDraining(node) { // we're not draining: ensure our annotation is not set and return if getAnnotation(&node.ObjectMeta, annotationDrainStartTime) == "" { - conditions.SetControllerConditions(ctx, r.client, cnds) + r.ClearConditions(ctx) return reconcile.Result{}, nil } delete(node.Annotations, annotationDrainStartTime) - err = r.client.Update(ctx, node) + err = r.Client.Update(ctx, node) if err != nil { - cnds.Degraded.Status = operatorv1.ConditionTrue - cnds.Degraded.Message = err.Error() - r.log.Error(err) + r.Log.Error(err) + r.SetDegraded(ctx, err) } - conditions.SetControllerConditions(ctx, r.client, cnds) return reconcile.Result{}, err } @@ -114,13 +103,11 @@ func (r *Reconciler) Reconcile(ctx context.Context, request ctrl.Request) (ctrl. t = time.Now().UTC() setAnnotation(&node.ObjectMeta, annotationDrainStartTime, t.Format(time.RFC3339)) - err = r.client.Update(ctx, node) + err = r.Client.Update(ctx, node) if err != nil { - r.log.Error(err) - cnds.Degraded.Status = operatorv1.ConditionTrue - cnds.Degraded.Message = err.Error() + r.Log.Error(err) + r.SetDegraded(ctx, err) - conditions.SetControllerConditions(ctx, r.client, cnds) return reconcile.Result{}, err } } @@ -129,10 +116,9 @@ func (r *Reconciler) Reconcile(ctx context.Context, request ctrl.Request) (ctrl. deadline := t.Add(gracePeriod) now := time.Now() if deadline.After(now) { - cnds.Progressing.Status = operatorv1.ConditionTrue - cnds.Progressing.Message = fmt.Sprintf("Draining node %s", request.Name) + r.SetProgressing(ctx, fmt.Sprintf("Draining node %s", request.Name)) - return reconcile.Result{RequeueAfter: deadline.Sub(now)}, conditions.SetControllerConditions(ctx, r.client, cnds) + return reconcile.Result{RequeueAfter: deadline.Sub(now)}, nil } // drain the node disabling eviction @@ -145,16 +131,14 @@ func (r *Reconciler) Reconcile(ctx context.Context, request ctrl.Request) (ctrl. DeleteEmptyDirData: true, DisableEviction: true, OnPodDeletedOrEvicted: func(pod *corev1.Pod, usingEviction bool) { - r.log.Printf("deleted pod %s/%s", pod.Namespace, pod.Name) + r.Log.Printf("deleted pod %s/%s", pod.Namespace, pod.Name) }, - Out: r.log.Writer(), - ErrOut: r.log.Writer(), + Out: r.Log.Writer(), + ErrOut: r.Log.Writer(), }, request.Name) if err != nil { - r.log.Error(err) - cnds.Degraded.Status = operatorv1.ConditionTrue - cnds.Degraded.Message = err.Error() - conditions.SetControllerConditions(ctx, r.client, cnds) + r.Log.Error(err) + r.SetDegraded(ctx, err) return reconcile.Result{}, err } @@ -162,24 +146,16 @@ func (r *Reconciler) Reconcile(ctx context.Context, request ctrl.Request) (ctrl. // ensure our annotation is not set and return delete(node.Annotations, annotationDrainStartTime) - err = r.client.Update(ctx, node) + err = r.Client.Update(ctx, node) if err != nil { - r.log.Error(err) - cnds.Degraded.Status = operatorv1.ConditionTrue - cnds.Degraded.Message = err.Error() - conditions.SetControllerConditions(ctx, r.client, cnds) + r.Log.Error(err) + r.SetDegraded(ctx, err) + return reconcile.Result{}, err } - // set all conditions to default state - cnds.Available.Status = operatorv1.ConditionTrue - cnds.Available.Message = "" - cnds.Progressing.Status = operatorv1.ConditionFalse - cnds.Progressing.Message = "" - cnds.Degraded.Status = operatorv1.ConditionFalse - cnds.Degraded.Message = "" - - return reconcile.Result{}, conditions.SetControllerConditions(ctx, r.client, cnds) + r.ClearConditions(ctx) + return reconcile.Result{}, nil } // SetupWithManager setup our mananger diff --git a/pkg/operator/controllers/node/node_controller_test.go b/pkg/operator/controllers/node/node_controller_test.go index 50473ba86..1b7411f34 100644 --- a/pkg/operator/controllers/node/node_controller_test.go +++ b/pkg/operator/controllers/node/node_controller_test.go @@ -27,21 +27,10 @@ import ( func TestReconciler(t *testing.T) { transitionTime := metav1.Time{Time: time.Now()} - defaultAvailable := operatorv1.OperatorCondition{ - Type: ControllerName + "Controller" + operatorv1.OperatorStatusTypeAvailable, - Status: operatorv1.ConditionTrue, - LastTransitionTime: transitionTime, - } - defaultProgressing := operatorv1.OperatorCondition{ - Type: ControllerName + "Controller" + operatorv1.OperatorStatusTypeProgressing, - Status: operatorv1.ConditionFalse, - LastTransitionTime: transitionTime, - } - defaultDegraded := operatorv1.OperatorCondition{ - Type: ControllerName + "Controller" + operatorv1.OperatorStatusTypeDegraded, - Status: operatorv1.ConditionFalse, - LastTransitionTime: transitionTime, - } + defaultAvailable := utilconditions.ControllerDefaultAvailable(ControllerName) + defaultProgressing := utilconditions.ControllerDefaultProgressing(ControllerName) + defaultDegraded := utilconditions.ControllerDefaultDegraded(ControllerName) + defaultConditions := []operatorv1.OperatorCondition{defaultAvailable, defaultProgressing, defaultDegraded} tests := []struct { name string @@ -64,9 +53,10 @@ func TestReconciler(t *testing.T) { }, }, }, - featureFlag: true, - wantErr: "", - wantConditions: []operatorv1.OperatorCondition{defaultAvailable, defaultProgressing, defaultDegraded}, + featureFlag: true, + wantErr: "", + startConditions: defaultConditions, + wantConditions: defaultConditions, }, { name: "node doesn't exist", @@ -76,8 +66,9 @@ func TestReconciler(t *testing.T) { Name: "aro-fake-node-0", }, }, - featureFlag: true, - wantErr: `nodes "nonexistent-node" not found`, + featureFlag: true, + wantErr: `nodes "nonexistent-node" not found`, + startConditions: defaultConditions, wantConditions: []operatorv1.OperatorCondition{defaultAvailable, defaultProgressing, { Type: ControllerName + "Controller" + operatorv1.OperatorStatusTypeDegraded, @@ -113,9 +104,10 @@ func TestReconciler(t *testing.T) { }, }, }, - featureFlag: true, - wantErr: "", - wantConditions: []operatorv1.OperatorCondition{defaultAvailable, defaultProgressing, defaultDegraded}, + featureFlag: true, + wantErr: "", + startConditions: defaultConditions, + wantConditions: defaultConditions, }, { name: "isDraining false, delete our annotation", @@ -128,9 +120,10 @@ func TestReconciler(t *testing.T) { }, }, }, - featureFlag: true, - wantErr: "", - wantConditions: []operatorv1.OperatorCondition{defaultAvailable, defaultProgressing, defaultDegraded}, + featureFlag: true, + wantErr: "", + startConditions: defaultConditions, + wantConditions: defaultConditions, }, { name: "isDraining false, node is unschedulable=false", @@ -151,9 +144,10 @@ func TestReconciler(t *testing.T) { Unschedulable: false, }, }, - featureFlag: true, - wantErr: "", - wantConditions: []operatorv1.OperatorCondition{defaultAvailable, defaultProgressing, defaultDegraded}, + featureFlag: true, + wantErr: "", + startConditions: defaultConditions, + wantConditions: defaultConditions, }, { name: "isDraining false, annotationDesiredConfig is blank", @@ -180,9 +174,10 @@ func TestReconciler(t *testing.T) { }, }, }, - featureFlag: true, - wantErr: "", - wantConditions: []operatorv1.OperatorCondition{defaultAvailable, defaultProgressing, defaultDegraded}, + featureFlag: true, + wantErr: "", + startConditions: defaultConditions, + wantConditions: defaultConditions, }, { name: "isDraining false, annotationCurrentConfig is blank", @@ -209,9 +204,10 @@ func TestReconciler(t *testing.T) { }, }, }, - featureFlag: true, - wantErr: "", - wantConditions: []operatorv1.OperatorCondition{defaultAvailable, defaultProgressing, defaultDegraded}, + featureFlag: true, + wantErr: "", + startConditions: defaultConditions, + wantConditions: defaultConditions, }, { name: "isDraining false, no conditions are met", @@ -238,9 +234,10 @@ func TestReconciler(t *testing.T) { }, }, }, - featureFlag: true, - wantErr: "", - wantConditions: []operatorv1.OperatorCondition{defaultAvailable, defaultProgressing, defaultDegraded}, + featureFlag: true, + wantErr: "", + startConditions: defaultConditions, + wantConditions: defaultConditions, }, { name: "isDraining false, current config matches desired", @@ -267,9 +264,10 @@ func TestReconciler(t *testing.T) { }, }, }, - featureFlag: true, - wantErr: "", - wantConditions: []operatorv1.OperatorCondition{defaultAvailable, defaultProgressing, defaultDegraded}, + featureFlag: true, + wantErr: "", + startConditions: defaultConditions, + wantConditions: defaultConditions, }, { name: "isDraining true, set annotation", @@ -295,8 +293,9 @@ func TestReconciler(t *testing.T) { }, }, }, - featureFlag: true, - wantErr: "", + featureFlag: true, + wantErr: "", + startConditions: defaultConditions, wantConditions: []operatorv1.OperatorCondition{ defaultAvailable, { @@ -333,8 +332,9 @@ func TestReconciler(t *testing.T) { }, }, }, - featureFlag: true, - wantErr: "", + featureFlag: true, + wantErr: "", + startConditions: defaultConditions, wantConditions: []operatorv1.OperatorCondition{ defaultAvailable, { @@ -355,8 +355,9 @@ func TestReconciler(t *testing.T) { Annotations: nil, }, }, - featureFlag: true, - wantErr: "", + featureFlag: true, + wantErr: "", + startConditions: defaultConditions, wantConditions: []operatorv1.OperatorCondition{ defaultAvailable, defaultProgressing, @@ -392,12 +393,14 @@ func TestReconciler(t *testing.T) { featureFlag: true, wantErr: "", startConditions: []operatorv1.OperatorCondition{ + defaultAvailable, { Type: ControllerName + "Controller" + operatorv1.OperatorStatusTypeProgressing, Status: operatorv1.ConditionTrue, LastTransitionTime: transitionTime, Message: `Draining node aro-fake-node-0`, }, + defaultDegraded, }, wantConditions: []operatorv1.OperatorCondition{defaultAvailable, defaultProgressing, defaultDegraded}, }, @@ -428,12 +431,7 @@ func TestReconciler(t *testing.T) { client := clientBuilder.Build() - r := &Reconciler{ - log: logrus.NewEntry(logrus.StandardLogger()), - - kubernetescli: fake.NewSimpleClientset(tt.nodeObject), - client: client, - } + r := NewReconciler(logrus.NewEntry(logrus.StandardLogger()), client, fake.NewSimpleClientset(tt.nodeObject)) request := ctrl.Request{} request.Name = tt.nodeName diff --git a/pkg/util/conditions/controllerconditions.go b/pkg/util/conditions/controllerconditions.go deleted file mode 100644 index f5b4ba0fb..000000000 --- a/pkg/util/conditions/controllerconditions.go +++ /dev/null @@ -1,93 +0,0 @@ -package conditions - -// Copyright (c) Microsoft Corporation. -// Licensed under the Apache License 2.0. - -import ( - "context" - "fmt" - - operatorv1 "github.com/openshift/api/operator/v1" - "github.com/openshift/library-go/pkg/operator/v1helpers" - "k8s.io/apimachinery/pkg/api/equality" - "k8s.io/apimachinery/pkg/types" - "sigs.k8s.io/controller-runtime/pkg/client" - - arov1alpha1 "github.com/Azure/ARO-RP/pkg/operator/apis/aro.openshift.io/v1alpha1" -) - -type ControllerConditions struct { - Available, Progressing, Degraded *operatorv1.OperatorCondition -} - -func GetControllerConditions(ctx context.Context, c client.Client, controllerName string) (ControllerConditions, error) { - conditions := ControllerConditions{ - Available: &operatorv1.OperatorCondition{ - Type: conditionName(controllerName, operatorv1.OperatorStatusTypeAvailable), - Status: operatorv1.ConditionTrue, - }, - Progressing: &operatorv1.OperatorCondition{ - Type: conditionName(controllerName, operatorv1.OperatorStatusTypeProgressing), - Status: operatorv1.ConditionFalse, - }, - Degraded: &operatorv1.OperatorCondition{ - Type: conditionName(controllerName, operatorv1.OperatorStatusTypeDegraded), - Status: operatorv1.ConditionFalse, - }, - } - - cluster, err := getCluster(ctx, c) - if err != nil { - return conditions, err - } - - for _, cond := range cluster.Status.Conditions { - switch cond.Type { - case conditionName(controllerName, operatorv1.OperatorStatusTypeAvailable): - conditions.Available = &cond - case conditionName(controllerName, operatorv1.OperatorStatusTypeProgressing): - conditions.Progressing = &cond - case conditionName(controllerName, operatorv1.OperatorStatusTypeDegraded): - conditions.Degraded = &cond - } - } - - return conditions, nil -} - -func SetControllerConditions(ctx context.Context, c client.Client, conditions ControllerConditions) error { - cluster, err := getCluster(ctx, c) - if err != nil { - return err - } - - newConditions := cluster.Status.DeepCopy().Conditions - v1helpers.SetOperatorCondition(&newConditions, *conditions.Available) - v1helpers.SetOperatorCondition(&newConditions, *conditions.Progressing) - v1helpers.SetOperatorCondition(&newConditions, *conditions.Degraded) - - if equality.Semantic.DeepEqual(cluster.Status.Conditions, newConditions) { - return nil - } - - cluster.Status.Conditions = newConditions - if err := c.Status().Update(ctx, cluster); err != nil { - return fmt.Errorf("error updating controller conditions: %w", err) - } - return nil -} - -func getCluster(ctx context.Context, c client.Client) (*arov1alpha1.Cluster, error) { - cluster := &arov1alpha1.Cluster{} - - err := c.Get(ctx, types.NamespacedName{Name: arov1alpha1.SingletonClusterName}, cluster) - if err != nil { - return nil, err - } - - return cluster, nil -} - -func conditionName(controllerName string, conditionType string) string { - return controllerName + "Controller" + conditionType -} diff --git a/pkg/util/conditions/controllerconditions_test.go b/pkg/util/conditions/controllerconditions_test.go deleted file mode 100644 index 18c4366de..000000000 --- a/pkg/util/conditions/controllerconditions_test.go +++ /dev/null @@ -1,227 +0,0 @@ -package conditions - -// Copyright (c) Microsoft Corporation. -// Licensed under the Apache License 2.0. - -import ( - "context" - "testing" - "time" - - "github.com/google/go-cmp/cmp/cmpopts" - operatorv1 "github.com/openshift/api/operator/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/types" - "k8s.io/apimachinery/pkg/util/clock" - "sigs.k8s.io/controller-runtime/pkg/client/fake" - - arov1alpha1 "github.com/Azure/ARO-RP/pkg/operator/apis/aro.openshift.io/v1alpha1" - "github.com/Azure/ARO-RP/pkg/util/cmp" - utilconditions "github.com/Azure/ARO-RP/test/util/conditions" -) - -func TestSetControllerConditions(t *testing.T) { - ctx := context.Background() - objectName := "cluster" - version := "unknown" - - kubeclock = clock.NewFakeClock(time.Date(2023, 1, 1, 0, 0, 0, 0, time.UTC)) - var now = metav1.NewTime(time.Now()) - var past = metav1.NewTime(now.Add(-1 * time.Hour)) - - for _, tt := range []struct { - name string - cluster arov1alpha1.Cluster - input ControllerConditions - expected []operatorv1.OperatorCondition - wantErr error - }{ - - { - name: "sets all provided conditions", - cluster: arov1alpha1.Cluster{ - ObjectMeta: metav1.ObjectMeta{Name: objectName}, - Status: arov1alpha1.ClusterStatus{ - Conditions: []operatorv1.OperatorCondition{ - { - Type: arov1alpha1.InternetReachableFromMaster, - Status: operatorv1.ConditionFalse, - LastTransitionTime: now, - }, - }, - OperatorVersion: version, - }, - }, - input: ControllerConditions{ - Available: &operatorv1.OperatorCondition{ - Type: "FakeController" + operatorv1.OperatorStatusTypeAvailable, - Status: operatorv1.ConditionTrue, - }, - Progressing: &operatorv1.OperatorCondition{ - Type: "FakeController" + operatorv1.OperatorStatusTypeProgressing, - Status: operatorv1.ConditionFalse, - }, - Degraded: &operatorv1.OperatorCondition{ - Type: "FakeController" + operatorv1.OperatorStatusTypeDegraded, - Status: operatorv1.ConditionFalse, - }, - }, - expected: []operatorv1.OperatorCondition{ - { - Type: arov1alpha1.InternetReachableFromMaster, - Status: operatorv1.ConditionFalse, - LastTransitionTime: now, - }, - { - Type: "FakeController" + operatorv1.OperatorStatusTypeAvailable, - Status: operatorv1.ConditionTrue, - LastTransitionTime: now, - }, - { - Type: "FakeController" + operatorv1.OperatorStatusTypeProgressing, - Status: operatorv1.ConditionFalse, - LastTransitionTime: now, - }, - { - Type: "FakeController" + operatorv1.OperatorStatusTypeDegraded, - Status: operatorv1.ConditionFalse, - LastTransitionTime: now, - }, - }, - }, - { - name: "if condition exists and status matches, does not update", - cluster: arov1alpha1.Cluster{ - ObjectMeta: metav1.ObjectMeta{Name: objectName}, - Status: arov1alpha1.ClusterStatus{ - Conditions: []operatorv1.OperatorCondition{ - { - Type: arov1alpha1.InternetReachableFromMaster, - Status: operatorv1.ConditionFalse, - LastTransitionTime: now, - }, - { - Type: "FakeController" + operatorv1.OperatorStatusTypeAvailable, - Status: operatorv1.ConditionTrue, - LastTransitionTime: past, - }, - }, - OperatorVersion: version, - }, - }, - input: ControllerConditions{ - Available: &operatorv1.OperatorCondition{ - Type: "FakeController" + operatorv1.OperatorStatusTypeAvailable, - Status: operatorv1.ConditionTrue, - }, - Progressing: &operatorv1.OperatorCondition{ - Type: "FakeController" + operatorv1.OperatorStatusTypeProgressing, - Status: operatorv1.ConditionFalse, - }, - Degraded: &operatorv1.OperatorCondition{ - Type: "FakeController" + operatorv1.OperatorStatusTypeDegraded, - Status: operatorv1.ConditionFalse, - }, - }, - expected: []operatorv1.OperatorCondition{ - { - Type: arov1alpha1.InternetReachableFromMaster, - Status: operatorv1.ConditionFalse, - LastTransitionTime: now, - }, - { - Type: "FakeController" + operatorv1.OperatorStatusTypeAvailable, - Status: operatorv1.ConditionTrue, - LastTransitionTime: past, - }, - { - Type: "FakeController" + operatorv1.OperatorStatusTypeProgressing, - Status: operatorv1.ConditionFalse, - LastTransitionTime: now, - }, - { - Type: "FakeController" + operatorv1.OperatorStatusTypeDegraded, - Status: operatorv1.ConditionFalse, - LastTransitionTime: now, - }, - }, - }, - { - name: "if condition exists and status does not match, updates", - cluster: arov1alpha1.Cluster{ - ObjectMeta: metav1.ObjectMeta{Name: objectName}, - Status: arov1alpha1.ClusterStatus{ - Conditions: []operatorv1.OperatorCondition{ - { - Type: arov1alpha1.InternetReachableFromMaster, - Status: operatorv1.ConditionFalse, - LastTransitionTime: now, - }, - { - Type: "FakeController" + operatorv1.OperatorStatusTypeAvailable, - Status: operatorv1.ConditionTrue, - LastTransitionTime: past, - }, - }, - OperatorVersion: version, - }, - }, - input: ControllerConditions{ - Available: &operatorv1.OperatorCondition{ - Type: "FakeController" + operatorv1.OperatorStatusTypeAvailable, - Status: operatorv1.ConditionFalse, - }, - Progressing: &operatorv1.OperatorCondition{ - Type: "FakeController" + operatorv1.OperatorStatusTypeProgressing, - Status: operatorv1.ConditionFalse, - }, - Degraded: &operatorv1.OperatorCondition{ - Type: "FakeController" + operatorv1.OperatorStatusTypeDegraded, - Status: operatorv1.ConditionFalse, - }, - }, - expected: []operatorv1.OperatorCondition{ - { - Type: arov1alpha1.InternetReachableFromMaster, - Status: operatorv1.ConditionFalse, - LastTransitionTime: now, - }, - { - Type: "FakeController" + operatorv1.OperatorStatusTypeAvailable, - Status: operatorv1.ConditionFalse, - LastTransitionTime: now, - }, - { - Type: "FakeController" + operatorv1.OperatorStatusTypeProgressing, - Status: operatorv1.ConditionFalse, - LastTransitionTime: now, - }, - { - Type: "FakeController" + operatorv1.OperatorStatusTypeDegraded, - Status: operatorv1.ConditionFalse, - LastTransitionTime: now, - }, - }, - }, - } { - t.Run(tt.name, func(t *testing.T) { - clientFake := fake.NewClientBuilder().WithObjects(&tt.cluster).Build() - - err := SetControllerConditions(ctx, clientFake, tt.input) - if err != tt.wantErr { - t.Fatalf("wanted error %v, got %v", tt.wantErr, err) - } - - utilconditions.AssertControllerConditions(t, ctx, clientFake, tt.expected) - - result := &arov1alpha1.Cluster{} - if err = clientFake.Get(ctx, types.NamespacedName{Name: arov1alpha1.SingletonClusterName}, result); err != nil { - t.Fatal(err.Error()) - } - - if diff := cmp.Diff(result.Status.Conditions, tt.expected, cmpopts.EquateApproxTime(time.Second)); diff != "" { - t.Fatal(diff) - } - }) - } -} diff --git a/test/util/conditions/conditions.go b/test/util/conditions/conditions.go index 2357e9785..4d1dc5b7e 100644 --- a/test/util/conditions/conditions.go +++ b/test/util/conditions/conditions.go @@ -11,6 +11,7 @@ import ( "github.com/google/go-cmp/cmp/cmpopts" operatorv1 "github.com/openshift/api/operator/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/client" @@ -20,6 +21,7 @@ import ( // AssertControllerConditions asserts that the ARO cluster resource contains the conditions expected in wantConditions. func AssertControllerConditions(t *testing.T, ctx context.Context, client client.Client, wantConditions []operatorv1.OperatorCondition) { + t.Helper() if len(wantConditions) == 0 { return } @@ -33,12 +35,36 @@ func AssertControllerConditions(t *testing.T, ctx context.Context, client client if err != nil { t.Error(err) } - if diff := cmp.Diff(gotCondition, &wantCondition, cmpopts.EquateApproxTime(time.Second)); diff != "" { + if diff := cmp.Diff(&wantCondition, gotCondition, cmpopts.EquateApproxTime(time.Second)); diff != "" { t.Error(diff) } } } +func ControllerDefaultAvailable(name string) operatorv1.OperatorCondition { + return operatorv1.OperatorCondition{ + Type: fmt.Sprintf("%sController%s", name, operatorv1.OperatorStatusTypeAvailable), + Status: operatorv1.ConditionTrue, + LastTransitionTime: metav1.NewTime(time.Now()), + } +} + +func ControllerDefaultProgressing(name string) operatorv1.OperatorCondition { + return operatorv1.OperatorCondition{ + Type: fmt.Sprintf("%sController%s", name, operatorv1.OperatorStatusTypeProgressing), + Status: operatorv1.ConditionFalse, + LastTransitionTime: metav1.NewTime(time.Now()), + } +} + +func ControllerDefaultDegraded(name string) operatorv1.OperatorCondition { + return operatorv1.OperatorCondition{ + Type: fmt.Sprintf("%sController%s", name, operatorv1.OperatorStatusTypeDegraded), + Status: operatorv1.ConditionFalse, + LastTransitionTime: metav1.NewTime(time.Now()), + } +} + func findCondition(conditions []operatorv1.OperatorCondition, conditionType string) (*operatorv1.OperatorCondition, error) { for _, cond := range conditions { if cond.Type == conditionType { diff --git a/test/util/error/error.go b/test/util/error/error.go index 78bc21bf1..1e1b1fc4a 100644 --- a/test/util/error/error.go +++ b/test/util/error/error.go @@ -7,6 +7,7 @@ import "testing" // AssertErrorMessage asserts that err.Error() is equal to wantMsg. func AssertErrorMessage(t *testing.T, err error, wantMsg string) { + t.Helper() if err == nil && wantMsg != "" { t.Errorf("did not get an error, but wanted error '%v'", wantMsg) }