Repository: Azure/ARO-RP
Pull request #2584 (2022-12-07 14:53:00)

Operator checker split - move internet checker into a separate controller
### Which issue this PR addresses:

Work item [16394925](https://msazure.visualstudio.com/AzureRedHatOpenShift/_workitems/edit/16394925/).

### What this PR does / why we need it:

Trying to enable the operator to run checks independently from each other.

### Test plan for issue:

Operator behaviour before the change should be the same as the behaviour with this change.

### Is there any documentation that needs to be updated for this PR?

No, refactoring.



Patch:
diff --git a/cmd/aro/operator.go b/cmd/aro/operator.go
index a5a2a83ce..5a23d54d0 100644
--- a/cmd/aro/operator.go
+++ b/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/internetchecker"
 	"github.com/Azure/ARO-RP/pkg/operator/controllers/clusteroperatoraro"
 	"github.com/Azure/ARO-RP/pkg/operator/controllers/dnsmasq"
 	"github.com/Azure/ARO-RP/pkg/operator/controllers/genevalogging"
@@ -237,12 +238,17 @@ func operator(ctx context.Context, log *logrus.Entry) error {
 			arocli, operatorcli)).SetupWithManager(mgr); err != nil {
 			return fmt.Errorf("unable to create controller %s: %v", ingress.ControllerName, err)
 		}
+		if err = (checker.NewReconciler(
+			log.WithField("controller", checker.ControllerName),
+			arocli, kubernetescli, maocli, operatorcli, configcli, role)).SetupWithManager(mgr); err != nil {
+			return fmt.Errorf("unable to create controller %s: %v", checker.ControllerName, err)
+		}
 	}
 
