Skip to content

Commit

Permalink
remove_np_sp_finalizer
Browse files Browse the repository at this point in the history
  • Loading branch information
@timdengyun
timdengyun committed Oct 6, 2024
1 parent 5c97290 commit 176c7bd
Show file tree
Hide file tree
Showing 10 changed files with 378 additions and 212 deletions.
100 changes: 61 additions & 39 deletions pkg/controllers/networkpolicy/networkpolicy_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ import (
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/controller"
"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"

"github.com/vmware-tanzu/nsx-operator/pkg/controllers/common"
"github.com/vmware-tanzu/nsx-operator/pkg/logger"
Expand Down Expand Up @@ -100,21 +99,21 @@ func (r *NetworkPolicyReconciler) Reconcile(ctx context.Context, req ctrl.Reques
metrics.CounterInc(r.Service.NSXConfig, metrics.ControllerSyncTotal, MetricResType)

if err := r.Client.Get(ctx, req.NamespacedName, networkPolicy); err != nil {
log.Error(err, "unable to fetch network policy", "req", req.NamespacedName)
return ResultNormal, client.IgnoreNotFound(err)
// IgnoreNotFound returns nil on NotFound errors.
if client.IgnoreNotFound(err) == nil {
if err := r.deleteNetworkPolicyByName(req.Name, req.Namespace); err != nil {
log.Error(err, "failed to delete NetworkPolicy", "networkpolicy", req.NamespacedName)
return ResultRequeue, err
}
return ResultNormal, nil
}
// In case that client is unable to check CR
log.Error(err, "client is unable to fetch NetworkPolicy CR", "req", req.NamespacedName)
return ResultRequeue, err
}

if networkPolicy.ObjectMeta.DeletionTimestamp.IsZero() {
metrics.CounterInc(r.Service.NSXConfig, metrics.ControllerUpdateTotal, MetricResType)
if !controllerutil.ContainsFinalizer(networkPolicy, servicecommon.NetworkPolicyFinalizerName) {
controllerutil.AddFinalizer(networkPolicy, servicecommon.NetworkPolicyFinalizerName)
if err := r.Client.Update(ctx, networkPolicy); err != nil {
log.Error(err, "add finalizer", "networkpolicy", req.NamespacedName)
updateFail(r, ctx, networkPolicy, &err)
return ResultRequeue, err
}
log.V(1).Info("added finalizer on networkpolicy", "networkpolicy", req.NamespacedName)
}

if err := r.Service.CreateOrUpdateSecurityPolicy(networkPolicy); err != nil {
if errors.As(err, &nsxutil.RestrictionError{}) {
Expand All @@ -135,25 +134,14 @@ func (r *NetworkPolicyReconciler) Reconcile(ctx context.Context, req ctrl.Reques
updateSuccess(r, ctx, networkPolicy)
cleanNetworkPolicyErrorAnnotation(ctx, networkPolicy, r.Client)
} else {
if controllerutil.ContainsFinalizer(networkPolicy, servicecommon.NetworkPolicyFinalizerName) {
metrics.CounterInc(r.Service.NSXConfig, metrics.ControllerDeleteTotal, MetricResType)
if err := r.Service.DeleteSecurityPolicy(networkPolicy, false, false, servicecommon.ResourceTypeNetworkPolicy); err != nil {
log.Error(err, "deletion failed, would retry exponentially", "networkpolicy", req.NamespacedName)
deleteFail(r, ctx, networkPolicy, &err)
return ResultRequeue, err
}
controllerutil.RemoveFinalizer(networkPolicy, servicecommon.NetworkPolicyFinalizerName)
if err := r.Client.Update(ctx, networkPolicy); err != nil {
log.Error(err, "deletion failed, would retry exponentially", "networkpolicy", req.NamespacedName)
deleteFail(r, ctx, networkPolicy, &err)
return ResultRequeue, err
}
log.V(1).Info("removed finalizer", "networkpolicy", req.NamespacedName)
deleteSuccess(r, ctx, networkPolicy)
} else {
// only print a message because it's not a normal case
log.Info("finalizers cannot be recognized", "networkpolicy", req.NamespacedName)
metrics.CounterInc(r.Service.NSXConfig, metrics.ControllerDeleteTotal, MetricResType)

if err := r.Service.DeleteSecurityPolicy(networkPolicy, false, false, servicecommon.ResourceTypeNetworkPolicy); err != nil {
log.Error(err, "deletion failed, would retry exponentially", "networkpolicy", req.NamespacedName)
deleteFail(r, ctx, networkPolicy, &err)
return ResultRequeue, err
}
deleteSuccess(r, ctx, networkPolicy)
}

return ResultNormal, nil
Expand Down Expand Up @@ -186,19 +174,12 @@ func (r *NetworkPolicyReconciler) CollectGarbage(ctx context.Context) {
if len(nsxPolicySet) == 0 {
return
}
policyList := &networkingv1.NetworkPolicyList{}
err := r.Client.List(ctx, policyList)

CRPolicySet, err := r.listNetworkPolciyCRIDs()
if err != nil {
log.Error(err, "failed to list NetworkPolicy")
return
}

CRPolicySet := sets.New[string]()
for _, policy := range policyList.Items {
CRPolicySet.Insert(r.Service.BuildNetworkPolicyAllowPolicyID(string(policy.UID)))
CRPolicySet.Insert(r.Service.BuildNetworkPolicyIsolationPolicyID(string(policy.UID)))
}

diffSet := nsxPolicySet.Difference(CRPolicySet)
for elem := range diffSet {
log.V(1).Info("GC collected NetworkPolicy", "ID", elem)
Expand All @@ -212,6 +193,47 @@ func (r *NetworkPolicyReconciler) CollectGarbage(ctx context.Context) {
}
}

func (r *NetworkPolicyReconciler) deleteNetworkPolicyByName(name, ns string) error {
nsxSecurityPolicies := r.Service.ListNetworkPolicyByNamespacedName(name, ns)

CRPolicySet, err := r.listNetworkPolciyCRIDs()
if err != nil {
return err
}

for _, item := range nsxSecurityPolicies {
uid := nsxutil.FindTag(item.Tags, servicecommon.TagScopeNetworkPolicyUID)
if CRPolicySet.Has(uid) {
log.Info("skipping deletion, NetworkPolicy CR still exists in K8s", "networkPolicyUID", uid, "nsxSecurityPolicyId", *item.Id)
continue
}

log.Info("deleting NetworkPolicy", "networkPolicyUID", uid, "nsxSecurityPolicyId", *item.Id)
if err := r.Service.DeleteSecurityPolicy(types.UID(uid), false, false, servicecommon.ResourceTypeNetworkPolicy); err != nil {
log.Error(err, "failed to delete NetworkPolicy", "networkPolicyUID", uid, "nsxSecurityPolicyId", *item.Id)
return err
}
log.Info("successfully deleted NetworkPolicy", "networkPolicyUID", uid, "nsxSecurityPolicyId", *item.Id)
}
return nil
}

func (r *NetworkPolicyReconciler) listNetworkPolciyCRIDs() (sets.Set[string], error) {
networkPolicyList := &networkingv1.NetworkPolicyList{}
err := r.Client.List(context.Background(), networkPolicyList)
if err != nil {
log.Error(err, "failed to list NetworkPolicy CRs")
return nil, err
}

CRPolicySet := sets.New[string]()
for _, policy := range networkPolicyList.Items {
CRPolicySet.Insert(r.Service.BuildNetworkPolicyAllowPolicyID(string(policy.UID)))
CRPolicySet.Insert(r.Service.BuildNetworkPolicyIsolationPolicyID(string(policy.UID)))
}
return CRPolicySet, nil
}

func StartNetworkPolicyController(mgr ctrl.Manager, commonService servicecommon.Service, vpcService servicecommon.VPCServiceProvider) {
networkPolicyReconcile := NetworkPolicyReconciler{
Client: mgr.GetClient(),
Expand Down
106 changes: 69 additions & 37 deletions pkg/controllers/securitypolicy/securitypolicy_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -137,12 +137,21 @@ func (r *SecurityPolicyReconciler) Reconcile(ctx context.Context, req ctrl.Reque
obj = &v1alpha1.SecurityPolicy{}
}

log.Info("reconciling securitypolicy CR", "securitypolicy", req.NamespacedName)
log.Info("reconciling SecurityPolicy CR", "securitypolicy", req.NamespacedName)
metrics.CounterInc(r.Service.NSXConfig, metrics.ControllerSyncTotal, MetricResType)

if err := r.Client.Get(ctx, req.NamespacedName, obj); err != nil {
log.Error(err, "unable to fetch security policy CR", "req", req.NamespacedName)
return ResultNormal, client.IgnoreNotFound(err)
// IgnoreNotFound returns nil on NotFound errors.
if client.IgnoreNotFound(err) == nil {
if err := r.deleteSecurityPolicyByName(req.Name, req.Namespace); err != nil {
log.Error(err, "failed to delete SecurityPolicy", "securitypolicy", req.NamespacedName)
return ResultRequeue, err
}
return ResultNormal, nil
}
// In case that client is unable to check CR
log.Error(err, "client is unable to fetch SecurityPolicy CR", "req", req.NamespacedName)
return ResultRequeue, err
}

isZero := false
Expand All @@ -152,7 +161,6 @@ func (r *SecurityPolicyReconciler) Reconcile(ctx context.Context, req ctrl.Reque
case *crdv1alpha1.SecurityPolicy:
o := obj.(*crdv1alpha1.SecurityPolicy)
isZero = o.ObjectMeta.DeletionTimestamp.IsZero()
finalizerName = servicecommon.SecurityPolicyFinalizerName
realObj = securitypolicy.VPCToT1(o)
case *v1alpha1.SecurityPolicy:
realObj = obj.(*v1alpha1.SecurityPolicy)
Expand All @@ -170,15 +178,6 @@ func (r *SecurityPolicyReconciler) Reconcile(ctx context.Context, req ctrl.Reque

if isZero {
metrics.CounterInc(r.Service.NSXConfig, metrics.ControllerUpdateTotal, MetricResType)
if !controllerutil.ContainsFinalizer(obj, finalizerName) {
controllerutil.AddFinalizer(obj, finalizerName)
if err := r.Client.Update(ctx, obj); err != nil {
log.Error(err, "add finalizer", "securitypolicy", req.NamespacedName)
updateFail(r, ctx, realObj, &err)
return ResultRequeue, err
}
log.V(1).Info("added finalizer on securitypolicy CR", "securitypolicy", req.NamespacedName)
}

if isCRInSysNs, err := util.IsSystemNamespace(r.Client, req.Namespace, nil); err != nil {
err = errors.New("fetch namespace associated with security policy CR failed")
Expand Down Expand Up @@ -213,25 +212,24 @@ func (r *SecurityPolicyReconciler) Reconcile(ctx context.Context, req ctrl.Reque
cleanSecurityPolicyErrorAnnotation(ctx, realObj, securitypolicy.IsVPCEnabled(r.Service), r.Client)
} else {
log.Info("reconciling CR to delete securitypolicy", "securitypolicy", req.NamespacedName)
metrics.CounterInc(r.Service.NSXConfig, metrics.ControllerDeleteTotal, MetricResType)

// For T1 upgrade, the upgraded CRs still has finalizer
if controllerutil.ContainsFinalizer(obj, finalizerName) {
metrics.CounterInc(r.Service.NSXConfig, metrics.ControllerDeleteTotal, MetricResType)
if err := r.Service.DeleteSecurityPolicy(realObj.UID, false, false, servicecommon.ResourceTypeSecurityPolicy); err != nil {
log.Error(err, "deletion failed, would retry exponentially", "securitypolicy", req.NamespacedName)
deleteFail(r, ctx, realObj, &err)
return ResultRequeue, err
}
controllerutil.RemoveFinalizer(obj, finalizerName)
if err := r.Client.Update(ctx, obj); err != nil {
log.Error(err, "deletion failed, would retry exponentially", "securitypolicy", req.NamespacedName)
log.Error(err, "finalizer remove failed, would retry exponentially", "securitypolicy", req.NamespacedName)
deleteFail(r, ctx, realObj, &err)
return ResultRequeue, err
}
log.V(1).Info("removed finalizer", "securitypolicy", req.NamespacedName)
deleteSuccess(r, ctx, realObj)
} else {
// only print a message because it's not a normal case
log.Info("finalizers cannot be recognized", "securitypolicy", req.NamespacedName)
}
if err := r.Service.DeleteSecurityPolicy(realObj.UID, false, false, servicecommon.ResourceTypeSecurityPolicy); err != nil {
log.Error(err, "deletion failed, would retry exponentially", "securitypolicy", req.NamespacedName)
deleteFail(r, ctx, realObj, &err)
return ResultRequeue, err
}
deleteSuccess(r, ctx, realObj)
}

return ResultNormal, nil
Expand Down Expand Up @@ -361,16 +359,60 @@ func (r *SecurityPolicyReconciler) CollectGarbage(ctx context.Context) {
return
}

CRPolicySet, err := r.listSecurityPolciyCRIDs()
if err != nil {
return
}

diffSet := nsxPolicySet.Difference(CRPolicySet)
for elem := range diffSet {
log.V(1).Info("GC collected SecurityPolicy CR", "securityPolicyUID", elem)
metrics.CounterInc(r.Service.NSXConfig, metrics.ControllerDeleteTotal, MetricResType)
err = r.Service.DeleteSecurityPolicy(types.UID(elem), true, false, servicecommon.ResourceTypeSecurityPolicy)
if err != nil {
metrics.CounterInc(r.Service.NSXConfig, metrics.ControllerDeleteFailTotal, MetricResType)
} else {
metrics.CounterInc(r.Service.NSXConfig, metrics.ControllerDeleteSuccessTotal, MetricResType)
}
}
}

func (r *SecurityPolicyReconciler) deleteSecurityPolicyByName(name, ns string) error {
nsxSecurityPolicies := r.Service.ListSecurityPolicyByNamespacedName(name, ns)

CRPolicySet, err := r.listSecurityPolciyCRIDs()
if err != nil {
return err
}

for _, item := range nsxSecurityPolicies {
uid := nsxutil.FindTag(item.Tags, servicecommon.TagValueScopeSecurityPolicyUID)
if CRPolicySet.Has(uid) {
log.Info("skipping deletion, SecurityPolicy CR still exists in K8s", "securityPolicyUID", uid, "nsxSecurityPolicyId", *item.Id)
continue
}

log.Info("deleting SecurityPolicy", "securityPolicyUID", uid, "nsxSecurityPolicyId", *item.Id)
if err := r.Service.DeleteSecurityPolicy(types.UID(uid), false, false, servicecommon.ResourceTypeSecurityPolicy); err != nil {
log.Error(err, "failed to delete SecurityPolicy", "securityPolicyUID", uid, "nsxSecurityPolicyId", *item.Id)
return err
}
log.Info("successfully deleted SecurityPolicy", "securityPolicyUID", uid, "nsxSecurityPolicyId", *item.Id)
}
return nil
}

func (r *SecurityPolicyReconciler) listSecurityPolciyCRIDs() (sets.Set[string], error) {
var objectList client.ObjectList
if securitypolicy.IsVPCEnabled(r.Service) {
objectList = &crdv1alpha1.SecurityPolicyList{}
} else {
objectList = &v1alpha1.SecurityPolicyList{}
}
err := r.Client.List(ctx, objectList)
err := r.Client.List(context.Background(), objectList)
if err != nil {
log.Error(err, "failed to list SecurityPolicy CR")
return
return nil, err
}

CRPolicySet := sets.New[string]()
Expand All @@ -387,17 +429,7 @@ func (r *SecurityPolicyReconciler) CollectGarbage(ctx context.Context) {
}
}

diffSet := nsxPolicySet.Difference(CRPolicySet)
for elem := range diffSet {
log.V(1).Info("GC collected SecurityPolicy CR", "securityPolicyUID", elem)
metrics.CounterInc(r.Service.NSXConfig, metrics.ControllerDeleteTotal, MetricResType)
err = r.Service.DeleteSecurityPolicy(types.UID(elem), true, false, servicecommon.ResourceTypeSecurityPolicy)
if err != nil {
metrics.CounterInc(r.Service.NSXConfig, metrics.ControllerDeleteFailTotal, MetricResType)
} else {
metrics.CounterInc(r.Service.NSXConfig, metrics.ControllerDeleteSuccessTotal, MetricResType)
}
}
return CRPolicySet, nil
}

// It is triggered by associated controller like pod, namespace, etc.
Expand Down
Loading

0 comments on commit 176c7bd

Please sign in to comment.