Skip to content

Commit

Permalink
Propagate errors of ARO DNSMasqCluster controller to ARO Operator
Browse files Browse the repository at this point in the history
  • Loading branch information
@SrinivasAtmakuri
SrinivasAtmakuri committed Jun 14, 2023
1 parent 07fa8b3 commit dffeee0
Show file tree
Hide file tree
Showing 2 changed files with 71 additions and 29 deletions.
32 changes: 17 additions & 15 deletions pkg/operator/controllers/dnsmasq/cluster_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,14 @@ 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"
"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"
"github.com/Azure/ARO-RP/pkg/util/dynamichelper"
)

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

Expand Down
68 changes: 54 additions & 14 deletions pkg/operator/controllers/dnsmasq/cluster_controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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",
Expand All @@ -44,24 +55,38 @@ 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",
},
},
},
},
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",
Expand All @@ -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",
Expand All @@ -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,
},
}

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

0 comments on commit dffeee0

Please sign in to comment.