Splits DNS checker into a separate controller

cmd/aro/operator.go

@@ -27,6 +27,7 @@ import (
 	"github.com/Azure/ARO-RP/pkg/operator/controllers/autosizednodes"
 	"github.com/Azure/ARO-RP/pkg/operator/controllers/banner"
 	"github.com/Azure/ARO-RP/pkg/operator/controllers/checker"
+	"github.com/Azure/ARO-RP/pkg/operator/controllers/checkers/clusterdnschecker"
 	"github.com/Azure/ARO-RP/pkg/operator/controllers/checkers/internetchecker"
 	"github.com/Azure/ARO-RP/pkg/operator/controllers/checkers/serviceprincipalchecker"
 	"github.com/Azure/ARO-RP/pkg/operator/controllers/clusteroperatoraro"
@@ -249,6 +250,11 @@ func operator(ctx context.Context, log *logrus.Entry) error {
 			arocli, kubernetescli, role)).SetupWithManager(mgr); err != nil {
 			return fmt.Errorf("unable to create controller %s: %v", serviceprincipalchecker.ControllerName, err)
 		}
+		if err = (clusterdnschecker.NewReconciler(
+			log.WithField("controller", clusterdnschecker.ControllerName),
+			arocli, operatorcli, role)).SetupWithManager(mgr); err != nil {
+			return fmt.Errorf("unable to create controller %s: %v", clusterdnschecker.ControllerName, err)
+		}
 	}
 
 	if err = (internetchecker.NewReconciler(

pkg/operator/controllers/checker/checker_controller.go

@@ -39,7 +39,6 @@ type Reconciler struct {
 func NewReconciler(log *logrus.Entry, arocli aroclient.Interface, kubernetescli kubernetes.Interface, machinecli machineclient.Interface, operatorcli operatorclient.Interface, configcli configclient.Interface, role string) *Reconciler {
 	checkers := []Checker{
 		NewIngressCertificateChecker(log, arocli, operatorcli, configcli, role),
-		NewClusterDNSChecker(log, arocli, operatorcli, role),
 	}
 
 	return &Reconciler{

pkg/operator/controllers/checker/clusterdns.go

@@ -1,89 +0,0 @@
-// Implements a check that provides detail on openshift-dns-operator
-// configurations.
-//
-// Included checks are:
-//  - existence of custom DNS entries
-//  - malformed zone forwarding configuration
-
-package checker
-
-// Copyright (c) Microsoft Corporation.
-// Licensed under the Apache License 2.0.
-
-import (
-	"context"
-	"fmt"
-	"strings"
-
-	operatorv1 "github.com/openshift/api/operator/v1"
-	operatorclient "github.com/openshift/client-go/operator/clientset/versioned"
-	"github.com/sirupsen/logrus"
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-
-	arov1alpha1 "github.com/Azure/ARO-RP/pkg/operator/apis/aro.openshift.io/v1alpha1"
-	aroclient "github.com/Azure/ARO-RP/pkg/operator/clientset/versioned"
-	"github.com/Azure/ARO-RP/pkg/util/conditions"
-)
-
-type ClusterDNSChecker struct {
-	log *logrus.Entry
-
-	arocli      aroclient.Interface
-	operatorcli operatorclient.Interface
-
-	role string
-}
-
-func NewClusterDNSChecker(log *logrus.Entry, arocli aroclient.Interface, operatorcli operatorclient.Interface, role string) *ClusterDNSChecker {
-	return &ClusterDNSChecker{
-		log:         log,
-		arocli:      arocli,
-		operatorcli: operatorcli,
-	}
-}
-
-func (r *ClusterDNSChecker) Name() string {
-	return "ClusterDNSChecker"
-}
-
-func (r *ClusterDNSChecker) Check(ctx context.Context) error {
-	cond := &operatorv1.OperatorCondition{
-		Type:    arov1alpha1.DefaultClusterDNS,
-		Status:  operatorv1.ConditionUnknown,
-		Message: "",
-		Reason:  "CheckDone",
-	}
-
-	dns, err := r.operatorcli.OperatorV1().DNSes().Get(ctx, "default", metav1.GetOptions{})
-	if err != nil {
-		cond.Message = err.Error()
-		cond.Reason = "CheckFailed"
-		return conditions.SetCondition(ctx, r.arocli, cond, r.role)
-	}
-
-	var upstreams []string
-	for _, s := range dns.Spec.Servers {
-		for _, z := range s.Zones {
-			if z == "." {
-				// If "." is set as a zone, bail out and warn about the
-				// malformed config, as this will prevent CoreDNS from rolling
-				// out
-				cond.Message = `Malformed config: "." in zones`
-				cond.Status = operatorv1.ConditionFalse
-				return conditions.SetCondition(ctx, r.arocli, cond, r.role)
-			}
-		}
-
-		upstreams = append(upstreams, s.ForwardPlugin.Upstreams...)
-	}
-
-	if len(upstreams) > 0 {
-		cond.Status = operatorv1.ConditionFalse
-		cond.Message = fmt.Sprintf("Custom upstream DNS servers in use: %s", strings.Join(upstreams, ", "))
-	} else {
-		cond.Status = operatorv1.ConditionTrue
-		cond.Message = "No in-cluster upstream DNS servers"
-	}
-
-	return conditions.SetCondition(ctx, r.arocli, cond, r.role)
-}

pkg/operator/controllers/checker/clusterdns_test.go

@@ -1,193 +0,0 @@
-package checker
-
-// Copyright (c) Microsoft Corporation.
-// Licensed under the Apache License 2.0.
-
-import (
-	"context"
-	"testing"
-	"time"
-
-	"github.com/go-test/deep"
-	operatorv1 "github.com/openshift/api/operator/v1"
-	operatorfake "github.com/openshift/client-go/operator/clientset/versioned/fake"
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-
-	arov1alpha1 "github.com/Azure/ARO-RP/pkg/operator/apis/aro.openshift.io/v1alpha1"
-	arofake "github.com/Azure/ARO-RP/pkg/operator/clientset/versioned/fake"
-)
-
-func TestDefaultClusterDNS(t *testing.T) {
-	ctx := context.Background()
-
-	for _, tt := range []struct {
-		name          string
-		aroCluster    *arov1alpha1.Cluster
-		dns           *operatorv1.DNS
-		expectedState operatorv1.OperatorCondition
-	}{
-		{
-			name: "run: has default DNS",
-			aroCluster: &arov1alpha1.Cluster{
-				ObjectMeta: metav1.ObjectMeta{
-					Name: arov1alpha1.SingletonClusterName,
-				},
-			},
-			dns: &operatorv1.DNS{
-				ObjectMeta: metav1.ObjectMeta{
-					Name: "default",
-				},
-				Spec: operatorv1.DNSSpec{},
-			},
-			expectedState: operatorv1.OperatorCondition{
-				Type:    arov1alpha1.DefaultClusterDNS,
-				Status:  operatorv1.ConditionTrue,
-				Reason:  "CheckDone",
-				Message: "No in-cluster upstream DNS servers",
-			},
-		},
-		{
-			name: "run: has changed DNS",
-			aroCluster: &arov1alpha1.Cluster{
-				ObjectMeta: metav1.ObjectMeta{
-					Name: arov1alpha1.SingletonClusterName,
-				},
-			},
-			dns: &operatorv1.DNS{
-				ObjectMeta: metav1.ObjectMeta{
-					Name: "default",
-				},
-				Spec: operatorv1.DNSSpec{
-					Servers: []operatorv1.Server{
-						{
-							Name:  "test-server",
-							Zones: []string{"example.com"},
-							ForwardPlugin: operatorv1.ForwardPlugin{
-								Upstreams: []string{
-									"1.2.3.4", "5.6.7.8",
-								},
-							},
-						},
-					},
-				},
-			},
-			expectedState: operatorv1.OperatorCondition{
-				Type:    arov1alpha1.DefaultClusterDNS,
-				Status:  operatorv1.ConditionFalse,
-				Reason:  "CheckDone",
-				Message: "Custom upstream DNS servers in use: 1.2.3.4, 5.6.7.8",
-			},
-		},
-		{
-			name: "run: malformed, servers but no forwardplugin",
-			aroCluster: &arov1alpha1.Cluster{
-				ObjectMeta: metav1.ObjectMeta{
-					Name: arov1alpha1.SingletonClusterName,
-				},
-			},
-			dns: &operatorv1.DNS{
-				ObjectMeta: metav1.ObjectMeta{
-					Name: "default",
-				},
-				Spec: operatorv1.DNSSpec{
-					Servers: []operatorv1.Server{
-						{
-							Name:  "test-server",
-							Zones: []string{"example.com"},
-						},
-					},
-				},
-			},
-			expectedState: operatorv1.OperatorCondition{
-				Type:    arov1alpha1.DefaultClusterDNS,
-				Status:  operatorv1.ConditionTrue,
-				Reason:  "CheckDone",
-				Message: "No in-cluster upstream DNS servers",
-			},
-		},
-		{
-			name: "fail: malformed, using . for zones",
-			aroCluster: &arov1alpha1.Cluster{
-				ObjectMeta: metav1.ObjectMeta{
-					Name: arov1alpha1.SingletonClusterName,
-				},
-			},
-			dns: &operatorv1.DNS{
-				ObjectMeta: metav1.ObjectMeta{
-					Name: "default",
-				},
-				Spec: operatorv1.DNSSpec{
-					Servers: []operatorv1.Server{
-						{
-							Name:  "test-server",
-							Zones: []string{"."},
-							ForwardPlugin: operatorv1.ForwardPlugin{
-								Upstreams: []string{
-									"1.2.3.4", "5.6.7.8",
-								},
-							},
-						},
-					},
-				},
-			},
-			expectedState: operatorv1.OperatorCondition{
-				Type:    arov1alpha1.DefaultClusterDNS,
-				Status:  operatorv1.ConditionFalse,
-				Reason:  "CheckDone",
-				Message: `Malformed config: "." in zones`,
-			},
-		},
-		{
-			name: "fail: no DNS",
-			aroCluster: &arov1alpha1.Cluster{
-				ObjectMeta: metav1.ObjectMeta{
-					Name: arov1alpha1.SingletonClusterName,
-				},
-			},
-			expectedState: operatorv1.OperatorCondition{
-				Type:    arov1alpha1.DefaultClusterDNS,
-				Status:  operatorv1.ConditionUnknown,
-				Reason:  "CheckFailed",
-				Message: `dnses.operator.openshift.io "default" not found`,
-			},
-		},
-	} {
-		arocli := arofake.NewSimpleClientset()
-		operatorcli := operatorfake.NewSimpleClientset()
-
-		if tt.aroCluster != nil {
-			arocli = arofake.NewSimpleClientset(tt.aroCluster)
-		}
-		if tt.dns != nil {
-			operatorcli = operatorfake.NewSimpleClientset(tt.dns)
-		}
-
-		sp := NewClusterDNSChecker(nil, arocli, operatorcli, "")
-
-		t.Run(tt.name, func(t *testing.T) {
-			err := sp.Check(ctx)
-
-			if err != nil {
-				t.Error(err)
-			}
-
-			cluster, err := arocli.AroV1alpha1().Clusters().Get(ctx, arov1alpha1.SingletonClusterName, metav1.GetOptions{})
-			if err != nil {
-				t.Error(err)
-			}
-
-			conds := []operatorv1.OperatorCondition{}
-
-			// nil out time
-			for _, c := range cluster.Status.Conditions {
-				c.LastTransitionTime = metav1.NewTime(time.Time{})
-				conds = append(conds, c)
-			}
-
-			errs := deep.Equal(conds, []operatorv1.OperatorCondition{tt.expectedState})
-			for _, err := range errs {
-				t.Error(err)
-			}
-		})
-	}
-}

pkg/operator/controllers/checkers/clusterdnschecker/checker.go

@@ -0,0 +1,54 @@
+package clusterdnschecker
+
+// Copyright (c) Microsoft Corporation.
+// Licensed under the Apache License 2.0.
+
+import (
+	"context"
+	"fmt"
+	"strings"
+
+	operatorclient "github.com/openshift/client-go/operator/clientset/versioned"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+)
+
+type clusterDNSChecker interface {
+	Check(ctx context.Context) error
+}
+
+type checker struct {
+	operatorcli operatorclient.Interface
+}
+
+func newClusterDNSChecker(operatorcli operatorclient.Interface) clusterDNSChecker {
+	return &checker{
+		operatorcli: operatorcli,
+	}
+}
+
+func (r *checker) Check(ctx context.Context) error {
+	dns, err := r.operatorcli.OperatorV1().DNSes().Get(ctx, "default", metav1.GetOptions{})
+	if err != nil {
+		return err
+	}
+
+	var upstreams []string
+	for _, s := range dns.Spec.Servers {
+		for _, z := range s.Zones {
+			if z == "." {
+				// If "." is set as a zone, bail out and warn about the
+				// malformed config, as this will prevent CoreDNS from rolling
+				// out
+				return fmt.Errorf("malformed config: %q in zones", z)
+			}
+		}
+
+		upstreams = append(upstreams, s.ForwardPlugin.Upstreams...)
+	}
+
+	if len(upstreams) > 0 {
+		return fmt.Errorf("custom upstream DNS servers in use: %s", strings.Join(upstreams, ", "))
+	}
+
+	return nil
+}

pkg/operator/controllers/checkers/clusterdnschecker/checker_test.go

@@ -0,0 +1,92 @@
+package clusterdnschecker
+
+// Copyright (c) Microsoft Corporation.
+// Licensed under the Apache License 2.0.
+
+import (
+	"context"
+	"testing"
+
+	operatorv1 "github.com/openshift/api/operator/v1"
+	operatorfake "github.com/openshift/client-go/operator/clientset/versioned/fake"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+)
+
+func TestCheck(t *testing.T) {
+	ctx := context.Background()
+
+	for _, tt := range []struct {
+		name    string
+		DNS     *operatorv1.DNS
+		wantErr string
+	}{
+		{
+			name: "valid dns config",
+			DNS: &operatorv1.DNS{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "default",
+				},
+			},
+		},
+		{
+			name: "invalid config: malformed dns config",
+			DNS: &operatorv1.DNS{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "default",
+				},
+				Spec: operatorv1.DNSSpec{
+					Servers: []operatorv1.Server{
+						{
+							Zones: []string{"."},
+						},
+					},
+				},
+			},
+			wantErr: `malformed config: "." in zones`,
+		},
+		{
+			name: "invalid config: forward plugin upstream is",
+			DNS: &operatorv1.DNS{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "default",
+				},
+				Spec: operatorv1.DNSSpec{
+					Servers: []operatorv1.Server{
+						{
+							ForwardPlugin: operatorv1.ForwardPlugin{
+								Upstreams: []string{"first-fake.io", "second-fake.io"},
+							},
+						},
+						{
+							ForwardPlugin: operatorv1.ForwardPlugin{
+								Upstreams: []string{"third-fake.io"},
+							},
+						},
+					},
+				},
+			},
+			wantErr: `custom upstream DNS servers in use: first-fake.io, second-fake.io, third-fake.io`,
+		},
+		{
+			name:    "default config not found",
+			wantErr: `dnses.operator.openshift.io "default" not found`,
+		},
+	} {
+		t.Run(tt.name, func(t *testing.T) {
+			operatorcliMock := operatorfake.NewSimpleClientset()
+			if tt.DNS != nil {
+				operatorcliMock.Tracker().Add(tt.DNS)
+			}
+
+			sp := &checker{
+				operatorcli: operatorcliMock,
+			}
+
+			err := sp.Check(ctx)
+			if err != nil && err.Error() != tt.wantErr ||
+				err == nil && tt.wantErr != "" {
+				t.Errorf("\n%s\n !=\n%s", err, tt.wantErr)
+			}
+		})
+	}
+}

pkg/operator/controllers/checkers/clusterdnschecker/controller.go

@@ -0,0 +1,131 @@
+package clusterdnschecker
+
+// Copyright (c) Microsoft Corporation.
+// Licensed under the Apache License 2.0.
+
+import (
+	"context"
+	"time"
+
+	operatorv1 "github.com/openshift/api/operator/v1"
+	operatorclient "github.com/openshift/client-go/operator/clientset/versioned"
+	"github.com/sirupsen/logrus"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	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/handler"
+	"sigs.k8s.io/controller-runtime/pkg/predicate"
+	"sigs.k8s.io/controller-runtime/pkg/reconcile"
+	"sigs.k8s.io/controller-runtime/pkg/source"
+
+	arov1alpha1 "github.com/Azure/ARO-RP/pkg/operator/apis/aro.openshift.io/v1alpha1"
+	aroclient "github.com/Azure/ARO-RP/pkg/operator/clientset/versioned"
+	checkercommon "github.com/Azure/ARO-RP/pkg/operator/controllers/checkers/common"
+	"github.com/Azure/ARO-RP/pkg/util/conditions"
+)
+
+// This is the permissions that this controller needs to work.
+// "make generate" will run kubebuilder and cause operator/deploy/staticresources/*/role.yaml to be updated
+// from the annotation below.
+// +kubebuilder:rbac:groups=aro.openshift.io,resources=clusters,verbs=get;list;watch
+// +kubebuilder:rbac:groups=aro.openshift.io,resources=clusters/status,verbs=get;update;patch
+
+const (
+	ControllerName = "ClusterDNSChecker"
+)
+
+// Reconciler runs a number of checkers
+type Reconciler struct {
+	log  *logrus.Entry
+	role string
+
+	arocli  aroclient.Interface
+	checker clusterDNSChecker
+}
+
+func NewReconciler(log *logrus.Entry, arocli aroclient.Interface, operatorcli operatorclient.Interface, role string) *Reconciler {
+	return &Reconciler{
+		log:  log,
+		role: role,
+
+		arocli:  arocli,
+		checker: newClusterDNSChecker(operatorcli),
+	}
+}
+
+// Reconcile will keep checking that the cluster has a valid DNS configuration.
+func (r *Reconciler) Reconcile(ctx context.Context, request ctrl.Request) (ctrl.Result, error) {
+	instance, err := r.arocli.AroV1alpha1().Clusters().Get(ctx, arov1alpha1.SingletonClusterName, metav1.GetOptions{})
+	if err != nil {
+		return reconcile.Result{}, err
+	}
+
+	if !instance.Spec.OperatorFlags.GetSimpleBoolean(checkercommon.ControllerEnabled) {
+		r.log.Debug("controller is disabled")
+		return r.reconcileDisabled(ctx)
+	}
+
+	r.log.Debug("running")
+	checkErr := r.checker.Check(ctx)
+	condition := r.condition(checkErr)
+
+	err = conditions.SetCondition(ctx, r.arocli, condition, r.role)
+	if err != nil {
+		return reconcile.Result{}, err
+	}
+
+	// We always requeue here:
+	// * Either immediately (with rate limiting) based on the error
+	//   when checkErr != nil.
+	// * Or based on RequeueAfter when err == nil.
+	return reconcile.Result{RequeueAfter: time.Hour}, checkErr
+}
+
+func (r *Reconciler) reconcileDisabled(ctx context.Context) (ctrl.Result, error) {
+	condition := &operatorv1.OperatorCondition{
+		Type:   arov1alpha1.DefaultClusterDNS,
+		Status: operatorv1.ConditionUnknown,
+	}
+
+	return reconcile.Result{}, conditions.SetCondition(ctx, r.arocli, condition, r.role)
+}
+
+func (r *Reconciler) condition(checkErr error) *operatorv1.OperatorCondition {
+	if checkErr != nil {
+		return &operatorv1.OperatorCondition{
+			Type:    arov1alpha1.DefaultClusterDNS,
+			Status:  operatorv1.ConditionFalse,
+			Message: checkErr.Error(),
+			Reason:  "CheckFailed",
+		}
+	}
+
+	return &operatorv1.OperatorCondition{
+		Type:    arov1alpha1.DefaultClusterDNS,
+		Status:  operatorv1.ConditionTrue,
+		Message: "No in-cluster upstream DNS servers",
+		Reason:  "CheckDone",
+	}
+}
+
+// SetupWithManager setup our manager
+func (r *Reconciler) SetupWithManager(mgr ctrl.Manager) error {
+	aroClusterPredicate := predicate.NewPredicateFuncs(func(o client.Object) bool {
+		return o.GetName() == arov1alpha1.SingletonClusterName
+	})
+
+	defaultClusterDNSPredicate := predicate.NewPredicateFuncs(func(o client.Object) bool {
+		return o.GetName() == "default"
+	})
+
+	builder := ctrl.NewControllerManagedBy(mgr).
+		For(&arov1alpha1.Cluster{}, builder.WithPredicates(aroClusterPredicate)).
+		Watches(
+			&source.Kind{Type: &operatorv1.DNS{}},
+			&handler.EnqueueRequestForObject{},
+			builder.WithPredicates(defaultClusterDNSPredicate),
+		)
+
+	return builder.Named(ControllerName).Complete(r)
+}

pkg/operator/controllers/checkers/clusterdnschecker/controller_test.go

@@ -0,0 +1,128 @@
+package clusterdnschecker
+
+// Copyright (c) Microsoft Corporation.
+// Licensed under the Apache License 2.0.
+
+import (
+	"context"
+	"errors"
+	"reflect"
+	"testing"
+	"time"
+
+	operatorv1 "github.com/openshift/api/operator/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	ctrl "sigs.k8s.io/controller-runtime"
+	"sigs.k8s.io/controller-runtime/pkg/reconcile"
+
+	arov1alpha1 "github.com/Azure/ARO-RP/pkg/operator/apis/aro.openshift.io/v1alpha1"
+	arofake "github.com/Azure/ARO-RP/pkg/operator/clientset/versioned/fake"
+	checkercommon "github.com/Azure/ARO-RP/pkg/operator/controllers/checkers/common"
+	"github.com/Azure/ARO-RP/pkg/util/azureclient"
+	"github.com/Azure/ARO-RP/pkg/util/cmp"
+	utillog "github.com/Azure/ARO-RP/pkg/util/log"
+)
+
+type fakeChecker func(ctx context.Context) error
+
+func (fc fakeChecker) Check(ctx context.Context) error {
+	return fc(ctx)
+}
+
+func TestReconcile(t *testing.T) {
+	ctx := context.Background()
+
+	tests := []struct {
+		name                 string
+		controllerDisabled   bool
+		checkerReturnErr     error
+		wantConditionStatus  operatorv1.ConditionStatus
+		wantConditionMessage string
+		wantErr              string
+		wantResult           reconcile.Result
+	}{
+		{
+			name:                 "no errors",
+			wantConditionStatus:  operatorv1.ConditionTrue,
+			wantConditionMessage: "No in-cluster upstream DNS servers",
+			wantResult:           reconcile.Result{RequeueAfter: time.Hour},
+		},
+		{
+			name:                 "check failed with an error",
+			wantConditionStatus:  operatorv1.ConditionFalse,
+			wantConditionMessage: "fake basic error",
+			checkerReturnErr:     errors.New("fake basic error"),
+			wantErr:              "fake basic error",
+			wantResult:           reconcile.Result{RequeueAfter: time.Hour},
+		},
+		{
+			name:                "controller disabled",
+			controllerDisabled:  true,
+			wantConditionStatus: operatorv1.ConditionUnknown,
+			wantResult:          reconcile.Result{},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			instance := &arov1alpha1.Cluster{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: arov1alpha1.SingletonClusterName,
+				},
+				Spec: arov1alpha1.ClusterSpec{
+					AZEnvironment: azureclient.PublicCloud.Environment.Name,
+					OperatorFlags: arov1alpha1.OperatorFlags{
+						checkercommon.ControllerEnabled: "true",
+					},
+				},
+			}
+			if tt.controllerDisabled {
+				instance.Spec.OperatorFlags[checkercommon.ControllerEnabled] = "false"
+			}
+
+			arocli := arofake.NewSimpleClientset(instance)
+
+			r := &Reconciler{
+				log:  utillog.GetLogger(),
+				role: "master",
+				checker: fakeChecker(func(ctx context.Context) error {
+					return tt.checkerReturnErr
+				}),
+				arocli: arocli,
+			}
+
+			result, err := r.Reconcile(ctx, ctrl.Request{})
+			if err != nil && err.Error() != tt.wantErr ||
+				err == nil && tt.wantErr != "" {
+				t.Error(err)
+			}
+
+			if !reflect.DeepEqual(tt.wantResult, result) {
+				t.Error(cmp.Diff(tt.wantResult, result))
+			}
+
+			instance, err = arocli.AroV1alpha1().Clusters().Get(ctx, arov1alpha1.SingletonClusterName, metav1.GetOptions{})
+			if err != nil {
+				t.Fatal(err)
+			}
+
+			var condition *operatorv1.OperatorCondition
+			for i := range instance.Status.Conditions {
+				if instance.Status.Conditions[i].Type == arov1alpha1.DefaultClusterDNS {
+					condition = &instance.Status.Conditions[i]
+				}
+			}
+			if condition == nil {
+				t.Fatal("no condition found")
+			}
+
+			if condition.Status != tt.wantConditionStatus {
+				t.Errorf(string(condition.Status))
+			}
+
+			if condition.Message != tt.wantConditionMessage {
+				t.Errorf(condition.Message)
+			}
+		})
+	}
+}

