Skip to content

Commit

Permalink
Propagate errors of ARO Ingress controller to ARO Operator (#2940)
Browse files Browse the repository at this point in the history 
* Propagate errors of ARO Ingress controller to Operator

* Modified the test cases to use the new conditions
  • Loading branch information
@SrinivasAtmakuri
SrinivasAtmakuri authored Jul 4, 2023
1 parent 0378d0e commit f1d9035
Show file tree
Hide file tree
Showing 2 changed files with 72 additions and 18 deletions.
30 changes: 17 additions & 13 deletions pkg/operator/controllers/ingress/ingress_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,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"
)

const (
Expand All @@ -31,47 +32,50 @@ const (
// Reconciler spots openshift ingress controllers has abnormal replica counts (less than 2)
// when happens, it tries to rescale the controller to 2 replicas, i.e., the minimum required replicas
type Reconciler struct {
log *logrus.Entry

client client.Client
base.AROController
}

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")
ingress := &operatorv1.IngressController{}
err = r.client.Get(ctx, types.NamespacedName{Namespace: openshiftIngressControllerNamespace, Name: openshiftIngressControllerName}, ingress)
err = r.Client.Get(ctx, types.NamespacedName{Namespace: openshiftIngressControllerNamespace, Name: openshiftIngressControllerName}, ingress)
if err != nil {
r.log.Error(err)
r.Log.Error(err)
r.SetDegraded(ctx, err)
return reconcile.Result{}, err
}

if ingress.Spec.Replicas != nil && *ingress.Spec.Replicas < minimumReplicas {
ingress.Spec.Replicas = to.Int32Ptr(minimumReplicas)
err := r.client.Update(ctx, ingress)
err := r.Client.Update(ctx, ingress)
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
}

Expand Down
60 changes: 55 additions & 5 deletions pkg/operator/controllers/ingress/ingress_controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ package ingress
import (
"context"
"testing"
"time"

"github.com/Azure/go-autorest/autorest/to"
operatorv1 "github.com/openshift/api/operator/v1"
Expand All @@ -17,10 +18,17 @@ import (

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"
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}

fakeCluster := func(controllerEnabledFlag string) *arov1alpha1.Cluster {
return &arov1alpha1.Cluster{
ObjectMeta: metav1.ObjectMeta{
Expand All @@ -40,15 +48,30 @@ func TestReconciler(t *testing.T) {
ingressController *operatorv1.IngressController
expectedReplica int32
expectedError string
startConditions []operatorv1.OperatorCondition
wantConditions []operatorv1.OperatorCondition
}{
{
name: "aro ingress controller disabled",
controllerEnabledFlag: "false",
startConditions: defaultConditions,
wantConditions: defaultConditions,
},
{
name: "openshift ingress controller not found",
controllerEnabledFlag: "true",
expectedError: "ingresscontrollers.operator.openshift.io \"default\" not found",
startConditions: defaultConditions,
wantConditions: []operatorv1.OperatorCondition{
defaultAvailable,
defaultProgressing,
{
Type: ControllerName + "Controller" + operatorv1.OperatorStatusTypeDegraded,
Status: operatorv1.ConditionTrue,
LastTransitionTime: transitionTime,
Message: `ingresscontrollers.operator.openshift.io "default" not found`,
},
},
},
{
name: "openshift ingress controller has 3 replicas",
Expand All @@ -63,6 +86,8 @@ func TestReconciler(t *testing.T) {
},
},
expectedReplica: 3,
startConditions: defaultConditions,
wantConditions: defaultConditions,
},
{
name: "openshift ingress controller has 2 replicas (minimum required replicas)",
Expand All @@ -77,6 +102,8 @@ func TestReconciler(t *testing.T) {
},
},
expectedReplica: minimumReplicas,
startConditions: defaultConditions,
wantConditions: defaultConditions,
},
{
name: "openshift ingress controller has 1 replica",
Expand All @@ -91,6 +118,17 @@ func TestReconciler(t *testing.T) {
},
},
expectedReplica: minimumReplicas,
startConditions: []operatorv1.OperatorCondition{
defaultAvailable,
defaultProgressing,
{
Type: ControllerName + "Controller" + operatorv1.OperatorStatusTypeDegraded,
Status: operatorv1.ConditionTrue,
LastTransitionTime: transitionTime,
Message: `ingresscontrollers.operator.openshift.io has 1 replica`,
},
},
wantConditions: defaultConditions,
},
{
name: "openshift ingress controller has 0 replica",
Expand All @@ -105,33 +143,45 @@ func TestReconciler(t *testing.T) {
},
},
expectedReplica: minimumReplicas,
startConditions: []operatorv1.OperatorCondition{
defaultAvailable,
defaultProgressing,
{
Type: ControllerName + "Controller" + operatorv1.OperatorStatusTypeDegraded,
Status: operatorv1.ConditionTrue,
LastTransitionTime: transitionTime,
Message: `ingresscontrollers.operator.openshift.io has 0 replica`,
},
},
wantConditions: defaultConditions,
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
clusterMock := fakeCluster(tt.controllerEnabledFlag)
if len(tt.startConditions) > 0 {
clusterMock.Status.Conditions = append(clusterMock.Status.Conditions, tt.startConditions...)
}

clientBuilder := ctrlfake.NewClientBuilder().WithObjects(clusterMock)
if tt.ingressController != nil {
clientBuilder = clientBuilder.WithObjects(tt.ingressController)
}
clientFake := clientBuilder.Build()

r := &Reconciler{
log: logrus.NewEntry(logrus.StandardLogger()),
client: clientFake,
}
r := NewReconciler(logrus.NewEntry(logrus.StandardLogger()), clientFake)

request := ctrl.Request{}
ctx := context.Background()

_, err := r.Reconcile(ctx, request)
utilerror.AssertErrorMessage(t, err, tt.expectedError)
utilconditions.AssertControllerConditions(t, ctx, clientFake, tt.wantConditions)

if tt.ingressController != nil {
ingress := &operatorv1.IngressController{}
err = r.client.Get(ctx, types.NamespacedName{Namespace: openshiftIngressControllerNamespace, Name: openshiftIngressControllerName}, ingress)
err = r.Client.Get(ctx, types.NamespacedName{Namespace: openshiftIngressControllerNamespace, Name: openshiftIngressControllerName}, ingress)
if err != nil {
t.Error(err)
}
Expand Down

0 comments on commit f1d9035

Please sign in to comment.