Merge pull request vmware-tanzu#578 from zhengxiexie/zhengxie/annotat…

…ion_shared

Replace shared vpc namespace annotation

pkg/controllers/common/utils.go

@@ -53,29 +53,23 @@ func AllocateSubnetFromSubnetSet(subnetSet *v1alpha1.SubnetSet, vpcService servi
 	return subnetService.CreateOrUpdateSubnet(subnetSet, vpcInfoList[0], tags)
 }
 
-func getSharedNamespaceAndVpcForNamespace(client k8sclient.Client, ctx context.Context, namespaceName string) (string, string, error) {
+func getSharedNamespaceForNamespace(client k8sclient.Client, ctx context.Context, namespaceName string) (string, error) {
 	namespace := &v1.Namespace{}
 	namespacedName := types.NamespacedName{Name: namespaceName}
 	if err := client.Get(ctx, namespacedName, namespace); err != nil {
 		log.Error(err, "failed to get target namespace during getting VPC for namespace")
-		return "", "", err
+		return "", err
 	}
-	vpcAnnotation, exists := namespace.Annotations[servicecommon.AnnotationVPCName]
+	sharedNamespaceName, exists := namespace.Annotations[servicecommon.AnnotationSharedVPCNamespace]
 	if !exists {
-		return "", "", nil
-	}
-	array := strings.Split(vpcAnnotation, "/")
-	if len(array) != 2 {
-		err := fmt.Errorf("invalid annotation value of '%s': %s", servicecommon.AnnotationVPCName, vpcAnnotation)
-		return "", "", err
+		return "", nil
 	}
-	sharedNamespaceName, sharedVpcName := array[0], array[1]
-	log.Info("got shared VPC for namespace", "current namespace", namespaceName, "shared VPC", sharedVpcName, "shared namespace", sharedNamespaceName)
-	return sharedNamespaceName, sharedVpcName, nil
+	log.Info("got shared VPC namespace", "current namespace", namespaceName, "shared namespace", sharedNamespaceName)
+	return sharedNamespaceName, nil
 }
 
 func GetDefaultSubnetSet(client k8sclient.Client, ctx context.Context, namespace string, resourceType string) (*v1alpha1.SubnetSet, error) {
-	targetNamespace, _, err := getSharedNamespaceAndVpcForNamespace(client, ctx, namespace)
+	targetNamespace, err := getSharedNamespaceForNamespace(client, ctx, namespace)
 	if err != nil {
 		return nil, err
 	}

pkg/controllers/namespace/namespace_controller.go

@@ -7,7 +7,6 @@ import (
 	"context"
 	"errors"
 	"fmt"
-	"strings"
 
 	v1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -184,12 +183,12 @@ func (r *NamespaceReconciler) insertNamespaceNetworkconfigBinding(ns string, ann
 	VPC creation strategy:
 
 We suppose namespace should have following annotations:
-  - "nsx.vmware.com/vpc_name": "<Namespace Name>/<Supervisor ID>"
-    If the ns contains this annotation, first check if	the namespace in annotation is the same as
+  - "nsx.vmware.com/shared_vpc_namespace": "<Namespace Name>"
+    If the ns contains this annotation, first check if the namespace in annotation is the same as
     the one in ns event, if yes, create an infra VPC for it. if	not, skip the whole ns event as the infra
     VPC will be created its corresponding ns creation event.
   - "nsx.vmware.com/vpc_network_config":"<Supervisor ID>"
-    If ns do not contains "nsx.vmware.com/vpc_name" annotation. Use this annotation to handle VPC creation.
+    If ns do not contains "nsx.vmware.com/shared_vpc_namespace" annotation. Use this annotation to handle VPC creation.
     VPC will locate the network config with the CR name, and create VPC using its config.
   - If the ns do not have either of the annotation above, then we believe it is using default VPC, try to search
     default VPC in network config CR store. The default VPC network config CR's name is "default".
@@ -217,20 +216,8 @@ func (r *NamespaceReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
 			log.Error(err, "failed to build namespace and network config bindings", "Namepspace", ns)
 			return common.ResultRequeueAfter10sec, nil
 		}
-		// read anno "nsx.vmware.com/vpc_name", if ns contains this annotation, it means it will share infra VPC
+		// read annotation "nsx.vmware.com/shared_vpc_namespace", if ns contains this annotation, it means it will share infra VPC
 		ncName, ncExist := annotations[types.AnnotationVPCNetworkConfig]
-		vpcName, nameExist := annotations[types.AnnotationVPCName]
-		if nameExist {
-			log.Info("read ns annotation vpcName", "VPCNAME", vpcName)
-			res := strings.Split(vpcName, "/")
-			// The format should be namespace/vpc_name
-			if len(res) != 2 {
-				message := fmt.Sprintf("incorrect vpcName annotation %s for namespace %s", vpcName, ns)
-				r.namespaceError(&ctx, obj, message, nil)
-				// If illegal format, skip handling this event?
-				return common.ResultNormal, nil
-			}
-		}
 
 		// If ns do not have network config name tag, then use default vpc network config name
 		if !ncExist {

pkg/nsx/services/common/types.go

@@ -73,7 +73,7 @@ const (
 	TagValueGroupDestination           string = "destination"
 	TagValueGroupAvi                   string = "avi"
 	AnnotationVPCNetworkConfig         string = "nsx.vmware.com/vpc_network_config"
-	AnnotationVPCName                  string = "nsx.vmware.com/vpc_name"
+	AnnotationSharedVPCNamespace       string = "nsx.vmware.com/shared_vpc_namespace"
 	AnnotationDefaultNetworkConfig     string = "nsx.vmware.com/default"
 	AnnotationAttachmentRef            string = "nsx.vmware.com/attachment_ref"
 	AnnotationPodMAC                   string = "nsx.vmware.com/mac"

pkg/nsx/services/vpc/vpc.go

@@ -382,16 +382,11 @@ func (s *VPCService) getSharedVPCNamespaceFromNS(ns string) (string, error) {
 		return "", nil
 	}
 
-	// If no annotation nsx.vmware.com/vpc_name on ns, this is not a shared vpc
-	ncName, exist := annos[common.AnnotationVPCName]
+	// If no annotation nsx.vmware.com/shared_vpc_namespace on ns, this is not a shared vpc
+	shared_ns, exist := annos[common.AnnotationSharedVPCNamespace]
 	if !exist {
 		return "", nil
 	}
-
-	// Retrieve the shared vpc namespace from annotation
-	// The format should be namespace/vpc_name, e.g. kube-system/infra-vpc
-	shared_ns := strings.Split(ncName, "/")[0]
-
 	return shared_ns, nil
 }
 

pkg/nsx/services/vpc/vpc_test.go

@@ -135,7 +135,7 @@ func TestGetSharedVPCNamespaceFromNS(t *testing.T) {
 		expected string
 	}{
 		{"1", "test-ns-1", map[string]string{"nsx.vmware.com/vpc_network_config": "default"}, ""},
-		{"2", "test-ns-2", map[string]string{"nsx.vmware.com/vpc_network_config": "infra", "nsx.vmware.com/vpc_name": "kube-system/fake_vpc"}, "kube-system"},
+		{"2", "test-ns-2", map[string]string{"nsx.vmware.com/vpc_network_config": "infra", "nsx.vmware.com/shared_vpc_namespace": "kube-system"}, "kube-system"},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {

test/e2e/framework.go

@@ -20,12 +20,13 @@ import (
 	"k8s.io/client-go/tools/clientcmd"
 
 	"github.com/vmware-tanzu/nsx-operator/pkg/client/clientset/versioned"
+
 	"github.com/vmware-tanzu/nsx-operator/pkg/nsx/services/common"
 	"github.com/vmware-tanzu/nsx-operator/test/e2e/providers"
 )
 
 const (
-	defaultTimeout         = 100 * time.Second
+	defaultTimeout         = 200 * time.Second
 	verifyNoneExistTimeout = 15 * time.Second
 	crdVersion             = "v1alpha1"
 )

test/e2e/manifest/testSubnet/shared_ns.yaml

@@ -2,7 +2,7 @@ apiVersion: v1
 kind: Namespace
 metadata:
   annotations:
-    nsx.vmware.com/vpc_name: target-ns/target-ns-vpc
+    nsx.vmware.com/shared_vpc_namespace: target-ns
   name: target-ns
 
 ---
@@ -11,5 +11,5 @@ apiVersion: v1
 kind: Namespace
 metadata:
   annotations:
-    nsx.vmware.com/vpc_name: target-ns/target-ns-vpc
+    nsx.vmware.com/shared_vpc_namespace: target-ns
   name: subnet-e2e-shared