Fix: delay clear pd finalizer and delete invalid printcolumn (#238)

* delay clear pd finalizer * delete pd printcolumn 'EFFECTIVE'
KusionStack · Aug 5, 2024 · d0e6766 · d0e6766
1 parent 36969bf
commit d0e6766
Show file tree

Hide file tree

Showing 5 changed files with 30 additions and 10 deletions.
diff --git a/apis/apps/v1alpha1/poddecoration_types.go b/apis/apps/v1alpha1/poddecoration_types.go
@@ -265,7 +265,6 @@ type PodDecorationCondition struct {
 // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object
 // +kubebuilder:resource:shortName=pd
 // +kubebuilder:subresource:status
-// +kubebuilder:printcolumn:name="EFFECTIVE",type="boolean",JSONPath=".status.isEffective",description="The number of pods updated."
 // +kubebuilder:printcolumn:name="MATCHED",type="integer",JSONPath=".status.matchedPods",description="The number of selected pods."
 // +kubebuilder:printcolumn:name="INJECTED",type="integer",JSONPath=".status.injectedPods",description="The number of injected pods."
 // +kubebuilder:printcolumn:name="UPDATED",type="integer",JSONPath=".status.updatedPods",description="The number of updated pods."

diff --git a/charts/templates/crd/apps.kusionstack.io_poddecorations.yaml b/charts/templates/crd/apps.kusionstack.io_poddecorations.yaml
@@ -18,10 +18,6 @@ spec:
   scope: Namespaced
   versions:
   - additionalPrinterColumns:
-    - description: The number of pods updated.
-      jsonPath: .status.isEffective
-      name: EFFECTIVE
-      type: boolean
     - description: The number of selected pods.
       jsonPath: .status.matchedPods
       name: MATCHED

diff --git a/config/crd/bases/apps.kusionstack.io_poddecorations.yaml b/config/crd/bases/apps.kusionstack.io_poddecorations.yaml
@@ -18,10 +18,6 @@ spec:
   scope: Namespaced
   versions:
   - additionalPrinterColumns:
-    - description: The number of pods updated.
-      jsonPath: .status.isEffective
-      name: EFFECTIVE
-      type: boolean
     - description: The number of selected pods.
       jsonPath: .status.matchedPods
       name: MATCHED

diff --git a/pkg/controllers/poddecoration/poddecoration_controller.go b/pkg/controllers/poddecoration/poddecoration_controller.go
@@ -20,6 +20,7 @@ import (
 	"context"
 	"fmt"
 	"sort"
+	"time"
 
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/equality"
@@ -45,6 +46,11 @@ import (
 	"kusionstack.io/operating/pkg/controllers/utils/poddecoration/strategy"
 	"kusionstack.io/operating/pkg/controllers/utils/revision"
 	"kusionstack.io/operating/pkg/utils"
+	"kusionstack.io/operating/pkg/utils/mixin"
+)
+
+const (
+	controllerName = "poddecoration-controller"
 )
 
 // Add creates a new PodDecoration Controller and adds it to the Manager with default RBAC.
@@ -56,6 +62,7 @@ func Add(mgr manager.Manager) error {
 // newReconciler returns a new reconcile.Reconciler
 func newReconciler(mgr manager.Manager) reconcile.Reconciler {
 	return &ReconcilePodDecoration{
+		ReconcilerMixin: mixin.NewReconcilerMixin(controllerName, mgr),
 		Client:          mgr.GetClient(),
 		revisionManager: revision.NewRevisionManager(mgr.GetClient(), mgr.GetScheme(), &revisionOwnerAdapter{}),
 	}
@@ -102,6 +109,7 @@ var (
 // ReconcilePodDecoration reconciles a PodDecoration object
 type ReconcilePodDecoration struct {
 	client.Client
+	*mixin.ReconcilerMixin
 	revisionManager *revision.RevisionManager
 }
 
@@ -133,6 +141,9 @@ func (r *ReconcilePodDecoration) Reconcile(ctx context.Context, request reconcil
 	if instance.DeletionTimestamp != nil {
 		strategy.SharedStrategyController.DeletePodDecoration(instance)
 		statusUpToDateExpectation.DeleteExpectations(key)
+		if !r.shouldEscape(ctx, instance) {
+			return reconcile.Result{RequeueAfter: 5 * time.Second}, nil
+		}
 		return reconcile.Result{}, r.clearProtection(ctx, instance)
 	}
 	if err := r.protectPD(ctx, instance); err != nil {
@@ -253,6 +264,18 @@ func (r *ReconcilePodDecoration) allCollaSetsSatisfyReplicas(collaSets sets.Stri
 	return true
 }
 
+func (r *ReconcilePodDecoration) shouldEscape(ctx context.Context, instance *appsv1alpha1.PodDecoration) bool {
+	podList := &corev1.PodList{}
+	if err := r.List(ctx, podList,
+		client.InNamespace(instance.Namespace),
+		client.HasLabels{appsv1alpha1.PodDecorationLabelPrefix + instance.Name},
+	); err != nil {
+		klog.Errorf("failed to list pods: %v", err)
+		return false
+	}
+	return len(podList.Items) == 0
+}
+
 func (r *ReconcilePodDecoration) updateStatus(
 	ctx context.Context,
 	instance *appsv1alpha1.PodDecoration,

diff --git a/pkg/controllers/utils/poddecoration/getter.go b/pkg/controllers/utils/poddecoration/getter.go
@@ -22,6 +22,7 @@ import (
 
 	appsv1 "k8s.io/api/apps/v1"
 	corev1 "k8s.io/api/core/v1"
+	"k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/apimachinery/pkg/util/sets"
 	"sigs.k8s.io/controller-runtime/pkg/client"
@@ -97,7 +98,9 @@ func (n *namespacedPodDecorationManager) GetByRevisions(ctx context.Context, rev
 			err = utils.Join(err, localErr)
 			continue
 		}
-		res[rev] = pd
+		if pd != nil {
+			res[rev] = pd
+		}
 	}
 	return res, err
 }
@@ -147,6 +150,9 @@ func (n *namespacedPodDecorationManager) getByRevision(ctx context.Context, rev
 	}
 	revision := &appsv1.ControllerRevision{}
 	if err := n.c.Get(ctx, types.NamespacedName{Namespace: n.namespace, Name: rev}, revision); err != nil {
+		if errors.IsNotFound(err) {
+			return nil, nil
+		}
 		return nil, fmt.Errorf("fail to get PodDecoration ControllerRevision %s/%s: %v", n.namespace, rev, err)
 	}
 	pd, err := anno.GetPodDecorationFromRevision(revision)