pkg/operator/controllers/checkers/internetchecker/checker_test.go

@@ -62,7 +62,7 @@ var (
 	timedoutReq = &fakeResponse{err: context.DeadlineExceeded}
 )
 
-func TestInternetCheckerCheck(t *testing.T) {
+func TestCheck(t *testing.T) {
 	var testCases = []struct {
 		name          string
 		responses     []*fakeResponse

pkg/operator/controllers/checkers/serviceprincipalchecker/checker_test.go

@@ -18,7 +18,7 @@ import (
 	mock_dynamic "github.com/Azure/ARO-RP/pkg/util/mocks/dynamic"
 )
 
-func TestServicePrincipalValid(t *testing.T) {
+func TestCheck(t *testing.T) {
 	ctx := context.Background()
 	log := logrus.NewEntry(logrus.StandardLogger())
 	mockCredentials := &clusterauthorizer.Credentials{

pkg/operator/controllers/checkers/serviceprincipalchecker/controller.go

@@ -56,7 +56,7 @@ func NewReconciler(log *logrus.Entry, arocli aroclient.Interface, kubernetescli
 	}
 }
 
-// Reconcile will keep checking that the has a valid cluster service principal.
+// Reconcile will keep checking that the cluster has a valid cluster service principal.
 func (r *Reconciler) Reconcile(ctx context.Context, request ctrl.Request) (ctrl.Result, error) {
 	instance, err := r.arocli.AroV1alpha1().Clusters().Get(ctx, arov1alpha1.SingletonClusterName, metav1.GetOptions{})
 	if err != nil {

pkg/util/scheme/scheme.go

@@ -7,6 +7,7 @@ import (
 	configv1 "github.com/openshift/api/config/v1"
 	consolev1 "github.com/openshift/api/console/v1"
 	machinev1beta1 "github.com/openshift/api/machine/v1beta1"
+	operatorv1 "github.com/openshift/api/operator/v1"
 	securityv1 "github.com/openshift/api/security/v1"
 	hivev1 "github.com/openshift/hive/apis/hive/v1"
 	mcv1 "github.com/openshift/machine-config-operator/pkg/apis/machineconfiguration.openshift.io/v1"
@@ -43,6 +44,7 @@ func init() {
 	utilruntime.Must(machinev1beta1.AddToScheme(scheme.Scheme))
 	utilruntime.Must(consolev1.AddToScheme(scheme.Scheme))
 	utilruntime.Must(monitoringv1.AddToScheme(scheme.Scheme))
+	utilruntime.Must(operatorv1.AddToScheme(scheme.Scheme))
 	// AzureMachineProviderSpec is not registered by default
 	scheme.Scheme.AddKnownTypes(machinev1beta1.GroupVersion, &machinev1beta1.AzureMachineProviderSpec{})
 	// AzureMachineProviderSpec type has been deleted from sigs.k8s.io/cluster-api-provider-azure.