feat(crr): implement TCPSocket preStop hook in ContainerRecreateRequest#2506
feat(crr): implement TCPSocket preStop hook in ContainerRecreateRequest#2506om7057 wants to merge 2 commits into
Conversation
The TCPSocket field in ProbeHandler was wired through the entire CRR
pipeline (webhook → spec → daemon) but was never executed, because the
upstream kubelet HandlerRunner only handles Exec, HTTPGet, and Sleep
for lifecycle hooks.
This commit adds first-class TCPSocket support:
- pkg/daemon/kuberuntime/kuberuntime_container.go
- executeTCPSocketHook: dials the target TCP address using
net.DialTimeout, treating a successful connection (immediately
closed) as success — matching Kubernetes probe semantics.
- resolveTCPSocketPort: resolves IntOrString ports; integer ports
are validated and used directly; named ports are looked up in
containerSpec.Ports.
- executePreStopHook: intercepts TCPSocket before delegating to
the upstream runner.Run so TCPSocket is no longer a silent no-op.
- pkg/webhook/containerrecreaterequest/mutating/crr_mutating_handler.go
- Populate c.Ports for TCPSocket hooks (same as HTTPGet) so that
named port references in TCPSocket.Port can be resolved by the
daemon.
- apis/apps/v1alpha1/containerrecreaterequest_types.go
- Remove stale TODO comments from ProbeHandler.TCPSocket and replace
with accurate documentation of the implemented behaviour.
Fixes openkruise#2505
Signed-off-by: om7057 <kulkarniom7057@gmail.com>
|
[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. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
Welcome @om7057! It looks like this is your first PR to openkruise/kruise 🎉 |
There was a problem hiding this comment.
Pull request overview
This PR adds execution support for TCPSocket preStop hooks in the kruise-daemon when processing ContainerRecreateRequest (CRR), so TCP-based lifecycle notifications are no longer silently treated as no-ops. It also ensures CRR mutation includes container ports needed for named-port resolution and updates API docs to reflect the now-supported behavior.
Changes:
- Add explicit handling of
TCPSocketinexecutePreStopHookby dialing the configured TCP host/port within the remaining grace period. - Populate CRR container
Portswhen the preStop hook isHTTPGetorTCPSocket, enabling named-port resolution by the daemon. - Update CRR API type comments to document actual
TCPSocketpreStop semantics.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| pkg/webhook/containerrecreaterequest/mutating/crr_mutating_handler.go | Includes container ports in CRR when preStop uses TCPSocket, enabling named port resolution. |
| pkg/daemon/kuberuntime/kuberuntime_container.go | Implements TCPSocket preStop execution via TCP dial and port resolution logic. |
| apis/apps/v1alpha1/containerrecreaterequest_types.go | Updates documentation/comments for TCPSocket hook support and semantics. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Fall back to the pod IP when no explicit host is provided. This mirrors | ||
| // how the HTTPGet probe handler resolves the target address. | ||
| if pod.Status.PodIP != "" { | ||
| host = pod.Status.PodIP | ||
| } else { | ||
| host = "localhost" | ||
| } |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #2506 +/- ##
==========================================
+ Coverage 49.91% 50.02% +0.11%
==========================================
Files 325 325
Lines 28437 28482 +45
==========================================
+ Hits 14194 14248 +54
+ Misses 12588 12577 -11
- Partials 1655 1657 +2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
… tests The fake pod built by convertCRRToPod had no Status.PodIP, so the TCPSocket hook executor would always fall back to 'localhost' when no explicit Host was set in TCPSocketAction — causing the hook to dial the kruise-daemon's node address instead of the container's pod IP. Changes: - crr_daemon_controller.go: fetch the real Pod before calling convertCRRToPod and pass its Status.PodIP. - crr_daemon_util.go: convertCRRToPod now accepts a podIP string and sets Status.PodIP on the fake pod. - crr_daemon_util_test.go: unit tests for convertCRRToPod covering pod IP propagation, grace period defaults, TCPSocket lifecycle reconstruction, and no-preStop cases. - kuberuntime_container_test.go: unit tests for executeTCPSocketHook and resolveTCPSocketPort covering success, failure, named ports, explicit host, pod IP fallback, localhost fallback, invalid ports, and zero grace period clamping. Signed-off-by: om7057 <kulkarniom7057@gmail.com>
E2E failure analysisThe Root cause: The only actual test failure is The other 3 Evidence that this is a pre-existing flake:
The failure is a CI timing flake on a shared runner. No code changes needed. |
What this PR does
Closes #2505
Adds support for executing
TCPSocketpreStop hooks in the kruise-daemon duringContainerRecreateRequesthandling.TCPSocketwas already supported through the CRR flow (webhook injection → CRR spec → daemon pod reconstruction), but the daemon-side lifecycle execution did not handle it. Since the upstream kubelet handler runner only executesExec,HTTPGet, andSleephooks,TCPSockethooks were skipped.This PR adds the missing execution path.
Changes
pkg/daemon/kuberuntime/kuberuntime_container.goTCPSockethooks before falling back to the existing lifecycle hook runner.pkg/webhook/containerrecreaterequest/mutating/crr_mutating_handler.goTCPSockethook is present, allowing named port references to be resolved correctly by the daemon.apis/apps/v1alpha1/containerrecreaterequest_types.goTCPSockethooks are supported.Behavior
A
TCPSocketpreStop hook now succeeds when a connection can be established to the configured host and port. No data is exchanged, matching the existing Kubernetes TCP probe behavior.This enables CRR lifecycle handling for workloads that rely on TCP-based shutdown coordination instead of HTTP or exec hooks.