From 861818ebd48e3c7b6c493d4b151d58f90af38bdb Mon Sep 17 00:00:00 2001 From: liuzhenwei Date: Wed, 13 Mar 2024 13:08:21 +0800 Subject: [PATCH] Sidecar terminator ignore the exit code of the sidecar container (#1303) add ut add some comments and simplified some code remove unnecessary pod status operations change pod to terminal phase before create crr reverse the checking to reduce code indentation simplified some logic remove unesd code and rename function avoid misleading Signed-off-by: liuzhenwei --- .../sidecar_terminator_controller.go | 80 +++++++-- .../sidecar_terminator_controller_test.go | 166 ++++++++++++++++-- .../sidecar_terminator_pod_event_handler.go | 8 +- ...decar_terminator_pod_event_handler_test.go | 58 +++++- .../mutating/crr_mutating_handler.go | 23 ++- pkg/webhook/util/util.go | 3 +- test/e2e/apps/sidecarterminator.go | 60 ++++++- 7 files changed, 357 insertions(+), 41 deletions(-) diff --git a/pkg/controller/sidecarterminator/sidecar_terminator_controller.go b/pkg/controller/sidecarterminator/sidecar_terminator_controller.go index 251baa8b34..68293216c3 100644 --- a/pkg/controller/sidecarterminator/sidecar_terminator_controller.go +++ b/pkg/controller/sidecarterminator/sidecar_terminator_controller.go @@ -18,19 +18,17 @@ 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/sets" "k8s.io/client-go/tools/record" "k8s.io/klog/v2" @@ -39,6 +37,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 +51,8 @@ func init() { } var ( - concurrentReconciles = 3 + concurrentReconciles = 3 + SidecarTerminated corev1.PodConditionType = "SidecarTerminated" ) /** @@ -131,17 +137,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 +147,10 @@ func (r *ReconcileSidecarTerminator) doReconcile(pod *corev1.Pod) (reconcile.Res return reconcile.Result{}, err } + if err := r.markJobPodTerminated(pod); err != nil { + return reconcile.Result{}, err + } + if err := r.executeKillContainerAction(pod, sidecarNeedToExecuteKillContainer); err != nil { return reconcile.Result{}, err } @@ -157,6 +158,51 @@ func (r *ReconcileSidecarTerminator) doReconcile(pod *corev1.Pod) (reconcile.Res return reconcile.Result{}, nil } +// markJobPodTerminated terminate the job pod and skip the state of the sidecar containers +// This method should only be called before the executeKillContainerAction +func (r *ReconcileSidecarTerminator) markJobPodTerminated(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 + if containersSucceeded(pod, getMain(pod)) { + status.Phase = corev1.PodSucceeded + } else { + status.Phase = corev1.PodFailed + } + 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..ee29c4c16c 100644 --- a/pkg/controller/sidecarterminator/sidecar_terminator_controller_test.go +++ b/pkg/controller/sidecarterminator/sidecar_terminator_controller_test.go @@ -24,15 +24,17 @@ 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/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 +77,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 +217,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 +385,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 +449,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 +477,10 @@ func TestKruiseDaemonStrategy(t *testing.T) { getCRR: func() *appsv1alpha1.ContainerRecreateRequest { return nil }, + expectedPod: func() *corev1.Pod { + pod := podDemo.DeepCopy() + return pod + }, }, } @@ -384,6 +517,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..fc9d85cbf9 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,9 @@ 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) { + // check isTerminatedBySidecarTerminator, + // because we need create CRR to kill sidecar container after change pod phase to terminal phase in SidecarTerminator Controller. + 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 +148,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 {