-	if err = (checker.NewReconciler(
-		log.WithField("controller", checker.ControllerName),
-		arocli, kubernetescli, maocli, operatorcli, configcli, role)).SetupWithManager(mgr); err != nil {
-		return fmt.Errorf("unable to create controller %s: %v", checker.ControllerName, err)
+	if err = (internetchecker.NewReconciler(
+		log.WithField("controller", internetchecker.ControllerName),
+		arocli, role)).SetupWithManager(mgr); err != nil {
+		return fmt.Errorf("unable to create controller %s: %v", internetchecker.ControllerName, err)
 	}
 
 	// +kubebuilder:scaffold:builder
diff --git a/docs/prepare-a-shared-rp-development-environment.md b/docs/prepare-a-shared-rp-development-environment.md
index 2b976ecb7..626b2d04d 100644
--- a/docs/prepare-a-shared-rp-development-environment.md
+++ b/docs/prepare-a-shared-rp-development-environment.md
@@ -27,7 +27,7 @@ locations.
    Set SECRET_SA_ACCOUNT_NAME to the name of the storage account:
 
    ```bash
-   SECRET_SA_ACCOUNT_NAME=rharosecretsdev
+   SECRET_SA_ACCOUNT_NAME=e2earosecrets
    ```
 
 1. You will need an AAD object (this could be your AAD user, or an AAD group of
@@ -368,7 +368,7 @@ az ad app credential reset \
 
 5. The RP makes API calls to kubernetes cluster via a proxy VMSS agent. For the agent to get the updated certificates, this vm needs to be redeployed. Proxy VM is currently deployed by the `deploy_env_dev` function in `deploy-shared-env.sh`. It makes use of `env-development.json`
 
-6. Run `[rharosecretsdev|aroe2esecrets] make secrets-update` to upload it to your
+6. Run `[rharosecretsdev|e2earosecrets] make secrets-update` to upload it to your
 storage account so other people on your team can access it via `make secrets`
 
 # Environment file
@@ -472,6 +472,16 @@ each of the bash functions below.
    when running the `deploy_env_dev_override` command, delete the `-pip` resource
    and re-run.
 
+1. Get the AKS kubeconfig and upload it to the storage account:
+   
+   ```bash
+   make aks.kubeconfig
+   mv aks.kubeconfig secrets/
+   make secrets-update
+   ```
+   
+1. [Install Hive on the new AKS](https://github.com/Azure/ARO-RP/blob/master/docs/hive.md)
+
 1. Load the keys/certificates into the key vault:
 
    ```bash
diff --git a/hack/fakecluster/fakecluster.go b/hack/fakecluster/fakecluster.go
index 9569dede1..9ee1001a1 100644
--- a/hack/fakecluster/fakecluster.go
+++ b/hack/fakecluster/fakecluster.go
@@ -71,9 +71,8 @@ func run(ctx context.Context, l *logrus.Entry) error {
 			tls.TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384,
 			tls.TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384,
 		},
-		PreferServerCipherSuites: true,
-		SessionTicketsDisabled:   true,
-		MinVersion:               tls.VersionTLS12,
+		SessionTicketsDisabled: true,
+		MinVersion:             tls.VersionTLS12,
 		CurvePreferences: []tls.CurveID{
 			tls.CurveP256,
 			tls.X25519,
diff --git a/hack/validate-imports/allowed-import-names.yaml b/hack/validate-imports/allowed-import-names.yaml
index fe66585d5..ff4779553 100644
--- a/hack/validate-imports/allowed-import-names.yaml
+++ b/hack/validate-imports/allowed-import-names.yaml
@@ -113,3 +113,5 @@ allowedImportNames:
     - hivev1azure
   github.com/gofrs/uuid:
     - gofrsuuid
+  github.com/Azure/ARO-RP/pkg/operator/controllers/checkers/common:
+    - checkercommon
diff --git a/pkg/dbtoken/server.go b/pkg/dbtoken/server.go
index 3f321fa99..6caff7f3e 100644
--- a/pkg/dbtoken/server.go
+++ b/pkg/dbtoken/server.go
@@ -71,9 +71,8 @@ func NewServer(
 			tls.TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384,
 			tls.TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384,
 		},
-		PreferServerCipherSuites: true,
-		SessionTicketsDisabled:   true,
-		MinVersion:               tls.VersionTLS12,
+		SessionTicketsDisabled: true,
+		MinVersion:             tls.VersionTLS12,
 		CurvePreferences: []tls.CurveID{
 			tls.CurveP256,
 			tls.X25519,
diff --git a/pkg/frontend/frontend.go b/pkg/frontend/frontend.go
index cce37d434..32b039ede 100644
--- a/pkg/frontend/frontend.go
+++ b/pkg/frontend/frontend.go
@@ -174,9 +174,8 @@ func NewFrontend(ctx context.Context,
 			tls.TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384,
 			tls.TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384,
 		},
-		PreferServerCipherSuites: true,
-		SessionTicketsDisabled:   true,
-		MinVersion:               tls.VersionTLS12,
+		SessionTicketsDisabled: true,
+		MinVersion:             tls.VersionTLS12,
 		CurvePreferences: []tls.CurveID{
 			tls.CurveP256,
 			tls.X25519,
diff --git a/pkg/operator/controllers/checker/checker_controller.go b/pkg/operator/controllers/checker/checker_controller.go
index e075a1f7f..315d2337c 100644
--- a/pkg/operator/controllers/checker/checker_controller.go
+++ b/pkg/operator/controllers/checker/checker_controller.go
@@ -19,40 +19,32 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/predicate"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 
-	"github.com/Azure/ARO-RP/pkg/operator"
 	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"
 )
 
 const (
 	ControllerName = "Checker"
-
-	controllerEnabled = "aro.checker.enabled"
 )
 
 // Reconciler runs a number of checkers
 type Reconciler struct {
 	log *logrus.Entry
 
-	role     string
 	checkers []Checker
 	arocli   aroclient.Interface
 }
 
 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{NewInternetChecker(log, arocli, role)}
-
-	if role == operator.RoleMaster {
-		checkers = append(checkers,
-			NewServicePrincipalChecker(log, arocli, kubernetescli, machinecli, role),
-			NewIngressCertificateChecker(log, arocli, operatorcli, configcli, role),
-			NewClusterDNSChecker(log, arocli, operatorcli, role),
-		)
+	checkers := []Checker{
+		NewServicePrincipalChecker(log, arocli, kubernetescli, machinecli, role),
+		NewIngressCertificateChecker(log, arocli, operatorcli, configcli, role),
+		NewClusterDNSChecker(log, arocli, operatorcli, role),
 	}
 
 	return &Reconciler{
 		log:      log,
-		role:     role,
 		checkers: checkers,
 		arocli:   arocli,
 	}
@@ -71,7 +63,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, request ctrl.Request) (ctrl.
 		return reconcile.Result{}, err
 	}
 
-	if !instance.Spec.OperatorFlags.GetSimpleBoolean(controllerEnabled) {
+	if !instance.Spec.OperatorFlags.GetSimpleBoolean(checkercommon.ControllerEnabled) {
 		r.log.Debug("controller is disabled")
 		return reconcile.Result{}, nil
 	}
diff --git a/pkg/operator/controllers/checker/internetchecker.go b/pkg/operator/controllers/checker/internetchecker.go
deleted file mode 100644
index 6ec10900a..000000000
--- a/pkg/operator/controllers/checker/internetchecker.go
+++ /dev/null
@@ -1,178 +0,0 @@
-package checker
-
-// Copyright (c) Microsoft Corporation.
-// Licensed under the Apache License 2.0.
-
-import (
-	"context"
-	"fmt"
-	"net/http"
-	"strings"
-	"time"
-
-	operatorv1 "github.com/openshift/api/operator/v1"
-	"github.com/sirupsen/logrus"
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-
-	"github.com/Azure/ARO-RP/pkg/operator"
-	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"
-)
-
-// InternetChecker reconciles a Cluster object
-type InternetChecker struct {
-	log          *logrus.Entry
-	role         string
-	checkTimeout time.Duration
-
-	arocli     aroclient.Interface
-	httpClient simpleHTTPClient
-}
-
-func NewInternetChecker(log *logrus.Entry, arocli aroclient.Interface, role string) *InternetChecker {
-	return &InternetChecker{
-		log:          log,
-		role:         role,
-		checkTimeout: time.Minute,
-
-		arocli: arocli,
-		httpClient: &http.Client{
-			Transport: &http.Transport{
-				// We set DisableKeepAlives for two reasons:
-				//
-				// 1. If we're talking HTTP/2 and the remote end blackholes traffic,
-				// Go has a bug whereby it doesn't reset the connection after a
-				// timeout (https://github.com/golang/go/issues/36026).  If this
-				// happens, we never have a chance to get healthy.  We have
-				// specifically seen this with gcs.prod.monitoring.core.windows.net
-				// in Korea Central, which currently has a bad server which when we
-				// hit it causes our cluster creations to fail.
-				//
-				// 2. We *want* to evaluate our capability to successfully create
-				// *new* connections to internet endpoints anyway.
-				DisableKeepAlives: true,
-			},
-		},
-	}
-}
-
-type simpleHTTPClient interface {
-	Do(req *http.Request) (*http.Response, error)
-}
-
-func (r *InternetChecker) Name() string {
-	return "InternetChecker"
-}
-
-// 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
-
-// Reconcile will keep checking that the cluster can connect to essential services.
-func (r *InternetChecker) Check(ctx context.Context) error {
-	instance, err := r.arocli.AroV1alpha1().Clusters().Get(ctx, arov1alpha1.SingletonClusterName, metav1.GetOptions{})
-	if err != nil {
-		return err
-	}
-
-	ch := make(chan error)
-	checkCount := 0
-	for _, url := range instance.Spec.InternetChecker.URLs {
-		checkCount++
-		go func(urlToCheck string) {
-			ch <- r.checkWithRetry(urlToCheck)
-		}(url)
-	}
-
-	sb := &strings.Builder{}
-	checkFailed := false
-
-	for i := 0; i < checkCount; i++ {
-		if err = <-ch; err != nil {
-			r.log.Infof("URL check failed with error %s", err)
-			fmt.Fprintf(sb, "%s\n", err)
-			checkFailed = true
-		}
-	}
-
-	var condition *operatorv1.OperatorCondition
-
-	if checkFailed {
-		condition = &operatorv1.OperatorCondition{
-			Type:    r.conditionType(),
-			Status:  operatorv1.ConditionFalse,
-			Message: sb.String(),
-			Reason:  "CheckFailed",
-		}
-	} else {
-		condition = &operatorv1.OperatorCondition{
-			Type:    r.conditionType(),
-			Status:  operatorv1.ConditionTrue,
-			Message: "Outgoing connection successful",
-			Reason:  "CheckDone",
-		}
-	}
-
-	err = conditions.SetCondition(ctx, r.arocli, condition, r.role)
-	if err != nil {
-		return err
-	}
-
-	if checkFailed {
-		return errRequeue
-	}
-
-	return nil
-}
-
-// check the URL, retrying a failed query a few times
-func (r *InternetChecker) checkWithRetry(url string) error {
-	var err error
-
-	for i := 0; i < 6; i++ {
-		err = r.checkOnce(url, r.checkTimeout/6)
-		if err == nil {
-			return nil
-		}
-	}
-
-	return err
-}
-
-// checkOnce checks a given url.  The check both times out after a given timeout
-// *and* will wait for the timeout if it fails, so that we don't hit endpoints
-// too much.
-func (r *InternetChecker) checkOnce(url string, timeout time.Duration) error {
-	ctx, cancel := context.WithTimeout(context.Background(), timeout)
-	defer cancel()
-
-	req, err := http.NewRequestWithContext(ctx, http.MethodHead, url, nil)
-	if err != nil {
-		<-ctx.Done()
-		return err
-	}
-
-	resp, err := r.httpClient.Do(req)
-	if err != nil {
-		<-ctx.Done()
-		return fmt.Errorf("%s: %s", url, err)
-	}
-
-	resp.Body.Close()
-	return nil
-}
-
-func (r *InternetChecker) conditionType() (ctype string) {
-	switch r.role {
-	case operator.RoleMaster:
-		return arov1alpha1.InternetReachableFromMaster
-	case operator.RoleWorker:
-		return arov1alpha1.InternetReachableFromWorker
-	default:
-		r.log.Warnf("unknown role %s, assuming worker role", r.role)
-		return arov1alpha1.InternetReachableFromWorker
-	}
-}
diff --git a/pkg/operator/controllers/checkers/common/const.go b/pkg/operator/controllers/checkers/common/const.go
new file mode 100644
index 000000000..8322ccd83
--- /dev/null
+++ b/pkg/operator/controllers/checkers/common/const.go
@@ -0,0 +1,8 @@
+package common
+
+// Copyright (c) Microsoft Corporation.
+// Licensed under the Apache License 2.0.
+
+const (
+	ControllerEnabled = "aro.checker.enabled"
+)
diff --git a/pkg/operator/controllers/checkers/internetchecker/checker.go b/pkg/operator/controllers/checkers/internetchecker/checker.go
new file mode 100644
index 000000000..6fe12f995
--- /dev/null
+++ b/pkg/operator/controllers/checkers/internetchecker/checker.go
@@ -0,0 +1,110 @@
+package internetchecker
+
+// Copyright (c) Microsoft Corporation.
+// Licensed under the Apache License 2.0.
+
+import (
+	"context"
+	"fmt"
+	"net/http"
+	"strings"
+	"time"
+)
+
+type simpleHTTPClient interface {
+	Do(req *http.Request) (*http.Response, error)
+}
+
+type internetChecker interface {
+	Check(URLs []string) error
+}
+
+// checker evaluates our capability to create new
+// connections to given internet endpoints.
+type checker struct {
+	checkTimeout time.Duration
+	httpClient   simpleHTTPClient
+}
+
+func newInternetChecker() internetChecker {
+	return &checker{
+		checkTimeout: time.Minute,
+		httpClient: &http.Client{
+			Transport: &http.Transport{
+				// We set DisableKeepAlives for two reasons:
+				//
+				// 1. If we're talking HTTP/2 and the remote end blackholes traffic,
+				// Go has a bug whereby it doesn't reset the connection after a
+				// timeout (https://github.com/golang/go/issues/36026).  If this
+				// happens, we never have a chance to get healthy.  We have
+				// specifically seen this with gcs.prod.monitoring.core.windows.net
+				// in Korea Central, which currently has a bad server which when we
+				// hit it causes our cluster creations to fail.
+				//
+				// 2. We *want* to evaluate our capability to successfully create
+				// *new* connections to internet endpoints anyway.
+				DisableKeepAlives: true,
+			},
+		},
+	}
+}
+func (r *checker) Check(URLs []string) error {
+	ch := make(chan error)
+	checkCount := 0
+	for _, url := range URLs {
+		checkCount++
+		go func(urlToCheck string) {
+			ch <- r.checkWithRetry(urlToCheck)
+		}(url)
+	}
+
+	errsAll := []string{}
+	for i := 0; i < checkCount; i++ {
+		if err := <-ch; err != nil {
+			errsAll = append(errsAll, err.Error())
+		}
+	}
+	if len(errsAll) != 0 {
+		// TODO: Consider replacing with multi error wrapping with Go 1.20: https://github.com/golang/go/issues/53435#issuecomment-1320343377
+		return fmt.Errorf(strings.Join(errsAll, "\n"))
+	}
+
+	return nil
+}
+
+// checkWithRetry checks the URL, retrying a failed query a few times
+func (r *checker) checkWithRetry(url string) error {
+	var err error
+
+	for i := 0; i < 6; i++ {
+		err = r.checkOnce(url, r.checkTimeout/6)
+		if err == nil {
+			return nil
+		}
+	}
+
+	return err
+}
+
+// checkOnce checks a given url.  The check both times out after a given timeout
+// *and* will wait for the timeout if it fails, so that we don't hit endpoints
+// too much.
+func (r *checker) checkOnce(url string, timeout time.Duration) error {
+	ctx, cancel := context.WithTimeout(context.Background(), timeout)
+	defer cancel()
+
+	req, err := http.NewRequestWithContext(ctx, http.MethodHead, url, nil)
+	if err != nil {
+		<-ctx.Done()
+		return err
+	}
+
+	resp, err := r.httpClient.Do(req)
+	if err != nil {
+		<-ctx.Done()
+		return fmt.Errorf("%s: %s", url, err)
+	}
+
+	resp.Body.Close()
+	return nil
+}
diff --git a/pkg/operator/controllers/checkers/internetchecker/controller.go b/pkg/operator/controllers/checkers/internetchecker/controller.go
new file mode 100644
index 000000000..4729ad7d3
--- /dev/null
+++ b/pkg/operator/controllers/checkers/internetchecker/controller.go
@@ -0,0 +1,131 @@
+package internetchecker
+
+// Copyright (c) Microsoft Corporation.
+// Licensed under the Apache License 2.0.
+
+import (
+	"context"
+	"time"
+
+	operatorv1 "github.com/openshift/api/operator/v1"
+	"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/predicate"
+	"sigs.k8s.io/controller-runtime/pkg/reconcile"
+
+	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 = "InternetChecker"
+)
+
+// Reconciler runs a number of checkers
+type Reconciler struct {
+	log  *logrus.Entry
+	role string
+
+	arocli  aroclient.Interface
+	checker internetChecker
+}
+
+func NewReconciler(log *logrus.Entry, arocli aroclient.Interface, role string) *Reconciler {
+	return &Reconciler{
+		log:  log,
+		role: role,
+
+		arocli:  arocli,
+		checker: newInternetChecker(),
+	}
+}
+
+// Reconcile will keep checking that the cluster can connect to essential services.
+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(instance.Spec.InternetChecker.URLs)
+	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:   r.conditionType(),
+		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:    r.conditionType(),
+			Status:  operatorv1.ConditionFalse,
+			Message: checkErr.Error(),
+			Reason:  "CheckFailed",
+		}
+	}
+
+	return &operatorv1.OperatorCondition{
+		Type:    r.conditionType(),
+		Status:  operatorv1.ConditionTrue,
+		Message: "Outgoing connection successful",
+		Reason:  "CheckDone",
+	}
+}
+
+func (r *Reconciler) conditionType() string {
+	switch r.role {
+	case "master":
+		return arov1alpha1.InternetReachableFromMaster
+	case "worker":
+		return arov1alpha1.InternetReachableFromWorker
+	default:
+		r.log.Warnf("unknown role %s, assuming worker role", r.role)
+		return arov1alpha1.InternetReachableFromWorker
+	}
+}
+
+// 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
+	})
+
+	builder := ctrl.NewControllerManagedBy(mgr).
+		For(&arov1alpha1.Cluster{}, builder.WithPredicates(aroClusterPredicate))
+
+	return builder.Named(ControllerName).Complete(r)
+}
diff --git a/pkg/operator/controllers/workaround/workaround_controller.go b/pkg/operator/controllers/workaround/workaround_controller.go
index c50d3f6c6..a6bb50987 100644
--- a/pkg/operator/controllers/workaround/workaround_controller.go
+++ b/pkg/operator/controllers/workaround/workaround_controller.go
@@ -33,11 +33,9 @@ const (
 type Reconciler struct {
 	log *logrus.Entry
 
-	arocli        aroclient.Interface
-	configcli     configclient.Interface
-	kubernetescli kubernetes.Interface
+	arocli    aroclient.Interface
+	configcli configclient.Interface
 
-	restConfig  *rest.Config
 	workarounds []Workaround
 }
 
@@ -48,12 +46,10 @@ func NewReconciler(log *logrus.Entry, arocli aroclient.Interface, configcli conf
 	}
 
 	return &Reconciler{
-		log:           log,
-		arocli:        arocli,
-		configcli:     configcli,
-		kubernetescli: kubernetescli,
-		restConfig:    restConfig,
-		workarounds:   []Workaround{NewSystemReserved(log, mcocli, dh), NewIfReload(log, kubernetescli)},
+		log:         log,
+		arocli:      arocli,
+		configcli:   configcli,
+		workarounds: []Workaround{NewSystemReserved(log, mcocli, dh), NewIfReload(log, kubernetescli)},
 	}
 }
 
diff --git a/pkg/portal/portal.go b/pkg/portal/portal.go
index 664450d0b..018bd1708 100644
--- a/pkg/portal/portal.go
+++ b/pkg/portal/portal.go
@@ -220,9 +220,8 @@ func (p *portal) Run(ctx context.Context) error {
 			tls.TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384,
 			tls.TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384,
 		},
-		PreferServerCipherSuites: true,
-		SessionTicketsDisabled:   true,
-		MinVersion:               tls.VersionTLS12,
+		SessionTicketsDisabled: true,
+		MinVersion:             tls.VersionTLS12,
 		CurvePreferences: []tls.CurveID{
 			tls.CurveP256,
 			tls.X25519,
diff --git a/pkg/proxy/proxy.go b/pkg/proxy/proxy.go
index 220de0b79..0d4453a3a 100644
--- a/pkg/proxy/proxy.go
+++ b/pkg/proxy/proxy.go
@@ -83,9 +83,8 @@ func (s *Server) Run() error {
 			tls.TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384,
 			tls.TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384,
 		},
-		PreferServerCipherSuites: true,
-		SessionTicketsDisabled:   true,
-		MinVersion:               tls.VersionTLS12,
+		SessionTicketsDisabled: true,
+		MinVersion:             tls.VersionTLS12,
 		CurvePreferences: []tls.CurveID{
 			tls.CurveP256,
 			tls.X25519,


Test patch:
diff --git a/.pipelines/e2e.yml b/.pipelines/e2e.yml
index 4a500a577..bed7fa28e 100644
--- a/.pipelines/e2e.yml
+++ b/.pipelines/e2e.yml
@@ -42,7 +42,7 @@ jobs:
       set -o pipefail
 
       . secrets/env
-      export HIVE_KUBE_CONFIG_PATH_1="secrets/e2e-aks-kubeconfig_1"
+      export HIVE_KUBE_CONFIG_PATH_1="secrets/aks.kubeconfig"
 
       az account set -s $AZURE_SUBSCRIPTION_ID
 
diff --git a/pkg/operator/controllers/checker/internetchecker_test.go b/pkg/operator/controllers/checkers/internetchecker/checker_test.go
similarity index 52%
rename from pkg/operator/controllers/checker/internetchecker_test.go
rename to pkg/operator/controllers/checkers/internetchecker/checker_test.go
index f90d20d22..a34bf0a35 100644
--- a/pkg/operator/controllers/checker/internetchecker_test.go
+++ b/pkg/operator/controllers/checkers/internetchecker/checker_test.go
@@ -1,4 +1,4 @@
-package checker
+package internetchecker
 
 // Copyright (c) Microsoft Corporation.
 // Licensed under the Apache License 2.0.
@@ -15,12 +15,6 @@ import (
 	"time"
 
 	operatorv1 "github.com/openshift/api/operator/v1"
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-
-	"github.com/Azure/ARO-RP/pkg/operator"
-	arov1alpha1 "github.com/Azure/ARO-RP/pkg/operator/apis/aro.openshift.io/v1alpha1"
-	arofake "github.com/Azure/ARO-RP/pkg/operator/clientset/versioned/fake"
-	utillog "github.com/Azure/ARO-RP/pkg/util/log"
 )
 
 type fakeResponse struct {
@@ -69,8 +63,6 @@ var (
 )
 
 func TestInternetCheckerCheck(t *testing.T) {
-	ctx := context.Background()
-
 	var testCases = []struct {
 		name          string
 		responses     []*fakeResponse
@@ -100,63 +92,21 @@ func TestInternetCheckerCheck(t *testing.T) {
 		{
 			name:          "timedout request",
 			responses:     []*fakeResponse{networkUnreach, timedoutReq, timedoutReq, timedoutReq, timedoutReq, timedoutReq},
-			wantErr:       "requeue",
+			wantErr:       "https://not-used-in-test.io: context deadline exceeded",
 			wantCondition: operatorv1.ConditionFalse,
 		},
 	}
 
-	roleToConditionTypeMap := map[string]string{
-		operator.RoleMaster: arov1alpha1.InternetReachableFromMaster,
-		operator.RoleWorker: arov1alpha1.InternetReachableFromWorker,
-	}
-
-	for _, testRole := range []string{operator.RoleMaster, operator.RoleWorker} {
-		t.Run(testRole, func(t *testing.T) {
-			for _, test := range testCases {
-				t.Run(test.name, func(t *testing.T) {
-					arocli := arofake.NewSimpleClientset(
-						&arov1alpha1.Cluster{
-							ObjectMeta: metav1.ObjectMeta{
-								Name: arov1alpha1.SingletonClusterName,
-							},
-							Spec: arov1alpha1.ClusterSpec{
-								InternetChecker: arov1alpha1.InternetCheckerSpec{
-									URLs: []string{urltocheck},
-								},
-							},
-						},
-					)
-
-					r := &InternetChecker{
-						log:          utillog.GetLogger(),
-						role:         testRole,
-						checkTimeout: 100 * time.Millisecond,
-
-						arocli:     arocli,
-						httpClient: &testClient{responses: test.responses},
-					}
-					err := r.Check(ctx)
-					if err != nil && err.Error() != test.wantErr ||
-						err == nil && test.wantErr != "" {
-						t.Error(err)
-					}
-
-					instance, err := arocli.AroV1alpha1().Clusters().Get(ctx, arov1alpha1.SingletonClusterName, metav1.GetOptions{})
-					if err != nil {
-						t.Error(err)
-					}
-
-					var condition operatorv1.OperatorCondition
-					for _, condition = range instance.Status.Conditions {
-						if condition.Type == roleToConditionTypeMap[testRole] {
-							break
-						}
-					}
-
-					if condition.Status != test.wantCondition {
-						t.Errorf(string(condition.Status))
-					}
-				})
+	for _, test := range testCases {
+		t.Run(test.name, func(t *testing.T) {
+			r := &checker{
+				checkTimeout: 100 * time.Millisecond,
+				httpClient:   &testClient{responses: test.responses},
+			}
+			err := r.Check([]string{urltocheck})
+			if err != nil && err.Error() != test.wantErr ||
+				err == nil && test.wantErr != "" {
+				t.Error(err)
 			}
 		})
 	}
diff --git a/pkg/operator/controllers/checkers/internetchecker/controller_test.go b/pkg/operator/controllers/checkers/internetchecker/controller_test.go
new file mode 100644
index 000000000..03f12ecff
--- /dev/null
+++ b/pkg/operator/controllers/checkers/internetchecker/controller_test.go
@@ -0,0 +1,136 @@
+package internetchecker
+
+// 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"
+
+	"github.com/Azure/ARO-RP/pkg/operator"
+	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/cmp"
+	utillog "github.com/Azure/ARO-RP/pkg/util/log"
+)
+
+type fakeChecker func(URLs []string) error
+
+func (fc fakeChecker) Check(URLs []string) error {
+	return fc(URLs)
+}
+
+func TestReconcile(t *testing.T) {
+	ctx := context.Background()
+	urlsToCheck := []string{"https://fake-url-for-test-only.xyz"}
+
+	tests := []struct {
+		name               string
+		controllerDisabled bool
+		checkerReturnErr   error
+		wantCondition      operatorv1.ConditionStatus
+		wantErr            string
+		wantResult         reconcile.Result
+	}{
+		{
+			name:          "no errors",
+			wantCondition: operatorv1.ConditionTrue,
+			wantResult:    reconcile.Result{RequeueAfter: time.Hour},
+		},
+		{
+			name:             "error making a request",
+			checkerReturnErr: errors.New("fake error from checker"),
+			wantCondition:    operatorv1.ConditionFalse,
+			wantErr:          "fake error from checker",
+			wantResult:       reconcile.Result{RequeueAfter: time.Hour},
+		},
+		{
+			name:               "controller disabled",
+			controllerDisabled: true,
+			wantCondition:      operatorv1.ConditionUnknown,
+			wantResult:         reconcile.Result{},
+		},
+	}
+
+	roleToConditionTypeMap := map[string]string{
+		"master":         arov1alpha1.InternetReachableFromMaster,
+		"worker":         arov1alpha1.InternetReachableFromWorker,
+		"incorrect-role": arov1alpha1.InternetReachableFromWorker,
+	}
+	for _, testRole := range []string{operator.RoleMaster, operator.RoleWorker, "incorrect-role"} {
+		t.Run(testRole, func(t *testing.T) {
+			for _, tt := range tests {
+				t.Run(tt.name, func(t *testing.T) {
+					instance := &arov1alpha1.Cluster{
+						ObjectMeta: metav1.ObjectMeta{
+							Name: arov1alpha1.SingletonClusterName,
+						},
+						Spec: arov1alpha1.ClusterSpec{
+							InternetChecker: arov1alpha1.InternetCheckerSpec{
+								URLs: urlsToCheck,
+							},
+							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: testRole,
+						checker: fakeChecker(func(URLs []string) error {
+							if !reflect.DeepEqual(urlsToCheck, URLs) {
+								t.Error(cmp.Diff(urlsToCheck, URLs))
+							}
+
+							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 == roleToConditionTypeMap[testRole] {
+							condition = &instance.Status.Conditions[i]
+						}
+					}
+					if condition == nil {
+						t.Fatal("no condition found")
+					}
+
+					if condition.Status != tt.wantCondition {
+						t.Error(condition.Status)
+					}
+				})
+			}
+		})
+	}
+}