From 83b53da4dd98b2e45e1b68f38383eaa6040ba35c Mon Sep 17 00:00:00 2001 From: Jian Qiu Date: Mon, 8 Jun 2020 18:41:06 +0800 Subject: [PATCH] Add klusterlet degraded status --- pkg/helpers/helpers.go | 14 + pkg/helpers/testing/assertion.go | 78 ++++++ pkg/helpers/testing/helpers.go | 27 ++ .../clustermanager/controller_test.go | 89 ++----- .../klusterlet_controller.go} | 42 +-- .../klusterlet_controller_test.go} | 190 +------------ .../klusterlet_status_controller.go | 249 ++++++++++++++++++ .../klusterlet_status_controller_test.go | 242 +++++++++++++++++ pkg/operators/manager.go | 18 +- test/integration/clustermanager_test.go | 2 +- test/integration/klusterlet_test.go | 79 +++++- test/integration/util/assertion.go | 13 +- test/integration/util/util.go | 7 +- 13 files changed, 737 insertions(+), 313 deletions(-) create mode 100644 pkg/helpers/testing/assertion.go create mode 100644 pkg/helpers/testing/helpers.go rename pkg/operators/klusterlet/{controller.go => controllers/klusterletcontroller/klusterlet_controller.go} (84%) rename pkg/operators/klusterlet/{controller_test.go => controllers/klusterletcontroller/klusterlet_controller_test.go} (63%) create mode 100644 pkg/operators/klusterlet/controllers/statuscontroller/klusterlet_status_controller.go create mode 100644 pkg/operators/klusterlet/controllers/statuscontroller/klusterlet_status_controller_test.go diff --git a/pkg/helpers/helpers.go b/pkg/helpers/helpers.go index 16c268017..db5bd047d 100644 --- a/pkg/helpers/helpers.go +++ b/pkg/helpers/helpers.go @@ -330,3 +330,17 @@ func ApplyDirectly( ret = append(ret, applyResults...) return ret } + +// NumOfUnavailablePod is to check if a deployment is in degraded state. +func NumOfUnavailablePod(deployment *appsv1.Deployment) int32 { + desiredReplicas := int32(1) + if deployment.Spec.Replicas != nil { + desiredReplicas = *(deployment.Spec.Replicas) + } + + if desiredReplicas <= deployment.Status.AvailableReplicas { + return 0 + } + + return desiredReplicas - deployment.Status.AvailableReplicas +} diff --git a/pkg/helpers/testing/assertion.go b/pkg/helpers/testing/assertion.go new file mode 100644 index 000000000..167f1eaa8 --- /dev/null +++ b/pkg/helpers/testing/assertion.go @@ -0,0 +1,78 @@ +package testing + +import ( + "testing" + + "github.com/davecgh/go-spew/spew" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" + clienttesting "k8s.io/client-go/testing" + + opratorapiv1 "github.com/open-cluster-management/api/operator/v1" + "github.com/open-cluster-management/registration-operator/pkg/helpers" +) + +func AssertAction(t *testing.T, actual clienttesting.Action, expected string) { + if actual.GetVerb() != expected { + t.Errorf("expected %s action but got: %#v", expected, actual) + } +} + +func AssertGet(t *testing.T, actual clienttesting.Action, group, version, resource string) { + t.Helper() + if actual.GetVerb() != "get" { + t.Error(spew.Sdump(actual)) + } + if actual.GetResource() != (schema.GroupVersionResource{Group: group, Version: version, Resource: resource}) { + t.Error(spew.Sdump(actual)) + } +} + +func NamedCondition(name, reason string, status metav1.ConditionStatus) opratorapiv1.StatusCondition { + return opratorapiv1.StatusCondition{Type: name, Status: status, Reason: reason} +} + +func AssertOnlyConditions(t *testing.T, actual runtime.Object, expectedConditions ...opratorapiv1.StatusCondition) { + t.Helper() + + var actualConditions []opratorapiv1.StatusCondition + if klusterlet, ok := actual.(*opratorapiv1.Klusterlet); ok { + actualConditions = klusterlet.Status.Conditions + } else { + clustermanager := actual.(*opratorapiv1.ClusterManager) + actualConditions = clustermanager.Status.Conditions + } + if len(actualConditions) != len(expectedConditions) { + t.Errorf("expected %v condition but got: %v", len(expectedConditions), spew.Sdump(actualConditions)) + } + + for _, expectedCondition := range expectedConditions { + actual := helpers.FindOperatorCondition(actualConditions, expectedCondition.Type) + if actual == nil { + t.Errorf("missing %v in %v", spew.Sdump(expectedCondition), spew.Sdump(actual)) + } + if actual.Status != expectedCondition.Status { + t.Errorf("wrong result for %v in %v", spew.Sdump(expectedCondition), spew.Sdump(actual)) + } + if actual.Reason != expectedCondition.Reason { + t.Errorf("wrong result for %v in %v", spew.Sdump(expectedCondition), spew.Sdump(actual)) + } + } +} + +func AssertEqualNumber(t *testing.T, actual, expected int) { + if actual != expected { + t.Errorf("expected %d number of actions but got: %d", expected, actual) + } +} + +func AssertEqualNameNamespace(t *testing.T, actualName, actualNamespace, name, namespace string) { + if actualName != name { + t.Errorf("Name of the object does not match, expected %s, actual %s", name, actualName) + } + + if actualNamespace != namespace { + t.Errorf("Namespace of the object does not match, expected %s, actual %s", namespace, actualNamespace) + } +} diff --git a/pkg/helpers/testing/helpers.go b/pkg/helpers/testing/helpers.go new file mode 100644 index 000000000..9a2b86564 --- /dev/null +++ b/pkg/helpers/testing/helpers.go @@ -0,0 +1,27 @@ +package testing + +import ( + "testing" + + "github.com/openshift/library-go/pkg/operator/events" + "github.com/openshift/library-go/pkg/operator/events/eventstesting" + "k8s.io/client-go/util/workqueue" +) + +type fakeSyncContext struct { + key string + queue workqueue.RateLimitingInterface + recorder events.Recorder +} + +func (f fakeSyncContext) Queue() workqueue.RateLimitingInterface { return f.queue } +func (f fakeSyncContext) QueueKey() string { return f.key } +func (f fakeSyncContext) Recorder() events.Recorder { return f.recorder } + +func NewFakeSyncContext(t *testing.T, key string) *fakeSyncContext { + return &fakeSyncContext{ + key: key, + queue: workqueue.NewRateLimitingQueue(workqueue.DefaultControllerRateLimiter()), + recorder: eventstesting.NewTestingEventRecorder(t), + } +} diff --git a/pkg/operators/clustermanager/controller_test.go b/pkg/operators/clustermanager/controller_test.go index ce344c822..94f264534 100644 --- a/pkg/operators/clustermanager/controller_test.go +++ b/pkg/operators/clustermanager/controller_test.go @@ -20,11 +20,9 @@ import ( fakekube "k8s.io/client-go/kubernetes/fake" clienttesting "k8s.io/client-go/testing" "k8s.io/client-go/util/cert" - "k8s.io/client-go/util/workqueue" fakeapiregistration "k8s.io/kube-aggregator/pkg/client/clientset_generated/clientset/fake" - "github.com/openshift/library-go/pkg/operator/events" - "github.com/openshift/library-go/pkg/operator/events/eventstesting" + testinghelper "github.com/open-cluster-management/registration-operator/pkg/helpers/testing" ) type testController struct { @@ -35,24 +33,6 @@ type testController struct { operatorClient *fakeoperatorlient.Clientset } -type fakeSyncContext struct { - key string - queue workqueue.RateLimitingInterface - recorder events.Recorder -} - -func (f fakeSyncContext) Queue() workqueue.RateLimitingInterface { return f.queue } -func (f fakeSyncContext) QueueKey() string { return f.key } -func (f fakeSyncContext) Recorder() events.Recorder { return f.recorder } - -func newFakeSyncContext(t *testing.T, key string) *fakeSyncContext { - return &fakeSyncContext{ - key: key, - queue: workqueue.NewRateLimitingQueue(workqueue.DefaultControllerRateLimiter()), - recorder: eventstesting.NewTestingEventRecorder(t), - } -} - func newClusterManager(name string) *operatorapiv1.ClusterManager { return &operatorapiv1.ClusterManager{ ObjectMeta: metav1.ObjectMeta{ @@ -105,43 +85,6 @@ func (t *testController) withAPIServiceObject(objects ...runtime.Object) *testCo return t } -func assertAction(t *testing.T, actual clienttesting.Action, expected string) { - if actual.GetVerb() != expected { - t.Errorf("expected %s action but got: %#v", expected, actual) - } -} - -func assertEqualNumber(t *testing.T, actual, expected int) { - if actual != expected { - t.Errorf("expected %d number of actions but got: %d", expected, actual) - } -} - -func assertCondition(t *testing.T, actual runtime.Object, expectedCondition string, expectedStatus metav1.ConditionStatus) { - hubCore := actual.(*operatorapiv1.ClusterManager) - conditions := hubCore.Status.Conditions - if len(conditions) != 1 { - t.Errorf("expected 1 condition but got: %#v", conditions) - } - condition := conditions[0] - if condition.Type != expectedCondition { - t.Errorf("expected %s but got: %s", expectedCondition, condition.Type) - } - if condition.Status != expectedStatus { - t.Errorf("expected %s but got: %s", expectedStatus, condition.Status) - } -} - -func ensureNameNamespace(t *testing.T, actualName, actualNamespace, name, namespace string) { - if actualName != name { - t.Errorf("Name of the object does not match, expected %s, actual %s", name, actualName) - } - - if actualNamespace != namespace { - t.Errorf("Namespace of the object does not match, expected %s, actual %s", namespace, actualNamespace) - } -} - func ensureObject(t *testing.T, object runtime.Object, hubCore *operatorapiv1.ClusterManager) { access, err := meta.Accessor(object) if err != nil { @@ -150,7 +93,7 @@ func ensureObject(t *testing.T, object runtime.Object, hubCore *operatorapiv1.Cl switch o := object.(type) { case *corev1.Namespace: - ensureNameNamespace(t, access.GetName(), "", clusterManagerNamespace, "") + testinghelper.AssertEqualNameNamespace(t, access.GetName(), "", clusterManagerNamespace, "") case *appsv1.Deployment: if hubCore.Spec.RegistrationImagePullSpec != o.Spec.Template.Spec.Containers[0].Image { t.Errorf("Image does not match to the expected.") @@ -162,7 +105,7 @@ func ensureObject(t *testing.T, object runtime.Object, hubCore *operatorapiv1.Cl func TestSyncDeploy(t *testing.T) { clusterManager := newClusterManager("testhub") controller := newTestController(clusterManager).withCRDObject().withKubeObject().withAPIServiceObject() - syncContext := newFakeSyncContext(t, "testhub") + syncContext := testinghelper.NewFakeSyncContext(t, "testhub") err := controller.controller.sync(nil, syncContext) if err != nil { @@ -179,7 +122,7 @@ func TestSyncDeploy(t *testing.T) { } // Check if resources are created as expected - assertEqualNumber(t, len(createKubeObjects), 12) + testinghelper.AssertEqualNumber(t, len(createKubeObjects), 12) for _, object := range createKubeObjects { ensureObject(t, object, clusterManager) } @@ -193,7 +136,7 @@ func TestSyncDeploy(t *testing.T) { } } // Check if resources are created as expected - assertEqualNumber(t, len(createCRDObjects), 2) + testinghelper.AssertEqualNumber(t, len(createCRDObjects), 2) createAPIServiceObjects := []runtime.Object{} apiServiceActions := controller.apiRegistrationClient.Actions() @@ -204,12 +147,14 @@ func TestSyncDeploy(t *testing.T) { } } // Check if resources are created as expected - assertEqualNumber(t, len(createAPIServiceObjects), 1) + testinghelper.AssertEqualNumber(t, len(createAPIServiceObjects), 1) clusterManagerAction := controller.operatorClient.Actions() - assertEqualNumber(t, len(clusterManagerAction), 2) - assertAction(t, clusterManagerAction[1], "update") - assertCondition(t, clusterManagerAction[1].(clienttesting.UpdateActionImpl).Object, clusterManagerApplied, metav1.ConditionTrue) + testinghelper.AssertEqualNumber(t, len(clusterManagerAction), 2) + testinghelper.AssertAction(t, clusterManagerAction[1], "update") + testinghelper.AssertOnlyConditions( + t, clusterManagerAction[1].(clienttesting.UpdateActionImpl).Object, + testinghelper.NamedCondition(clusterManagerApplied, "ClusterManagerApplied", metav1.ConditionTrue)) } // TestSyncDelete test cleanup hub deploy @@ -218,7 +163,7 @@ func TestSyncDelete(t *testing.T) { now := metav1.Now() clusterManager.ObjectMeta.SetDeletionTimestamp(&now) controller := newTestController(clusterManager).withCRDObject().withKubeObject().withAPIServiceObject() - syncContext := newFakeSyncContext(t, "testhub") + syncContext := testinghelper.NewFakeSyncContext(t, "testhub") err := controller.controller.sync(nil, syncContext) if err != nil { @@ -233,7 +178,7 @@ func TestSyncDelete(t *testing.T) { deleteKubeActions = append(deleteKubeActions, deleteKubeAction) } } - assertEqualNumber(t, len(deleteKubeActions), 10) + testinghelper.AssertEqualNumber(t, len(deleteKubeActions), 10) deleteCRDActions := []clienttesting.DeleteActionImpl{} crdActions := controller.apiExtensionClient.Actions() @@ -244,7 +189,7 @@ func TestSyncDelete(t *testing.T) { } } // Check if resources are created as expected - assertEqualNumber(t, len(deleteCRDActions), 4) + testinghelper.AssertEqualNumber(t, len(deleteCRDActions), 4) deleteAPIServiceActions := []clienttesting.DeleteActionImpl{} apiServiceActions := controller.apiRegistrationClient.Actions() @@ -255,12 +200,12 @@ func TestSyncDelete(t *testing.T) { } } // Check if resources are created as expected - assertEqualNumber(t, len(deleteAPIServiceActions), 1) + testinghelper.AssertEqualNumber(t, len(deleteAPIServiceActions), 1) for _, action := range deleteKubeActions { switch action.Resource.Resource { case "namespaces": - ensureNameNamespace(t, action.Name, "", clusterManagerNamespace, "") + testinghelper.AssertEqualNameNamespace(t, action.Name, "", clusterManagerNamespace, "") } } } @@ -288,7 +233,7 @@ func TestDeleteCRD(t *testing.T) { apiextensionsv1.Resource("customresourcedefinitions"), crdNames[0]) }) - syncContext := newFakeSyncContext(t, "testhub") + syncContext := testinghelper.NewFakeSyncContext(t, "testhub") err := controller.controller.sync(nil, syncContext) if err == nil { t.Errorf("Expected error when sync") diff --git a/pkg/operators/klusterlet/controller.go b/pkg/operators/klusterlet/controllers/klusterletcontroller/klusterlet_controller.go similarity index 84% rename from pkg/operators/klusterlet/controller.go rename to pkg/operators/klusterlet/controllers/klusterletcontroller/klusterlet_controller.go index 36274d481..27bfa9e83 100644 --- a/pkg/operators/klusterlet/controller.go +++ b/pkg/operators/klusterlet/controllers/klusterletcontroller/klusterlet_controller.go @@ -1,4 +1,4 @@ -package klusterlet +package klusterletcontroller import ( "context" @@ -167,17 +167,6 @@ func (n *klusterletController) sync(ctx context.Context, controllerContext facto return err } - // Check if bootstrap secret exists - _, err = n.kubeClient.CoreV1().Secrets(config.KlusterletNamespace).Get( - ctx, config.BootStrapKubeConfigSecret, metav1.GetOptions{}) - if err != nil { - helpers.UpdateKlusterletStatus(ctx, n.klusterletClient, klusterletName, helpers.UpdateKlusterletConditionFn(operatorapiv1.StatusCondition{ - Type: klusterletApplied, Status: metav1.ConditionFalse, Reason: "KlusterletApplyFailed", - Message: fmt.Sprintf("Failed to get bootstrap secret -n %q %q: %v", config.KlusterletNamespace, config.BootStrapKubeConfigSecret, err), - })) - return err - } - // Deploy the static resources // Apply static files resourceResults := resourceapply.ApplyDirectly( @@ -283,35 +272,6 @@ func (n *klusterletController) sync(ctx context.Context, controllerContext facto Type: klusterletApplied, Status: metav1.ConditionTrue, Reason: "KlusterletApplied", Message: "Klusterlet Component Applied", })) - - // now that we have applied all of our logic, we can check to see if the data we expect to have present as indications of - // proper functioning of registration controller is working - // TODO this should be moved into a separate loop since it is independent of the application of the eventually consistent - // resources above - - // If cluster name is empty, return err - if config.ClusterName == "" { - helpers.UpdateKlusterletStatus(ctx, n.klusterletClient, klusterletName, helpers.UpdateKlusterletConditionFn(operatorapiv1.StatusCondition{ - Type: klusterletRegistrationDegraded, Status: metav1.ConditionTrue, Reason: "ClusterNameMissing", - Message: fmt.Sprintf("Failed to get cluster name from `kubectl get secret -n %q %q -ojsonpath='{.data.cluster-name}`. This is set by the klusterlet registration deployment.", hubSecret.Namespace, hubSecret.Name), - })) - return fmt.Errorf("Failed to get cluster name") - } - - // If hub kubeconfig does not exist, return err. - if hubSecret.Data["kubeconfig"] == nil { - helpers.UpdateKlusterletStatus(ctx, n.klusterletClient, klusterletName, helpers.UpdateKlusterletConditionFn(operatorapiv1.StatusCondition{ - Type: klusterletRegistrationDegraded, Status: metav1.ConditionTrue, Reason: "HubKubeconfigMissing", - Message: fmt.Sprintf("Failed to get kubeconfig from `kubectl get secret -n %q %q -ojsonpath='{.data.kubeconfig}`. This is set by the klusterlet registration deployment, but the CSR must be approved by the cluster-admin on the hub.", hubSecret.Namespace, hubSecret.Name), - })) - return fmt.Errorf("Failed to get kubeconfig from hub kubeconfig secret") - } - // TODO it is possible to verify the kubeconfig actually works. - - helpers.UpdateKlusterletStatus(ctx, n.klusterletClient, klusterletName, helpers.UpdateKlusterletConditionFn(operatorapiv1.StatusCondition{ - Type: klusterletRegistrationDegraded, Status: metav1.ConditionFalse, Reason: "RegistrationFunctional", - Message: "Registration is managing credentials", - })) return nil } diff --git a/pkg/operators/klusterlet/controller_test.go b/pkg/operators/klusterlet/controllers/klusterletcontroller/klusterlet_controller_test.go similarity index 63% rename from pkg/operators/klusterlet/controller_test.go rename to pkg/operators/klusterlet/controllers/klusterletcontroller/klusterlet_controller_test.go index 80e27b0ce..4da709523 100644 --- a/pkg/operators/klusterlet/controller_test.go +++ b/pkg/operators/klusterlet/controllers/klusterletcontroller/klusterlet_controller_test.go @@ -1,4 +1,4 @@ -package klusterlet +package klusterletcontroller import ( "fmt" @@ -6,24 +6,19 @@ import ( "testing" "time" - "github.com/davecgh/go-spew/spew" fakeoperatorclient "github.com/open-cluster-management/api/client/operator/clientset/versioned/fake" operatorinformers "github.com/open-cluster-management/api/client/operator/informers/externalversions" opratorapiv1 "github.com/open-cluster-management/api/operator/v1" - "github.com/open-cluster-management/registration-operator/pkg/helpers" - "github.com/openshift/library-go/pkg/operator/events" - "github.com/openshift/library-go/pkg/operator/events/eventstesting" + testinghelper "github.com/open-cluster-management/registration-operator/pkg/helpers/testing" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" - "k8s.io/apimachinery/pkg/runtime/schema" fakekube "k8s.io/client-go/kubernetes/fake" clienttesting "k8s.io/client-go/testing" "k8s.io/client-go/tools/cache" - "k8s.io/client-go/util/workqueue" ) type testController struct { @@ -33,24 +28,6 @@ type testController struct { operatorStore cache.Store } -type fakeSyncContext struct { - key string - queue workqueue.RateLimitingInterface - recorder events.Recorder -} - -func (f fakeSyncContext) Queue() workqueue.RateLimitingInterface { return f.queue } -func (f fakeSyncContext) QueueKey() string { return f.key } -func (f fakeSyncContext) Recorder() events.Recorder { return f.recorder } - -func newFakeSyncContext(t *testing.T, key string) *fakeSyncContext { - return &fakeSyncContext{ - key: key, - queue: workqueue.NewRateLimitingQueue(workqueue.DefaultControllerRateLimiter()), - recorder: eventstesting.NewTestingEventRecorder(t), - } -} - func newSecret(name, namespace string) *corev1.Secret { return &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ @@ -107,56 +84,6 @@ func newTestController(klusterlet *opratorapiv1.Klusterlet, objects ...runtime.O } } -func assertAction(t *testing.T, actual clienttesting.Action, expected string) { - if actual.GetVerb() != expected { - t.Errorf("expected %s action but got: %#v", expected, actual) - } -} - -func assertGet(t *testing.T, actual clienttesting.Action, group, version, resource string) { - t.Helper() - if actual.GetVerb() != "get" { - t.Error(spew.Sdump(actual)) - } - if actual.GetResource() != (schema.GroupVersionResource{Group: group, Version: version, Resource: resource}) { - t.Error(spew.Sdump(actual)) - } -} - -func namedCondition(name string, status metav1.ConditionStatus) opratorapiv1.StatusCondition { - return opratorapiv1.StatusCondition{Type: name, Status: status} -} - -func assertOnlyConditions(t *testing.T, actual runtime.Object, expectedConditions ...opratorapiv1.StatusCondition) { - t.Helper() - - klusterlet := actual.(*opratorapiv1.Klusterlet) - actualConditions := klusterlet.Status.Conditions - if len(actualConditions) != len(expectedConditions) { - t.Errorf("expected %v condition but got: %v", len(expectedConditions), spew.Sdump(actualConditions)) - } - - for _, expectedCondition := range expectedConditions { - actual := helpers.FindOperatorCondition(actualConditions, expectedCondition.Type) - if actual == nil { - t.Errorf("missing %v in %v", spew.Sdump(expectedCondition), spew.Sdump(actual)) - } - if actual.Status != expectedCondition.Status { - t.Errorf("wrong result for %v in %v", spew.Sdump(expectedCondition), spew.Sdump(actual)) - } - } -} - -func ensureNameNamespace(t *testing.T, actualName, actualNamespace, name, namespace string) { - if actualName != name { - t.Errorf("Name of the object does not match, expected %s, actual %s", name, actualName) - } - - if actualNamespace != namespace { - t.Errorf("Namespace of the object does not match, expected %s, actual %s", namespace, actualNamespace) - } -} - func ensureDeployments(t *testing.T, actions []clienttesting.Action, verb, serverURL, registrationClusterName, workClusterName string) { deployments := []*appsv1.Deployment{} for _, action := range actions { @@ -255,14 +182,14 @@ func ensureObject(t *testing.T, object runtime.Object, klusterlet *opratorapiv1. switch o := object.(type) { case *appsv1.Deployment: if strings.Contains(access.GetName(), "registration") { - ensureNameNamespace( + testinghelper.AssertEqualNameNamespace( t, access.GetName(), access.GetNamespace(), fmt.Sprintf("%s-registration-agent", klusterlet.Name), klusterlet.Spec.Namespace) if klusterlet.Spec.RegistrationImagePullSpec != o.Spec.Template.Spec.Containers[0].Image { t.Errorf("Image does not match to the expected.") } } else if strings.Contains(access.GetName(), "work") { - ensureNameNamespace( + testinghelper.AssertEqualNameNamespace( t, access.GetName(), access.GetNamespace(), fmt.Sprintf("%s-work-agent", klusterlet.Name), klusterlet.Spec.Namespace) if klusterlet.Spec.WorkImagePullSpec != o.Spec.Template.Spec.Containers[0].Image { @@ -282,7 +209,7 @@ func TestSyncDeploy(t *testing.T) { hubKubeConfigSecret.Data["kubeconfig"] = []byte("dummuykubeconnfig") namespace := newNamespace("testns") controller := newTestController(klusterlet, bootStrapSecret, hubKubeConfigSecret, namespace) - syncContext := newFakeSyncContext(t, "klusterlet") + syncContext := testinghelper.NewFakeSyncContext(t, "klusterlet") err := controller.controller.sync(nil, syncContext) if err != nil { @@ -306,109 +233,16 @@ func TestSyncDeploy(t *testing.T) { ensureObject(t, object, klusterlet) } - operatorAction := controller.operatorClient.Actions() - if len(operatorAction) != 4 { - t.Errorf("Expect 4 actions in the sync loop, actual %#v", operatorAction) - } - - assertGet(t, operatorAction[0], "operator.open-cluster-management.io", "v1", "klusterlets") - assertAction(t, operatorAction[1], "update") - assertOnlyConditions(t, operatorAction[1].(clienttesting.UpdateActionImpl).Object, - namedCondition(klusterletApplied, metav1.ConditionTrue)) - assertGet(t, operatorAction[2], "operator.open-cluster-management.io", "v1", "klusterlets") - assertAction(t, operatorAction[3], "update") - assertOnlyConditions(t, operatorAction[3].(clienttesting.UpdateActionImpl).Object, - namedCondition(klusterletApplied, metav1.ConditionTrue), namedCondition(klusterletRegistrationDegraded, metav1.ConditionFalse)) -} - -// TestSyncWithNoSecret test the scenario that bootstrap secret and hub config secret does not exist -func TestSyncWithNoSecret(t *testing.T) { - klusterlet := newKlusterlet("klusterlet", "testns", "") - bootStrapSecret := newSecret(bootstrapHubKubeConfigSecret, "testns") - hubSecret := newSecret(hubKubeConfigSecret, "testns") - namespace := newNamespace("testns") - controller := newTestController(klusterlet, namespace) - syncContext := newFakeSyncContext(t, "klusterlet") - - // Return err since bootstrap secret does not exist - err := controller.controller.sync(nil, syncContext) - if err == nil { - t.Errorf("Expected error when sync") - } operatorAction := controller.operatorClient.Actions() if len(operatorAction) != 2 { t.Errorf("Expect 2 actions in the sync loop, actual %#v", operatorAction) } - assertGet(t, operatorAction[0], "operator.open-cluster-management.io", "v1", "klusterlets") - assertAction(t, operatorAction[1], "update") - assertOnlyConditions(t, operatorAction[1].(clienttesting.UpdateActionImpl).Object, namedCondition(klusterletApplied, metav1.ConditionFalse)) - - // reset for round 2 - controller.operatorClient.ClearActions() - // Add bootstrap secret and sync again - controller.kubeClient.PrependReactor("get", "secrets", func(action clienttesting.Action) (handled bool, ret runtime.Object, err error) { - if action.GetVerb() != "get" { - return false, nil, nil - } - - getAction := action.(clienttesting.GetActionImpl) - if getAction.Name != bootstrapHubKubeConfigSecret { - return false, nil, errors.NewNotFound( - corev1.Resource("secrets"), bootstrapHubKubeConfigSecret) - } - return true, bootStrapSecret, nil - }) - // Return err since cluster-name cannot be found in hubkubeconfig secret - err = controller.controller.sync(nil, syncContext) - if err == nil { - t.Errorf("Expected error when sync") - } - operatorAction = controller.operatorClient.Actions() - if len(operatorAction) != 4 { - t.Errorf("Expect 4 actions in the sync loop, actual %#v", operatorAction) - } - - assertGet(t, operatorAction[0], "operator.open-cluster-management.io", "v1", "klusterlets") - assertAction(t, operatorAction[1], "update") - assertOnlyConditions(t, operatorAction[1].(clienttesting.UpdateActionImpl).Object, - namedCondition(klusterletApplied, metav1.ConditionTrue)) - assertGet(t, operatorAction[2], "operator.open-cluster-management.io", "v1", "klusterlets") - assertAction(t, operatorAction[3], "update") - assertOnlyConditions(t, operatorAction[3].(clienttesting.UpdateActionImpl).Object, - namedCondition(klusterletApplied, metav1.ConditionTrue), namedCondition(klusterletRegistrationDegraded, metav1.ConditionTrue)) - - // reset for round 3 - controller.operatorClient.ClearActions() - // Add hub config secret and sync again - hubSecret.Data["kubeconfig"] = []byte("dummykubeconfig") - hubSecret.Data["cluster-name"] = []byte("cluster1") - controller.kubeClient.PrependReactor("get", "secrets", func(action clienttesting.Action) (handled bool, ret runtime.Object, err error) { - if action.GetVerb() != "get" { - return false, nil, nil - } - - getAction := action.(clienttesting.GetActionImpl) - if getAction.Name != hubKubeConfigSecret { - return false, nil, errors.NewNotFound( - corev1.Resource("secrets"), hubKubeConfigSecret) - } - return true, hubSecret, nil - }) - err = controller.controller.sync(nil, syncContext) - if err != nil { - t.Errorf("Expected no error when sync: %v", err) - } - operatorAction = controller.operatorClient.Actions() - if len(operatorAction) != 3 { - t.Errorf("Expect 3 actions in the sync loop, actual %#v", operatorAction) - } - - assertGet(t, operatorAction[0], "operator.open-cluster-management.io", "v1", "klusterlets") - assertGet(t, operatorAction[1], "operator.open-cluster-management.io", "v1", "klusterlets") - assertAction(t, operatorAction[2], "update") - assertOnlyConditions(t, operatorAction[2].(clienttesting.UpdateActionImpl).Object, - namedCondition(klusterletApplied, metav1.ConditionTrue), namedCondition(klusterletRegistrationDegraded, metav1.ConditionFalse)) + testinghelper.AssertGet(t, operatorAction[0], "operator.open-cluster-management.io", "v1", "klusterlets") + testinghelper.AssertAction(t, operatorAction[1], "update") + testinghelper.AssertOnlyConditions( + t, operatorAction[1].(clienttesting.UpdateActionImpl).Object, + testinghelper.NamedCondition(klusterletApplied, "KlusterletApplied", metav1.ConditionTrue)) } // TestSyncDelete test cleanup hub deploy @@ -418,7 +252,7 @@ func TestSyncDelete(t *testing.T) { klusterlet.ObjectMeta.SetDeletionTimestamp(&now) namespace := newNamespace("testns") controller := newTestController(klusterlet, namespace) - syncContext := newFakeSyncContext(t, "klusterlet") + syncContext := testinghelper.NewFakeSyncContext(t, "klusterlet") err := controller.controller.sync(nil, syncContext) if err != nil { @@ -491,7 +325,7 @@ func TestClusterNameChange(t *testing.T) { hubSecret.Data["kubeconfig"] = []byte("dummuykubeconnfig") hubSecret.Data["cluster-name"] = []byte("cluster1") controller := newTestController(klusterlet, bootStrapSecret, hubSecret, namespace) - syncContext := newFakeSyncContext(t, "klusterlet") + syncContext := testinghelper.NewFakeSyncContext(t, "klusterlet") err := controller.controller.sync(nil, syncContext) if err != nil { t.Errorf("Expected non error when sync, %v", err) diff --git a/pkg/operators/klusterlet/controllers/statuscontroller/klusterlet_status_controller.go b/pkg/operators/klusterlet/controllers/statuscontroller/klusterlet_status_controller.go new file mode 100644 index 000000000..1424f74c4 --- /dev/null +++ b/pkg/operators/klusterlet/controllers/statuscontroller/klusterlet_status_controller.go @@ -0,0 +1,249 @@ +package statuscontroller + +import ( + "context" + "fmt" + "strings" + + "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/api/meta" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/labels" + "k8s.io/apimachinery/pkg/runtime" + appsinformer "k8s.io/client-go/informers/apps/v1" + coreinformer "k8s.io/client-go/informers/core/v1" + "k8s.io/client-go/kubernetes" + appslister "k8s.io/client-go/listers/apps/v1" + corelister "k8s.io/client-go/listers/core/v1" + "k8s.io/klog" + + "github.com/openshift/library-go/pkg/controller/factory" + "github.com/openshift/library-go/pkg/operator/events" + "github.com/openshift/library-go/pkg/operator/resource/resourcehelper" + + operatorv1client "github.com/open-cluster-management/api/client/operator/clientset/versioned/typed/operator/v1" + operatorinformer "github.com/open-cluster-management/api/client/operator/informers/externalversions/operator/v1" + operatorlister "github.com/open-cluster-management/api/client/operator/listers/operator/v1" + operatorapiv1 "github.com/open-cluster-management/api/operator/v1" + "github.com/open-cluster-management/registration-operator/pkg/helpers" +) + +type klusterletStatusController struct { + kubeClient kubernetes.Interface + secretLister corelister.SecretLister + deploymentLister appslister.DeploymentLister + klusterletClient operatorv1client.KlusterletInterface + klusterletLister operatorlister.KlusterletLister +} + +const ( + klusterletNamespace = "open-cluster-management-agent" + bootstrapHubKubeConfigSecret = "bootstrap-hub-kubeconfig" + hubKubeConfigSecret = "hub-kubeconfig-secret" + klusterletRegistrationDegraded = "KlusterletRegistrationDegraded" + klusterletWorKDegraded = "KlusterletWorkDegraded" +) + +// NewKlusterletStatusController returns a klusterletStatusController +func NewKlusterletStatusController( + kubeClient kubernetes.Interface, + klusterletClient operatorv1client.KlusterletInterface, + klusterletInformer operatorinformer.KlusterletInformer, + secretInformer coreinformer.SecretInformer, + deploymentInformer appsinformer.DeploymentInformer, + recorder events.Recorder) factory.Controller { + controller := &klusterletStatusController{ + kubeClient: kubeClient, + klusterletClient: klusterletClient, + secretLister: secretInformer.Lister(), + deploymentLister: deploymentInformer.Lister(), + klusterletLister: klusterletInformer.Lister(), + } + return factory.New().WithSync(controller.sync). + WithInformersQueueKeyFunc(controller.queueKeyFunc, secretInformer.Informer(), deploymentInformer.Informer()). + WithInformersQueueKeyFunc(func(obj runtime.Object) string { + accessor, _ := meta.Accessor(obj) + return accessor.GetName() + }, klusterletInformer.Informer()). + ToController("KlusterletStatusController", recorder) +} + +func (k *klusterletStatusController) sync(ctx context.Context, controllerContext factory.SyncContext) error { + klusterletName := controllerContext.QueueKey() + if klusterletName == "" { + return nil + } + klog.V(4).Infof("Reconciling Klusterlet %q", klusterletName) + + klusterlet, err := k.klusterletLister.Get(klusterletName) + switch { + case errors.IsNotFound(err): + return nil + case err != nil: + return err + } + klusterlet = klusterlet.DeepCopy() + + klusterletNS := klusterlet.Spec.Namespace + if klusterletNS == "" { + klusterletNS = klusterletNamespace + } + + registrationDegradedCondition := operatorapiv1.StatusCondition{ + Type: klusterletRegistrationDegraded, + Status: metav1.ConditionFalse, + Reason: "RegistrationFunctional", + Message: "Registration is managing credentials", + } + workDegradedCondition := operatorapiv1.StatusCondition{ + Type: klusterletWorKDegraded, + Status: metav1.ConditionFalse, + Reason: "WorkFunctional", + Message: "Work is managing manifests", + } + + // Check if bootstrap secret exists + _, err = k.kubeClient.CoreV1().Secrets(klusterletNS).Get(ctx, bootstrapHubKubeConfigSecret, metav1.GetOptions{}) + if err != nil { + registrationDegradedCondition.Message = fmt.Sprintf("Failed to get bootstrap secret %q %q: %v", klusterletNS, bootstrapHubKubeConfigSecret, err) + registrationDegradedCondition.Status = metav1.ConditionTrue + registrationDegradedCondition.Reason = "BootStrapSecretMissing" + _, _, err := helpers.UpdateKlusterletStatus(ctx, k.klusterletClient, klusterletName, + helpers.UpdateKlusterletConditionFn(registrationDegradedCondition), + ) + return err + } + // TODO verify if bootstrap secret works + + // Check if hub kubeconfig secret exists + hubConfigSecret, err := k.kubeClient.CoreV1().Secrets(klusterletNS).Get(ctx, hubKubeConfigSecret, metav1.GetOptions{}) + if err != nil { + registrationDegradedCondition.Message = fmt.Sprintf("Failed to get hub kubeconfig secret %q %q: %v", klusterletNS, hubKubeConfigSecret, err) + registrationDegradedCondition.Status = metav1.ConditionTrue + registrationDegradedCondition.Reason = "HubKubeConfigSecretMissing" + // Work condition will be the same as registration + workDegradedCondition.Message = registrationDegradedCondition.Message + workDegradedCondition.Status = registrationDegradedCondition.Status + workDegradedCondition.Reason = registrationDegradedCondition.Reason + + _, _, err := helpers.UpdateKlusterletStatus(ctx, k.klusterletClient, klusterletName, + helpers.UpdateKlusterletConditionFn(registrationDegradedCondition), + helpers.UpdateKlusterletConditionFn(workDegradedCondition), + ) + return err + } + + // If cluster name is empty, read cluster name from hub config secret + if klusterlet.Spec.ClusterName == "" { + clusterName := hubConfigSecret.Data["cluster-name"] + if clusterName == nil { + registrationDegradedCondition.Message = fmt.Sprintf( + "Failed to get cluster name from `kubectl get secret -n %q %q -ojsonpath='{.data.cluster-name}`. This is set by the klusterlet registration deployment.", hubConfigSecret.Namespace, hubConfigSecret.Name) + registrationDegradedCondition.Status = metav1.ConditionTrue + registrationDegradedCondition.Reason = "ClusterNameMissing" + // Work condition will be the same as registration + workDegradedCondition.Message = registrationDegradedCondition.Message + workDegradedCondition.Status = registrationDegradedCondition.Status + workDegradedCondition.Reason = registrationDegradedCondition.Reason + + _, _, err := helpers.UpdateKlusterletStatus(ctx, k.klusterletClient, klusterletName, + helpers.UpdateKlusterletConditionFn(registrationDegradedCondition), + helpers.UpdateKlusterletConditionFn(workDegradedCondition), + ) + return err + } + } + + // If hub kubeconfig does not exist, return err. + if hubConfigSecret.Data["kubeconfig"] == nil { + registrationDegradedCondition.Message = fmt.Sprintf( + "Failed to get kubeconfig from `kubectl get secret -n %q %q -ojsonpath='{.data.kubeconfig}`. This is set by the klusterlet registration deployment, but the CSR must be approved by the cluster-admin on the hub.", hubConfigSecret.Namespace, hubConfigSecret.Name) + registrationDegradedCondition.Status = metav1.ConditionTrue + registrationDegradedCondition.Reason = "KubeConfigMissing" + // Work condition will be the same as registration + workDegradedCondition.Message = registrationDegradedCondition.Message + workDegradedCondition.Status = registrationDegradedCondition.Status + workDegradedCondition.Reason = registrationDegradedCondition.Reason + + _, _, err := helpers.UpdateKlusterletStatus(ctx, k.klusterletClient, klusterletName, + helpers.UpdateKlusterletConditionFn(registrationDegradedCondition), + helpers.UpdateKlusterletConditionFn(workDegradedCondition), + ) + return err + } + // TODO it is possible to verify the kubeconfig actually works. + + // Check deployment status + registrationDeploymentName := fmt.Sprintf("%s-registration-agent", klusterlet.Name) + registrationDeployment, err := k.kubeClient.AppsV1().Deployments(klusterletNS).Get(ctx, registrationDeploymentName, metav1.GetOptions{}) + if err != nil { + registrationDegradedCondition.Message = fmt.Sprintf("Failed to get registration deployment %q %q: %v", klusterletNS, registrationDeploymentName, err) + registrationDegradedCondition.Status = metav1.ConditionTrue + registrationDegradedCondition.Reason = "GetRegistrationDeploymentFailed" + } else if unavailablePod := helpers.NumOfUnavailablePod(registrationDeployment); unavailablePod > 0 { + registrationDegradedCondition.Message = fmt.Sprintf("%v of requested instances are unavailable of registration deployment %q %q", unavailablePod, klusterletNS, registrationDeploymentName) + registrationDegradedCondition.Status = metav1.ConditionTrue + registrationDegradedCondition.Reason = "UnavailableRegistrationPod" + } + + workDeploymentName := fmt.Sprintf("%s-work-agent", klusterlet.Name) + workDeployment, err := k.kubeClient.AppsV1().Deployments(klusterletNS).Get(ctx, workDeploymentName, metav1.GetOptions{}) + if err != nil { + workDegradedCondition.Message = fmt.Sprintf("Failed to get work deployment %q %q: %v", klusterletNS, workDeploymentName, err) + workDegradedCondition.Status = metav1.ConditionTrue + workDegradedCondition.Reason = "GetWorkDeploymentFailed" + } else if unavailablePod := helpers.NumOfUnavailablePod(workDeployment); unavailablePod > 0 { + workDegradedCondition.Message = fmt.Sprintf("%v of requested instances are unavailable of work deployment %q %q", unavailablePod, klusterletNS, workDeploymentName) + workDegradedCondition.Status = metav1.ConditionTrue + workDegradedCondition.Reason = "UnavailableWorkPod" + } + + helpers.UpdateKlusterletStatus(ctx, k.klusterletClient, klusterletName, + helpers.UpdateKlusterletConditionFn(registrationDegradedCondition), + helpers.UpdateKlusterletConditionFn(workDegradedCondition), + ) + return nil +} + +func (k *klusterletStatusController) queueKeyFunc(obj runtime.Object) string { + accessor, _ := meta.Accessor(obj) + namespace := accessor.GetNamespace() + name := accessor.GetName() + + // return empty key if secret ot deployment is not interesting + gvk := resourcehelper.GuessObjectGroupVersionKind(obj) + interestedObjectFound := false + switch gvk.Kind { + case "Secret": + if name == hubKubeConfigSecret || name == bootstrapHubKubeConfigSecret { + interestedObjectFound = true + } + case "Deployment": + if strings.HasSuffix(name, "registration-agent") || strings.HasSuffix(name, "work-agent") { + interestedObjectFound = true + } + } + + if !interestedObjectFound { + return "" + } + + klusterlets, err := k.klusterletLister.List(labels.Everything()) + if err != nil { + return "" + } + + for _, klusterlet := range klusterlets { + klusterletNS := klusterlet.Spec.Namespace + if klusterletNS == "" { + klusterletNS = klusterletNamespace + } + if namespace == klusterletNS { + return klusterlet.Name + } + + return "" + } + + return "" +} diff --git a/pkg/operators/klusterlet/controllers/statuscontroller/klusterlet_status_controller_test.go b/pkg/operators/klusterlet/controllers/statuscontroller/klusterlet_status_controller_test.go new file mode 100644 index 000000000..75d522430 --- /dev/null +++ b/pkg/operators/klusterlet/controllers/statuscontroller/klusterlet_status_controller_test.go @@ -0,0 +1,242 @@ +package statuscontroller + +import ( + "testing" + "time" + + appsv1 "k8s.io/api/apps/v1" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + kubeinformers "k8s.io/client-go/informers" + fakekube "k8s.io/client-go/kubernetes/fake" + clienttesting "k8s.io/client-go/testing" + + fakeoperatorclient "github.com/open-cluster-management/api/client/operator/clientset/versioned/fake" + operatorinformers "github.com/open-cluster-management/api/client/operator/informers/externalversions" + operatorapiv1 "github.com/open-cluster-management/api/operator/v1" + testinghelper "github.com/open-cluster-management/registration-operator/pkg/helpers/testing" +) + +type testController struct { + controller *klusterletStatusController + operatorClient *fakeoperatorclient.Clientset +} + +func newSecret(name, namespace string) *corev1.Secret { + return &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: namespace, + }, + Data: map[string][]byte{}, + } +} + +func newSecretWithKubeConfig(name, namespace string) *corev1.Secret { + secret := newSecret(name, namespace) + secret.Data["kubeconfig"] = []byte("kubeconfig") + return secret +} + +func newKlusterlet(name, namespace, clustername string) *operatorapiv1.Klusterlet { + return &operatorapiv1.Klusterlet{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + }, + Spec: operatorapiv1.KlusterletSpec{ + RegistrationImagePullSpec: "testregistration", + WorkImagePullSpec: "testwork", + ClusterName: clustername, + Namespace: namespace, + ExternalServerURLs: []operatorapiv1.ServerURL{}, + }, + } +} + +func newDeployment(name, namespace string, desiredReplica, availableReplica int32) *appsv1.Deployment { + return &appsv1.Deployment{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: namespace, + }, + Spec: appsv1.DeploymentSpec{ + Replicas: &desiredReplica, + }, + Status: appsv1.DeploymentStatus{ + AvailableReplicas: availableReplica, + }, + } +} + +func newTestController(klusterlet *operatorapiv1.Klusterlet, objects ...runtime.Object) *testController { + fakeKubeClient := fakekube.NewSimpleClientset(objects...) + fakeOperatorClient := fakeoperatorclient.NewSimpleClientset(klusterlet) + operatorInformers := operatorinformers.NewSharedInformerFactory(fakeOperatorClient, 5*time.Minute) + kubeInformers := kubeinformers.NewSharedInformerFactory(fakeKubeClient, 5*time.Minute) + + klusterletController := &klusterletStatusController{ + kubeClient: fakeKubeClient, + klusterletClient: fakeOperatorClient.OperatorV1().Klusterlets(), + secretLister: kubeInformers.Core().V1().Secrets().Lister(), + deploymentLister: kubeInformers.Apps().V1().Deployments().Lister(), + klusterletLister: operatorInformers.Operator().V1().Klusterlets().Lister(), + } + + store := operatorInformers.Operator().V1().Klusterlets().Informer().GetStore() + store.Add(klusterlet) + + return &testController{ + controller: klusterletController, + operatorClient: fakeOperatorClient, + } +} + +// TestQueueKeyFunc test queueKeyFunc +func TestQueueKeyFunc(t *testing.T) { + cases := []struct { + name string + object runtime.Object + klusterlet *operatorapiv1.Klusterlet + expectedKey string + }{ + { + name: "key by secret", + object: newSecret(hubKubeConfigSecret, "test"), + klusterlet: newKlusterlet("testklusterlet", "test", ""), + expectedKey: "testklusterlet", + }, + { + name: "key by deployment", + object: newDeployment("test-work-agent", "test", 0, 0), + klusterlet: newKlusterlet("testklusterlet", "test", ""), + expectedKey: "testklusterlet", + }, + { + name: "key by wrong secret", + object: newSecret("dummy", "test"), + klusterlet: newKlusterlet("testklusterlet", "test", ""), + expectedKey: "", + }, + { + name: "key by wrong deployment", + object: newDeployment("dummy", "test", 0, 0), + klusterlet: newKlusterlet("testklusterlet", "test", ""), + expectedKey: "", + }, + { + name: "key by wrong klusterlet", + object: newDeployment("test-work-agent", "test", 0, 0), + klusterlet: newKlusterlet("testklusterlet", "test1", ""), + expectedKey: "", + }, + { + name: "key by klusterlet with empty namespace", + object: newSecret(bootstrapHubKubeConfigSecret, klusterletNamespace), + klusterlet: newKlusterlet("testklusterlet", "", ""), + expectedKey: "testklusterlet", + }, + } + + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + controller := newTestController(c.klusterlet, c.object) + actualKey := controller.controller.queueKeyFunc(c.object) + if actualKey != c.expectedKey { + t.Errorf("Queued key is not correct: actual %s, expected %s", actualKey, c.expectedKey) + } + }) + } +} + +func TestSync(t *testing.T) { + cases := []struct { + name string + object []runtime.Object + klusterlet *operatorapiv1.Klusterlet + expectedConditions []operatorapiv1.StatusCondition + }{ + { + name: "No bootstrap secret", + object: []runtime.Object{}, + klusterlet: newKlusterlet("testklusterlet", "test", ""), + expectedConditions: []operatorapiv1.StatusCondition{ + testinghelper.NamedCondition(klusterletRegistrationDegraded, "BootStrapSecretMissing", metav1.ConditionTrue), + }, + }, + { + name: "No hubconfig secret", + object: []runtime.Object{newSecret(bootstrapHubKubeConfigSecret, "test")}, + klusterlet: newKlusterlet("testklusterlet", "test", ""), + expectedConditions: []operatorapiv1.StatusCondition{ + testinghelper.NamedCondition(klusterletRegistrationDegraded, "HubKubeConfigSecretMissing", metav1.ConditionTrue), + testinghelper.NamedCondition(klusterletWorKDegraded, "HubKubeConfigSecretMissing", metav1.ConditionTrue), + }, + }, + { + name: "No cluster name secret", + object: []runtime.Object{newSecret(bootstrapHubKubeConfigSecret, "test"), newSecret(hubKubeConfigSecret, "test")}, + klusterlet: newKlusterlet("testklusterlet", "test", ""), + expectedConditions: []operatorapiv1.StatusCondition{ + testinghelper.NamedCondition(klusterletRegistrationDegraded, "ClusterNameMissing", metav1.ConditionTrue), + testinghelper.NamedCondition(klusterletWorKDegraded, "ClusterNameMissing", metav1.ConditionTrue), + }, + }, + { + name: "No kubeconfig secret", + object: []runtime.Object{newSecret(bootstrapHubKubeConfigSecret, "test"), newSecret(hubKubeConfigSecret, "test")}, + klusterlet: newKlusterlet("testklusterlet", "test", "cluster1"), + expectedConditions: []operatorapiv1.StatusCondition{ + testinghelper.NamedCondition(klusterletRegistrationDegraded, "KubeConfigMissing", metav1.ConditionTrue), + testinghelper.NamedCondition(klusterletWorKDegraded, "KubeConfigMissing", metav1.ConditionTrue), + }, + }, + { + name: "Unavailable pod in deployments", + object: []runtime.Object{ + newSecret(bootstrapHubKubeConfigSecret, "test"), + newSecretWithKubeConfig(hubKubeConfigSecret, "test"), + newDeployment("testklusterlet-registration-agent", "test", 3, 0), + newDeployment("testklusterlet-work-agent", "test", 3, 0), + }, + klusterlet: newKlusterlet("testklusterlet", "test", "cluster1"), + expectedConditions: []operatorapiv1.StatusCondition{ + testinghelper.NamedCondition(klusterletRegistrationDegraded, "UnavailableRegistrationPod", metav1.ConditionTrue), + testinghelper.NamedCondition(klusterletWorKDegraded, "UnavailableWorkPod", metav1.ConditionTrue), + }, + }, + { + name: "Operator functional", + object: []runtime.Object{ + newSecret(bootstrapHubKubeConfigSecret, "test"), + newSecretWithKubeConfig(hubKubeConfigSecret, "test"), + newDeployment("testklusterlet-registration-agent", "test", 3, 3), + newDeployment("testklusterlet-work-agent", "test", 3, 3), + }, + klusterlet: newKlusterlet("testklusterlet", "test", "cluster1"), + expectedConditions: []operatorapiv1.StatusCondition{ + testinghelper.NamedCondition(klusterletRegistrationDegraded, "RegistrationFunctional", metav1.ConditionFalse), + testinghelper.NamedCondition(klusterletWorKDegraded, "WorkFunctional", metav1.ConditionFalse), + }, + }, + } + + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + controller := newTestController(c.klusterlet, c.object...) + syncContext := testinghelper.NewFakeSyncContext(t, c.klusterlet.Name) + err := controller.controller.sync(nil, syncContext) + if err != nil { + t.Errorf("Expected no error when update status: %v", err) + } + + operatorActions := controller.operatorClient.Actions() + + testinghelper.AssertEqualNumber(t, len(operatorActions), 2) + testinghelper.AssertGet(t, operatorActions[0], "operator.open-cluster-management.io", "v1", "klusterlets") + testinghelper.AssertAction(t, operatorActions[1], "update") + testinghelper.AssertOnlyConditions( + t, operatorActions[1].(clienttesting.UpdateActionImpl).Object, c.expectedConditions...) + }) + } +} diff --git a/pkg/operators/manager.go b/pkg/operators/manager.go index 78f4ee630..94aaee8f0 100644 --- a/pkg/operators/manager.go +++ b/pkg/operators/manager.go @@ -5,6 +5,7 @@ import ( "time" apiextensionsclient "k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset" + "k8s.io/client-go/informers" "k8s.io/client-go/kubernetes" apiregistrationclient "k8s.io/kube-aggregator/pkg/client/clientset_generated/clientset" @@ -13,7 +14,8 @@ import ( operatorclient "github.com/open-cluster-management/api/client/operator/clientset/versioned" operatorinformer "github.com/open-cluster-management/api/client/operator/informers/externalversions" "github.com/open-cluster-management/registration-operator/pkg/operators/clustermanager" - "github.com/open-cluster-management/registration-operator/pkg/operators/klusterlet" + "github.com/open-cluster-management/registration-operator/pkg/operators/klusterlet/controllers/klusterletcontroller" + "github.com/open-cluster-management/registration-operator/pkg/operators/klusterlet/controllers/statuscontroller" ) // RunClusterManagerOperator starts a new cluster manager operator @@ -61,6 +63,8 @@ func RunKlusterletOperator(ctx context.Context, controllerContext *controllercmd return err } + kubeInformer := informers.NewSharedInformerFactory(kubeClient, 5*time.Minute) + // Build operator client and informer operatorClient, err := operatorclient.NewForConfig(controllerContext.KubeConfig) if err != nil { @@ -68,14 +72,24 @@ func RunKlusterletOperator(ctx context.Context, controllerContext *controllercmd } operatorInformer := operatorinformer.NewSharedInformerFactory(operatorClient, 5*time.Minute) - klusterletController := klusterlet.NewKlusterletController( + klusterletController := klusterletcontroller.NewKlusterletController( kubeClient, operatorClient.OperatorV1().Klusterlets(), operatorInformer.Operator().V1().Klusterlets(), controllerContext.EventRecorder) + statusController := statuscontroller.NewKlusterletStatusController( + kubeClient, + operatorClient.OperatorV1().Klusterlets(), + operatorInformer.Operator().V1().Klusterlets(), + kubeInformer.Core().V1().Secrets(), + kubeInformer.Apps().V1().Deployments(), + controllerContext.EventRecorder, + ) go operatorInformer.Start(ctx.Done()) + go kubeInformer.Start(ctx.Done()) go klusterletController.Run(ctx, 1) + go statusController.Run(ctx, 1) <-ctx.Done() return nil } diff --git a/test/integration/clustermanager_test.go b/test/integration/clustermanager_test.go index cfb6950bd..462bc0261 100644 --- a/test/integration/clustermanager_test.go +++ b/test/integration/clustermanager_test.go @@ -168,7 +168,7 @@ var _ = ginkgo.Describe("ClusterManager", func() { return true }, eventuallyTimeout, eventuallyInterval).Should(gomega.BeTrue()) - util.AssertClusterManagerCondition(clusterManagerName, operatorClient, "Applied", metav1.ConditionTrue, eventuallyTimeout, eventuallyInterval) + util.AssertClusterManagerCondition(clusterManagerName, operatorClient, "Applied", "ClusterManagerApplied", metav1.ConditionTrue) err := operatorClient.OperatorV1().ClusterManagers().Delete(context.Background(), clusterManagerName, metav1.DeleteOptions{}) gomega.Expect(err).NotTo(gomega.HaveOccurred()) diff --git a/test/integration/klusterlet_test.go b/test/integration/klusterlet_test.go index 305769f22..a1cdd2585 100644 --- a/test/integration/klusterlet_test.go +++ b/test/integration/klusterlet_test.go @@ -50,19 +50,6 @@ var _ = ginkgo.Describe("Klusterlet", func() { _, err := kubeClient.CoreV1().Namespaces().Create(context.Background(), ns, metav1.CreateOptions{}) gomega.Expect(err).ToNot(gomega.HaveOccurred()) - // Create a dummy bootstrap secret - bootStrapSecret := &corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Name: "bootstrap-hub-kubeconfig", - Namespace: klusterletNamespace, - }, - Data: map[string][]byte{ - "kubeconfig": []byte("dummy"), - }, - } - _, err = kubeClient.CoreV1().Secrets(klusterletNamespace).Create(context.Background(), bootStrapSecret, metav1.CreateOptions{}) - gomega.Expect(err).ToNot(gomega.HaveOccurred()) - klusterlet = &operatorapiv1.Klusterlet{ ObjectMeta: metav1.ObjectMeta{ Name: fmt.Sprintf("klusterlet-%s", rand.String(6)), @@ -103,6 +90,10 @@ var _ = ginkgo.Describe("Klusterlet", func() { workSAName = fmt.Sprintf("%s-work-sa", klusterlet.Name) }) + ginkgo.AfterEach(func() { + operatorClient.OperatorV1().Klusterlets().Delete(context.Background(), klusterlet.Name, metav1.DeleteOptions{}) + }) + ginkgo.It("should have expected resource created successfully", func() { _, err := operatorClient.OperatorV1().Klusterlets().Create(context.Background(), klusterlet, metav1.CreateOptions{}) gomega.Expect(err).NotTo(gomega.HaveOccurred()) @@ -175,7 +166,7 @@ var _ = ginkgo.Describe("Klusterlet", func() { return true }, eventuallyTimeout, eventuallyInterval).Should(gomega.BeTrue()) - util.AssertKlusterletCondition(klusterlet.Name, operatorClient, "Applied", metav1.ConditionTrue, eventuallyTimeout, eventuallyInterval) + util.AssertKlusterletCondition(klusterlet.Name, operatorClient, "Applied", "KlusterletApplied", metav1.ConditionTrue) }) ginkgo.It("should have correct registration deployment when server url is empty", func() { @@ -222,4 +213,64 @@ var _ = ginkgo.Describe("Klusterlet", func() { } }) }) + + ginkgo.Context("klusterlet statuses", func() { + ginkgo.BeforeEach(func() { + registrationDeploymentName = fmt.Sprintf("%s-registration-agent", klusterlet.Name) + workDeploymentName = fmt.Sprintf("%s-work-agent", klusterlet.Name) + }) + ginkgo.It("should have correct degraded conditions", func() { + _, err := operatorClient.OperatorV1().Klusterlets().Create(context.Background(), klusterlet, metav1.CreateOptions{}) + gomega.Expect(err).NotTo(gomega.HaveOccurred()) + + util.AssertKlusterletCondition(klusterlet.Name, operatorClient, "KlusterletRegistrationDegraded", "BootStrapSecretMissing", metav1.ConditionTrue) + + // Create a dummy bootstrap secret + bootStrapSecret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "bootstrap-hub-kubeconfig", + Namespace: klusterletNamespace, + }, + Data: map[string][]byte{ + "kubeconfig": []byte("dummy"), + }, + } + _, err = kubeClient.CoreV1().Secrets(klusterletNamespace).Create(context.Background(), bootStrapSecret, metav1.CreateOptions{}) + gomega.Expect(err).ToNot(gomega.HaveOccurred()) + + util.AssertKlusterletCondition(klusterlet.Name, operatorClient, "KlusterletRegistrationDegraded", "KubeConfigMissing", metav1.ConditionTrue) + util.AssertKlusterletCondition(klusterlet.Name, operatorClient, "KlusterletWorkDegraded", "KubeConfigMissing", metav1.ConditionTrue) + + hubSecret, err := kubeClient.CoreV1().Secrets(klusterletNamespace).Get(context.Background(), "hub-kubeconfig-secret", metav1.GetOptions{}) + gomega.Expect(err).ToNot(gomega.HaveOccurred()) + + // Update hub secret + hubSecret.Data["cluster-name"] = []byte("testcluster") + hubSecret.Data["kubeconfig"] = []byte("dummy") + _, err = kubeClient.CoreV1().Secrets(klusterletNamespace).Update(context.Background(), hubSecret, metav1.UpdateOptions{}) + gomega.Expect(err).ToNot(gomega.HaveOccurred()) + + util.AssertKlusterletCondition(klusterlet.Name, operatorClient, "KlusterletRegistrationDegraded", "UnavailableRegistrationPod", metav1.ConditionTrue) + util.AssertKlusterletCondition(klusterlet.Name, operatorClient, "KlusterletWorkDegraded", "UnavailableWorkPod", metav1.ConditionTrue) + + // Update replica of deployment + registrationDeployment, err := kubeClient.AppsV1().Deployments(klusterletNamespace).Get(context.Background(), registrationDeploymentName, metav1.GetOptions{}) + gomega.Expect(err).ToNot(gomega.HaveOccurred()) + registrationDeployment.Status.AvailableReplicas = 3 + registrationDeployment.Status.Replicas = 3 + registrationDeployment.Status.ReadyReplicas = 3 + _, err = kubeClient.AppsV1().Deployments(klusterletNamespace).UpdateStatus(context.Background(), registrationDeployment, metav1.UpdateOptions{}) + gomega.Expect(err).ToNot(gomega.HaveOccurred()) + workDeployment, err := kubeClient.AppsV1().Deployments(klusterletNamespace).Get(context.Background(), workDeploymentName, metav1.GetOptions{}) + gomega.Expect(err).ToNot(gomega.HaveOccurred()) + workDeployment.Status.AvailableReplicas = 3 + workDeployment.Status.Replicas = 3 + workDeployment.Status.ReadyReplicas = 3 + _, err = kubeClient.AppsV1().Deployments(klusterletNamespace).UpdateStatus(context.Background(), workDeployment, metav1.UpdateOptions{}) + gomega.Expect(err).ToNot(gomega.HaveOccurred()) + + util.AssertKlusterletCondition(klusterlet.Name, operatorClient, "KlusterletRegistrationDegraded", "RegistrationFunctional", metav1.ConditionFalse) + util.AssertKlusterletCondition(klusterlet.Name, operatorClient, "KlusterletWorkDegraded", "WorkFunctional", metav1.ConditionFalse) + }) + }) }) diff --git a/test/integration/util/assertion.go b/test/integration/util/assertion.go index 8f8753e45..2de1ed85f 100644 --- a/test/integration/util/assertion.go +++ b/test/integration/util/assertion.go @@ -10,8 +10,13 @@ import ( operatorclientset "github.com/open-cluster-management/api/client/operator/clientset/versioned" ) +const ( + eventuallyTimeout = 30 // seconds + eventuallyInterval = 1 // seconds +) + func AssertKlusterletCondition( - name string, operatorClient operatorclientset.Interface, expectedType string, expectedWorkStatus metav1.ConditionStatus, eventuallyTimeout, eventuallyInterval int) { + name string, operatorClient operatorclientset.Interface, expectedType, expectedReason string, expectedWorkStatus metav1.ConditionStatus) { gomega.Eventually(func() bool { klusterlet, err := operatorClient.OperatorV1().Klusterlets().Get(context.Background(), name, metav1.GetOptions{}) if err != nil { @@ -19,12 +24,12 @@ func AssertKlusterletCondition( } // check work status condition - return HasCondition(klusterlet.Status.Conditions, expectedType, expectedWorkStatus) + return HasCondition(klusterlet.Status.Conditions, expectedType, expectedReason, expectedWorkStatus) }, eventuallyTimeout, eventuallyInterval).Should(gomega.BeTrue()) } func AssertClusterManagerCondition( - name string, operatorClient operatorclientset.Interface, expectedType string, expectedWorkStatus metav1.ConditionStatus, eventuallyTimeout, eventuallyInterval int) { + name string, operatorClient operatorclientset.Interface, expectedType, expectedReason string, expectedWorkStatus metav1.ConditionStatus) { gomega.Eventually(func() bool { klusterlet, err := operatorClient.OperatorV1().ClusterManagers().Get(context.Background(), name, metav1.GetOptions{}) if err != nil { @@ -32,6 +37,6 @@ func AssertClusterManagerCondition( } // check work status condition - return HasCondition(klusterlet.Status.Conditions, expectedType, expectedWorkStatus) + return HasCondition(klusterlet.Status.Conditions, expectedType, expectedReason, expectedWorkStatus) }, eventuallyTimeout, eventuallyInterval).Should(gomega.BeTrue()) } diff --git a/test/integration/util/util.go b/test/integration/util/util.go index a38b6d171..77ab6f099 100644 --- a/test/integration/util/util.go +++ b/test/integration/util/util.go @@ -47,7 +47,7 @@ func (r *IntegrationTestEventRecorder) Warningf(reason, messageFmt string, args r.Warning(reason, fmt.Sprintf(messageFmt, args...)) } -func HasCondition(conditions []operatorapiv1.StatusCondition, expectedType string, expectedStatus metav1.ConditionStatus) bool { +func HasCondition(conditions []operatorapiv1.StatusCondition, expectedType, expectedReason string, expectedStatus metav1.ConditionStatus) bool { found := false for _, condition := range conditions { if condition.Type != expectedType { @@ -58,6 +58,11 @@ func HasCondition(conditions []operatorapiv1.StatusCondition, expectedType strin if condition.Status != expectedStatus { return false } + + if condition.Reason != expectedReason { + return false + } + return true }