From 9f938344d840e74c0606d99620a69213b6957ab9 Mon Sep 17 00:00:00 2001 From: shaofan-hs <133250733+shaofan-hs@users.noreply.github.com> Date: Thu, 28 Mar 2024 19:45:13 +0800 Subject: [PATCH] refactor: optimize the events about pods (#173) update event reasons of podopslifecycle --- apis/apps/v1alpha1/well_known_vars.go | 6 ++ .../poddeletion/lifecycle_adapter.go | 1 - .../poddeletion/poddeletion_controller.go | 7 ++- .../podopslifecycle_controller.go | 19 +++--- .../podopslifecycle_controller_test.go | 61 ++++++++++++++++++- 5 files changed, 83 insertions(+), 11 deletions(-) diff --git a/apis/apps/v1alpha1/well_known_vars.go b/apis/apps/v1alpha1/well_known_vars.go index b1084c3d..2fbc18db 100644 --- a/apis/apps/v1alpha1/well_known_vars.go +++ b/apis/apps/v1alpha1/well_known_vars.go @@ -27,6 +27,12 @@ const ( ProtectFinalizer = "finalizer.operating.kusionstack.io/protected" ) +// well known event reasons +const ( + PodDeletionEvent = "PodDeletion" + ServiceReadyEvent = "ServiceReady" +) + // well known variables const ( PodOpsLifecyclePreCheckStage = "PreCheck" diff --git a/pkg/controllers/poddeletion/lifecycle_adapter.go b/pkg/controllers/poddeletion/lifecycle_adapter.go index 95a41f56..6428a66d 100644 --- a/pkg/controllers/poddeletion/lifecycle_adapter.go +++ b/pkg/controllers/poddeletion/lifecycle_adapter.go @@ -52,6 +52,5 @@ func (a *PodDeleteOpsLifecycleAdapter) WhenBegin(obj client.Object) (bool, error // WhenFinish will be executed when finish a lifecycle func (a *PodDeleteOpsLifecycleAdapter) WhenFinish(_ client.Object) (bool, error) { - return false, nil } diff --git a/pkg/controllers/poddeletion/poddeletion_controller.go b/pkg/controllers/poddeletion/poddeletion_controller.go index 1c84fbf9..18da9cc2 100644 --- a/pkg/controllers/poddeletion/poddeletion_controller.go +++ b/pkg/controllers/poddeletion/poddeletion_controller.go @@ -29,6 +29,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/reconcile" "sigs.k8s.io/controller-runtime/pkg/source" + "kusionstack.io/operating/apis/apps/v1alpha1" appsv1alpha1 "kusionstack.io/operating/apis/apps/v1alpha1" "kusionstack.io/operating/pkg/controllers/collaset/pvccontrol" "kusionstack.io/operating/pkg/controllers/utils/expectations" @@ -123,14 +124,16 @@ func (r *PodDeletionReconciler) Reconcile(ctx context.Context, req ctrl.Request) } } - // if Pod is allow to operate, delete it + // if Pod is allowed to operate, delete it if _, allowed := podopslifecycle.AllowOps(OpsLifecycleAdapter, 0, instance); allowed { logger.Info("try to delete Pod with deletion indication") if err := r.Client.Delete(context.TODO(), instance); err != nil { return ctrl.Result{}, fmt.Errorf("fail to delete Pod %s with deletion indication: %s", req, err) } else { + r.Recorder.Event(instance, corev1.EventTypeNormal, v1alpha1.PodDeletionEvent, "Deleted pod") + if err := activeExpectations.ExpectDelete(instance, expectations.Pod, instance.Name); err != nil { - return ctrl.Result{}, fmt.Errorf("fail to expect Pod %s deleted: %s", req, err) + return ctrl.Result{}, fmt.Errorf("fail to expect Pod %s to be deleted: %s", req, err) } } // if this pod in replaced update, delete pvcs diff --git a/pkg/controllers/podopslifecycle/podopslifecycle_controller.go b/pkg/controllers/podopslifecycle/podopslifecycle_controller.go index 1031f092..a76464b7 100644 --- a/pkg/controllers/podopslifecycle/podopslifecycle_controller.go +++ b/pkg/controllers/podopslifecycle/podopslifecycle_controller.go @@ -126,19 +126,23 @@ func (r *ReconcilePodOpsLifecycle) Reconcile(ctx context.Context, request reconc return reconcile.Result{}, err } - // If all lifecycle is finished, or the is no lifecycle begined + // All lifecycles are finished, or no lifecycle begined if len(idToLabelsMap) == 0 { updated, err := r.addServiceAvailable(pod) - if updated { - r.Recorder.Eventf(pod, corev1.EventTypeNormal, "ServiceAvailable", "Label pod as service available, error: %v", err) + if err != nil { return reconcile.Result{}, err } + if updated { + return reconcile.Result{}, nil + } updated, err = r.updateServiceReadiness(ctx, pod, true) - if updated { - r.Recorder.Eventf(pod, corev1.EventTypeNormal, "ReadinessGate", "Set service ready readiness gate to true, error: %v", err) + if err != nil { return reconcile.Result{}, err } + if updated { + return reconcile.Result{}, nil + } } // Get the state of pod managed by TransitionRule @@ -181,7 +185,6 @@ func (r *ReconcilePodOpsLifecycle) Reconcile(ctx context.Context, request reconc return reconcile.Result{}, err // Only need set once } if updated { - r.Recorder.Eventf(pod, corev1.EventTypeNormal, "ReadinessGate", "Set service ready readiness gate to %v", v) return reconcile.Result{}, nil } } @@ -325,6 +328,9 @@ func (r *ReconcilePodOpsLifecycle) updateServiceReadiness(ctx context.Context, p return false, err } + + r.Recorder.Eventf(pod, corev1.EventTypeNormal, v1alpha1.ServiceReadyEvent, "Set ReadinessGate service-ready to %v", isReady) + return true, nil } @@ -436,7 +442,6 @@ func (r *ReconcilePodOpsLifecycle) addLabels(ctx context.Context, pod *corev1.Po r.Logger.Error(err, "failed to update pod with labels", "pod", utils.ObjectKeyString(pod), "labels", labels) r.expectation.DeleteExpectations(key) } - r.Recorder.Eventf(pod, corev1.EventTypeNormal, "UpdatePod", "Succeed to update labels: %v", labels) return err } diff --git a/pkg/controllers/podopslifecycle/podopslifecycle_controller_test.go b/pkg/controllers/podopslifecycle/podopslifecycle_controller_test.go index 686635c3..d1e67d5c 100644 --- a/pkg/controllers/podopslifecycle/podopslifecycle_controller_test.go +++ b/pkg/controllers/podopslifecycle/podopslifecycle_controller_test.go @@ -31,7 +31,9 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" + "k8s.io/klog/klogr" "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" "sigs.k8s.io/controller-runtime/pkg/envtest" logf "sigs.k8s.io/controller-runtime/pkg/log" "sigs.k8s.io/controller-runtime/pkg/log/zap" @@ -41,6 +43,8 @@ import ( "kusionstack.io/operating/apis/apps/v1alpha1" "kusionstack.io/operating/pkg/controllers/podtransitionrule" "kusionstack.io/operating/pkg/controllers/podtransitionrule/checker" + "kusionstack.io/operating/pkg/controllers/utils/expectations" + "kusionstack.io/operating/pkg/utils/mixin" ) var ( @@ -411,7 +415,7 @@ var _ = Describe("Stage processing", func() { }) }) -var _ = Describe("Expected finalizer processng", func() { +var _ = Describe("Finalizer processing", func() { var ( name = "test" namespace = "default" @@ -506,6 +510,61 @@ var _ = Describe("Expected finalizer processng", func() { }) }) +var _ = Describe("Label service-available processing", func() { + scheme := runtime.NewScheme() + err := corev1.AddToScheme(scheme) + Expect(err).NotTo(HaveOccurred()) + + pod := &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test1", + Namespace: "default", + Labels: map[string]string{ + v1alpha1.ControlledByKusionStackLabelKey: "true", + }, + }, + Status: corev1.PodStatus{ + Phase: corev1.PodRunning, + Conditions: []corev1.PodCondition{ + { + Type: corev1.PodReady, + Status: corev1.ConditionTrue, + }, + }, + }, + } + + fakeClient := fake.NewClientBuilder(). + WithScheme(scheme). + WithObjects(pod). + Build() + + podOpsLifecycle := &ReconcilePodOpsLifecycle{ + ReconcilerMixin: &mixin.ReconcilerMixin{ + Client: fakeClient, + Logger: klogr.New().WithName(controllerName), + }, + expectation: expectations.NewResourceVersionExpectation(), + } + + It("Service is available", func() { + _, err := podOpsLifecycle.Reconcile(context.Background(), reconcile.Request{ + NamespacedName: types.NamespacedName{ + Name: "test1", + Namespace: "default", + }, + }) + Expect(err).NotTo(HaveOccurred()) + + pod1 := &corev1.Pod{} + fakeClient.Get(context.Background(), client.ObjectKey{ + Name: "test1", + Namespace: "default", + }, pod1) + Expect(pod1.Labels[v1alpha1.PodServiceAvailableLabel]).NotTo(BeEmpty()) + }) +}) + func testReconcile(inner reconcile.Reconciler) (reconcile.Reconciler, chan reconcile.Request) { requests := make(chan reconcile.Request, 5) fn := reconcile.Func(func(ctx context.Context, req reconcile.Request) (reconcile.Result, error) {