🐛 handle the unknown status of the previously accepted cluster

Signed-off-by: Yang Le <yangle@redhat.com>
open-cluster-management-io · Oct 18, 2024 · e516eff · e516eff
1 parent 6d68522
commit e516eff
Show file tree

Hide file tree

Showing 4 changed files with 69 additions and 49 deletions.
diff --git a/pkg/registration/helpers/testing/testinghelpers.go b/pkg/registration/helpers/testing/testinghelpers.go
@@ -19,6 +19,7 @@ import (
 	coordv1 "k8s.io/api/coordination/v1"
 	corev1 "k8s.io/api/core/v1"
 	rbacv1 "k8s.io/api/rbac/v1"
+	"k8s.io/apimachinery/pkg/api/meta"
 	"k8s.io/apimachinery/pkg/api/resource"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	kubeversion "k8s.io/client-go/pkg/version"
@@ -128,9 +129,17 @@ func NewManagedClusterWithStatus(capacity, allocatable clusterv1.ResourceList) *
 	return managedCluster
 }
 
-func NewDeniedManagedCluster() *clusterv1.ManagedCluster {
+func NewDeniedManagedCluster(acceptedConditionStatus string) *clusterv1.ManagedCluster {
 	managedCluster := NewAcceptedManagedCluster()
 	managedCluster.Spec.HubAcceptsClient = false
+
+	meta.SetStatusCondition(&managedCluster.Status.Conditions, NewManagedClusterCondition(
+		clusterv1.ManagedClusterConditionHubAccepted,
+		acceptedConditionStatus,
+		"",
+		"",
+		nil,
+	))
 	return managedCluster
 }
 
@@ -182,16 +191,6 @@ func NewAddOnLease(namespace, name string, renewTime time.Time) *coordv1.Lease {
 	}
 }
 
