diff --git a/pkg/controllers/namespace/namespace_controller.go b/pkg/controllers/namespace/namespace_controller.go index 372e7f660..1d26c1615 100644 --- a/pkg/controllers/namespace/namespace_controller.go +++ b/pkg/controllers/namespace/namespace_controller.go @@ -210,7 +210,6 @@ 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 annotation "nsx.vmware.com/shared_vpc_namespace", if ns contains this annotation, it means it will share infra VPC ncName, ncExist := annotations[types.AnnotationVPCNetworkConfig] // If ns do not have network config name tag, then use default vpc network config name diff --git a/pkg/controllers/networkinfo/networkinfo_controller.go b/pkg/controllers/networkinfo/networkinfo_controller.go index ae5958812..ac3574f70 100644 --- a/pkg/controllers/networkinfo/networkinfo_controller.go +++ b/pkg/controllers/networkinfo/networkinfo_controller.go @@ -69,8 +69,18 @@ func (r *NetworkInfoReconciler) Reconcile(ctx context.Context, req ctrl.Request) return common.ResultRequeueAfter10sec, err } + var privateIPs []string + var vpcConnectivityProfilePath string + if vpc.IsPreCreatedVPC(*nc) { + privateIPs = createdVpc.PrivateIps + vpcConnectivityProfilePath = *createdVpc.VpcConnectivityProfile + } else { + privateIPs = nc.PrivateIPs + vpcConnectivityProfilePath = nc.VPCConnectivityProfile + } + snatIP, path, cidr := "", "", "" - parts := strings.Split(nc.VPCConnectivityProfile, "/") + parts := strings.Split(vpcConnectivityProfilePath, "/") if len(parts) < 1 { log.Error(err, "failed to check VPCConnectivityProfile length", "VPCConnectivityProfile", nc.VPCConnectivityProfile) return common.ResultRequeue, err @@ -104,7 +114,7 @@ func (r *NetworkInfoReconciler) Reconcile(ctx context.Context, req ctrl.Request) VPCPath: *createdVpc.Path, DefaultSNATIP: "", LoadBalancerIPAddresses: "", - PrivateIPs: nc.PrivateIPs, + PrivateIPs: privateIPs, } updateFail(r, &ctx, obj, &err, r.Client, state) return common.ResultRequeueAfter10sec, err @@ -123,7 +133,7 @@ func (r *NetworkInfoReconciler) Reconcile(ctx context.Context, req ctrl.Request) VPCPath: *createdVpc.Path, DefaultSNATIP: snatIP, LoadBalancerIPAddresses: "", - PrivateIPs: nc.PrivateIPs, + PrivateIPs: privateIPs, } updateFail(r, &ctx, obj, &err, r.Client, state) return common.ResultRequeueAfter10sec, err @@ -135,7 +145,7 @@ func (r *NetworkInfoReconciler) Reconcile(ctx context.Context, req ctrl.Request) VPCPath: *createdVpc.Path, DefaultSNATIP: snatIP, LoadBalancerIPAddresses: cidr, - PrivateIPs: nc.PrivateIPs, + PrivateIPs: privateIPs, } updateSuccess(r, &ctx, obj, r.Client, state, nc.Name, path, r.Service.GetNSXLBSPath(*createdVpc.Id)) } else { diff --git a/pkg/controllers/networkinfo/networkinfo_utils.go b/pkg/controllers/networkinfo/networkinfo_utils.go index 81978ce24..6e9778232 100644 --- a/pkg/controllers/networkinfo/networkinfo_utils.go +++ b/pkg/controllers/networkinfo/networkinfo_utils.go @@ -4,6 +4,7 @@ import ( "context" "fmt" "reflect" + "slices" "github.com/vmware/vsphere-automation-sdk-go/services/nsxt/model" v1 "k8s.io/api/core/v1" @@ -42,21 +43,31 @@ func deleteSuccess(r *NetworkInfoReconciler, _ *context.Context, o *v1alpha1.Net metrics.CounterInc(r.Service.NSXConfig, metrics.ControllerDeleteSuccessTotal, common.MetricResTypeNetworkInfo) } -func setNetworkInfoVPCStatus(ctx *context.Context, networkInfo *v1alpha1.NetworkInfo, client client.Client, createdVPC *v1alpha1.VPCState) { +func setNetworkInfoVPCStatus(ctx *context.Context, networkInfo *v1alpha1.NetworkInfo, client client.Client, createdVPC *v1alpha1.VPCState) bool { + updateNetworkInfo := func(ctx context.Context, networkInfo *v1alpha1.NetworkInfo) bool { + if err := client.Update(ctx, networkInfo); err != nil { + log.Error(err, "Failed to update NetworkInfo's VPC state") + return false + } + return true + } + // if createdVPC is empty, remove the VPC from networkInfo if createdVPC == nil { networkInfo.VPCs = []v1alpha1.VPCState{} - client.Update(*ctx, networkInfo) - return + return updateNetworkInfo(*ctx, networkInfo) } existingVPC := &v1alpha1.VPCState{} if len(networkInfo.VPCs) > 0 { existingVPC = &networkInfo.VPCs[0] } - if !reflect.DeepEqual(*existingVPC, *createdVPC) { - networkInfo.VPCs = []v1alpha1.VPCState{*createdVPC} - client.Update(*ctx, networkInfo) + slices.Sort(existingVPC.PrivateIPs) + slices.Sort(createdVPC.PrivateIPs) + if reflect.DeepEqual(*existingVPC, *createdVPC) { + return false } + networkInfo.VPCs = []v1alpha1.VPCState{*createdVPC} + return updateNetworkInfo(*ctx, networkInfo) } func setVPCNetworkConfigurationStatus(ctx *context.Context, client client.Client, ncName string, vpcName string, aviSubnetPath string, nsxLBSPath string) { diff --git a/pkg/controllers/networkinfo/vpcnetworkconfig_handler.go b/pkg/controllers/networkinfo/vpcnetworkconfig_handler.go index e10ff944f..94922e77c 100644 --- a/pkg/controllers/networkinfo/vpcnetworkconfig_handler.go +++ b/pkg/controllers/networkinfo/vpcnetworkconfig_handler.go @@ -115,6 +115,7 @@ func buildNetworkConfigInfo(vpcConfigCR v1alpha1.VPCNetworkConfiguration) (*comm DefaultSubnetSize: vpcConfigCR.Spec.DefaultSubnetSize, PodSubnetAccessMode: vpcConfigCR.Spec.PodSubnetAccessMode, ShortID: vpcConfigCR.Spec.ShortID, + VPCPath: vpcConfigCR.Spec.VPC, } return ninfo, nil } diff --git a/pkg/controllers/networkinfo/vpcnetworkconfig_handler_test.go b/pkg/controllers/networkinfo/vpcnetworkconfig_handler_test.go index e29297b76..7ad2fcdb4 100644 --- a/pkg/controllers/networkinfo/vpcnetworkconfig_handler_test.go +++ b/pkg/controllers/networkinfo/vpcnetworkconfig_handler_test.go @@ -85,6 +85,12 @@ func TestBuildNetworkConfigInfo(t *testing.T) { PodSubnetAccessMode: "Private", NSXProject: "/orgs/anotherOrg/projects/anotherProject", } + spec3 := v1alpha1.VPCNetworkConfigurationSpec{ + DefaultSubnetSize: 28, + PodSubnetAccessMode: "Private", + NSXProject: "/orgs/anotherOrg/projects/anotherProject", + VPC: "vpc33", + } testCRD1 := v1alpha1.VPCNetworkConfiguration{ Spec: spec1, } @@ -104,6 +110,16 @@ func TestBuildNetworkConfigInfo(t *testing.T) { } testCRD3.Name = "test-3" + testCRD4 := v1alpha1.VPCNetworkConfiguration{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + types.AnnotationDefaultNetworkConfig: "false", + }, + }, + Spec: spec3, + } + testCRD3.Name = "test-4" + tests := []struct { name string nc v1alpha1.VPCNetworkConfiguration @@ -115,10 +131,12 @@ func TestBuildNetworkConfigInfo(t *testing.T) { accessMode string isDefault bool vpcConnectivityProfile string + vpcPath string }{ - {"test-nsxtProjectPathToId", testCRD1, "test-gw-path-1", "test-edge-path-1", "default", "nsx_operator_e2e_test", 64, "Public", false, ""}, - {"with-VPCConnectivityProfile", testCRD2, "test-gw-path-2", "test-edge-path-2", "anotherOrg", "anotherProject", 32, "Private", false, "test-VPCConnectivityProfile"}, - {"with-defaultNetworkConfig", testCRD3, "test-gw-path-2", "test-edge-path-2", "anotherOrg", "anotherProject", 32, "Private", true, ""}, + {"test-nsxtProjectPathToId", testCRD1, "test-gw-path-1", "test-edge-path-1", "default", "nsx_operator_e2e_test", 64, "Public", false, "", ""}, + {"with-VPCConnectivityProfile", testCRD2, "test-gw-path-2", "test-edge-path-2", "anotherOrg", "anotherProject", 32, "Private", false, "test-VPCConnectivityProfile", ""}, + {"with-defaultNetworkConfig", testCRD3, "test-gw-path-2", "test-edge-path-2", "anotherOrg", "anotherProject", 32, "Private", true, "", ""}, + {"with-preCreatedVPC", testCRD4, "", "", "anotherOrg", "anotherProject", 28, "Private", false, "", "vpc33"}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -131,7 +149,7 @@ func TestBuildNetworkConfigInfo(t *testing.T) { 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) }) } - } diff --git a/pkg/controllers/securitypolicy/pod_hanlder_test.go b/pkg/controllers/securitypolicy/pod_handler_test.go similarity index 100% rename from pkg/controllers/securitypolicy/pod_hanlder_test.go rename to pkg/controllers/securitypolicy/pod_handler_test.go diff --git a/pkg/nsx/services/common/types.go b/pkg/nsx/services/common/types.go index 85044ac6e..50ac0492d 100644 --- a/pkg/nsx/services/common/types.go +++ b/pkg/nsx/services/common/types.go @@ -59,6 +59,8 @@ const ( TagScopeIPSubnetName string = "nsx-op/ipsubnet_name" TagScopeVMNamespaceUID string = "nsx-op/vm_namespace_uid" TagScopeVMNamespace string = "nsx-op/vm_namespace" + TagScopeVPCManagedBy string = "nsx/managed-by" + AutoCreatedVPCTagValue string = "nsx-op" LabelDefaultSubnetSet string = "nsxoperator.vmware.com/default-subnetset-for" LabelDefaultVMSubnetSet string = "VirtualMachine" LabelDefaultPodSubnetSet string = "Pod" @@ -212,4 +214,5 @@ type VPCNetworkConfigInfo struct { DefaultSubnetSize int PodSubnetAccessMode string ShortID string + VPCPath string } diff --git a/pkg/nsx/services/vpc/builder.go b/pkg/nsx/services/vpc/builder.go index 4b8c09176..1996da549 100644 --- a/pkg/nsx/services/vpc/builder.go +++ b/pkg/nsx/services/vpc/builder.go @@ -74,6 +74,8 @@ func buildNSXVPC(obj *v1alpha1.NetworkInfo, nsObj *v1.Namespace, nc common.VPCNe vpc.LoadBalancerVpcEndpoint = &model.LoadBalancerVPCEndpoint{Enabled: &loadBalancerVPCEndpointEnabled} } vpc.Tags = util.BuildBasicTags(cluster, obj, nsObj.UID) + vpc.Tags = append(vpc.Tags, model.Tag{ + Scope: common.String(common.TagScopeVPCManagedBy), Tag: common.String(common.AutoCreatedVPCTagValue)}) } if nc.VPCConnectivityProfile != "" { diff --git a/pkg/nsx/services/vpc/builder_test.go b/pkg/nsx/services/vpc/builder_test.go index f925e4104..68ab81060 100644 --- a/pkg/nsx/services/vpc/builder_test.go +++ b/pkg/nsx/services/vpc/builder_test.go @@ -134,6 +134,7 @@ func TestBuildNSXVPC(t *testing.T) { {Scope: common.String("nsx-op/version"), Tag: common.String("1.0.0")}, {Scope: common.String("nsx-op/namespace"), Tag: common.String("ns1")}, {Scope: common.String("nsx-op/namespace_uid"), Tag: common.String("nsuid1")}, + {Scope: common.String("nsx/managed-by"), Tag: common.String("nsx-op")}, }, }, }, @@ -152,6 +153,7 @@ func TestBuildNSXVPC(t *testing.T) { {Scope: common.String("nsx-op/version"), Tag: common.String("1.0.0")}, {Scope: common.String("nsx-op/namespace"), Tag: common.String("ns1")}, {Scope: common.String("nsx-op/namespace_uid"), Tag: common.String("nsuid1")}, + {Scope: common.String("nsx/managed-by"), Tag: common.String("nsx-op")}, }, }, }, diff --git a/pkg/nsx/services/vpc/vpc.go b/pkg/nsx/services/vpc/vpc.go index 0f3622c68..361d9775a 100644 --- a/pkg/nsx/services/vpc/vpc.go +++ b/pkg/nsx/services/vpc/vpc.go @@ -49,7 +49,7 @@ var ( ) type VPCNetworkInfoStore struct { - sync.Mutex + sync.RWMutex VPCNetworkConfigMap map[string]common.VPCNetworkConfigInfo } @@ -143,6 +143,10 @@ 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 { + if nc.VPCPath != "" { + // if network config is using a pre-created VPC, skip the check on PrivateIPs. + return true + } return nc.PrivateIPs != nil && len(nc.PrivateIPs) != 0 } @@ -534,6 +538,16 @@ func (s *VPCService) CreateOrUpdateVPC(obj *v1alpha1.NetworkInfo) (*model.Vpc, * return nil, nil, errors.New(message) } + // Return pre-created VPC resource if it is used in the VPCNetworkConfiguration + if IsPreCreatedVPC(nc) { + preVPC, err := s.GetVPCFromNSXByPath(nc.VPCPath) + if err != nil { + log.Error(err, "Failed to get existing VPC from NSX", "vpcPath", nc.VPCPath) + return nil, nil, err + } + return preVPC, &nc, nil + } + // check if this namespace vpc share from others, if yes // then check if the shared vpc created or not, if yes // then directly return this vpc, if not, requeue @@ -716,6 +730,19 @@ func (s *VPCService) Cleanup(ctx context.Context) error { func (service *VPCService) ListVPCInfo(ns string) []common.VPCResourceInfo { var VPCInfoList []common.VPCResourceInfo + nc := service.GetVPCNetworkConfigByNamespace(ns) + // Return the pre-created VPC resource info if it is set in VPCNetworkConfiguration. + if nc != nil && IsPreCreatedVPC(*nc) { + vpcResourceInfo, err := common.ParseVPCResourcePath(nc.VPCPath) + if err != nil { + log.Error(err, "Failed to get vpc info from vpc path", "vpc path", nc.VPCPath) + } else { + VPCInfoList = append(VPCInfoList, vpcResourceInfo) + } + return VPCInfoList + } + + // List VPCs from local store. vpcs := service.GetVPCsByNamespace(ns) // Transparently call the VPCService.GetVPCsByNamespace method for _, v := range vpcs { vpcResourceInfo, err := common.ParseVPCResourcePath(*v.Path) @@ -781,3 +808,44 @@ func (vpcService *VPCService) getLBProvider() string { } return LBProviderAVI } + +// ListNamespacesWithPreCreatedVPC returns a mapping of the pre-created VPC path and the Namespaces +// which use the VPC. The key is the pre-created VPC path, the value is a slice of Namespaces that +// use the VPC. +func (service *VPCService) ListNamespacesWithPreCreatedVPC() map[string][]string { + vpcNSMappings := make(map[string][]string) + service.VPCNetworkConfigStore.RLock() + defer service.VPCNetworkConfigStore.RUnlock() + for name, config := range service.VPCNetworkConfigStore.VPCNetworkConfigMap { + if IsPreCreatedVPC(config) { + vpcPath := config.VPCPath + namespaces := service.GetNamespacesByNetworkconfigName(name) + existingNamespaces, found := vpcNSMappings[vpcPath] + if !found { + existingNamespaces = make([]string, 0) + } + existingNamespaces = append(existingNamespaces, namespaces...) + vpcNSMappings[vpcPath] = existingNamespaces + } + } + return vpcNSMappings +} + +func (service *VPCService) GetVPCFromNSXByPath(vpcPath string) (*model.Vpc, error) { + vpcResInfo, err := common.ParseVPCResourcePath(vpcPath) + if err != nil { + return nil, err + } + vpc, err := service.NSXClient.VPCClient.Get(vpcResInfo.OrgID, vpcResInfo.ProjectID, vpcResInfo.VPCID) + err = nsxutil.NSXApiError(err) + if err != nil { + log.Error(err, "failed to read VPC object from NSX", "VPC", vpcPath) + return nil, err + } + + return &vpc, nil +} + +func IsPreCreatedVPC(nc common.VPCNetworkConfigInfo) bool { + return nc.VPCPath != "" +} diff --git a/pkg/nsx/services/vpc/vpc_test.go b/pkg/nsx/services/vpc/vpc_test.go index f65996c08..b0eeea721 100644 --- a/pkg/nsx/services/vpc/vpc_test.go +++ b/pkg/nsx/services/vpc/vpc_test.go @@ -391,3 +391,44 @@ func TestGetLbProvider(t *testing.T) { assert.Equal(t, LBProviderAVI, lbProvider) patch.Reset() } + +func TestListNamespacesWithPreCreatedVPC(t *testing.T) { + service := &VPCService{ + VPCNetworkConfigStore: VPCNetworkInfoStore{ + VPCNetworkConfigMap: make(map[string]common.VPCNetworkConfigInfo), + }, + VPCNSNetworkConfigStore: VPCNsNetworkConfigStore{ + VPCNSNetworkConfigMap: make(map[string]string), + }, + } + vpcPath1 := "vpc-path1" + vpcPath2 := "vpc-path2" + vpcCR1 := "vpc-cr1" + vpcCR2 := "vpc-cr2" + vpcCR3 := "vpc-cr3" + service.RegisterNamespaceNetworkconfigBinding("ns1", vpcCR1) + service.RegisterNamespaceNetworkconfigBinding("ns2", vpcCR2) + service.RegisterNamespaceNetworkconfigBinding("ns3", vpcCR3) + service.RegisterNamespaceNetworkconfigBinding("ns4", vpcCR2) + service.RegisterVPCNetworkConfig(vpcCR1, common.VPCNetworkConfigInfo{ + Name: "vpc1", + VPCPath: vpcPath1, + }) + service.RegisterVPCNetworkConfig(vpcCR2, common.VPCNetworkConfigInfo{ + Name: "vpc2", + VPCPath: vpcPath2, + }) + // Auto-created VPC. + service.RegisterVPCNetworkConfig(vpcCR3, common.VPCNetworkConfigInfo{ + Name: "vpc-3", + }) + + vpcNSMappings := service.ListNamespacesWithPreCreatedVPC() + assert.Equal(t, 2, len(vpcNSMappings)) + nss1, found := vpcNSMappings[vpcPath1] + assert.True(t, found) + assert.ElementsMatch(t, []string{"ns1"}, nss1) + nss2, found := vpcNSMappings[vpcPath2] + assert.True(t, found) + assert.ElementsMatch(t, []string{"ns2", "ns4"}, nss2) +}