New version of creation/deletion of remote Namespaces

This new version doesn't consider the status of the NamespaceMap CurrentMappings in order to decide how to procede, it get the actual status of the cluster in order to do it
liqotech · Jun 1, 2021 · 47ddaf4 · 47ddaf4
1 parent dc422b1
commit 47ddaf4
Show file tree

Hide file tree

Showing 5 changed files with 93 additions and 49 deletions.
diff --git a/pkg/consts/discovery.go b/pkg/consts/discovery.go
@@ -2,6 +2,6 @@ package consts
 
 const (
 	// ClusterIDForeignLabelKey is the label key used to get the ForeignCluster resource referred
-	// to a specific remote cluster
+	// to a specific remote cluster.
 	ClusterIDForeignLabelKey = "discovery.liqo.io/cluster-id"
 )
diff --git a/pkg/liqo-controller-manager/namespaceMap-controller/ensure_remote_namespaces.go b/pkg/liqo-controller-manager/namespaceMap-controller/ensure_remote_namespaces.go
@@ -13,23 +13,59 @@ import (
 
 	mapsv1alpha1 "github.com/liqotech/liqo/apis/virtualKubelet/v1alpha1"
 	liqoconst "github.com/liqotech/liqo/pkg/consts"
+	liqoutils "github.com/liqotech/liqo/pkg/utils"
 )
 
