Add startup probe support to Knative Service #15309

ReToCode · 2024-06-06T12:04:09Z

Context

see feature document: https://docs.google.com/document/d/1TmimPy7qNLtc5IHVFKEme8X-NiIUBtVgR44GlaTqoWs/edit?usp=sharing

Proposed Changes

Adds startup probe support to Knative Service
Startup Probes disable the Knative's probe optimisation, same as exec probes do, as they are executed on the user-container by the Kubelet

Release Note

Knative Service now supports setting startup probes in the spec. Please note that this increases the cold-start time of your service (more info in docs).

ReToCode · 2024-06-06T12:05:50Z

/assign @skonto
/assign @dprotaso
/assign @izabelacg

Codecov Report

Attention: Patch coverage is 33.33333% with 2 lines in your changes missing coverage. Please review.

Project coverage is 84.61%. Comparing base (a310476) to head (be536f3).
Report is 1 commits behind head on main.

Files	Patch %	Lines
pkg/activator/net/revision_backends.go	0.00%	1 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #15309      +/-   ##
==========================================
+ Coverage   84.56%   84.61%   +0.05%     
==========================================
  Files         219      219              
  Lines       13584    13587       +3     
==========================================
+ Hits        11487    11497      +10     
+ Misses       1727     1724       -3     
+ Partials      370      366       -4

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

skonto · 2024-06-19T07:50:01Z

@dprotaso gentle ping.

dprotaso · 2024-06-19T18:41:46Z

pkg/reconciler/revision/resources/deploy.go

+	startupProbeMaxDuration := int32(0)
+	for _, container := range podSpec.Containers {
+		if container.StartupProbe != nil {
+			maxSuccessDuration := container.StartupProbe.PeriodSeconds *
+				container.StartupProbe.SuccessThreshold *
+				container.StartupProbe.TimeoutSeconds
+
+			maxFailDuration := container.StartupProbe.PeriodSeconds *
+				container.StartupProbe.FailureThreshold *
+				container.StartupProbe.TimeoutSeconds
+
+			maxDuration := max(maxSuccessDuration, maxFailDuration)
+			if maxDuration > startupProbeMaxDuration {
+				startupProbeMaxDuration = container.StartupProbe.InitialDelaySeconds + maxDuration
+			}
+		}
+	}
+


I'm sorta inclined to think that the progressdeadline encapsulates all the (startup time + ready time).

So I'm thinking of not adding this extra time here just because there's a startup probe.

Probably we can omit that and keep the K8s UX. It seems that the only validation that happens at the K8s side for progressdeadline is about MinReadySeconds, https://github.com/kubernetes/kubernetes/blob/master/pkg/apis/apps/validation/validation.go#L592. I am wondering how that field affects our stuff in case would want to set it (right now it is not exposed). 🤔

Hm I think we can, just means a user has to be aware of it and set that field himself accordingly. As startup probes are potentially used for slow-starting things the increase is quite necessary. Otherwise we will never see the pod starting up. Im fine with both ways.

@dprotaso @skonto I removed the increase. I'll add it to the docs. PTAL.

dprotaso · 2024-07-08T17:39:59Z

/lgtm
/approve

knative-prow · 2024-07-08T17:40:13Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dprotaso, ReToCode

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [ReToCode,dprotaso]
~~pkg/apis/OWNERS~~ [dprotaso]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

dprotaso · 2024-07-08T17:42:22Z

A follow question I have is - what happens when a user container restarts and it has a startup probe?

ReToCode · 2024-07-09T05:09:18Z

A follow question I have is - what happens when a user container restarts and it has a startup probe?

I think K8s runs the startup probe on every container creating (including a restarting one): kubernetes/kubernetes#102230

* Add startup probe to CRDs * Allow startup probes in Knative Services * Increase ProgressDeadlineSeconds when startup probe is present * Remove ProgressDeadlineSeconds increase when startup probe is present (cherry picked from commit ac7f585)

