Address comments

Signed-off-by: Wenqi Qiu <[email protected]>
vmware-tanzu · Aug 8, 2024 · dd55c2c · dd55c2c
1 parent 77cacc1
commit dd55c2c
Show file tree

Hide file tree

Showing 5 changed files with 20 additions and 30 deletions.
diff --git a/pkg/controllers/networkinfo/vpcnetworkconfig_handler.go b/pkg/controllers/networkinfo/vpcnetworkconfig_handler.go
@@ -101,7 +101,7 @@ var VPCNetworkConfigurationPredicate = predicate.Funcs{
 func buildNetworkConfigInfo(vpcConfigCR v1alpha1.VPCNetworkConfiguration) (*commontypes.VPCNetworkConfigInfo, error) {
 	org, project, err := nsxtProjectPathToId(vpcConfigCR.Spec.NSXProject)
 	if err != nil {
-		log.Error(err, "failed to parse nsx-t project in network config", "Project Path", vpcConfigCR.Spec.NSXProject)
+		log.Error(err, "failed to parse NSX project in network config", "Project Path", vpcConfigCR.Spec.NSXProject)
 		return nil, err
 	}
 

diff --git a/pkg/nsx/services/vpc/builder.go b/pkg/nsx/services/vpc/builder.go
@@ -50,7 +50,7 @@ func buildPrivateIpBlock(networkInfo *v1alpha1.NetworkInfo, nsObj *v1.Namespace,
 	return block
 }
 
-func buildNSXVPC(obj *v1alpha1.NetworkInfo, nsObj *v1.Namespace, nc common.VPCNetworkConfigInfo, cluster string, pathMap map[string]string,
+func buildNSXVPC(obj *v1alpha1.NetworkInfo, nsObj *v1.Namespace, nc common.VPCNetworkConfigInfo, cluster string,
 	nsxVPC *model.Vpc, useAVILB bool) (*model.Vpc,
 	error) {
 	vpc := &model.Vpc{}
@@ -80,9 +80,9 @@ func buildNSXVPC(obj *v1alpha1.NetworkInfo, nsObj *v1.Namespace, nc common.VPCNe
 		vpc.VpcConnectivityProfile = &nc.VPCConnectivityProfile
 	}
 
-	// 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.PrivateIPs != nil {
+		vpc.PrivateIps = nc.PrivateIPs
+	}
 	if nc.ShortID != "" {
 		vpc.ShortId = &nc.ShortID
 	}

diff --git a/pkg/nsx/services/vpc/builder_test.go b/pkg/nsx/services/vpc/builder_test.go
@@ -95,38 +95,35 @@ func TestBuildNSXVPC(t *testing.T) {
 	for _, tc := range []struct {
 		name        string
 		existingVPC *model.Vpc
-		pathMap     map[string]string
 		useAVILB    bool
 		expVPC      *model.Vpc
 	}{
 		{
 			name: "existing VPC not change",
 			existingVPC: &model.Vpc{
-				PrivateIpv4Blocks: []string{"192.168.1.0/24"},
+				PrivateIps: []string{"192.168.1.0/24"},
 			},
 			useAVILB: true,
 		},
 		{
-			name: "existing VPC changes private IPv4 blocks",
+			name: "existing VPC changes private IPv4",
 			existingVPC: &model.Vpc{
-				PrivateIpv4Blocks: []string{},
+				PrivateIps: []string{},
 			},
-			pathMap:  map[string]string{"vpc1": "192.168.3.0/24"},
 			useAVILB: false,
 			expVPC: &model.Vpc{
-				PrivateIpv4Blocks: []string{"192.168.3.0/24"},
-				ShortId:           common.String("short1"),
+				PrivateIps: []string{"192.168.1.0/24"},
+				ShortId:    common.String("short1"),
 			},
 		},
 		{
 			name:     "create new VPC with AVI load balancer enabled",
-			pathMap:  map[string]string{"vpc1": "192.168.3.0/24"},
 			useAVILB: true,
 			expVPC: &model.Vpc{
 				Id:                      common.String("ns1-netinfouid1"),
 				DisplayName:             common.String("ns1-netinfouid1"),
 				LoadBalancerVpcEndpoint: &model.LoadBalancerVPCEndpoint{Enabled: common.Bool(true)},
-				PrivateIpv4Blocks:       []string{"192.168.3.0/24"},
+				PrivateIps:              []string{"192.168.1.0/24"},
 				IpAddressType:           common.String("IPV4"),
 				ShortId:                 common.String("short1"),
 				Tags: []model.Tag{
@@ -139,14 +136,13 @@ func TestBuildNSXVPC(t *testing.T) {
 		},
 		{
 			name:     "create new VPC with AVI load balancer disabled",
-			pathMap:  map[string]string{"vpc1": "192.168.3.0/24"},
 			useAVILB: false,
 			expVPC: &model.Vpc{
-				Id:                common.String("ns1-netinfouid1"),
-				DisplayName:       common.String("ns1-netinfouid1"),
-				PrivateIpv4Blocks: []string{"192.168.3.0/24"},
-				IpAddressType:     common.String("IPV4"),
-				ShortId:           common.String("short1"),
+				Id:            common.String("ns1-netinfouid1"),
+				DisplayName:   common.String("ns1-netinfouid1"),
+				PrivateIps:    []string{"192.168.1.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")},
@@ -157,7 +153,7 @@ func TestBuildNSXVPC(t *testing.T) {
 		},
 	} {
 		t.Run(tc.name, func(t *testing.T) {
-			got, err := buildNSXVPC(netInfoObj, nsObj, nc, clusterStr, tc.pathMap, tc.existingVPC, tc.useAVILB)
+			got, err := buildNSXVPC(netInfoObj, nsObj, nc, clusterStr, tc.existingVPC, tc.useAVILB)
 			assert.Nil(t, err)
 			assert.Equal(t, tc.expVPC, got)
 		})

diff --git a/pkg/nsx/services/vpc/compare.go b/pkg/nsx/services/vpc/compare.go
@@ -9,7 +9,7 @@ import (
 // currently we only support appending public/private cidrs
 // so only comparing list size is enough to identify if vcp changed
 func IsVPCChanged(nc common.VPCNetworkConfigInfo, vpc *model.Vpc) bool {
-	if len(nc.PrivateIPs) != len(vpc.PrivateIpv4Blocks) {
+	if len(nc.PrivateIPs) != len(vpc.PrivateIps) {
 		return true
 	}
 

diff --git a/pkg/nsx/services/vpc/vpc.go b/pkg/nsx/services/vpc/vpc.go
@@ -141,7 +141,7 @@ func (s *VPCService) GetVPCNetworkConfigByNamespace(ns string) *common.VPCNetwor
 // TBD: for now, if network config info do not contains private cidr, we consider this is
 // incorrect configuration, and skip creating this VPC CR
 func (s *VPCService) ValidateNetworkConfig(nc common.VPCNetworkConfigInfo) bool {
-	return nc.PrivateIPs != nil && len(nc.PrivateIPs) != 0
+	return true
 }
 
 // InitializeVPC sync NSX resources
@@ -555,12 +555,6 @@ func (s *VPCService) CreateOrUpdateVPC(obj *v1alpha1.NetworkInfo) (*model.Vpc, *
 
 	log.Info("read network config from store", "NetworkConfig", ncName)
 
-	paths, err := s.CreateOrUpdatePrivateIPBlock(obj, nsObj, nc)
-	if err != nil {
-		log.Error(err, "failed to process private ip blocks, push event back to queue")
-		return nil, nil, err
-	}
-
 	// if all private ip blocks are created, then create nsx vpc resource.
 	nsxVPC := &model.Vpc{}
 	if updateVpc {
@@ -571,7 +565,7 @@ func (s *VPCService) CreateOrUpdateVPC(obj *v1alpha1.NetworkInfo) (*model.Vpc, *
 		nsxVPC = nil
 	}
 
-	createdVpc, err := buildNSXVPC(obj, nsObj, nc, s.NSXConfig.Cluster, paths, nsxVPC, s.NSXConfig.NsxConfig.UseAVILoadBalancer)
+	createdVpc, err := buildNSXVPC(obj, nsObj, nc, s.NSXConfig.Cluster, nsxVPC, s.NSXConfig.NsxConfig.UseAVILoadBalancer)
 	if err != nil {
 		log.Error(err, "failed to build NSX VPC object")
 		return nil, nil, err