Remove PodSubnetAccessMode and ShortID in VPCNetworkConfigurationSpec (…

…#703) (#713)


Signed-off-by: Wenqi Qiu <wenqiq@vmware.com>

build/yaml/crd/vpc/crd.nsx.vmware.com_vpcnetworkconfigurations.yaml

@@ -63,26 +63,11 @@ spec:
               nsxProject:
                 description: NSX Project the Namespace associated with.
                 type: string
-              podSubnetAccessMode:
-                description: |-
-                  PodSubnetAccessMode defines the access mode of the default SubnetSet for PodVMs.
-                  Must be Public, Private or PrivateTGW.
-                enum:
-                - Public
-                - Private
-                - PrivateTGW
-                type: string
               privateIPs:
                 description: Private IPs.
                 items:
                   type: string
                 type: array
-              shortID:
-                description: |-
-                  ShortID specifies Identifier to use when displaying VPC context in logs.
-                  Less than equal to 8 characters.
-                maxLength: 8
-                type: string
               vpc:
                 description: |-
                   NSX path of the VPC the Namespace associated with.
@@ -148,8 +133,6 @@ spec:
                   - name
                   type: object
                 type: array
-            required:
-            - conditions
             type: object
         type: object
     served: true

build/yaml/samples/nsx_v1alpha1_vpcnetworkconfigurations.yaml

@@ -8,7 +8,6 @@ spec:
   privateIPs:
     - 172.26.0.0/16
     - 172.36.0.0/16
-  podSubnetAccessMode: Private
 ---
 # Sample to create VPCNetworkConfiguration CR using a pre-created NSX VPC.
 apiVersion: crd.nsx.vmware.com/v1alpha1
@@ -18,5 +17,4 @@ metadata:
 spec:
   vpc: /orgs/default/projects/proj-1/vpcs/vpc-1
   defaultSubnetSize: 32
-  podSubnetAccessMode: Private
   vpcConnectivityProfile: /orgs/default/projects/wenqi-test/vpc-connectivity-profiles/default

go.mod

@@ -13,6 +13,7 @@ replace (
 )
 
 require (
+	github.com/agiledragon/gomonkey v2.0.2+incompatible
 	github.com/agiledragon/gomonkey/v2 v2.9.0
 	github.com/apparentlymart/go-cidr v1.1.0
 	github.com/deckarep/golang-set v1.8.0
@@ -48,7 +49,6 @@ require (
 )
 
 require (
-	github.com/agiledragon/gomonkey v2.0.2+incompatible // indirect
 	github.com/beevik/etree v1.1.0 // indirect
 	github.com/beorn7/perks v1.0.1 // indirect
 	github.com/cespare/xxhash/v2 v2.2.0 // indirect

pkg/apis/vpc/v1alpha1/vpcnetworkconfiguration_types.go

@@ -29,29 +29,18 @@ type VPCNetworkConfigurationSpec struct {
 	// Private IPs.
 	PrivateIPs []string `json:"privateIPs,omitempty"`
 
-	// ShortID specifies Identifier to use when displaying VPC context in logs.
-	// Less than equal to 8 characters.
-	// +kubebuilder:validation:MaxLength=8
-	// +optional
-	ShortID string `json:"shortID,omitempty"`
-
 	// Default size of Subnets.
 	// Defaults to 32.
 	// +kubebuilder:default=32
 	DefaultSubnetSize int `json:"defaultSubnetSize,omitempty"`
-
-	// PodSubnetAccessMode defines the access mode of the default SubnetSet for PodVMs.
-	// Must be Public, Private or PrivateTGW.
-	// +kubebuilder:validation:Enum=Public;Private;PrivateTGW
-	PodSubnetAccessMode string `json:"podSubnetAccessMode,omitempty"`
 }
 
 // VPCNetworkConfigurationStatus defines the observed state of VPCNetworkConfiguration
 type VPCNetworkConfigurationStatus struct {
 	// VPCs describes VPC info, now it includes lb Subnet info which are needed for AKO.
 	VPCs []VPCInfo `json:"vpcs,omitempty"`
 	// Conditions describe current state of VPCNetworkConfiguration.
-	Conditions []Condition `json:"conditions"`
+	Conditions []Condition `json:"conditions,omitempty"`
 }
 
 // VPCInfo defines VPC info needed by tenant admin.

pkg/controllers/namespace/namespace_controller.go

@@ -84,7 +84,7 @@ func (r *NamespaceReconciler) createNetworkInfoCR(ctx *context.Context, obj clie
 	return networkInfoCR, nil
 }
 
-func (r *NamespaceReconciler) createDefaultSubnetSet(ns string, defaultPodAccessMode string) error {
+func (r *NamespaceReconciler) createDefaultSubnetSet(ns string) error {
 	defaultSubnetSets := map[string]string{
 		types.DefaultVMSubnetSet:  types.LabelDefaultVMSubnetSet,
 		types.DefaultPodSubnetSet: types.LabelDefaultPodSubnetSet,
@@ -121,10 +121,9 @@ func (r *NamespaceReconciler) createDefaultSubnetSet(ns string, defaultPodAccess
 				},
 			}
 			if name == types.DefaultVMSubnetSet {
-				// use "Private" type for VM
-				obj.Spec.AccessMode = v1alpha1.AccessMode("Private")
+				obj.Spec.AccessMode = v1alpha1.AccessMode(v1alpha1.AccessModePrivate)
 			} else if name == types.DefaultPodSubnetSet {
-				obj.Spec.AccessMode = v1alpha1.AccessMode(defaultPodAccessMode)
+				obj.Spec.AccessMode = v1alpha1.AccessMode(v1alpha1.AccessModeProject)
 			}
 			if err := r.Client.Create(context.Background(), obj); err != nil {
 				return err
@@ -238,7 +237,7 @@ func (r *NamespaceReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
 		if _, err := r.createNetworkInfoCR(&ctx, obj, ns, ncName); err != nil {
 			return common.ResultRequeueAfter10sec, nil
 		}
-		if err := r.createDefaultSubnetSet(ns, nc.PodSubnetAccessMode); err != nil {
+		if err := r.createDefaultSubnetSet(ns); err != nil {
 			return common.ResultRequeueAfter10sec, nil
 		}
 		return common.ResultNormal, nil

pkg/controllers/networkinfo/networkinfo_utils.go

@@ -69,6 +69,7 @@ func setVPCNetworkConfigurationStatusWithLBS(ctx *context.Context, client client
 	err := client.Get(*ctx, apitypes.NamespacedName{Name: ncName}, nc)
 	if err != nil {
 		log.Error(err, "failed to get VPCNetworkConfiguration", "Name", ncName)
+		return
 	}
 	createdVPCInfo := &v1alpha1.VPCInfo{
 		Name:                vpcName,
@@ -83,12 +84,12 @@ func setVPCNetworkConfigurationStatusWithLBS(ctx *context.Context, client client
 			return
 		}
 	}
-	// else append the new VPCInfo
-	if nc.Status.VPCs == nil {
-		nc.Status.VPCs = []v1alpha1.VPCInfo{}
-	}
 	nc.Status.VPCs = append(nc.Status.VPCs, *createdVPCInfo)
-	client.Status().Update(*ctx, nc)
+	err = client.Status().Update(*ctx, nc)
+	if err != nil {
+		log.Error(err, "Update VPCNetworkConfiguration status failed", "ncName", ncName, "vpcName", vpcName, "nc.Status.VPCs", nc.Status.VPCs)
+	}
+	log.Info("Update VPCNetworkConfiguration status success", "ncName", ncName, "vpcName", vpcName, "nc.Status.VPCs", nc.Status.VPCs)
 }
 
 func setVPCNetworkConfigurationStatusWithGatewayConnection(ctx *context.Context, client client.Client, nc *v1alpha1.VPCNetworkConfiguration, gatewayConnectionReady bool, reason string) {

pkg/controllers/networkinfo/vpcnetworkconfig_handler.go

@@ -113,8 +113,6 @@ func buildNetworkConfigInfo(vpcConfigCR v1alpha1.VPCNetworkConfiguration) (*comm
 		NSXProject:             project,
 		PrivateIPs:             vpcConfigCR.Spec.PrivateIPs,
 		DefaultSubnetSize:      vpcConfigCR.Spec.DefaultSubnetSize,
-		PodSubnetAccessMode:    vpcConfigCR.Spec.PodSubnetAccessMode,
-		ShortID:                vpcConfigCR.Spec.ShortID,
 		VPCPath:                vpcConfigCR.Spec.VPC,
 	}
 	return ninfo, nil

pkg/controllers/networkinfo/vpcnetworkconfig_handler_test.go

@@ -76,20 +76,17 @@ func TestBuildNetworkConfigInfo(t *testing.T) {
 		PrivateIPs:             []string{"private-ipb-1", "private-ipb-2"},
 		DefaultSubnetSize:      64,
 		VPCConnectivityProfile: "test-VPCConnectivityProfile",
-		PodSubnetAccessMode:    "Public",
 		NSXProject:             "/orgs/default/projects/nsx_operator_e2e_test",
 	}
 	spec2 := v1alpha1.VPCNetworkConfigurationSpec{
-		PrivateIPs:          []string{"private-ipb-1", "private-ipb-2"},
-		DefaultSubnetSize:   32,
-		PodSubnetAccessMode: "Private",
-		NSXProject:          "/orgs/anotherOrg/projects/anotherProject",
+		PrivateIPs:        []string{"private-ipb-1", "private-ipb-2"},
+		DefaultSubnetSize: 32,
+		NSXProject:        "/orgs/anotherOrg/projects/anotherProject",
 	}
 	spec3 := v1alpha1.VPCNetworkConfigurationSpec{
-		DefaultSubnetSize:   28,
-		PodSubnetAccessMode: "Private",
-		NSXProject:          "/orgs/anotherOrg/projects/anotherProject",
-		VPC:                 "vpc33",
+		DefaultSubnetSize: 28,
+		NSXProject:        "/orgs/anotherOrg/projects/anotherProject",
+		VPC:               "vpc33",
 	}
 	testCRD1 := v1alpha1.VPCNetworkConfiguration{
 		Spec: spec1,
@@ -147,7 +144,6 @@ func TestBuildNetworkConfigInfo(t *testing.T) {
 			assert.Equal(t, tt.org, nc.Org)
 			assert.Equal(t, tt.project, nc.NSXProject)
 			assert.Equal(t, tt.subnetSize, nc.DefaultSubnetSize)
-			assert.Equal(t, tt.accessMode, nc.PodSubnetAccessMode)
 			assert.Equal(t, tt.isDefault, nc.IsDefault)
 			assert.Equal(t, tt.vpcPath, nc.VPCPath)
 		})

pkg/controllers/subnetset/subnetset_controller.go

@@ -71,9 +71,6 @@ func (r *SubnetSetReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
 				}
 				if obj.Spec.AccessMode == "" {
 					obj.Spec.AccessMode = v1alpha1.AccessMode(v1alpha1.AccessModePrivate)
-					if obj.Name == servicecommon.DefaultPodSubnetSet {
-						obj.Spec.AccessMode = v1alpha1.AccessMode(vpcNetworkConfig.PodSubnetAccessMode)
-					}
 				}
 				if obj.Spec.IPv4SubnetSize == 0 {
 					obj.Spec.IPv4SubnetSize = vpcNetworkConfig.DefaultSubnetSize

pkg/nsx/services/common/types.go

@@ -218,7 +218,5 @@ type VPCNetworkConfigInfo struct {
 	NSXProject             string
 	PrivateIPs             []string
 	DefaultSubnetSize      int
-	PodSubnetAccessMode    string
-	ShortID                string
 	VPCPath                string
 }

pkg/nsx/services/vpc/builder.go

@@ -85,9 +85,6 @@ func buildNSXVPC(obj *v1alpha1.NetworkInfo, nsObj *v1.Namespace, nc common.VPCNe
 	// TODO: add PrivateIps and remove PrivateIpv4Blocks once the NSX VPC API support private_ips field.
 	// vpc.PrivateIps = nc.PrivateIPs
 	vpc.PrivateIpv4Blocks = util.GetMapValues(pathMap)
-	if nc.ShortID != "" {
-		vpc.ShortId = &nc.ShortID
-	}
 
 	return vpc, nil
 }

pkg/nsx/services/vpc/builder_test.go

@@ -81,7 +81,6 @@ func Test_buildNSXLBS(t *testing.T) {
 func TestBuildNSXVPC(t *testing.T) {
 	nc := common.VPCNetworkConfigInfo{
 		PrivateIPs: []string{"192.168.1.0/24"},
-		ShortID:    "short1",
 	}
 	netInfoObj := &v1alpha1.NetworkInfo{
 		ObjectMeta: metav1.ObjectMeta{Namespace: "ns1", Name: "ns1", UID: "netinfouid1"},
@@ -115,7 +114,6 @@ func TestBuildNSXVPC(t *testing.T) {
 			useAVILB: false,
 			expVPC: &model.Vpc{
 				PrivateIpv4Blocks: []string{"192.168.3.0/24"},
-				ShortId:           common.String("short1"),
 			},
 		},
 		{
@@ -128,7 +126,6 @@ func TestBuildNSXVPC(t *testing.T) {
 				LoadBalancerVpcEndpoint: &model.LoadBalancerVPCEndpoint{Enabled: common.Bool(true)},
 				PrivateIpv4Blocks:       []string{"192.168.3.0/24"},
 				IpAddressType:           common.String("IPV4"),
-				ShortId:                 common.String("short1"),
 				Tags: []model.Tag{
 					{Scope: common.String("nsx-op/cluster"), Tag: common.String("cluster1")},
 					{Scope: common.String("nsx-op/version"), Tag: common.String("1.0.0")},
@@ -147,7 +144,6 @@ func TestBuildNSXVPC(t *testing.T) {
 				DisplayName:       common.String("ns1-netinfouid1"),
 				PrivateIpv4Blocks: []string{"192.168.3.0/24"},
 				IpAddressType:     common.String("IPV4"),
-				ShortId:           common.String("short1"),
 				Tags: []model.Tag{
 					{Scope: common.String("nsx-op/cluster"), Tag: common.String("cluster1")},
 					{Scope: common.String("nsx-op/version"), Tag: common.String("1.0.0")},

test/e2e/manifest/testVPC/customize_networkconfig.yaml

@@ -10,5 +10,4 @@ spec:
   privateIPs:
     - 172.29.0.0/16
     - 172.39.0.0/16
-  podSubnetAccessMode: Public
   vpcConnectivityProfile: /orgs/default/projects/nsx_operator_e2e_test/vpc-connectivity-profiles/default

test/e2e/manifest/testVPC/customize_networkconfig_updated.yaml

@@ -11,5 +11,4 @@ spec:
     - 172.29.0.0/16
     - 172.39.0.0/16
     - 172.49.0.0/16
-  podSubnetAccessMode: Public
   vpcConnectivityProfile: /orgs/default/projects/nsx_operator_e2e_test/vpc-connectivity-profiles/default

test/e2e/manifest/testVPC/default_networkconfig.yaml

@@ -14,5 +14,4 @@ spec:
   privateIPs:
     - 172.28.0.0/16
     - 172.38.0.0/16
-  podSubnetAccessMode: Public
   vpcConnectivityProfile: /orgs/default/projects/nsx_operator_e2e_test/vpc-connectivity-profiles/default

test/e2e/manifest/testVPC/system_networkconfig.yaml

@@ -11,5 +11,4 @@ spec:
   privateIPs:
     - 172.27.0.0/16
     - 172.37.0.0/16
-  podSubnetAccessMode: Public
   vpcConnectivityProfile: /orgs/default/projects/nsx_operator_e2e_test/vpc-connectivity-profiles/default

test/e2e/nsx_subnet_test.go

@@ -45,13 +45,6 @@ func verifySubnetSetCR(subnetSet string) bool {
 		return false
 	}
 
-	if subnetSet == common.DefaultPodSubnetSet {
-		if string(subnetSetCR.Spec.AccessMode) != vpcNetworkConfig.Spec.PodSubnetAccessMode {
-			log.Printf("AccessMode is %s, while it's expected to be %s", subnetSetCR.Spec.AccessMode, vpcNetworkConfig.Spec.PodSubnetAccessMode)
-			return false
-		}
-	}
-
 	if subnetSetCR.Spec.IPv4SubnetSize != vpcNetworkConfig.Spec.DefaultSubnetSize {
 		log.Printf("IPv4SubnetSize is %d, while it's expected to be %d", subnetSetCR.Spec.IPv4SubnetSize, vpcNetworkConfig.Spec.DefaultSubnetSize)
 		return false