diff --git a/pkg/controllers/common/utils.go b/pkg/controllers/common/utils.go index 018e5e30e..ce812491c 100644 --- a/pkg/controllers/common/utils.go +++ b/pkg/controllers/common/utils.go @@ -40,7 +40,7 @@ func AllocateSubnetFromSubnetSet(subnetSet *v1alpha1.SubnetSet, vpcService servi return *nsxSubnet.Path, nil } } - tags := subnetService.GenerateSubnetNSTags(subnetSet, subnetSet.Namespace) + tags := subnetService.GenerateSubnetNSTags(subnetSet, subnetSet.Name, subnetSet.Namespace) if tags == nil { return "", errors.New("failed to generate subnet tags") } diff --git a/pkg/controllers/subnet/subnet_controller.go b/pkg/controllers/subnet/subnet_controller.go index 4a5daf27c..aa227d90f 100644 --- a/pkg/controllers/subnet/subnet_controller.go +++ b/pkg/controllers/subnet/subnet_controller.go @@ -5,6 +5,7 @@ import ( "errors" "fmt" "reflect" + "time" "github.com/vmware/vsphere-automation-sdk-go/services/nsxt/model" v1 "k8s.io/api/core/v1" @@ -16,12 +17,8 @@ import ( "sigs.k8s.io/controller-runtime/pkg/builder" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller" - "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" - "sigs.k8s.io/controller-runtime/pkg/event" - "sigs.k8s.io/controller-runtime/pkg/predicate" "github.com/vmware-tanzu/nsx-operator/pkg/apis/vpc/v1alpha1" - "github.com/vmware-tanzu/nsx-operator/pkg/controllers/common" "github.com/vmware-tanzu/nsx-operator/pkg/logger" "github.com/vmware-tanzu/nsx-operator/pkg/metrics" @@ -50,102 +47,130 @@ type SubnetReconciler struct { } func (r *SubnetReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { - obj := &v1alpha1.Subnet{} - log.Info("reconciling subnet CR", "subnet", req.NamespacedName) + startTime := time.Now() + defer func() { + log.Info("Finished reconciling Subnet", "Subnet", req.NamespacedName, "time", time.Since(startTime)) + }() metrics.CounterInc(r.SubnetService.NSXConfig, metrics.ControllerSyncTotal, MetricResTypeSubnet) - if err := r.Client.Get(ctx, req.NamespacedName, obj); err != nil { + subnetCR := &v1alpha1.Subnet{} + if err := r.Client.Get(ctx, req.NamespacedName, subnetCR); err != nil { + if err := r.DeleteSubnetByName(req.Name, req.Namespace); err != nil { + return ResultRequeue, err + } log.Error(err, "unable to fetch Subnet CR", "req", req.NamespacedName) return ResultNormal, client.IgnoreNotFound(err) } - - if obj.ObjectMeta.DeletionTimestamp.IsZero() { - metrics.CounterInc(r.SubnetService.NSXConfig, metrics.ControllerUpdateTotal, MetricResTypeSubnet) - if !controllerutil.ContainsFinalizer(obj, servicecommon.SubnetFinalizerName) { - controllerutil.AddFinalizer(obj, servicecommon.SubnetFinalizerName) - - if obj.Spec.AccessMode == "" { - log.Info("obj.Spec.AccessMode set", "subnet", req.NamespacedName) - obj.Spec.AccessMode = v1alpha1.AccessMode(v1alpha1.AccessModePrivate) - } - - if obj.Spec.IPv4SubnetSize == 0 { - vpcNetworkConfig := r.VPCService.GetVPCNetworkConfigByNamespace(obj.Namespace) - if vpcNetworkConfig == nil { - err := fmt.Errorf("operate failed: cannot get configuration for Subnet CR") - log.Error(nil, "failed to find VPCNetworkConfig for Subnet CR", "subnet", req.NamespacedName, "namespace %s", obj.Namespace) - updateFail(r, ctx, obj, err.Error()) - return ResultRequeue, err - } - obj.Spec.IPv4SubnetSize = vpcNetworkConfig.DefaultSubnetSize - } - if err := r.Client.Update(ctx, obj); err != nil { - log.Error(err, "add finalizer", "subnet", req.NamespacedName) - updateFail(r, ctx, obj, err.Error()) - return ResultRequeue, err - } - log.V(1).Info("added finalizer on subnet CR", "subnet", req.NamespacedName) - } - tags := r.SubnetService.GenerateSubnetNSTags(obj, obj.Namespace) - if tags == nil { - return ResultRequeue, errors.New("failed to generate subnet tags") + if err := r.cleanStaleSubnets(subnetCR.Name, subnetCR.Namespace, string(subnetCR.UID)); err != nil { + return ResultRequeue, err + } + if !subnetCR.DeletionTimestamp.IsZero() { + metrics.CounterInc(r.SubnetService.NSXConfig, metrics.ControllerDeleteTotal, MetricResTypeSubnet) + if err := r.DeleteSubnetByID(*subnetCR); err != nil { + log.Error(err, "deletion NSX Subnet failed, would retry exponentially", "Subnet", req.NamespacedName) + deleteFail(r, ctx, subnetCR, err.Error()) + return ResultRequeue, err } - vpcInfoList := r.VPCService.ListVPCInfo(req.Namespace) - if len(vpcInfoList) == 0 { - return ResultRequeueAfter10sec, nil + if err := r.Client.Delete(ctx, subnetCR); err != nil { + log.Error(err, "deletion Subnet CR failed, would retry exponentially", "Subnet", req.NamespacedName) + deleteFail(r, ctx, subnetCR, err.Error()) + return ResultRequeue, err } - if _, err := r.SubnetService.CreateOrUpdateSubnet(obj, vpcInfoList[0], tags); err != nil { - if errors.As(err, &util.ExceedTagsError{}) { - log.Error(err, "exceed tags limit, would not retry", "subnet", req.NamespacedName) - updateFail(r, ctx, obj, err.Error()) - return ResultNormal, nil - } - log.Error(err, "operate failed, would retry exponentially", "subnet", req.NamespacedName) - updateFail(r, ctx, obj, err.Error()) + log.V(1).Info("Deleted Subnet", "Subnet", req.NamespacedName) + deleteSuccess(r, ctx, subnetCR) + return ResultNormal, nil + } + + metrics.CounterInc(r.SubnetService.NSXConfig, metrics.ControllerUpdateTotal, MetricResTypeSubnet) + + specChanged := false + if subnetCR.Spec.AccessMode == "" { + subnetCR.Spec.AccessMode = v1alpha1.AccessMode(v1alpha1.AccessModePrivate) + specChanged = true + } + + if subnetCR.Spec.IPv4SubnetSize == 0 { + vpcNetworkConfig := r.VPCService.GetVPCNetworkConfigByNamespace(subnetCR.Namespace) + if vpcNetworkConfig == nil { + err := fmt.Errorf("operate failed: cannot get configuration for Subnet CR") + log.Error(nil, "failed to find VPCNetworkConfig for Subnet CR", "Subnet", req.NamespacedName, "Namespace", subnetCR.Namespace) + updateFail(r, ctx, subnetCR, err.Error()) return ResultRequeue, err } - if err := r.updateSubnetStatus(obj); err != nil { - log.Error(err, "update subnet status failed, would retry exponentially", "subnet", req.NamespacedName) - updateFail(r, ctx, obj, err.Error()) + subnetCR.Spec.IPv4SubnetSize = vpcNetworkConfig.DefaultSubnetSize + specChanged = true + } + if specChanged { + err := r.Client.Update(ctx, subnetCR) + if err != nil { + log.Error(err, "update Subnet failed", "Subnet", req.NamespacedName) + updateFail(r, ctx, subnetCR, err.Error()) return ResultRequeue, err } - updateSuccess(r, ctx, obj) - } else { - if controllerutil.ContainsFinalizer(obj, servicecommon.SubnetFinalizerName) { - metrics.CounterInc(r.SubnetService.NSXConfig, metrics.ControllerDeleteTotal, MetricResTypeSubnet) - if err := r.DeleteSubnet(*obj); err != nil { - log.Error(err, "deletion failed, would retry exponentially", "subnet", req.NamespacedName) - deleteFail(r, ctx, obj, err.Error()) - return ResultRequeue, err - } - controllerutil.RemoveFinalizer(obj, servicecommon.SubnetFinalizerName) - if err := r.Client.Update(ctx, obj); err != nil { - log.Error(err, "deletion failed, would retry exponentially", "subnet", req.NamespacedName) - deleteFail(r, ctx, obj, err.Error()) - return ResultRequeue, err - } - log.V(1).Info("removed finalizer", "subnet", req.NamespacedName) - deleteSuccess(r, ctx, obj) - } else { - log.Info("finalizers cannot be recognized", "subnet", req.NamespacedName) + } + + tags := r.SubnetService.GenerateSubnetNSTags(subnetCR, subnetCR.Name, subnetCR.Namespace) + if tags == nil { + return ResultRequeue, errors.New("failed to generate Subnet tags") + } + vpcInfoList := r.VPCService.ListVPCInfo(req.Namespace) + if len(vpcInfoList) == 0 { + return ResultRequeueAfter10sec, nil + } + if _, err := r.SubnetService.CreateOrUpdateSubnet(subnetCR, vpcInfoList[0], tags); err != nil { + if errors.As(err, &util.ExceedTagsError{}) { + log.Error(err, "exceed tags limit, would not retry", "Subnet", req.NamespacedName) + updateFail(r, ctx, subnetCR, err.Error()) + return ResultNormal, nil } + log.Error(err, "operate failed, would retry exponentially", "Subnet", req.NamespacedName) + updateFail(r, ctx, subnetCR, err.Error()) + return ResultRequeue, err + } + if err := r.updateSubnetStatus(subnetCR); err != nil { + log.Error(err, "update Subnet status failed, would retry exponentially", "Subnet", req.NamespacedName) + updateFail(r, ctx, subnetCR, err.Error()) + return ResultRequeue, err } + updateSuccess(r, ctx, subnetCR) return ctrl.Result{}, nil } -func (r *SubnetReconciler) DeleteSubnet(obj v1alpha1.Subnet) error { +func (r *SubnetReconciler) DeleteSubnetByID(obj v1alpha1.Subnet) error { nsxSubnets := r.SubnetService.SubnetStore.GetByIndex(servicecommon.TagScopeSubnetCRUID, string(obj.GetUID())) - if len(nsxSubnets) == 0 { - log.Info("no subnet found for subnet CR", "uid", string(obj.GetUID())) - return nil + return r.deleteSubnets(nsxSubnets) +} + +func (r *SubnetReconciler) deleteSubnets(nsxSubnets []*model.VpcSubnet) error { + for _, nsxSubnet := range nsxSubnets { + portNums := len(r.SubnetPortService.GetPortsOfSubnet(*nsxSubnet.Id)) + if portNums > 0 { + err := errors.New("subnet still attached by port") + log.Error(err, "delete Subnet from NSX failed", "ID", *nsxSubnet.Id) + return err + } + if err := r.SubnetService.DeleteSubnet(*nsxSubnet); err != nil { + return err + } } - portNums := len(r.SubnetPortService.GetPortsOfSubnet(*nsxSubnets[0].Id)) - if portNums > 0 { - err := errors.New("subnet still attached by port") - log.Error(err, "", "ID", *nsxSubnets[0].Id) - return err + return nil +} + +func (r *SubnetReconciler) DeleteSubnetByName(name, namespace string) error { + nsxSubnets := r.SubnetService.SubnetStore.GetByIndex(servicecommon.TagScopeSubnetCRNamespacedName, subnet.SubnetNamespacedName(name, namespace)) + return r.deleteSubnets(nsxSubnets) +} + +func (r *SubnetReconciler) cleanStaleSubnets(name, namespace, id string) error { + subnetsToDelete := []*model.VpcSubnet{} + nsxSubnets := r.SubnetService.SubnetStore.GetByIndex(servicecommon.TagScopeSubnetCRNamespacedName, subnet.SubnetNamespacedName(name, namespace)) + for i, nsxSubnet := range nsxSubnets { + if *nsxSubnet.Id == id { + continue + } + subnetsToDelete = append(subnetsToDelete, nsxSubnets[i]) } - return r.SubnetService.DeleteSubnet(*nsxSubnets[0]) + return r.deleteSubnets(subnetsToDelete) } func (r *SubnetReconciler) updateSubnetStatus(obj *v1alpha1.Subnet) error { @@ -163,7 +188,7 @@ func (r *SubnetReconciler) updateSubnetStatus(obj *v1alpha1.Subnet) error { for _, status := range statusList { obj.Status.NetworkAddresses = append(obj.Status.NetworkAddresses, *status.NetworkAddress) obj.Status.GatewayAddresses = append(obj.Status.GatewayAddresses, *status.GatewayAddress) - // DHCPServerAddress is only for the subnet with DHCP enabled + // DHCPServerAddress is only for the Subnet with DHCP enabled if status.DhcpServerAddress != nil { obj.Status.DHCPServerAddresses = append(obj.Status.DHCPServerAddresses, *status.DhcpServerAddress) } @@ -209,7 +234,7 @@ func (r *SubnetReconciler) updateSubnetStatusConditions(ctx context.Context, sub } if conditionsUpdated { if err := r.Client.Status().Update(ctx, subnet); err != nil { - log.Error(err, "failed to update subnet status", "Name", subnet.Name, "Namespace", subnet.Namespace) + log.Error(err, "failed to update Subnet status", "Name", subnet.Name, "Namespace", subnet.Namespace) } else { log.Info("updated Subnet", "Name", subnet.Name, "Namespace", subnet.Namespace, "New Conditions", newConditions) } @@ -269,12 +294,7 @@ func deleteSuccess(r *SubnetReconciler, _ context.Context, o *v1alpha1.Subnet) { func (r *SubnetReconciler) setupWithManager(mgr ctrl.Manager) error { return ctrl.NewControllerManagedBy(mgr). For(&v1alpha1.Subnet{}). - WithEventFilter(predicate.Funcs{ - DeleteFunc: func(e event.DeleteEvent) bool { - // Suppress Delete events to avoid filtering them out in the Reconcile function - return false - }, - }). + // WithEventFilter(). Watches( &v1.Namespace{}, &EnqueueRequestForNamespace{Client: mgr.GetClient()}, @@ -315,11 +335,11 @@ func (r *SubnetReconciler) Start(mgr ctrl.Manager) error { // CollectGarbage implements the interface GarbageCollector method. func (r *SubnetReconciler) CollectGarbage(ctx context.Context) { - log.Info("subnet garbage collector started") + log.Info("Subnet garbage collector started") crdSubnetList := &v1alpha1.SubnetList{} err := r.Client.List(ctx, crdSubnetList) if err != nil { - log.Error(err, "failed to list subnet CR") + log.Error(err, "failed to list Subnet CR") return } var nsxSubnetList []*model.VpcSubnet diff --git a/pkg/controllers/subnetset/subnetset_controller.go b/pkg/controllers/subnetset/subnetset_controller.go index 84d688526..4d73fb2cd 100644 --- a/pkg/controllers/subnetset/subnetset_controller.go +++ b/pkg/controllers/subnetset/subnetset_controller.go @@ -5,6 +5,7 @@ import ( "errors" "fmt" "reflect" + "time" "github.com/vmware/vsphere-automation-sdk-go/services/nsxt/model" v1 "k8s.io/api/core/v1" @@ -16,7 +17,6 @@ import ( "sigs.k8s.io/controller-runtime/pkg/builder" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller" - "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" "sigs.k8s.io/controller-runtime/pkg/webhook" "sigs.k8s.io/controller-runtime/pkg/webhook/admission" @@ -49,94 +49,94 @@ type SubnetSetReconciler struct { } func (r *SubnetSetReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { - obj := &v1alpha1.SubnetSet{} - log.Info("reconciling subnetset CR", "subnetset", req.NamespacedName) + startTime := time.Now() + defer func() { + log.Info("Finished reconciling SubnetSet", "SubnetSet", req.NamespacedName, "time", time.Since(startTime)) + }() + + subnetsetCR := &v1alpha1.SubnetSet{} metrics.CounterInc(r.SubnetService.NSXConfig, metrics.ControllerSyncTotal, MetricResTypeSubnetSet) - if err := r.Client.Get(ctx, req.NamespacedName, obj); err != nil { - log.Error(err, "unable to fetch subnetset CR", "req", req.NamespacedName) + if err := r.Client.Get(ctx, req.NamespacedName, subnetsetCR); err != nil { + if err := r.deleteSubnetBySubnetSetName(req.Name); err != nil { + return ResultRequeue, err + } + log.Error(err, "unable to fetch SubnetSet CR", "req", req.NamespacedName) return ResultNormal, client.IgnoreNotFound(err) } + if err := r.cleanStaleSubnetsForSubnetSet(subnetsetCR.Name, string(subnetsetCR.UID)); err != nil { + return ResultRequeue, err + } + if !subnetsetCR.ObjectMeta.DeletionTimestamp.IsZero() { + metrics.CounterInc(r.SubnetService.NSXConfig, metrics.ControllerDeleteTotal, MetricResTypeSubnetSet) + err := r.deleteSubnetForSubnetSet(*subnetsetCR, false) + if err != nil { + log.Error(err, "deletion NSX Subnet failed, would retry exponentially", "SubnetSet", req.NamespacedName) + deleteFail(r, ctx, subnetsetCR, err.Error()) + return ResultRequeue, err + } + if err := r.Client.Delete(ctx, subnetsetCR); err != nil { + log.Error(err, "deletion Subnet CR failed, would retry exponentially", "SubnetSet", req.NamespacedName) + deleteFail(r, ctx, subnetsetCR, err.Error()) + return ResultRequeue, err + } + deleteSuccess(r, ctx, subnetsetCR) + } - if obj.ObjectMeta.DeletionTimestamp.IsZero() { - metrics.CounterInc(r.SubnetService.NSXConfig, metrics.ControllerUpdateTotal, MetricResTypeSubnetSet) - if !controllerutil.ContainsFinalizer(obj, servicecommon.SubnetSetFinalizerName) { - controllerutil.AddFinalizer(obj, servicecommon.SubnetSetFinalizerName) - - if obj.Spec.AccessMode == "" { - obj.Spec.AccessMode = v1alpha1.AccessMode(v1alpha1.AccessModePrivate) - } - if obj.Spec.IPv4SubnetSize == 0 { - vpcNetworkConfig := r.VPCService.GetVPCNetworkConfigByNamespace(obj.Namespace) - if vpcNetworkConfig == nil { - err := fmt.Errorf("failed to find VPCNetworkConfig for namespace %s", obj.Namespace) - log.Error(err, "operate failed, would retry exponentially", "subnetset", req.NamespacedName) - updateFail(r, ctx, obj, err.Error()) - return ResultRequeue, err - } - obj.Spec.IPv4SubnetSize = vpcNetworkConfig.DefaultSubnetSize - } - if !util.IsPowerOfTwo(obj.Spec.IPv4SubnetSize) { - errorMsg := fmt.Sprintf("ipv4SubnetSize has invalid size %d, which needs to be >= 16 and power of 2", obj.Spec.IPv4SubnetSize) - log.Error(nil, errorMsg, "subnetset", req.NamespacedName) - updateFail(r, ctx, obj, errorMsg) - return ResultNormal, nil - } - - if err := r.Client.Update(ctx, obj); err != nil { - log.Error(err, "add finalizer", "subnetset", req.NamespacedName) - updateFail(r, ctx, obj, err.Error()) - return ResultRequeue, err - } - log.V(1).Info("added finalizer on subnetset CR", "subnetset", req.NamespacedName) + metrics.CounterInc(r.SubnetService.NSXConfig, metrics.ControllerUpdateTotal, MetricResTypeSubnetSet) + + specChanged := false + if subnetsetCR.Spec.AccessMode == "" { + subnetsetCR.Spec.AccessMode = v1alpha1.AccessMode(v1alpha1.AccessModePrivate) + specChanged = true + } + if subnetsetCR.Spec.IPv4SubnetSize == 0 { + vpcNetworkConfig := r.VPCService.GetVPCNetworkConfigByNamespace(subnetsetCR.Namespace) + if vpcNetworkConfig == nil { + err := fmt.Errorf("failed to find VPCNetworkConfig for namespace %s", subnetsetCR.Namespace) + log.Error(err, "operate failed, would retry exponentially", "SubnetSet", req.NamespacedName) + updateFail(r, ctx, subnetsetCR, err.Error()) + return ResultRequeue, err } + subnetsetCR.Spec.IPv4SubnetSize = vpcNetworkConfig.DefaultSubnetSize + specChanged = true + } + if !util.IsPowerOfTwo(subnetsetCR.Spec.IPv4SubnetSize) { + errorMsg := fmt.Sprintf("ipv4SubnetSize has invalid size %d, which needs to be >= 16 and power of 2", subnetsetCR.Spec.IPv4SubnetSize) + log.Error(nil, errorMsg, "SubnetSet", req.NamespacedName) + updateFail(r, ctx, subnetsetCR, errorMsg) + return ResultNormal, nil + } - // update subnetset tags if labels of namespace changed - nsxSubnets := r.SubnetService.SubnetStore.GetByIndex(servicecommon.TagScopeSubnetSetCRUID, string(obj.UID)) - if len(nsxSubnets) > 0 { - tags := r.SubnetService.GenerateSubnetNSTags(obj, obj.Namespace) - if tags == nil { - return ResultRequeue, errors.New("failed to generate subnet tags") - } - // tags cannot exceed maximum size 26 - if len(tags) > servicecommon.TagsCountMax { - errorMsg := fmt.Sprintf("tags cannot exceed maximum size 26, tags length: %d", len(tags)) - log.Error(nil, "exceed tags limit, would not retry", "subnet", req.NamespacedName) - updateFail(r, ctx, obj, errorMsg) - return ResultNormal, nil - } - if err := r.SubnetService.UpdateSubnetSetTags(obj.Namespace, nsxSubnets, tags); err != nil { - log.Error(err, "failed to update subnetset tags") - } + if specChanged { + err := r.Client.Update(ctx, subnetsetCR) + if err != nil { + log.Error(err, "update SubnetSet failed", "SubnetSet", req.NamespacedName) + updateFail(r, ctx, subnetsetCR, err.Error()) + return ResultRequeue, err } - updateSuccess(r, ctx, obj) - } else { - if controllerutil.ContainsFinalizer(obj, servicecommon.SubnetSetFinalizerName) { - metrics.CounterInc(r.SubnetService.NSXConfig, metrics.ControllerDeleteTotal, MetricResTypeSubnetSet) - hasStaleSubnetPorts, err := r.DeleteSubnetForSubnetSet(*obj, false) - if err != nil { - log.Error(err, "deletion failed, would retry exponentially", "subnetset", req.NamespacedName) - deleteFail(r, ctx, obj, err.Error()) - return ResultRequeue, err - } - if hasStaleSubnetPorts { - err := fmt.Errorf("stale subnet ports found while deleting subnetset %v", req.NamespacedName) - log.Error(err, "deletion failed, delete all the subnetports first", "subnetset", req.NamespacedName) - updateFail(r, ctx, obj, err.Error()) - return ResultRequeue, err - } - controllerutil.RemoveFinalizer(obj, servicecommon.SubnetSetFinalizerName) - if err := r.Client.Update(ctx, obj); err != nil { - log.Error(err, "deletion failed, would retry exponentially", "subnetset", req.NamespacedName) - deleteFail(r, ctx, obj, err.Error()) - return ResultRequeue, err - } - log.V(1).Info("removed finalizer", "subnetset", req.NamespacedName) - deleteSuccess(r, ctx, obj) - } else { - log.Info("finalizers cannot be recognized", "subnetset", req.NamespacedName) + } + + // update SubnetSet tags if labels of namespace changed + nsxSubnets := r.SubnetService.SubnetStore.GetByIndex(servicecommon.TagScopeSubnetSetCRUID, string(subnetsetCR.UID)) + if len(nsxSubnets) > 0 { + tags := r.SubnetService.GenerateSubnetNSTags(subnetsetCR, subnetsetCR.Name, subnetsetCR.Namespace) + if tags == nil { + return ResultRequeue, errors.New("failed to generate SubnetSet tags") + } + // tags cannot exceed maximum size 26 + if len(tags) > servicecommon.TagsCountMax { + errorMsg := fmt.Sprintf("tags cannot exceed maximum size 26, tags length: %d", len(tags)) + log.Error(nil, "exceed tags limit, would not retry", "subnet", req.NamespacedName) + updateFail(r, ctx, subnetsetCR, errorMsg) + return ResultNormal, nil + } + if err := r.SubnetService.UpdateSubnetSetTags(subnetsetCR.Namespace, nsxSubnets, tags); err != nil { + log.Error(err, "failed to update SubnetSet tags") } } + updateSuccess(r, ctx, subnetsetCR) + return ctrl.Result{}, nil } @@ -163,7 +163,7 @@ func deleteSuccess(r *SubnetSetReconciler, _ context.Context, o *v1alpha1.Subnet metrics.CounterInc(r.SubnetService.NSXConfig, metrics.ControllerDeleteSuccessTotal, MetricResTypeSubnetSet) } -func (r *SubnetSetReconciler) setSubnetSetReadyStatusTrue(ctx context.Context, subnetset *v1alpha1.SubnetSet, transitionTime metav1.Time) { +func (r *SubnetSetReconciler) setSubnetSetReadyStatusTrue(ctx context.Context, subnetSet *v1alpha1.SubnetSet, transitionTime metav1.Time) { newConditions := []v1alpha1.Condition{ { Type: v1alpha1.Ready, @@ -173,10 +173,10 @@ func (r *SubnetSetReconciler) setSubnetSetReadyStatusTrue(ctx context.Context, s LastTransitionTime: transitionTime, }, } - r.updateSubnetSetStatusConditions(ctx, subnetset, newConditions) + r.updateSubnetSetStatusConditions(ctx, subnetSet, newConditions) } -func (r *SubnetSetReconciler) setSubnetSetReadyStatusFalse(ctx context.Context, subnetset *v1alpha1.SubnetSet, transitionTime metav1.Time, m string) { +func (r *SubnetSetReconciler) setSubnetSetReadyStatusFalse(ctx context.Context, subnetSet *v1alpha1.SubnetSet, transitionTime metav1.Time, m string) { newConditions := []v1alpha1.Condition{ { Type: v1alpha1.Ready, @@ -189,27 +189,27 @@ func (r *SubnetSetReconciler) setSubnetSetReadyStatusFalse(ctx context.Context, if m != "" { newConditions[0].Message = m } - r.updateSubnetSetStatusConditions(ctx, subnetset, newConditions) + r.updateSubnetSetStatusConditions(ctx, subnetSet, newConditions) } -func (r *SubnetSetReconciler) updateSubnetSetStatusConditions(ctx context.Context, subnetset *v1alpha1.SubnetSet, newConditions []v1alpha1.Condition) { +func (r *SubnetSetReconciler) updateSubnetSetStatusConditions(ctx context.Context, subnetSet *v1alpha1.SubnetSet, newConditions []v1alpha1.Condition) { conditionsUpdated := false for i := range newConditions { - if r.mergeSubnetSetStatusCondition(ctx, subnetset, &newConditions[i]) { + if r.mergeSubnetSetStatusCondition(ctx, subnetSet, &newConditions[i]) { conditionsUpdated = true } } if conditionsUpdated { - if err := r.Client.Status().Update(ctx, subnetset); err != nil { - log.Error(err, "failed to update status", "Name", subnetset.Name, "Namespace", subnetset.Namespace) + if err := r.Client.Status().Update(ctx, subnetSet); err != nil { + log.Error(err, "failed to update status", "Name", subnetSet.Name, "Namespace", subnetSet.Namespace) } else { - log.Info("updated SubnetSet", "Name", subnetset.Name, "Namespace", subnetset.Namespace, "New Conditions", newConditions) + log.Info("updated SubnetSet", "Name", subnetSet.Name, "Namespace", subnetSet.Namespace, "New Conditions", newConditions) } } } -func (r *SubnetSetReconciler) mergeSubnetSetStatusCondition(ctx context.Context, subnetset *v1alpha1.SubnetSet, newCondition *v1alpha1.Condition) bool { - matchedCondition := getExistingConditionOfType(newCondition.Type, subnetset.Status.Conditions) +func (r *SubnetSetReconciler) mergeSubnetSetStatusCondition(ctx context.Context, subnetSet *v1alpha1.SubnetSet, newCondition *v1alpha1.Condition) bool { + matchedCondition := getExistingConditionOfType(newCondition.Type, subnetSet.Status.Conditions) if reflect.DeepEqual(matchedCondition, newCondition) { log.V(2).Info("conditions already match", "New Condition", newCondition, "Existing Condition", matchedCondition) @@ -221,7 +221,7 @@ func (r *SubnetSetReconciler) mergeSubnetSetStatusCondition(ctx context.Context, matchedCondition.Message = newCondition.Message matchedCondition.Status = newCondition.Status } else { - subnetset.Status.Conditions = append(subnetset.Status.Conditions, *newCondition) + subnetSet.Status.Conditions = append(subnetSet.Status.Conditions, *newCondition) } return true } @@ -252,7 +252,7 @@ func (r *SubnetSetReconciler) setupWithManager(mgr ctrl.Manager) error { // CollectGarbage collect Subnet which there is no port attached on it. // it implements the interface GarbageCollector method. func (r *SubnetSetReconciler) CollectGarbage(ctx context.Context) { - log.Info("subnetset garbage collector started") + log.Info("SubnetSet garbage collector started") subnetSetList := &v1alpha1.SubnetSetList{} err := r.Client.List(ctx, subnetSetList) if err != nil { @@ -269,7 +269,7 @@ func (r *SubnetSetReconciler) CollectGarbage(ctx context.Context) { subnetSetIDs := sets.New[string]() for _, subnetSet := range subnetSetList.Items { - if _, err := r.DeleteSubnetForSubnetSet(subnetSet, true); err != nil { + if err := r.deleteSubnetForSubnetSet(subnetSet, true); err != nil { metrics.CounterInc(r.SubnetService.NSXConfig, metrics.ControllerDeleteFailTotal, MetricResTypeSubnetSet) } else { metrics.CounterInc(r.SubnetService.NSXConfig, metrics.ControllerDeleteSuccessTotal, MetricResTypeSubnetSet) @@ -288,33 +288,62 @@ func (r *SubnetSetReconciler) CollectGarbage(ctx context.Context) { } } -func (r *SubnetSetReconciler) DeleteSubnetForSubnetSet(obj v1alpha1.SubnetSet, updataStatus bool) (bool, error) { - nsxSubnets := r.SubnetService.SubnetStore.GetByIndex(servicecommon.TagScopeSubnetSetCRUID, string(obj.GetUID())) - hitError := false - hasStaleSubnetPorts := false +func (r *SubnetSetReconciler) cleanStaleSubnetsForSubnetSet(subnetSetName, subnetsetID string) error { + nsxStaleSubnets := r.SubnetService.SubnetStore.GetByIndex(servicecommon.TagScopeSubnetSetCRName, subnetSetName) + nsxSubnets := r.SubnetService.SubnetStore.GetByIndex(servicecommon.TagScopeSubnetSetCRUID, subnetsetID) + subnetIDs := sets.New[string]() for _, subnet := range nsxSubnets { - r.SubnetService.LockSubnet(subnet.Path) - portNums := len(r.SubnetPortService.GetPortsOfSubnet(*subnet.Id)) - if portNums > 0 { - hasStaleSubnetPorts = true - r.SubnetService.UnlockSubnet(subnet.Path) - continue - } - if err := r.SubnetService.DeleteSubnet(*subnet); err != nil { - log.Error(err, "fail to delete subnet from subnetset cr", "ID", *subnet.Id) - hitError = true + subnetIDs.Insert(*subnet.Id) + } + subnetToDelete := []*model.VpcSubnet{} + for _, staleSubnet := range nsxStaleSubnets { + if !subnetIDs.Has(*staleSubnet.Id) { + subnetToDelete = append(subnetToDelete, staleSubnet) } - r.SubnetService.UnlockSubnet(subnet.Path) } - if updataStatus { - if err := r.SubnetService.UpdateSubnetSetStatus(&obj); err != nil { - return hasStaleSubnetPorts, err + if err := r.deleteSubnets(subnetToDelete); err != nil { + return err + } + return nil +} + +func (r *SubnetSetReconciler) deleteSubnetBySubnetSetName(subnetSetName string) error { + nsxSubnets := r.SubnetService.SubnetStore.GetByIndex(servicecommon.TagScopeSubnetSetCRName, subnetSetName) + if err := r.deleteSubnets(nsxSubnets); err != nil { + return err + } + return nil +} + +func (r *SubnetSetReconciler) deleteSubnetForSubnetSet(obj v1alpha1.SubnetSet, updateStatus bool) error { + nsxSubnets := r.SubnetService.SubnetStore.GetByIndex(servicecommon.TagScopeSubnetSetCRUID, string(obj.GetUID())) + if err := r.deleteSubnets(nsxSubnets); err != nil { + return err + } + if updateStatus { + err := r.SubnetService.UpdateSubnetSetStatus(&obj) + if err != nil { + return err } } - if hitError { - return hasStaleSubnetPorts, errors.New("error occurs when deleting subnet") + return nil +} + +func (r *SubnetSetReconciler) deleteSubnets(nsxSubnets []*model.VpcSubnet) error { + for _, nsxSubnet := range nsxSubnets { + portNums := len(r.SubnetPortService.GetPortsOfSubnet(*nsxSubnet.Id)) + if portNums > 0 { + return fmt.Errorf("fail to delete subnet/%s from SubnetSet cr, there is stale ports", *nsxSubnet.Id) + } + r.SubnetService.LockSubnet(nsxSubnet.Path) + err := r.SubnetService.DeleteSubnet(*nsxSubnet) + if err != nil { + r.SubnetService.UnlockSubnet(nsxSubnet.Path) + return fmt.Errorf("fail to delete subnet/%s from SubnetSet cr: %+v", *nsxSubnet.Id, err) + } + r.SubnetService.UnlockSubnet(nsxSubnet.Path) } - return hasStaleSubnetPorts, nil + return nil } func StartSubnetSetController(mgr ctrl.Manager, subnetService *subnet.SubnetService, @@ -330,7 +359,7 @@ func StartSubnetSetController(mgr ctrl.Manager, subnetService *subnet.SubnetServ Recorder: mgr.GetEventRecorderFor("subnetset-controller"), } if err := subnetsetReconciler.Start(mgr, enableWebhook); err != nil { - log.Error(err, "failed to create controller", "controller", "Subnet") + log.Error(err, "failed to create controller", "controller", "SubnetSet") return err } go common.GenericGarbageCollector(make(chan bool), servicecommon.GCInterval, subnetsetReconciler.CollectGarbage) diff --git a/pkg/nsx/services/common/services.go b/pkg/nsx/services/common/services.go index b42eb3094..447ae6398 100644 --- a/pkg/nsx/services/common/services.go +++ b/pkg/nsx/services/common/services.go @@ -29,7 +29,7 @@ type SubnetServiceProvider interface { GetSubnetByPath(path string) (*model.VpcSubnet, error) GetSubnetsByIndex(key, value string) []*model.VpcSubnet CreateOrUpdateSubnet(obj client.Object, vpcInfo VPCResourceInfo, tags []model.Tag) (string, error) - GenerateSubnetNSTags(obj client.Object, nsUID string) []model.Tag + GenerateSubnetNSTags(obj client.Object, name, nsUID string) []model.Tag LockSubnet(path *string) UnlockSubnet(path *string) } diff --git a/pkg/nsx/services/common/types.go b/pkg/nsx/services/common/types.go index 31ec87ff3..6b5021f19 100644 --- a/pkg/nsx/services/common/types.go +++ b/pkg/nsx/services/common/types.go @@ -72,6 +72,7 @@ const ( SystemVPCNetworkConfigurationName string = "system" TagScopeSubnetCRUID string = "nsx-op/subnet_uid" TagScopeSubnetCRName string = "nsx-op/subnet_name" + TagScopeSubnetCRNamespacedName string = "nsx-op/subnet_namespaced_name" TagScopeSubnetSetCRName string = "nsx-op/subnetset_name" TagScopeSubnetSetCRUID string = "nsx-op/subnetset_uid" TagValueGroupScope string = "scope" diff --git a/pkg/nsx/services/subnet/store.go b/pkg/nsx/services/subnet/store.go index bcd12a4cb..76ab49a53 100644 --- a/pkg/nsx/services/subnet/store.go +++ b/pkg/nsx/services/subnet/store.go @@ -49,6 +49,15 @@ func subnetSetIndexFunc(obj interface{}) ([]string, error) { } } +func subnetNamespacedNameIndexFunc(obj interface{}) ([]string, error) { + switch o := obj.(type) { + case *model.VpcSubnet: + return filterTag(o.Tags, common.TagScopeSubnetCRNamespacedName), nil + default: + return nil, errors.New("subnetSetIndexFunc doesn't support unknown type") + } +} + // SubnetStore is a store for subnet. type SubnetStore struct { common.ResourceStore diff --git a/pkg/nsx/services/subnet/store_test.go b/pkg/nsx/services/subnet/store_test.go index 4db3c39c1..6e72323cd 100644 --- a/pkg/nsx/services/subnet/store_test.go +++ b/pkg/nsx/services/subnet/store_test.go @@ -43,13 +43,14 @@ func Test_IndexFunc(t *testing.T) { if !reflect.DeepEqual(got, []string{"cr_uid"}) { t.Errorf("subnetCRUIDScopeIndexFunc() = %v, want %v", got, model.Tag{Tag: &tag, Scope: &scope}) } + // }) } func Test_KeyFunc(t *testing.T) { id := "test_id" subnet := model.VpcSubnet{Id: &id} - t.Run("1", func(t *testing.T) { + t.Run("subnetKeyFunc", func(t *testing.T) { got, _ := keyFunc(&subnet) if got != "test_id" { t.Errorf("keyFunc() = %v, want %v", got, "test_id") @@ -62,7 +63,10 @@ func Test_InitializeSubnetStore(t *testing.T) { cluster, _ := nsx.NewCluster(config2) rc, _ := cluster.NewRestConnector() - subnetCacheIndexer := cache.NewIndexer(keyFunc, cache.Indexers{common.TagScopeSubnetCRUID: subnetIndexFunc}) + subnetCacheIndexer := cache.NewIndexer(keyFunc, cache.Indexers{ + common.TagScopeSubnetCRUID: subnetIndexFunc, + common.TagScopeSubnetCRNamespacedName: subnetNamespacedNameIndexFunc, + }) service := SubnetService{ Service: common.Service{ NSXClient: &nsx.Client{ @@ -107,7 +111,10 @@ func Test_InitializeSubnetStore(t *testing.T) { } func TestSubnetStore_Apply(t *testing.T) { - subnetCacheIndexer := cache.NewIndexer(keyFunc, cache.Indexers{common.TagScopeSubnetCRUID: subnetIndexFunc}) + subnetCacheIndexer := cache.NewIndexer(keyFunc, cache.Indexers{ + common.TagScopeSubnetCRUID: subnetIndexFunc, + common.TagScopeSubnetCRNamespacedName: subnetNamespacedNameIndexFunc, + }) resourceStore := common.ResourceStore{ Indexer: subnetCacheIndexer, BindingType: model.SecurityPolicyBindingType(), diff --git a/pkg/nsx/services/subnet/subnet.go b/pkg/nsx/services/subnet/subnet.go index 29806d0af..f31740e78 100644 --- a/pkg/nsx/services/subnet/subnet.go +++ b/pkg/nsx/services/subnet/subnet.go @@ -56,8 +56,9 @@ func InitializeSubnetService(service common.Service) (*SubnetService, error) { SubnetStore: &SubnetStore{ ResourceStore: common.ResourceStore{ Indexer: cache.NewIndexer(keyFunc, cache.Indexers{ - common.TagScopeSubnetCRUID: subnetIndexFunc, - common.TagScopeSubnetSetCRUID: subnetSetIndexFunc, + common.TagScopeSubnetCRUID: subnetIndexFunc, + common.TagScopeSubnetSetCRUID: subnetSetIndexFunc, + common.TagScopeSubnetCRNamespacedName: subnetNamespacedNameIndexFunc, }), BindingType: model.VpcSubnetBindingType(), }, @@ -339,7 +340,11 @@ func (service *SubnetService) GetSubnetsByIndex(key, value string) []*model.VpcS return service.SubnetStore.GetByIndex(key, value) } -func (service *SubnetService) GenerateSubnetNSTags(obj client.Object, ns string) []model.Tag { +func SubnetNamespacedName(name, namespace string) string { + return fmt.Sprintf("%s/%s", namespace, name) +} + +func (service *SubnetService) GenerateSubnetNSTags(obj client.Object, name, ns string) []model.Tag { namespace := &v1.Namespace{} namespacedName := types.NamespacedName{ Name: ns, @@ -353,6 +358,7 @@ func (service *SubnetService) GenerateSubnetNSTags(obj client.Object, ns string) case *v1alpha1.Subnet: tags = append(tags, model.Tag{Scope: String(common.TagScopeVMNamespaceUID), Tag: String(nsUID)}, + model.Tag{Scope: String(common.TagScopeSubnetCRNamespacedName), Tag: String(SubnetNamespacedName(name, ns))}, model.Tag{Scope: String(common.TagScopeVMNamespace), Tag: String(obj.GetNamespace())}) case *v1alpha1.SubnetSet: findLabelDefaultPodSubnetSet := false