Fix Construction of Monitor for Clusters with kube-apiservers Down (#…

…3201)

pkg/monitor/cluster/cluster.go

@@ -18,6 +18,7 @@ import (
 	"k8s.io/client-go/kubernetes"
 	"k8s.io/client-go/rest"
 	"sigs.k8s.io/controller-runtime/pkg/client"
+	"sigs.k8s.io/controller-runtime/pkg/client/apiutil"
 
 	"github.com/Azure/ARO-RP/pkg/api"
 	"github.com/Azure/ARO-RP/pkg/metrics"
@@ -92,7 +93,15 @@ func NewMonitor(log *logrus.Entry, restConfig *rest.Config, oc *api.OpenShiftClu
 		return nil, err
 	}
 
-	ocpclientset, err := client.New(restConfig, client.Options{})
+	// lazy discovery will not attempt to reach out to the apiserver immediately
+	mapper, err := apiutil.NewDynamicRESTMapper(restConfig, apiutil.WithLazyDiscovery)
+	if err != nil {
+		return nil, err
+	}
+
+	ocpclientset, err := client.New(restConfig, client.Options{
+		Mapper: mapper,
+	})
 	if err != nil {
 		return nil, err
 	}
@@ -126,7 +135,15 @@ func getHiveClientSet(hiveRestConfig *rest.Config) (client.Client, error) {
 		return nil, nil
 	}
 
-	hiveclientset, err := client.New(hiveRestConfig, client.Options{})
+	// lazy discovery will not attempt to reach out to the apiserver immediately
+	mapper, err := apiutil.NewDynamicRESTMapper(hiveRestConfig, apiutil.WithLazyDiscovery)
+	if err != nil {
+		return nil, err
+	}
+
+	hiveclientset, err := client.New(hiveRestConfig, client.Options{
+		Mapper: mapper,
+	})
 	if err != nil {
 		return nil, err
 	}

pkg/monitor/cluster/hiveregistrationstatus_panics_test.go

@@ -112,37 +112,18 @@ func TestEmitHiveRegistrationDoesNotPanicWhenWeAssignLiteralNil(t *testing.T) {
 	_ = mon.emitHiveRegistrationStatus(context.Background())
 }
 
-// TestEmitHiveRegistrationDoesNotPanicWhenWeAssignLiteralNilUsingAuxFunction shows the exact same
-// behavior as TestEmitHiveRegistrationDoesNotPanicWhenWeAssignLiteralNil but encapsulating the
-// initialisation of hiveclientset in a function. The conclusion is the same, assign literal nil
-// to the interface when we get an error: the difference is that in this test this assignment is
-// coming from a function the prod code uses (so it is a good think) while the previous tests was done inline in the test.
-func TestEmitHiveRegistrationDoesNotPanicWhenWeAssignLiteralNilUsingAuxFunction(t *testing.T) {
-	hiveclientset, err := getHiveClientSet(&rest.Config{})
-	if err == nil {
-		t.Fatalf("we want to make sure err is not nil for this test")
-	}
-
-	mon := &Monitor{
-		hiveclientset: hiveclientset,
-		oc: &api.OpenShiftCluster{
-			Properties: api.OpenShiftClusterProperties{
-				HiveProfile: api.HiveProfile{
-					Namespace: "something",
-				},
-				NetworkProfile: api.NetworkProfile{
-					APIServerPrivateEndpointIP: "something",
-				},
-			},
-		},
-		log: utillog.GetLogger(),
+// TestGetHiveClientSetNeverReturnsError will test that the creation
+// of the non-typed kubernetes client will not return an error as it's
+// now leveraging a lazy fetch mechanism where it will only discover
+// the preexisting schema and apiversions during explicit CRUD operations
+// against the apiserver
+func TestGetHiveClientSetNeverReturnsError(t *testing.T) {
+	hiveClientSet, err := getHiveClientSet(&rest.Config{})
+	if err != nil {
+		t.Fatalf("error should not be nil")
 	}
 
-	fmt.Printf("mon.hiveclientset: interface type is %T, interface value is: %v\n", mon.hiveclientset, mon.hiveclientset)
-	if mon.hiveclientset != nil {
-		t.Fatalf("mon.hiveclientset should be nil")
+	if hiveClientSet == nil {
+		t.Fatalf("hive client set should not be nil")
 	}
-
-	// no panic
-	_ = mon.emitHiveRegistrationStatus(context.Background())
 }

pkg/monitor/worker.go

@@ -249,6 +249,9 @@ func (mon *monitor) workOne(ctx context.Context, log *logrus.Entry, doc *api.Ope
 	c, err := cluster.NewMonitor(log, restConfig, doc.OpenShiftCluster, mon.clusterm, hiveRestConfig, hourlyRun)
 	if err != nil {
 		log.Error(err)
+		mon.m.EmitGauge("monitor.cluster.failedworker", 1, map[string]string{
+			"resourceId": doc.OpenShiftCluster.ID,
+		})
 		return
 	}