Skip to content

Commit

Permalink
Add status conditions to operator MachineSet controller (#2899)
Browse files Browse the repository at this point in the history
* Add status conditions to operator MachineSet controller
* Use base ARO GetCluster method for retrieving cluster resource in derived controllers
* Add helper functions for status
* Refactor operator controller conditions tests to use shared default conditions
  • Loading branch information
tsatam authored Jun 6, 2023
1 parent 477c735 commit a8012e5
Show file tree
Hide file tree
Showing 10 changed files with 486 additions and 497 deletions.
105 changes: 105 additions & 0 deletions pkg/operator/controllers/base/aro_controller.go
Original file line number Diff line number Diff line change
@@ -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
}
143 changes: 143 additions & 0 deletions pkg/operator/controllers/base/aro_controller_test.go
Original file line number Diff line number Diff line change
@@ -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)
})
}
}
41 changes: 27 additions & 14 deletions pkg/operator/controllers/machineset/machineset_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand All @@ -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
}

Expand All @@ -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 {
Expand All @@ -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
}

Expand Down
Loading

0 comments on commit a8012e5

Please sign in to comment.