From a1465b5725fa0a0e1b74b28c2e7b243c6babe3ca Mon Sep 17 00:00:00 2001
From: liuzhenwei <dui_zhang@163.com>
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 <dui_zhang@163.com>
add ut
Signed-off-by: liuzhenwei <dui_zhang@163.com>
add some comments and simplified some code
Signed-off-by: liuzhenwei <dui_zhang@163.com>
remove unnecessary pod status operations
Signed-off-by: liuzhenwei <dui_zhang@163.com>
change pod to terminal phase before create crr
Signed-off-by: liuzhenwei <dui_zhang@163.com>
reverse the checking to reduce code indentation
Signed-off-by: liuzhenwei <dui_zhang@163.com>
simplified some logic
Signed-off-by: liuzhenwei <dui_zhang@163.com>
---
.../sidecar_terminator_controller.go | 83 +++++++--
.../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, 360 insertions(+), 41 deletions(-)
diff --git a/pkg/controller/sidecarterminator/sidecar_terminator_controller.go b/pkg/controller/sidecarterminator/sidecar_terminator_controller.go
index 251baa8b34..248e1eb894 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,51 @@ 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
+ 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..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 {