add enhanced livenessprobe controller #1535
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
BH4AWS
force-pushed
the
add_enhanced_livenessprobe_controller_v1
branch
from
05ebc64
to
11b35e2
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1535 +/- ##
==========================================
+ Coverage 47.91% 47.93% +0.01%
==========================================
Files 162 165 +3
Lines 23483 23763 +280
==========================================
+ Hits 11252 11390 +138
- Misses 11011 11131 +120
- Partials 1220 1242 +22
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
|
@BH4AWS Would you like to explain what the |
BH4AWS
force-pushed
the
add_enhanced_livenessprobe_controller_v1
branch
8 times, most recently
from
a5e88a3
to
4f9bbfa
Compare
Signed-off-by: jicheng.sk <jicheng.sk@alibaba-inc.com>
BH4AWS
force-pushed
the
add_enhanced_livenessprobe_controller_v1
branch
from
4f9bbfa
to
10b0f9f
Compare
new link is : #1544 There is the standard livenessProbe feature in Kubernetes opensource community. For the applications configured with the liveness probe, the Kubernetes kubelet component will periodically check whether the liveness probe service is normal. If being negative, the kubelet component will directly trigger the restart of the service container. However, this is a deadly operation or resilience protections for online applications, expecially for the incorrect probe configurations. For example, the configurations of liveness probes are often incorrect due to complicated configurations or contents. Once the probe takes effect, the full service container is triggered to restart, this condition casues service outage(or even triggering an avalanche of services during the restart). Secondly, there is no standard concept in the design idea of community method, which completely relies on the machine node detection and machine node restart mechanism. Furthermore, for a stability perspective, community-native solutions lack application-level global high availability and any resilience protection policies. Combined with the existing defects, it proposes to design and implement the enhanced livnessProbe application survival solution in the OpenKruise suite, and plans to open source release as the core capability in the future, so that the innovative technology can be used by more people. details info can be found in our proposal~~~ |
|
@BH4AWS plz submit design proposal first for more feedback from the community |
| } | ||
| }() | ||
|
|
||
| err = r.syncPodContainersLivenessProbe(request.Namespace, request.Name) |
There was a problem hiding this comment.
plz double check whether pod is using enhanced liveness probe
There was a problem hiding this comment.
pkg/controller/livenessprobemapnodeprobe/probemapnodeprobe_pod_event_handler.go
the event handler checks that the pods has the annotation for 'usingEnhancedLivenessProbe' function.
| } | ||
|
|
||
| func (r *ReconcileEnhancedLivenessProbe2NodeProbe) GetPodNodeName(pod *v1.Pod) string { | ||
| return pod.Spec.NodeName |
There was a problem hiding this comment.
this func is too trivial , just use pod.spec.nodeName instead
There was a problem hiding this comment.
this function has been removed, using pod.spec.nodeName.
| podContainerProbe := podNewNppCloneSpec.PodProbes[index] | ||
| if podContainerProbe.Name == pod.Name && podContainerProbe.Namespace == pod.Namespace && | ||
| podContainerProbe.UID == fmt.Sprintf("%v", pod.UID) { | ||
| continue |
There was a problem hiding this comment.
we can mark the newPodProbeTmp is changed, so that no deep equal comparison is needed in L90
| return err | ||
| } | ||
|
|
||
| podNewNppClone := nppClone.DeepCopy() |
There was a problem hiding this comment.
the deep copy is not necessary since nppClone and podNewNppClone is not used here
| isHit = true | ||
| // diff the current pod container probes vs the npp container probes | ||
| newPodContainerProbes := generatePodContainersProbe(pod, containersLivenessProbeConfig) | ||
| podContainerProbe.Probes = newPodContainerProbes |
There was a problem hiding this comment.
can we break the loop if hit ?
| podNewNppCloneSpec := podNewNppClone.Spec.DeepCopy() | ||
|
|
||
| isHit := false | ||
| for index := range podNewNppCloneSpec.PodProbes { |
There was a problem hiding this comment.
replace this loop with a func called getOrCreatePodProbe(), so that the func can be greatly simplified
podNewNppCloneSpec.PodProbes = append( podNewNppCloneSpec.PodProbes, getOrCreatePodProbe(pod, containersLivenessProbeConfig))
| return err | ||
| } | ||
|
|
||
| podNewNppClone := nppClone.DeepCopy() |
There was a problem hiding this comment.
this clone is unnecessary since podNewNppClone is not used other than assign to podNewNppCloneSpec
|
@BH4AWS: PR needs rebase. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
Ⅰ. Describe what this PR does
A controller converts the pod livenessprobe config to nodePodProbe config.
This controller is the part of the enhanced livenessProbe solutions.
Ⅱ. Does this pull request fix one issue?
Ⅲ. Describe how to verify it
when a pod with livenessProbe config is created, the native livenessProbe will be replaced to pod annotation and this controller converts to nodePodProbe config immediately.
Ⅳ. Special notes for reviews