* Add startup probe support to Knative Service (knative#15309) * Add startup probe to CRDs * Allow startup probes in Knative Services * Increase ProgressDeadlineSeconds when startup probe is present * Remove ProgressDeadlineSeconds increase when startup probe is present (cherry picked from commit ac7f585) * Run make generated files

dprotaso · 2024-07-09T18:07:32Z

A follow question I have is - what happens when a user container restarts and it has a startup probe?

I think K8s runs the startup probe on every container creating (including a restarting one): kubernetes/kubernetes#102230

I guess to confirm does Knative behave correctly with your changes?

ReToCode · 2024-07-10T07:21:45Z

I guess to confirm does Knative behave correctly with your changes?

I think it looks good: https://github.com/ReToCode/knative-multicontainer-probing/blob/main/TESTING_STARTUP_PROBES.md#testing-startup-probe-with-liveness-failures

# Logs of the user container
user-container Liveness probe called, responding with:  true
user-container Liveness is now:  false
user-container Liveness probe called, responding with:  false
user-container Liveness probe called, responding with:  false
user-container Liveness probe called, responding with:  false
queue-proxy {"severity":"INFO","timestamp":"2024-07-10T07:18:02.386311375Z","logger":"queueproxy","caller":"sharedmain/handlers.go:109","message":"Attached drain handler from user-container&{GET /wait-for-drain HTTP/1.1 1 1 map[Accept:[*/*] Accept-Encoding:[gzip] User-Agent:[kube-lifecycle/1.28]] {} <nil> 0 ] false 10.244.2.47:8022 map] map] <nil> map] 10.244.2.1:57410 /wait-for-drain <nil> <nil> <nil> 0x4000289cc0 0x400013f260 ] map]}","commit":"2156812","knative.dev/key":"default/runtime-00001","knative.dev/pod":"runtime-00001-deployment-5d6cf5bbc9-brpdg"}
Stream closed EOF for default/runtime-00001-deployment-5d6cf5bbc9-brpdg (user-container) # restarted by K8s

# restarted user-container
user-container Starting server. Listening on port:  8080
queue-proxy {"severity":"INFO","timestamp":"2024-07-10T07:11:02.57859614Z","logger":"queueproxy","caller":"sharedmain/main.go:271","message":"Starting queue-proxy","commit":"2156812","knative.dev/key":"default/runtime-00001","knative.dev/pod":"runtime-00001-deployment-5d6cf5bbc9-brpdg"}
queue-proxy {"severity":"INFO","timestamp":"2024-07-10T07:11:02.57867039Z","logger":"queueproxy","caller":"sharedmain/main.go:277","message":"Starting http server metrics:9090","commit":"2156812","knative.dev/key":"default/runtime-00001","knative.dev/pod":"runtime-00001-deployment-5d6cf5bbc9-brpdg"}
queue-proxy {"severity":"INFO","timestamp":"2024-07-10T07:11:02.57870039Z","logger":"queueproxy","caller":"sharedmain/main.go:277","message":"Starting http server admin:8022","commit":"2156812","knative.dev/key":"default/runtime-00001","knative.dev/pod":"runtime-00001-deployment-5d6cf5bbc9-brpdg"}
queue-proxy {"severity":"INFO","timestamp":"2024-07-10T07:11:02.578705224Z","logger":"queueproxy","caller":"sharedmain/main.go:277","message":"Starting http server main:8012","commit":"2156812","knative.dev/key":"default/runtime-00001","knative.dev/pod":"runtime-00001-deployment-5d6cf5bbc9-brpdg"}
user-container Startup probe called, responding with:  false
user-container Startup probe called, responding with:  false

After the user-container is restarted, the startup probe is executed again.

knative-prow bot added the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label Jun 6, 2024

knative-prow bot requested review from dprotaso and skonto June 6, 2024 12:04

knative-prow bot assigned dprotaso, izabelacg and skonto Jun 6, 2024

knative-prow-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 6, 2024

ReToCode force-pushed the startup-probes branch from e2f4c8a to 0888fb9 Compare June 7, 2024 06:36

knative-prow-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 7, 2024

ReToCode mentioned this pull request Jun 10, 2024

How to add initial delay for Activator Probing time #15316

Closed

dprotaso reviewed Jun 19, 2024

View reviewed changes

ReToCode added 4 commits July 8, 2024 07:37

Add startup probe to CRDs

4b18053

Allow startup probes in Knative Services

4e2c269

Increase ProgressDeadlineSeconds when startup probe is present

9af692f

Remove ProgressDeadlineSeconds increase when startup probe is present

be536f3

ReToCode force-pushed the startup-probes branch from 0888fb9 to be536f3 Compare July 8, 2024 05:40

knative-prow bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Jul 8, 2024

ReToCode mentioned this pull request Jul 8, 2024

Adding startup probes knative/docs#6042

Merged

knative-prow bot added the lgtm Indicates that a PR is ready to be merged. label Jul 8, 2024

knative-prow bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 8, 2024

knative-prow bot merged commit ac7f585 into knative:main Jul 8, 2024
67 checks passed

ReToCode mentioned this pull request Jul 9, 2024

[RELEASE-1.14] add startup probe support to Knative Service openshift-knative/serving#758

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add startup probe support to Knative Service #15309

Add startup probe support to Knative Service #15309

ReToCode commented Jun 6, 2024 •

edited

Loading

ReToCode commented Jun 6, 2024

codecov bot commented Jun 6, 2024 •

edited

Loading

skonto commented Jun 19, 2024

dprotaso Jun 19, 2024

skonto Jun 20, 2024 •

edited

Loading

ReToCode Jul 1, 2024

ReToCode Jul 8, 2024

dprotaso commented Jul 8, 2024

knative-prow bot commented Jul 8, 2024

dprotaso commented Jul 8, 2024

ReToCode commented Jul 9, 2024

dprotaso commented Jul 9, 2024

ReToCode commented Jul 10, 2024 •

edited

Loading

Add startup probe support to Knative Service #15309

Add startup probe support to Knative Service #15309

Conversation

ReToCode commented Jun 6, 2024 • edited Loading

Context

Proposed Changes

ReToCode commented Jun 6, 2024

codecov bot commented Jun 6, 2024 • edited Loading

Codecov Report

skonto commented Jun 19, 2024

dprotaso Jun 19, 2024

Choose a reason for hiding this comment

skonto Jun 20, 2024 • edited Loading

Choose a reason for hiding this comment

ReToCode Jul 1, 2024

Choose a reason for hiding this comment

ReToCode Jul 8, 2024

Choose a reason for hiding this comment

dprotaso commented Jul 8, 2024

knative-prow bot commented Jul 8, 2024

dprotaso commented Jul 8, 2024

ReToCode commented Jul 9, 2024

dprotaso commented Jul 9, 2024

ReToCode commented Jul 10, 2024 • edited Loading

ReToCode commented Jun 6, 2024 •

edited

Loading

codecov bot commented Jun 6, 2024 •

edited

Loading

skonto Jun 20, 2024 •

edited

Loading

ReToCode commented Jul 10, 2024 •

edited

Loading