From 4855c5e5694bbb8679e13509403add7a84e7c0ff Mon Sep 17 00:00:00 2001 From: Wenqi Qiu Date: Thu, 8 Aug 2024 16:14:36 +0800 Subject: [PATCH] Address comments Signed-off-by: Wenqi Qiu --- .../networkinfo/vpcnetworkconfig_handler.go | 2 +- pkg/nsx/services/vpc/builder.go | 8 ++++---- pkg/nsx/services/vpc/builder_test.go | 6 +----- pkg/nsx/services/vpc/compare.go | 2 +- pkg/nsx/services/vpc/vpc.go | 10 ++-------- 5 files changed, 9 insertions(+), 19 deletions(-) diff --git a/pkg/controllers/networkinfo/vpcnetworkconfig_handler.go b/pkg/controllers/networkinfo/vpcnetworkconfig_handler.go index b84ac39f9..21bc9c402 100644 --- 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 index d22fef1f9..4c4c25da8 100644 --- 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 index f5a2785ee..6b80292e8 100644 --- a/pkg/nsx/services/vpc/builder_test.go +++ b/pkg/nsx/services/vpc/builder_test.go @@ -95,7 +95,6 @@ func TestBuildNSXVPC(t *testing.T) { for _, tc := range []struct { name string existingVPC *model.Vpc - pathMap map[string]string useAVILB bool expVPC *model.Vpc }{ @@ -111,7 +110,6 @@ func TestBuildNSXVPC(t *testing.T) { existingVPC: &model.Vpc{ PrivateIpv4Blocks: []string{}, }, - pathMap: map[string]string{"vpc1": "192.168.3.0/24"}, useAVILB: false, expVPC: &model.Vpc{ PrivateIpv4Blocks: []string{"192.168.3.0/24"}, @@ -120,7 +118,6 @@ func TestBuildNSXVPC(t *testing.T) { }, { 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"), @@ -139,7 +136,6 @@ 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"), @@ -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 index 3a5354c6c..45686a7bd 100644 --- 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 index 698c16b1a..6ccb8e6be 100644 --- 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