Debug probes #5474
Debug probes #5474
Conversation
Codecov Report
@@ Coverage Diff @@
## master #5474 +/- ##
==========================================
- Coverage 71.48% 71.25% -0.23%
==========================================
Files 397 400 +3
Lines 14573 14968 +395
==========================================
+ Hits 10418 10666 +248
- Misses 3387 3513 +126
- Partials 768 789 +21
Continue to review full report at Codecov.
|
pkg/skaffold/debug/transform.go
Outdated
| } | ||
|
|
||
| changed := false | ||
| var minTimeout int32 = 10 * 60 // make it configurable? |
There was a problem hiding this comment.
Add seconds to clarify this is in seconds as default duration is Nanosecond
| var minTimeout int32 = 10 * 60 // make it configurable? | |
| var minTimeoutSeconds int32 = 10 * 60 // make it configurable? |
There was a problem hiding this comment.
Will change to a duration.
pkg/skaffold/debug/transform.go
Outdated
| func rewriteProbes(metadata *metav1.ObjectMeta, podSpec *v1.PodSpec) bool { | ||
| annotation, found := metadata.Annotations[DebugConfigAnnotation] | ||
| if !found { | ||
| logrus.Warn("no debug config found") |
There was a problem hiding this comment.
Debug annotation not present on a pod was not surfaced as a warning before. Is there a reason why we want to show it now?
There was a problem hiding this comment.
Can we move checking the pod debug annotation at L202 and skip both if no annotation is found?
func transformPodSpec(metadata *metav1.ObjectMeta, podSpec *v1.PodSpec, retrieveImageConfiguration configurationRetriever, debugHelpersRegistry string) bool {
// skip annotated podspecs — allows users to customize their own image
if _, found := metadata.Annotations[DebugConfigAnnotation]; found {
return false
}
containers := rewriteContainers(metadata, podSpec, retrieveImageConfiguration, debugHelpersRegistry)
timeouts := rewriteProbes(metadata, podSpec) // must rewriteProbes after
return containers || timeouts
}
There was a problem hiding this comment.
You're right: this shouldn't be a warning. There's an assumption that the probes are only rewritten for containers that were transformed for debugging (I'll make that explicit).
|
@tejal29 could you take a quick peek: I added support for disabling the probes and also overriding the probe timeout value too. |
Fixes: #5425
Description
Cause
skaffold debugto rewrite any readiness/liveness/startup probes using HTTP-Get to set the timeout value to 10 minutes (from the default of 1s). This change allows debugging calls from such probes, and also avoids timeouts when a probe is made but the application is suspended because of an ongoing debug session.User facing changes (remove if N/A)
skaffold debugalters Kubernetes HTTP-Get probes to have a 10 minute timeout to allow debugging probe callsFollow-up Work (remove if N/A)
Consider making this configurable somehow.