-func NewNamespace(name string, terminated bool) *corev1.Namespace {
-	namespace := &corev1.Namespace{}
-	namespace.Name = name
-	if terminated {
-		now := metav1.Now()
-		namespace.DeletionTimestamp = &now
-	}
-	return namespace
-}
-
 func NewManifestWork(namespace, name string, finalizers []string,
 	labels, annotations map[string]string, deletionTimestamp *metav1.Time) *workapiv1.ManifestWork {
 	work := &workapiv1.ManifestWork{

diff --git a/pkg/registration/hub/lease/controller.go b/pkg/registration/hub/lease/controller.go
@@ -71,7 +71,8 @@ func NewClusterLeaseController(
 		ToController("ManagedClusterLeaseController", recorder)
 }
 
-// sync checks the lease of each accepted cluster on hub to determine whether a managed cluster is available.
+// sync checks the lease of each cluster on hub, which is accepted or previously accepted, to determine whether
+// the managed cluster is available.
 func (c *leaseController) sync(ctx context.Context, syncCtx factory.SyncContext) error {
 	clusterName := syncCtx.QueueKey()
 
@@ -84,8 +85,9 @@ func (c *leaseController) sync(ctx context.Context, syncCtx factory.SyncContext)
 		return err
 	}
 
-	if !meta.IsStatusConditionTrue(cluster.Status.Conditions, clusterv1.ManagedClusterConditionHubAccepted) {
-		// cluster is not accepted, skip it.
+	if cond := meta.FindStatusCondition(cluster.Status.Conditions,
+		clusterv1.ManagedClusterConditionHubAccepted); cond == nil {
+		// cluster was never accepted, skip it.
 		return nil
 	}
 

diff --git a/pkg/registration/hub/lease/controller_test.go b/pkg/registration/hub/lease/controller_test.go
@@ -17,7 +17,6 @@ import (
 	clusterscheme "open-cluster-management.io/api/client/cluster/clientset/versioned/scheme"
 	clusterinformers "open-cluster-management.io/api/client/cluster/informers/externalversions"
 	clusterv1 "open-cluster-management.io/api/cluster/v1"
-	v1 "open-cluster-management.io/api/cluster/v1"
 	"open-cluster-management.io/sdk-go/pkg/patcher"
 
 	"open-cluster-management.io/ocm/pkg/common/helpers"
@@ -28,6 +27,39 @@ import (
 var now = time.Now()
 
 func TestSync(t *testing.T) {
+	leaseStopUpdatingValidateActions := func(t *testing.T, hubKubeActions, clusterActions []clienttesting.Action) {
+		expected := metav1.Condition{
+			Type:    clusterv1.ManagedClusterConditionAvailable,
+			Status:  metav1.ConditionUnknown,
+			Reason:  "ManagedClusterLeaseUpdateStopped",
+			Message: "Registration agent stopped updating its lease.",
+		}
+		testingcommon.AssertActions(t, clusterActions, "patch")
+		patch := clusterActions[0].(clienttesting.PatchAction).GetPatch()
+		managedCluster := &clusterv1.ManagedCluster{}
+		err := json.Unmarshal(patch, managedCluster)
+		if err != nil {
+			t.Fatal(err)
+		}
+		testingcommon.AssertCondition(t, managedCluster.Status.Conditions, expected)
+
+		if len(hubKubeActions) != 1 {
+			t.Errorf("Expected 1 event created in the sync loop, actual %d",
+				len(hubKubeActions))
+		}
+		actionEvent := hubKubeActions[0]
+		if actionEvent.GetResource().Resource != "events" {
+			t.Errorf("Expected event created, actual %s", actionEvent.GetResource())
+		}
+		if actionEvent.GetNamespace() != testinghelpers.TestManagedClusterName {
+			t.Errorf("Expected event created in namespace %s, actual %s",
+				testinghelpers.TestManagedClusterName, actionEvent.GetNamespace())
+		}
+		if actionEvent.GetVerb() != "create" {
+			t.Errorf("Expected event created, actual %s", actionEvent.GetVerb())
+		}
+	}
+
 	cases := []struct {
 		name            string
 		clusters        []runtime.Object
@@ -43,6 +75,15 @@ func TestSync(t *testing.T) {
 				testingcommon.AssertNoActions(t, clusterActions)
 			},
 		},
+		{
+			name:          "sync previously accepted managed cluster",
+			clusters:      []runtime.Object{testinghelpers.NewDeniedManagedCluster("False")},
+			clusterLeases: []runtime.Object{},
+			validateActions: func(t *testing.T, leaseActions, clusterActions []clienttesting.Action) {
+				testingcommon.AssertActions(t, leaseActions, "create")
+				testingcommon.AssertNoActions(t, clusterActions)
+			},
+		},
 		{
 			name:          "there is no lease for a managed cluster",
 			clusters:      []runtime.Object{testinghelpers.NewAcceptedManagedCluster()},
@@ -53,44 +94,22 @@ func TestSync(t *testing.T) {
 			},
 		},
 		{
-			name:     "managed cluster stop update lease",
+			name:     "accepted managed cluster stop update lease",
 			clusters: []runtime.Object{testinghelpers.NewAvailableManagedCluster()},
 			clusterLeases: []runtime.Object{
 				testinghelpers.NewManagedClusterLease("managed-cluster-lease", now.Add(-5*time.Minute)),
 				testinghelpers.NewManagedClusterLease(fmt.Sprintf("cluster-lease-%s", testinghelpers.TestManagedClusterName), now.Add(-5*time.Minute)),
 			},
-			validateActions: func(t *testing.T, hubKubeActions, clusterActions []clienttesting.Action) {
-				expected := metav1.Condition{
-					Type:    clusterv1.ManagedClusterConditionAvailable,
-					Status:  metav1.ConditionUnknown,
-					Reason:  "ManagedClusterLeaseUpdateStopped",
-					Message: "Registration agent stopped updating its lease.",
-				}
-				testingcommon.AssertActions(t, clusterActions, "patch")
-				patch := clusterActions[0].(clienttesting.PatchAction).GetPatch()
-				managedCluster := &v1.ManagedCluster{}
-				err := json.Unmarshal(patch, managedCluster)
-				if err != nil {
-					t.Fatal(err)
-				}
-				testingcommon.AssertCondition(t, managedCluster.Status.Conditions, expected)
-
-				if len(hubKubeActions) != 1 {
-					t.Errorf("Expected 1 event created in the sync loop, actual %d",
-						len(hubKubeActions))
-				}
-				actionEvent := hubKubeActions[0]
-				if actionEvent.GetResource().Resource != "events" {
-					t.Errorf("Expected event created, actual %s", actionEvent.GetResource())
-				}
-				if actionEvent.GetNamespace() != testinghelpers.TestManagedClusterName {
-					t.Errorf("Expected event created in namespace %s, actual %s",
-						testinghelpers.TestManagedClusterName, actionEvent.GetNamespace())
-				}
-				if actionEvent.GetVerb() != "create" {
-					t.Errorf("Expected event created, actual %s", actionEvent.GetVerb())
-				}
+			validateActions: leaseStopUpdatingValidateActions,
+		},
+		{
+			name:     "previously accepted managed cluster stop update lease",
+			clusters: []runtime.Object{testinghelpers.NewDeniedManagedCluster("False")},
+			clusterLeases: []runtime.Object{
+				testinghelpers.NewManagedClusterLease("managed-cluster-lease", now.Add(-5*time.Minute)),
+				testinghelpers.NewManagedClusterLease(fmt.Sprintf("cluster-lease-%s", testinghelpers.TestManagedClusterName), now.Add(-5*time.Minute)),
 			},
+			validateActions: leaseStopUpdatingValidateActions,
 		},
 		{
 			name:          "managed cluster is available",
@@ -112,7 +131,7 @@ func TestSync(t *testing.T) {
 				}
 				testingcommon.AssertActions(t, clusterActions, "patch")
 				patch := clusterActions[0].(clienttesting.PatchAction).GetPatch()
-				managedCluster := &v1.ManagedCluster{}
+				managedCluster := &clusterv1.ManagedCluster{}
 				err := json.Unmarshal(patch, managedCluster)
 				if err != nil {
 					t.Fatal(err)

diff --git a/pkg/registration/hub/managedcluster/controller_test.go b/pkg/registration/hub/managedcluster/controller_test.go
@@ -77,7 +77,7 @@ func TestSyncManagedCluster(t *testing.T) {
 		},
 		{
 			name:            "deny an accepted spoke cluster",
-			startingObjects: []runtime.Object{testinghelpers.NewDeniedManagedCluster()},
+			startingObjects: []runtime.Object{testinghelpers.NewDeniedManagedCluster("True")},
 			validateActions: func(t *testing.T, actions []clienttesting.Action) {
 				expectedCondition := metav1.Condition{
 					Type:    v1.ManagedClusterConditionHubAccepted,