From 14ae299a993edc5fe6c174d04ebeffe6565c344d Mon Sep 17 00:00:00 2001 From: liuzhenwei Date: Wed, 24 May 2023 22:36:46 +0800 Subject: [PATCH] Sidecar terminator ignore the exit code of the sidecar container Signed-off-by: liuzhenwei add ut Signed-off-by: liuzhenwei add some comments and simplified some code Signed-off-by: liuzhenwei remove unnecessary pod status operations Signed-off-by: liuzhenwei change pod to terminal phase before create crr Signed-off-by: liuzhenwei reverse the checking to reduce code indentation Signed-off-by: liuzhenwei simplified some logic Signed-off-by: liuzhenwei --- .../sidecar_terminator_controller.go | 97 ++++++++-- .../sidecar_terminator_controller_test.go | 168 ++++++++++++++++-- .../sidecar_terminator_pod_event_handler.go | 8 +- ...decar_terminator_pod_event_handler_test.go | 58 +++++- .../mutating/crr_mutating_handler.go | 21 ++- pkg/webhook/util/util.go | 3 +- test/e2e/apps/sidecarterminator.go | 60 ++++++- 7 files changed, 374 insertions(+), 41 deletions(-) diff --git a/pkg/controller/sidecarterminator/sidecar_terminator_controller.go b/pkg/controller/sidecarterminator/sidecar_terminator_controller.go index 251baa8b34..fed9135435 100644 --- a/pkg/controller/sidecarterminator/sidecar_terminator_controller.go +++ b/pkg/controller/sidecarterminator/sidecar_terminator_controller.go @@ -18,19 +18,18 @@ package sidecarterminator import ( "context" + "encoding/json" "flag" + "fmt" "strings" "time" - appsv1alpha1 "github.com/openkruise/kruise/apis/apps/v1alpha1" - "github.com/openkruise/kruise/pkg/features" - "github.com/openkruise/kruise/pkg/util" - utilclient "github.com/openkruise/kruise/pkg/util/client" - utilfeature "github.com/openkruise/kruise/pkg/util/feature" - "github.com/openkruise/kruise/pkg/util/ratelimiter" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" + "k8s.io/apimachinery/pkg/util/clock" "k8s.io/apimachinery/pkg/util/sets" "k8s.io/client-go/tools/record" "k8s.io/klog/v2" @@ -39,6 +38,13 @@ import ( "sigs.k8s.io/controller-runtime/pkg/manager" "sigs.k8s.io/controller-runtime/pkg/reconcile" "sigs.k8s.io/controller-runtime/pkg/source" + + appsv1alpha1 "github.com/openkruise/kruise/apis/apps/v1alpha1" + "github.com/openkruise/kruise/pkg/features" + "github.com/openkruise/kruise/pkg/util" + utilclient "github.com/openkruise/kruise/pkg/util/client" + utilfeature "github.com/openkruise/kruise/pkg/util/feature" + "github.com/openkruise/kruise/pkg/util/ratelimiter" ) func init() { @@ -46,7 +52,8 @@ func init() { } var ( - concurrentReconciles = 3 + concurrentReconciles = 3 + SidecarTerminated corev1.PodConditionType = "SidecarTerminated" ) /** @@ -71,6 +78,7 @@ func newReconciler(mgr manager.Manager) reconcile.Reconciler { Client: cli, recorder: recorder, scheme: mgr.GetScheme(), + clock: clock.RealClock{}, } } @@ -101,6 +109,7 @@ type ReconcileSidecarTerminator struct { client.Client recorder record.EventRecorder scheme *runtime.Scheme + clock clock.Clock } // Reconcile get the pod whose sidecar containers should be stopped, and stop them. @@ -131,17 +140,8 @@ func (r *ReconcileSidecarTerminator) doReconcile(pod *corev1.Pod) (reconcile.Res return reconcile.Result{}, nil } - if containersCompleted(pod, getSidecar(pod)) { - klog.V(3).Infof("SidecarTerminator -- all sidecars of pod(%v/%v) have been completed, no need to process", pod.Namespace, pod.Name) - return reconcile.Result{}, nil - } - - if pod.Spec.RestartPolicy == corev1.RestartPolicyOnFailure && !containersSucceeded(pod, getMain(pod)) { - klog.V(3).Infof("SidecarTerminator -- pod(%v/%v) is trying to restart, no need to process", pod.Namespace, pod.Name) - return reconcile.Result{}, nil - } - sidecarNeedToExecuteKillContainer, sidecarNeedToExecuteInPlaceUpdate, err := r.groupSidecars(pod) + if err != nil { return reconcile.Result{}, err } @@ -150,6 +150,10 @@ func (r *ReconcileSidecarTerminator) doReconcile(pod *corev1.Pod) (reconcile.Res return reconcile.Result{}, err } + if err := r.terminateJobPod(pod); err != nil { + return reconcile.Result{}, err + } + if err := r.executeKillContainerAction(pod, sidecarNeedToExecuteKillContainer); err != nil { return reconcile.Result{}, err } @@ -157,6 +161,65 @@ func (r *ReconcileSidecarTerminator) doReconcile(pod *corev1.Pod) (reconcile.Res return reconcile.Result{}, nil } +// terminateJobPod terminate the job pod and skip the state of the sidecar containers +// This method should only be called before the executeKillContainerAction +func (r *ReconcileSidecarTerminator) terminateJobPod(pod *corev1.Pod) error { + if pod.Status.Phase == corev1.PodFailed || pod.Status.Phase == corev1.PodSucceeded { + return nil + } + + // after the pod is terminated by the sidecar terminator, kubelet will kill the containers that are not in the terminal phase + // 1. sidecar container terminate with non-zero exit code + // 2. sidecar container is not in a terminal phase (still running or waiting) + klog.V(3).Infof("all of the main containers are completed, will terminate the job pod %s/%s", pod.Namespace, pod.Name) + // terminate the pod, ignore the status of the sidecar containers. + // in kubelet,pods are not allowed to transition out of terminal phases. + + // patch pod condition + status := corev1.PodStatus{ + Conditions: []corev1.PodCondition{ + { + Type: SidecarTerminated, + Status: corev1.ConditionTrue, + LastTransitionTime: metav1.Now(), + Message: "Terminated by Sidecar Terminator", + }, + }, + } + + // patch pod phase & podReady and containersReady condition + if containersSucceeded(pod, getMain(pod)) { + status.Phase = corev1.PodSucceeded + for _, condition := range pod.Status.Conditions { + if condition.Type == corev1.PodReady || condition.Type == corev1.ContainersReady { + condition.Reason = "PodCompleted" + condition.Status = corev1.ConditionTrue + status.Conditions = append(status.Conditions, condition) + } + } + } else { + status.Phase = corev1.PodFailed + for _, condition := range pod.Status.Conditions { + if condition.Type == corev1.PodReady || condition.Type == corev1.ContainersReady { + condition.Reason = "PodFailed" + condition.Status = corev1.ConditionFalse + status.Conditions = append(status.Conditions, condition) + } + } + } + klog.V(3).Infof("terminate the job pod %s/%s phase=%s", pod.Namespace, pod.Name, status.Phase) + + by, _ := json.Marshal(status) + patchCondition := fmt.Sprintf(`{"status":%s}`, string(by)) + rcvObject := &corev1.Pod{ObjectMeta: metav1.ObjectMeta{Namespace: pod.Namespace, Name: pod.Name}} + + if err := r.Status().Patch(context.TODO(), rcvObject, client.RawPatch(types.StrategicMergePatchType, []byte(patchCondition))); err != nil { + return fmt.Errorf("failed to patch pod status: %v", err) + } + + return nil +} + func (r *ReconcileSidecarTerminator) groupSidecars(pod *corev1.Pod) (sets.String, sets.String, error) { runningOnVK, err := IsPodRunningOnVirtualKubelet(pod, r.Client) if err != nil { diff --git a/pkg/controller/sidecarterminator/sidecar_terminator_controller_test.go b/pkg/controller/sidecarterminator/sidecar_terminator_controller_test.go index ad62205649..5709827f57 100644 --- a/pkg/controller/sidecarterminator/sidecar_terminator_controller_test.go +++ b/pkg/controller/sidecarterminator/sidecar_terminator_controller_test.go @@ -24,15 +24,18 @@ import ( utilruntime "k8s.io/apimachinery/pkg/util/runtime" - appsv1alpha1 "github.com/openkruise/kruise/apis/apps/v1alpha1" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" + "k8s.io/apimachinery/pkg/util/clock" "k8s.io/client-go/tools/record" + "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/fake" "sigs.k8s.io/controller-runtime/pkg/reconcile" + + appsv1alpha1 "github.com/openkruise/kruise/apis/apps/v1alpha1" ) const ( @@ -75,12 +78,28 @@ var ( }, } + failedSidecarContainerStatus = corev1.ContainerStatus{ + Name: "sidecar", + State: corev1.ContainerState{ + Terminated: &corev1.ContainerStateTerminated{ + ExitCode: int32(137), + }, + }, + } uncompletedSidecarContainerStatus = corev1.ContainerStatus{ Name: "sidecar", State: corev1.ContainerState{ Terminated: nil, }, } + runningSidecarContainerStatus = corev1.ContainerStatus{ + Name: "sidecar", + State: corev1.ContainerState{ + Running: &corev1.ContainerStateRunning{ + StartedAt: metav1.Now(), + }, + }, + } podDemo = &corev1.Pod{ TypeMeta: metav1.TypeMeta{ @@ -199,81 +218,150 @@ func sidecarContainerFactory(name string, strategy string) corev1.Container { func TestKruiseDaemonStrategy(t *testing.T) { cases := []struct { - name string - getIn func() *corev1.Pod - getCRR func() *appsv1alpha1.ContainerRecreateRequest + name string + getIn func() *corev1.Pod + getCRR func() *appsv1alpha1.ContainerRecreateRequest + expectedPod func() *corev1.Pod }{ { name: "normal pod with sidecar, restartPolicy=Never, main containers have not been completed", getIn: func() *corev1.Pod { podIn := podDemo.DeepCopy() podIn.Status.ContainerStatuses[0] = uncompletedMainContainerStatus + podIn.Status.ContainerStatuses[1] = runningSidecarContainerStatus return podIn }, getCRR: func() *appsv1alpha1.ContainerRecreateRequest { return nil }, + expectedPod: func() *corev1.Pod { + return podDemo.DeepCopy() + }, + }, + { + name: "normal pod with sidecar, restartPolicy=Never, main containers failed and sidecar running", + getIn: func() *corev1.Pod { + podIn := podDemo.DeepCopy() + podIn.Status.ContainerStatuses[0] = failedMainContainerStatus + podIn.Status.ContainerStatuses[1] = runningSidecarContainerStatus + return podIn + }, + getCRR: func() *appsv1alpha1.ContainerRecreateRequest { + return crrDemo.DeepCopy() + }, + expectedPod: func() *corev1.Pod { + pod := podDemo.DeepCopy() + pod.Status.Phase = corev1.PodFailed + return pod + }, }, { - name: "normal pod with sidecar, restartPolicy=Never, main containers failed", + name: "normal pod with sidecar, restartPolicy=Never, main containers failed and sidecar running", getIn: func() *corev1.Pod { podIn := podDemo.DeepCopy() podIn.Status.ContainerStatuses[0] = failedMainContainerStatus + podIn.Status.ContainerStatuses[1] = runningSidecarContainerStatus return podIn }, getCRR: func() *appsv1alpha1.ContainerRecreateRequest { return crrDemo.DeepCopy() }, + expectedPod: func() *corev1.Pod { + pod := podDemo.DeepCopy() + pod.Status.Phase = corev1.PodFailed + return pod + }, + }, + { + name: "normal pod with sidecar, restartPolicy=Never, main containers failed and sidecar failed", + getIn: func() *corev1.Pod { + podIn := podDemo.DeepCopy() + podIn.Status.ContainerStatuses[0] = failedMainContainerStatus + podIn.Status.ContainerStatuses[1] = failedSidecarContainerStatus + podIn.Status.Phase = corev1.PodFailed //todo + return podIn + }, + getCRR: func() *appsv1alpha1.ContainerRecreateRequest { + return nil + }, + expectedPod: func() *corev1.Pod { + pod := podDemo.DeepCopy() + pod.Status.Phase = corev1.PodFailed + return pod + }, }, { - name: "normal pod with sidecar, restartPolicy=Never, main containers succeeded", + name: "normal pod with sidecar, restartPolicy=Never, main containers succeeded and sidecar running", getIn: func() *corev1.Pod { podIn := podDemo.DeepCopy() podIn.Status.ContainerStatuses[0] = succeededMainContainerStatus + podIn.Status.ContainerStatuses[1] = runningSidecarContainerStatus return podIn }, getCRR: func() *appsv1alpha1.ContainerRecreateRequest { return crrDemo.DeepCopy() }, + expectedPod: func() *corev1.Pod { + pod := podDemo.DeepCopy() + pod.Status.Phase = corev1.PodSucceeded + return pod + }, }, { - name: "normal pod with sidecar, restartPolicy=OnFailure, main containers have not been completed", + name: "normal pod with sidecar, restartPolicy=OnFailure, main containers have not been completed and sidecar running", getIn: func() *corev1.Pod { podIn := podDemo.DeepCopy() podIn.Spec.RestartPolicy = corev1.RestartPolicyOnFailure podIn.Status.ContainerStatuses[0] = uncompletedMainContainerStatus + podIn.Status.ContainerStatuses[1] = runningSidecarContainerStatus return podIn }, getCRR: func() *appsv1alpha1.ContainerRecreateRequest { return nil }, + expectedPod: func() *corev1.Pod { + pod := podDemo.DeepCopy() + return pod + }, }, { - name: "normal pod with sidecar, restartPolicy=OnFailure, main containers failed", + name: "normal pod with sidecar, restartPolicy=OnFailure, main containers failed and sidecar succeeded", getIn: func() *corev1.Pod { podIn := podDemo.DeepCopy() podIn.Spec.RestartPolicy = corev1.RestartPolicyOnFailure podIn.Status.ContainerStatuses[0] = failedMainContainerStatus + podIn.Status.ContainerStatuses[1] = completedSidecarContainerStatus return podIn }, getCRR: func() *appsv1alpha1.ContainerRecreateRequest { return nil }, + expectedPod: func() *corev1.Pod { + pod := podDemo.DeepCopy() + return pod + }, }, { - name: "normal pod with sidecar, restartPolicy=OnFailure, main containers succeeded", + name: "normal pod with sidecar, restartPolicy=OnFailure, main containers succeeded and sidecar succeeded", getIn: func() *corev1.Pod { podIn := podDemo.DeepCopy() podIn.Spec.RestartPolicy = corev1.RestartPolicyOnFailure + podIn.Status.Phase = corev1.PodSucceeded podIn.Status.ContainerStatuses[0] = succeededMainContainerStatus + podIn.Status.ContainerStatuses[1] = completedSidecarContainerStatus return podIn }, getCRR: func() *appsv1alpha1.ContainerRecreateRequest { - return crrDemo.DeepCopy() + return nil + }, + expectedPod: func() *corev1.Pod { + pod := podDemo.DeepCopy() + pod.Status.Phase = corev1.PodSucceeded + return pod }, }, { - name: "normal pod with sidecar, restartPolicy=OnFailure, 2 succeeded main containers, 2 sidecars", + name: "normal pod with sidecar, restartPolicy=OnFailure, 2 succeeded main containers, 2 sidecars uncompleted", getIn: func() *corev1.Pod { podIn := podDemo.DeepCopy() podIn.Spec.Containers = []corev1.Container{ @@ -298,6 +386,43 @@ func TestKruiseDaemonStrategy(t *testing.T) { } return crr }, + expectedPod: func() *corev1.Pod { + pod := podDemo.DeepCopy() + pod.Status.Phase = corev1.PodSucceeded + return pod + }, + }, + { + name: "normal pod with sidecar, restartPolicy=OnFailure, 2 succeeded main containers, 2 sidecars running", + getIn: func() *corev1.Pod { + podIn := podDemo.DeepCopy() + podIn.Spec.Containers = []corev1.Container{ + mainContainerFactory("main-1"), + mainContainerFactory("main-2"), + sidecarContainerFactory("sidecar-1", "true"), + sidecarContainerFactory("sidecar-2", "true"), + } + podIn.Spec.RestartPolicy = corev1.RestartPolicyOnFailure + podIn.Status.ContainerStatuses = []corev1.ContainerStatus{ + rename(succeededMainContainerStatus.DeepCopy(), "main-1"), + rename(succeededMainContainerStatus.DeepCopy(), "main-2"), + rename(runningSidecarContainerStatus.DeepCopy(), "sidecar-1"), + rename(runningSidecarContainerStatus.DeepCopy(), "sidecar-2"), + } + return podIn + }, + getCRR: func() *appsv1alpha1.ContainerRecreateRequest { + crr := crrDemo.DeepCopy() + crr.Spec.Containers = []appsv1alpha1.ContainerRecreateRequestContainer{ + {Name: "sidecar-1"}, {Name: "sidecar-2"}, + } + return crr + }, + expectedPod: func() *corev1.Pod { + pod := podDemo.DeepCopy() + pod.Status.Phase = corev1.PodSucceeded + return pod + }, }, { name: "normal pod with sidecar, restartPolicy=OnFailure, 2 succeeded main containers, 2 sidecars but 1 completed", @@ -325,6 +450,11 @@ func TestKruiseDaemonStrategy(t *testing.T) { } return crr }, + expectedPod: func() *corev1.Pod { + pod := podDemo.DeepCopy() + pod.Status.Phase = corev1.PodSucceeded + return pod + }, }, { name: "normal pod with sidecar, restartPolicy=OnFailure, 2 main containers but 1 uncompleted, 2 sidecars but 1 completed", @@ -348,6 +478,10 @@ func TestKruiseDaemonStrategy(t *testing.T) { getCRR: func() *appsv1alpha1.ContainerRecreateRequest { return nil }, + expectedPod: func() *corev1.Pod { + pod := podDemo.DeepCopy() + return pod + }, }, } @@ -359,6 +493,7 @@ func TestKruiseDaemonStrategy(t *testing.T) { r := ReconcileSidecarTerminator{ Client: fakeClient, recorder: fakeRecord, + clock: clock.RealClock{}, } _, err := r.Reconcile(context.Background(), reconcile.Request{ @@ -384,6 +519,17 @@ func TestKruiseDaemonStrategy(t *testing.T) { if !(expectCRR == nil && errors.IsNotFound(err) || reflect.DeepEqual(realBy, expectBy)) { t.Fatal("Get unexpected CRR") } + + pod := &corev1.Pod{} + err = fakeClient.Get(context.TODO(), client.ObjectKey{Namespace: podDemo.Namespace, Name: podDemo.Name}, pod) + if err != nil { + t.Fatalf("Get pod error: %v", err) + } + expectPod := cs.expectedPod() + if pod.Status.Phase != expectPod.Status.Phase { + t.Fatalf("Get an expected pod phase : expectd=%s,got=%s", expectPod.Status.Phase, pod.Status.Phase) + } + }) } } diff --git a/pkg/controller/sidecarterminator/sidecar_terminator_pod_event_handler.go b/pkg/controller/sidecarterminator/sidecar_terminator_pod_event_handler.go index 32789898cd..59d5be58d6 100644 --- a/pkg/controller/sidecarterminator/sidecar_terminator_pod_event_handler.go +++ b/pkg/controller/sidecarterminator/sidecar_terminator_pod_event_handler.go @@ -19,7 +19,6 @@ package sidecarterminator import ( "strings" - appsv1alpha1 "github.com/openkruise/kruise/apis/apps/v1alpha1" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" @@ -28,6 +27,8 @@ import ( "sigs.k8s.io/controller-runtime/pkg/event" "sigs.k8s.io/controller-runtime/pkg/handler" "sigs.k8s.io/controller-runtime/pkg/reconcile" + + appsv1alpha1 "github.com/openkruise/kruise/apis/apps/v1alpha1" ) var _ handler.EventHandler = &enqueueRequestForPod{} @@ -74,12 +75,13 @@ func (p *enqueueRequestForPod) handlePodUpdate(q workqueue.RateLimitingInterface func isInterestingPod(pod *corev1.Pod) bool { if pod.DeletionTimestamp != nil || - pod.Status.Phase != corev1.PodRunning || + pod.Status.Phase == corev1.PodPending || pod.Spec.RestartPolicy == corev1.RestartPolicyAlways { return false } - if containersCompleted(pod, getSidecar(pod)) { + sidecars := getSidecar(pod) + if sidecars.Len() == 0 || containersCompleted(pod, sidecars) { return false } diff --git a/pkg/controller/sidecarterminator/sidecar_terminator_pod_event_handler_test.go b/pkg/controller/sidecarterminator/sidecar_terminator_pod_event_handler_test.go index 5002db990f..f3c863b6c0 100644 --- a/pkg/controller/sidecarterminator/sidecar_terminator_pod_event_handler_test.go +++ b/pkg/controller/sidecarterminator/sidecar_terminator_pod_event_handler_test.go @@ -142,6 +142,48 @@ func TestEnqueueRequestForPodUpdate(t *testing.T) { }, expectLen: 0, }, + { + name: "Pod, main container completed -> completed, sidecar container completed and pod has reached succeeded phase", + getOldPod: func() *corev1.Pod { + oldPod := oldPodDemo.DeepCopy() + oldPod.Status.ContainerStatuses = []corev1.ContainerStatus{ + succeededMainContainerStatus, + completedSidecarContainerStatus, + } + return oldPod + }, + getNewPod: func() *corev1.Pod { + newPod := newPodDemo.DeepCopy() + newPod.Status.ContainerStatuses = []corev1.ContainerStatus{ + succeededMainContainerStatus, + completedSidecarContainerStatus, + } + newPod.Status.Phase = corev1.PodSucceeded + return newPod + }, + expectLen: 0, + }, + { + name: "Pod, main container completed -> completed, sidecar container failed and pod has reached succeeded phase", + getOldPod: func() *corev1.Pod { + oldPod := oldPodDemo.DeepCopy() + oldPod.Status.ContainerStatuses = []corev1.ContainerStatus{ + succeededMainContainerStatus, + completedSidecarContainerStatus, + } + return oldPod + }, + getNewPod: func() *corev1.Pod { + newPod := newPodDemo.DeepCopy() + newPod.Status.ContainerStatuses = []corev1.ContainerStatus{ + succeededMainContainerStatus, + failedSidecarContainerStatus, + } + newPod.Status.Phase = corev1.PodSucceeded + return newPod + }, + expectLen: 0, + }, { name: "Pod, main container completed -> uncompleted, sidecar container completed", getOldPod: func() *corev1.Pod { @@ -260,13 +302,27 @@ func TestEnqueueRequestForPodCreate(t *testing.T) { expectLen: 1, }, { - name: "Pod, main container completed, sidecar container completed", + name: "Pod, main container completed, sidecar container completed and pod has reached succeeded phase", getPod: func() *corev1.Pod { newPod := demoPod.DeepCopy() newPod.Status.ContainerStatuses = []corev1.ContainerStatus{ succeededMainContainerStatus, completedSidecarContainerStatus, } + newPod.Status.Phase = corev1.PodSucceeded + return newPod + }, + expectLen: 0, + }, + { + name: "Pod, main container completed, sidecar container failed and pod has reached succeeded phase", + getPod: func() *corev1.Pod { + newPod := demoPod.DeepCopy() + newPod.Status.ContainerStatuses = []corev1.ContainerStatus{ + succeededMainContainerStatus, + failedSidecarContainerStatus, + } + newPod.Status.Phase = corev1.PodSucceeded return newPod }, expectLen: 0, diff --git a/pkg/webhook/containerrecreaterequest/mutating/crr_mutating_handler.go b/pkg/webhook/containerrecreaterequest/mutating/crr_mutating_handler.go index 2985d88e93..58db07e77a 100644 --- a/pkg/webhook/containerrecreaterequest/mutating/crr_mutating_handler.go +++ b/pkg/webhook/containerrecreaterequest/mutating/crr_mutating_handler.go @@ -23,10 +23,6 @@ import ( "net/http" "reflect" - appsv1alpha1 "github.com/openkruise/kruise/apis/apps/v1alpha1" - "github.com/openkruise/kruise/pkg/features" - "github.com/openkruise/kruise/pkg/util" - utilfeature "github.com/openkruise/kruise/pkg/util/feature" admissionv1 "k8s.io/api/admission/v1" v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/errors" @@ -37,6 +33,12 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/runtime/inject" "sigs.k8s.io/controller-runtime/pkg/webhook/admission" + + appsv1alpha1 "github.com/openkruise/kruise/apis/apps/v1alpha1" + "github.com/openkruise/kruise/pkg/controller/sidecarterminator" + "github.com/openkruise/kruise/pkg/features" + "github.com/openkruise/kruise/pkg/util" + utilfeature "github.com/openkruise/kruise/pkg/util/feature" ) const ( @@ -123,7 +125,7 @@ func (h *ContainerRecreateRequestHandler) Handle(ctx context.Context, req admiss } return admission.Errored(http.StatusInternalServerError, fmt.Errorf("failed to find Pod %s: %v", obj.Spec.PodName, err)) } - if !kubecontroller.IsPodActive(pod) { + if !kubecontroller.IsPodActive(pod) && !isTerminatedBySidecarTerminator(pod) { return admission.Errored(http.StatusBadRequest, fmt.Errorf("not allowed to recreate containers in an inactive Pod")) } else if pod.Spec.NodeName == "" { return admission.Errored(http.StatusBadRequest, fmt.Errorf("not allowed to recreate containers in a pending Pod")) @@ -144,6 +146,15 @@ func (h *ContainerRecreateRequestHandler) Handle(ctx context.Context, req admiss return admission.PatchResponseFromRaw(req.AdmissionRequest.Object.Raw, marshalled) } +func isTerminatedBySidecarTerminator(pod *v1.Pod) bool { + for _, c := range pod.Status.Conditions { + if c.Type == sidecarterminator.SidecarTerminated { + return true + } + } + return false +} + func injectPodIntoContainerRecreateRequest(obj *appsv1alpha1.ContainerRecreateRequest, pod *v1.Pod) error { obj.Labels[appsv1alpha1.ContainerRecreateRequestNodeNameKey] = pod.Spec.NodeName obj.Labels[appsv1alpha1.ContainerRecreateRequestPodUIDKey] = string(pod.UID) diff --git a/pkg/webhook/util/util.go b/pkg/webhook/util/util.go index c4a3f4f7a9..442c25b268 100644 --- a/pkg/webhook/util/util.go +++ b/pkg/webhook/util/util.go @@ -20,8 +20,9 @@ import ( "os" "strconv" - "github.com/openkruise/kruise/pkg/util" "k8s.io/klog/v2" + + "github.com/openkruise/kruise/pkg/util" ) func GetHost() string { diff --git a/test/e2e/apps/sidecarterminator.go b/test/e2e/apps/sidecarterminator.go index d49a57b752..2730f3e961 100644 --- a/test/e2e/apps/sidecarterminator.go +++ b/test/e2e/apps/sidecarterminator.go @@ -7,15 +7,16 @@ import ( "github.com/onsi/ginkgo" "github.com/onsi/gomega" - appsv1alpha1 "github.com/openkruise/kruise/apis/apps/v1alpha1" - kruiseclientset "github.com/openkruise/kruise/pkg/client/clientset/versioned" - "github.com/openkruise/kruise/test/e2e/framework" batchv1 "k8s.io/api/batch/v1" v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/rand" clientset "k8s.io/client-go/kubernetes" "k8s.io/utils/pointer" + + appsv1alpha1 "github.com/openkruise/kruise/apis/apps/v1alpha1" + kruiseclientset "github.com/openkruise/kruise/pkg/client/clientset/versioned" + "github.com/openkruise/kruise/test/e2e/framework" ) var _ = SIGDescribe("SidecarTerminator", func() { @@ -51,6 +52,18 @@ var _ = SIGDescribe("SidecarTerminator", func() { }, }, } + sidecarContainerNeverStop := v1.Container{ + Name: "sidecar", + Image: "busybox:latest", + ImagePullPolicy: v1.PullIfNotPresent, + Command: []string{"/bin/sh", "-c", "sleep 10000"}, + Env: []v1.EnvVar{ + { + Name: appsv1alpha1.KruiseTerminateSidecarEnv, + Value: "true", + }, + }, + } cases := []struct { name string @@ -90,6 +103,47 @@ var _ = SIGDescribe("SidecarTerminator", func() { return false }, }, + { + name: "native job, restartPolicy=Never, main succeeded, sidecar never stop", + createJob: func(str string) metav1.Object { + job := &batchv1.Job{ + ObjectMeta: metav1.ObjectMeta{Namespace: ns, Name: "job-" + str}, + Spec: batchv1.JobSpec{ + Template: v1.PodTemplateSpec{ + ObjectMeta: metav1.ObjectMeta{Labels: map[string]string{"with-sidecar-neverstop": "true"}}, + Spec: v1.PodSpec{ + Containers: []v1.Container{ + *mainContainer.DeepCopy(), + *sidecarContainerNeverStop.DeepCopy(), + }, + RestartPolicy: v1.RestartPolicyNever, + }, + }, + }, + } + job, err := c.BatchV1().Jobs(ns).Create(context.TODO(), job, metav1.CreateOptions{}) + gomega.Expect(err).NotTo(gomega.HaveOccurred()) + return job + }, + checkStatus: func(object metav1.Object) bool { + // assert job status + job, err := c.BatchV1().Jobs(object.GetNamespace()). + Get(context.TODO(), object.GetName(), metav1.GetOptions{}) + gomega.Expect(err).NotTo(gomega.HaveOccurred()) + for _, cond := range job.Status.Conditions { + if cond.Type == batchv1.JobComplete { + return true + } + } + // assert pod status + pods, err := c.CoreV1().Pods(object.GetNamespace()).List(context.TODO(), metav1.ListOptions{LabelSelector: "with-sidecar-neverstop=true"}) + gomega.Expect(err).NotTo(gomega.HaveOccurred()) + if len(pods.Items) == 1 && pods.Items[0].Status.Phase == v1.PodSucceeded { + return true + } + return false + }, + }, { name: "native job, restartPolicy=OnFailure, main failed", createJob: func(str string) metav1.Object {