+// This constants are used to compose the annotation that is inserted on remote Namespace at creation time.
+const (
+	liqoSuffix                      = "liqo.io"
+	remoteNamespaceAnnotationPrefix = "remote-namespace"
+	remoteNamespaceAnnotationValue  = "This Namespace has been created through Liqo offloading mechanism"
+)
+
+// This function gets the clusted-id of the local cluster.
+func (r *NamespaceMapReconciler) checkLocalClusterID() error {
+	if r.LocalClusterID == "" {
+		clusterID, err := liqoutils.GetClusterID(r.Client)
+		if err != nil || clusterID == "" {
+			return err
+		}
+		r.LocalClusterID = clusterID
+	}
+	return nil
+}
+
 // This function creates a remote Namespace inside the remote cluster, if it doesn't exist yet.
 // The right client to use is chosen by means of NamespaceMap's cluster-id.
 func (r *NamespaceMapReconciler) createRemoteNamespace(clusterID, remoteName string) error {
 	if err := r.checkRemoteClientPresence(clusterID); err != nil {
 		return err
 	}
+	if err := r.checkLocalClusterID(); err != nil {
+		return err
+	}
 
+	// This annotation is used to recognize the remote namespaces that have been created by this controller.
+	annotationKey := fmt.Sprintf("%s-%s/%s", remoteNamespaceAnnotationPrefix, r.LocalClusterID, liqoSuffix)
 	remoteNamespace := &corev1.Namespace{
 		ObjectMeta: metav1.ObjectMeta{
 			Name: remoteName,
+			Annotations: map[string]string{
+				annotationKey: remoteNamespaceAnnotationValue,
+			},
 		},
 	}
-	// Here I don't know if there is a conflict because i have already created this remote Namespace
-	// or because there is a namespace with the same name in the remote cluster.
+
 	if err := r.RemoteClients[clusterID].Create(context.TODO(), remoteNamespace); err != nil {
+		if apierrors.IsAlreadyExists(err) {
+			if errGet := r.RemoteClients[clusterID].Get(context.TODO(), types.NamespacedName{Name: remoteName}, remoteNamespace); errGet == nil {
+				if value, ok := remoteNamespace.Annotations[annotationKey]; ok && value == remoteNamespaceAnnotationValue {
+					klog.Infof("Namespace '%s' already created inside remote cluster: '%s'", remoteNamespace.Name, clusterID)
+					return nil
+				}
+			}
+		}
+		klog.Error(err)
 		return err
 	}
 
@@ -42,20 +78,13 @@ func (r *NamespaceMapReconciler) ensureRemoteNamespacesExistence(nm *mapsv1alpha
 	errorCondition := 0
 	for localName, remoteName := range nm.Spec.DesiredMapping {
 		if err := r.createRemoteNamespace(nm.Labels[liqoconst.RemoteClusterID], remoteName); err != nil {
-			// todo: in order to force cache to update the NamespaceMap status, the internal cache of the controller
-			//       doesn't have the updated status of NamespaceMap.
-			//       Alternative: I can create the remote Namespace with a distinctive characteristic "liqo-offloaded"
-			//         in order to recognize it
-			//fmt.Println(nm.Status.CurrentMapping[localName].Phase)
-			if apierrors.IsAlreadyExists(err) && nm.Status.CurrentMapping[localName].Phase == mapsv1alpha1.MappingAccepted {
-				continue
-			}
 			nm.Status.CurrentMapping[localName] = mapsv1alpha1.RemoteNamespaceStatus{
 				RemoteNamespace: remoteName,
 				Phase:           mapsv1alpha1.MappingCreationLoopBackOff,
 			}
 			errorCondition = 1
-			klog.Errorf("%s -> Namespace '%s' cannot be created on cluster '%s'", err, localName, nm.Labels[liqoconst.RemoteClusterID])
+			klog.Errorf("%s -> Namespace '%s' cannot be created inside cluster '%s'", err,
+				localName, nm.Labels[liqoconst.RemoteClusterID])
 			continue
 		}
 		nm.Status.CurrentMapping[localName] = mapsv1alpha1.RemoteNamespaceStatus{
@@ -73,10 +102,12 @@ func (r *NamespaceMapReconciler) deleteRemoteNamespace(clusterID, remoteName str
 	if err := r.checkRemoteClientPresence(clusterID); err != nil {
 		return err
 	}
+	if err := r.checkLocalClusterID(); err != nil {
+		return err
+	}
 
 	remoteNamespace := &corev1.Namespace{}
-	if err := r.RemoteClients[clusterID].Get(context.TODO(),
-		types.NamespacedName{Name: remoteName}, remoteNamespace); err != nil {
+	if err := r.RemoteClients[clusterID].Get(context.TODO(), types.NamespacedName{Name: remoteName}, remoteNamespace); err != nil {
 		if apierrors.IsNotFound(err) {
 			klog.Infof("The namespace '%s' is correctly deleted from the remote cluster: '%s'", remoteName, clusterID)
 			return nil
@@ -85,6 +116,14 @@ func (r *NamespaceMapReconciler) deleteRemoteNamespace(clusterID, remoteName str
 		return err
 	}
 
+	// Check if it is a namespace created by the NamespaceMap controller.
+	annotationKey := fmt.Sprintf("%s-%s/%s", remoteNamespaceAnnotationPrefix, r.LocalClusterID, liqoSuffix)
+	if value, ok := remoteNamespace.Annotations[annotationKey]; !ok || value != remoteNamespaceAnnotationValue {
+		klog.Infof("No remote namespaces with name '%s' created through Liqo mechanism inside the remote cluster: '%s'",
+			remoteName, clusterID)
+		return nil
+	}
+
 	if remoteNamespace.DeletionTimestamp.IsZero() {
 		if err := r.RemoteClients[clusterID].Delete(context.TODO(), remoteNamespace); err != nil {
 			klog.Errorf("unable to delete the namespace '%s' from the remote cluster: '%s'", remoteName, clusterID)
@@ -99,19 +138,18 @@ func (r *NamespaceMapReconciler) deleteRemoteNamespace(clusterID, remoteName str
 func (r *NamespaceMapReconciler) ensureRemoteNamespacesDeletion(nm *mapsv1alpha1.NamespaceMap) int {
 	errorCondition := 0
 	for localName, remoteStatus := range nm.Status.CurrentMapping {
-		if _, ok := nm.Spec.DesiredMapping[localName]; ok && nm.DeletionTimestamp.IsZero(){
+		if _, ok := nm.Spec.DesiredMapping[localName]; ok && nm.DeletionTimestamp.IsZero() {
 			continue
 		}
-		if remoteStatus.Phase == mapsv1alpha1.MappingAccepted || remoteStatus.Phase == mapsv1alpha1.MappingTerminating {
-			if err := r.deleteRemoteNamespace(nm.Labels[liqoconst.RemoteClusterID], remoteStatus.RemoteNamespace); err != nil {
-				nm.Status.CurrentMapping[localName] = mapsv1alpha1.RemoteNamespaceStatus{
-					RemoteNamespace: remoteStatus.RemoteNamespace,
-					Phase:           mapsv1alpha1.MappingTerminating,
-				}
-				errorCondition = 1
-				klog.Info(err)
-				continue
+		if err := r.deleteRemoteNamespace(nm.Labels[liqoconst.RemoteClusterID], remoteStatus.RemoteNamespace); err != nil {
+			nm.Status.CurrentMapping[localName] = mapsv1alpha1.RemoteNamespaceStatus{
+				RemoteNamespace: remoteStatus.RemoteNamespace,
+				Phase:           mapsv1alpha1.MappingTerminating,
 			}
+			errorCondition = 1
+			klog.Errorf("%s -> Namespace '%s' cannot be deleted from cluster '%s'", err,
+				localName, nm.Labels[liqoconst.RemoteClusterID])
+			continue
 		}
 		delete(nm.Status.CurrentMapping, localName)
 	}
@@ -128,9 +166,9 @@ func (r *NamespaceMapReconciler) ensureRemoteNamespaces(ctx context.Context, nm
 	// Object used as base for client.MergeFrom
 	patch := nm.DeepCopy()
 	// For every entry of DesiredMapping create remote Namespace if it has not already being created
-	errorCondition = errorCondition + r.ensureRemoteNamespacesExistence(nm)
+	errorCondition += r.ensureRemoteNamespacesExistence(nm)
 	// If DesiredMapping field has less entries than CurrentMapping, is necessary to remove some remote namespaces
-	errorCondition = errorCondition + r.ensureRemoteNamespacesDeletion(nm)
+	errorCondition += r.ensureRemoteNamespacesDeletion(nm)
 	// MergeFrom used to avoid conflicts, the NamespaceMap controller has the ownership of NamespaceMap status
 	if err := r.Patch(ctx, nm, client.MergeFrom(patch)); err != nil {
 		klog.Errorf("%s -> unable to update NamespaceMap '%s' Status", err, nm.Name)
@@ -152,7 +190,8 @@ func (r *NamespaceMapReconciler) namespaceMapDeletionProcess(ctx context.Context
 	nm *mapsv1alpha1.NamespaceMap) error {
 	patch := nm.DeepCopy()
 	r.ensureRemoteNamespacesDeletion(nm)
-	if removed, err := r.TryToRemoveNamespaceMapControllerFinalizer(ctx, nm); err == nil && removed == true {
+	// If the NamespaceMap Status is empty, it is possible to remove the finalizer.
+	if removed, err := r.TryToRemoveNamespaceMapControllerFinalizer(ctx, nm); err == nil && removed {
 		return nil
 	}
 	if err := r.Patch(ctx, nm, client.MergeFrom(patch)); err != nil {

diff --git a/pkg/liqo-controller-manager/namespaceMap-controller/namespaceMap_controller_test.go b/pkg/liqo-controller-manager/namespaceMap-controller/namespaceMap_controller_test.go
@@ -22,7 +22,6 @@ import (
 	"time"
 
 	corev1 "k8s.io/api/core/v1"
-	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	ctrlutils "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
 
@@ -82,8 +81,10 @@ var _ = Describe("NamespaceMap controller", func() {
 			By(" 2 - Checking remote namespace existence")
 			Eventually(func() bool {
 				remoteNamespace := &corev1.Namespace{}
-				err := remoteClient2.Get(context.TODO(), types.NamespacedName{Name: namespace1Name}, remoteNamespace)
-				return err == nil
+				if err := remoteClient2.Get(context.TODO(), types.NamespacedName{Name: namespace1Name}, remoteNamespace); err != nil {
+					return false
+				}
+				return remoteNamespace.Annotations[liqoAnnotationKey] == remoteNamespaceAnnotationValue
 			}, timeout, interval).Should(BeTrue())
 
 			By(" 3 - Checking status of CurrentMapping entry: must be 'Accepted'")
@@ -169,24 +170,24 @@ var _ = Describe("NamespaceMap controller", func() {
 
 		It(fmt.Sprintf("Check NamespaceMap status when a remote namespace with the same name '%s' "+
 			"already exists on remote cluster '%s'", namespace1Name, remoteClusterId1), func() {
-
-			By(fmt.Sprintf(" 1 - Create remote namespace '%s' if it isn't already there", namespace1Name))
+			remoteName := "pippo"
+			By(fmt.Sprintf(" 1 - Create remote namespace '%s' if it isn't already there", remoteName))
 			Eventually(func() bool {
 				remoteNamespace := &corev1.Namespace{
 					ObjectMeta: metav1.ObjectMeta{
-						Name: namespace1Name,
+						Name: remoteName,
 					},
 				}
 				err := remoteClient2.Create(context.TODO(), remoteNamespace)
-				return err == nil || apierrors.IsAlreadyExists(err)
+				return err == nil
 			}, timeout, interval).Should(BeTrue())
 
 			By(" 2 - Adding desired mapping entry to the NamespaceMap")
 			Eventually(func() bool {
 				Expect(homeClient.List(context.TODO(), nms, client.MatchingLabels{liqoconst.RemoteClusterID: remoteClusterId1})).To(Succeed())
 				Expect(len(nms.Items) == 1).To(BeTrue())
 				nms.Items[0].Spec.DesiredMapping = map[string]string{}
-				nms.Items[0].Spec.DesiredMapping[namespace1Name] = namespace1Name
+				nms.Items[0].Spec.DesiredMapping[remoteName] = remoteName
 				err := homeClient.Update(context.TODO(), nms.Items[0].DeepCopy())
 				return err == nil
 			}, timeout, interval).Should(BeTrue())
@@ -199,8 +200,8 @@ var _ = Describe("NamespaceMap controller", func() {
 				if len(nms.Items) == 0 {
 					return false
 				}
-				return nms.Items[0].Status.CurrentMapping[namespace1Name].RemoteNamespace == namespace1Name &&
-					nms.Items[0].Status.CurrentMapping[namespace1Name].Phase == mapsv1alpha1.MappingCreationLoopBackOff
+				return nms.Items[0].Status.CurrentMapping[remoteName].RemoteNamespace == remoteName &&
+					nms.Items[0].Status.CurrentMapping[remoteName].Phase == mapsv1alpha1.MappingCreationLoopBackOff
 			}, timeout, interval).Should(BeTrue())
 
 			By(" 4 - Finalizer of namespaceMap controller should be here")
@@ -218,7 +219,7 @@ var _ = Describe("NamespaceMap controller", func() {
 			Eventually(func() bool {
 				Expect(homeClient.List(context.TODO(), nms, client.MatchingLabels{liqoconst.RemoteClusterID: remoteClusterId1})).To(Succeed())
 				Expect(len(nms.Items) == 1).To(BeTrue())
-				delete(nms.Items[0].Spec.DesiredMapping, namespace1Name)
+				delete(nms.Items[0].Spec.DesiredMapping, remoteName)
 				err := homeClient.Update(context.TODO(), nms.Items[0].DeepCopy())
 				return err == nil
 			}, timeout, interval).Should(BeTrue())
@@ -307,8 +308,8 @@ var _ = Describe("NamespaceMap controller", func() {
 				Expect(len(nms.Items) == 1).To(BeTrue())
 				return nms.Items[0].Status.CurrentMapping[namespace2Name].Phase == mapsv1alpha1.MappingTerminating &&
 					nms.Items[0].Status.CurrentMapping[namespace3Name].Phase == mapsv1alpha1.MappingTerminating &&
-				nms.Items[0].Status.CurrentMapping[namespace4Name].Phase == mapsv1alpha1.MappingTerminating &&
-				nms.Items[0].Status.CurrentMapping[namespace5Name].Phase == mapsv1alpha1.MappingTerminating
+					nms.Items[0].Status.CurrentMapping[namespace4Name].Phase == mapsv1alpha1.MappingTerminating &&
+					nms.Items[0].Status.CurrentMapping[namespace5Name].Phase == mapsv1alpha1.MappingTerminating
 			}, timeout, interval).Should(BeTrue())
 
 			By(" 7 - Check if NamespaceMap Controller finalizer is still there")

diff --git a/pkg/liqo-controller-manager/namespaceMap-controller/namespacemap_controller.go b/pkg/liqo-controller-manager/namespaceMap-controller/namespacemap_controller.go
@@ -35,9 +35,10 @@ import (
 // NamespaceMapReconciler creates remote namespaces and updates NamespaceMaps Status.
 type NamespaceMapReconciler struct {
 	client.Client
-	Scheme        *runtime.Scheme
-	Mapper        meta.RESTMapper
-	RemoteClients map[string]client.Client
+	Scheme         *runtime.Scheme
+	Mapper         meta.RESTMapper
+	RemoteClients  map[string]client.Client
+	LocalClusterID string
 }
 
 // Reconcile adds/removes NamespaceMap finalizer, and checks differences

diff --git a/pkg/liqo-controller-manager/namespaceMap-controller/suite_test.go b/pkg/liqo-controller-manager/namespaceMap-controller/suite_test.go
@@ -39,8 +39,10 @@ import (
 
 const (
 	// Namespace where the NamespaceMaps are created.
-	mapNamespaceName = "default"
-	remoteClusterId1 = "899890-dsd-323s"
+	mapNamespaceName  = "default"
+	remoteClusterId1  = "899890-dsd-323s"
+	localClusterID    = "478374-dsa-432dd"
+	liqoAnnotationKey = "remote-namespace-478374-dsa-432dd/liqo.io"
 )
 
 var (
@@ -151,10 +153,11 @@ var _ = BeforeSuite(func(done Done) {
 	Expect(homeClient.Create(context.TODO(), nm1)).Should(Succeed())
 
 	err = (&NamespaceMapReconciler{
-		Client:        homeClient,
-		Scheme:        k8sManager.GetScheme(),
-		Mapper:        k8sManager.GetRESTMapper(),
-		RemoteClients: controllerClients,
+		Client:         homeClient,
+		Scheme:         k8sManager.GetScheme(),
+		Mapper:         k8sManager.GetRESTMapper(),
+		RemoteClients:  controllerClients,
+		LocalClusterID: localClusterID,
 	}).SetupWithManager(k8sManager)
 	Expect(err).ToNot(HaveOccurred())