kruise
kruise copied to clipboard
Sidecar terminator ignore the exit code of the sidecar container
Ⅰ. Describe what this PR does
Pods are not allowed to transition out of terminal phases when kubelet report the pod status.(ref) Base on this context, the proposal as bellow: In the sidecar terminator controller: 1,Calculate the Pod phase from the exit-code of the main container: the pod phase will be Succeeded, if all main containers are succeeded . the pod phase will be failed ,if at least one main container is failed(the exitCode is not zero) . 2, Terminate it if the job pod is complete.
3, Terminate the pod and set the condition of the pod as the bellow:
- lastProbeTime: null
lastTransitionTime: "2023-06-06T14:06:08Z"
status: "True"
type: SidecarTerminated
4, If the job pod is completed, we will skip terminating the pod. Check whether a job is complete by:
func isJobPodCompleted(pod *corev1.Pod) bool {
mainContainers := getMain(pod)
if !containersCompleted(pod, mainContainers) {
return false
}
if containersSucceeded(pod, mainContainers) {
return pod.Status.Phase == corev1.PodSucceeded
} else {
return pod.Status.Phase == corev1.PodFailed
}
}
Native controller actions:
1 .Job controller update the status of the job when pod phase is updated. 2. As we said above, kubelet only update the container status and do not update the pod phase since it is terminal phase. And the pod will reach a final state. 3. Kubelet will kill the containers that are not in terminal phase, after the pod the terminated by sidecar terminator.
Ⅱ. Does this pull request fix one issue?
fixes #1285
Ⅲ. Describe how to verify it
Ⅳ. Special notes for reviews
Codecov Report
Attention: Patch coverage is 79.06977%
with 9 lines
in your changes are missing coverage. Please review.
Project coverage is 47.89%. Comparing base (
209d476
) to head (a66d98f
).
Files | Patch % | Lines |
---|---|---|
...sidecarterminator/sidecar_terminator_controller.go | 77.50% | 6 Missing and 3 partials :warning: |
Additional details and impacted files
@@ Coverage Diff @@
## master #1303 +/- ##
==========================================
+ Coverage 47.82% 47.89% +0.06%
==========================================
Files 161 161
Lines 23395 23428 +33
==========================================
+ Hits 11188 11220 +32
- Misses 10993 10995 +2
+ Partials 1214 1213 -1
Flag | Coverage Δ | |
---|---|---|
unittests | 47.89% <79.06%> (+0.06%) |
:arrow_up: |
Flags with carried forward coverage won't be shown. Click here to find out more.
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
@diannaowa Can we set Pod.Status.Phase
to Succeeded/Failed
BEFORE killing sidecars?
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.
will do
/lgtm
/approve
[APPROVALNOTIFIER] This PR is APPROVED
This pull-request has been approved by: furykerry
The full list of commands accepted by this bot can be found here.
The pull request process is described here
- ~~OWNERS~~ [furykerry]
Approvers can indicate their approval by writing /approve
in a comment
Approvers can cancel approval by writing /approve cancel
in a comment