Skip to content

Commit

Permalink
Propagate errors of ARO DNSMasqMachineConfig controller
Browse files Browse the repository at this point in the history 
JIRA: https://issues.redhat.com/browse/ARO-3254
  • Loading branch information
@SrinivasAtmakuri
SrinivasAtmakuri committed Jul 12, 2023
1 parent f87f198 commit b025e27
Show file tree
Hide file tree
Showing 2 changed files with 60 additions and 29 deletions.
32 changes: 18 additions & 14 deletions pkg/operator/controllers/dnsmasq/machineconfig_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,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/operator/controllers/base"
"github.com/Azure/ARO-RP/pkg/util/dynamichelper"
)

Expand All @@ -24,51 +24,53 @@ const (
)

type MachineConfigReconciler struct {
log *logrus.Entry
base.AROController

dh dynamichelper.Interface

client client.Client
}

var rxARODNS = regexp.MustCompile("^99-(.*)-aro-dns$")

func NewMachineConfigReconciler(log *logrus.Entry, client client.Client, dh dynamichelper.Interface) *MachineConfigReconciler {
return &MachineConfigReconciler{
log: log,
dh: dh,
client: client,
AROController: base.AROController{
Log: log,
Client: client,
Name: MachineConfigControllerName,
},
dh: dh,
}
}

// Reconcile watches ARO DNS MachineConfig objects, and if any changes,
// reconciles it
func (r *MachineConfigReconciler) 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")
m := rxARODNS.FindStringSubmatch(request.Name)
if m == nil {
return reconcile.Result{}, nil
}
role := m[1]

mcp := &mcv1.MachineConfigPool{}
err = r.client.Get(ctx, types.NamespacedName{Name: role}, mcp)
err = r.Client.Get(ctx, types.NamespacedName{Name: role}, mcp)
if kerrors.IsNotFound(err) {
r.ClearDegraded(ctx)
return reconcile.Result{}, nil
}
if err != nil {
r.log.Error(err)
r.Log.Error(err)
r.SetDegraded(ctx, err)
return reconcile.Result{}, err
}
if mcp.GetDeletionTimestamp() != nil {
Expand All @@ -77,10 +79,12 @@ func (r *MachineConfigReconciler) Reconcile(ctx context.Context, request ctrl.Re

err = reconcileMachineConfigs(ctx, instance, r.dh, *mcp)
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
57 changes: 42 additions & 15 deletions pkg/operator/controllers/dnsmasq/machineconfig_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 @@ -18,20 +20,27 @@ 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 TestMachineConfigReconciler(t *testing.T) {
transitionTime := metav1.Time{Time: time.Now()}
defaultAvailable := utilconditions.ControllerDefaultAvailable(MachineConfigControllerName)
defaultProgressing := utilconditions.ControllerDefaultProgressing(MachineConfigControllerName)
defaultDegraded := utilconditions.ControllerDefaultDegraded(MachineConfigControllerName)
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
wantConditions []operatorv1.OperatorCondition
}{
{
name: "no cluster",
Expand All @@ -45,24 +54,37 @@ func TestMachineConfigReconciler(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: "",
wantConditions: defaultConditions,
},
{
name: "no MachineConfigPool for MachineConfig does nothing",
objects: []client.Object{
&arov1alpha1.Cluster{
ObjectMeta: metav1.ObjectMeta{Name: "cluster"},
Status: arov1alpha1.ClusterStatus{},
Status: arov1alpha1.ClusterStatus{
Conditions: []operatorv1.OperatorCondition{
defaultAvailable,
defaultProgressing,
{
Type: MachineConfigControllerName + "Controller" + operatorv1.OperatorStatusTypeDegraded,
Status: operatorv1.ConditionTrue,
LastTransitionTime: transitionTime,
},
},
},
Spec: arov1alpha1.ClusterSpec{
OperatorFlags: arov1alpha1.OperatorFlags{
controllerEnabled: "true",
Expand All @@ -77,14 +99,17 @@ func TestMachineConfigReconciler(t *testing.T) {
Name: "99-custom-aro-dns",
},
},
wantErrMsg: "",
wantErrMsg: "",
wantConditions: defaultConditions,
},
{
name: "valid MachineConfigPool for MachineConfig reconciles 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 @@ -106,7 +131,8 @@ func TestMachineConfigReconciler(t *testing.T) {
Name: "99-custom-aro-dns",
},
},
wantErrMsg: "",
wantErrMsg: "",
wantConditions: defaultConditions,
},
}

Expand All @@ -126,10 +152,11 @@ func TestMachineConfigReconciler(t *testing.T) {
client,
dh,
)

_, err := r.Reconcile(context.Background(), tt.request)
ctx := context.Background()
_, err := r.Reconcile(ctx, tt.request)

utilerror.AssertErrorMessage(t, err, tt.wantErrMsg)
utilconditions.AssertControllerConditions(t, ctx, client, tt.wantConditions)
})
}
}

0 comments on commit b025e27

Please sign in to comment.