Skip default probe if container freezer in use #12967
Conversation
Signed-off-by: Paul S. Schweigert <paulschw@us.ibm.com> Paused containers and readiness probes are not the best of friends. Kubernetes has no concept of a paused container, and so the container freezer does a bit of an end around to actually freeze the container (using the container runtime rather than doing it through Kubernetes). However, because we're doing an end around, paused containers show as not ready in Kubernetes... and not ready means that the readiness probe we default in will eventually fail the container. In practice what happens is a bunch of "aggressive probe errors" before queue proxy basically becomes unresponsive. To prevent this from happening, this PR proposes disabling the default probe when the freezer is enabled (which is done by setting a value for the concurrency state endpoint). There's future work in the pipeline that may render this change moot at some point down the line (such as switching the default probe to a startup one), but for now this provides a quick solution to make the container freezer usable outside of demos / toy environments.
knative-prow
bot
added
size/XS
|
/approve cancel |
knative-prow
bot
removed
the
approved
|
Not entirely convinced this approach is the right way to go, but given that there's been some interest in the freezer since KnativeCon, I thought it would be good to remove one of the main barriers from folks being able to use it... |
Codecov Report
@@ Coverage Diff @@
## main #12967 +/- ##
==========================================
- Coverage 87.01% 87.00% -0.02%
==========================================
Files 197 197
Lines 14437 14439 +2
==========================================
Hits 12562 12562
- Misses 1580 1582 +2
Partials 295 295
Continue to review full report at Codecov.
|
| probe := func() bool { return true } | ||
| if env.ServingReadinessProbe != "" { | ||
| if env.ServingReadinessProbe != "" && env.ConcurrencyStateEndpoint == "" { |
There was a problem hiding this comment.
How about putting this behind a flag since you are not 100% convinced with it, even if enabled by default, but at least allows users to turn it off if they want to?
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: dprotaso 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:
Approvers can indicate their approval by writing |
knative-prow
bot
added
the
approved
Signed-off-by: Paul S. Schweigert paulschw@us.ibm.com
Paused containers and readiness probes are not the best of friends.
Kubernetes has no concept of a paused container, and so the container
freezer does a bit of an end around to actually freeze the
container (using the container runtime rather than doing it
through Kubernetes).
However, because we're doing an end around, paused containers show as
not ready in Kubernetes... and not ready means that the readiness
probe we default in will eventually fail the container. In practice
what happens is a bunch of "aggressive probe errors" before queue
proxy basically becomes unresponsive.
To prevent this from happening, this PR proposes disabling the default
probe when the freezer is enabled (which is done by setting a value
for the concurrency state endpoint). There's future work in the
pipeline that may render this change moot at some point down the
line (such as switching the default probe to a startup one), but for
now this provides a quick solution to make the container freezer
usable outside of demos / toy environments.
/assign @dprotaso @nader-ziada
Release Note