You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@camel.apache.org by "mertdotcc (via GitHub)" <gi...@apache.org> on 2023/03/28 15:13:18 UTC
[GitHub] [camel-k] mertdotcc opened a new pull request, #4182: add startup probes into the health trait
mertdotcc opened a new pull request, #4182:
URL: https://github.com/apache/camel-k/pull/4182
Completes (?) [#4146](https://github.com/apache/camel-k/issues/4146)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] squakez commented on a diff in pull request #4182: add startup probes into the health trait
Posted by "squakez (via GitHub)" <gi...@apache.org>.
squakez commented on code in PR #4182:
URL: https://github.com/apache/camel-k/pull/4182#discussion_r1150841810
##########
pkg/apis/camel/v1/trait/health.go:
##########
@@ -54,4 +54,19 @@ type HealthTrait struct {
ReadinessSuccessThreshold int32 `property:"readiness-success-threshold" json:"readinessSuccessThreshold,omitempty"`
// Minimum consecutive failures for the readiness probe to be considered failed after having succeeded.
ReadinessFailureThreshold int32 `property:"readiness-failure-threshold" json:"readinessFailureThreshold,omitempty"`
+
+ // Configures the startup probe for the integration container (default `true`).
+ StartupProbeEnabled *bool `property:"startup-probe-enabled" json:"startupProbeEnabled,omitempty"`
Review Comment:
I think the Kubernetes doc page [1] has the better explaination:
> The [kubelet](https://kubernetes.io/docs/reference/command-line-tools-reference/kubelet/) uses liveness probes to know when to restart a container. For example, liveness probes could catch a deadlock, where an application is running, but unable to make progress. Restarting a container in such a state can help to make the application more available despite bugs.
>
>The kubelet uses readiness probes to know when a container is ready to start accepting traffic. A Pod is considered ready when all of its containers are ready. One use of this signal is to control which Pods are used as backends for Services. When a Pod is not ready, it is removed from Service load balancers.
>
>The kubelet uses startup probes to know when a container application has started. If such a probe is configured, it disables liveness and readiness checks until it succeeds, making sure those probes don't interfere with the application startup. This can be used to adopt liveness checks on slow starting containers, avoiding them getting killed by the kubelet before they are up and running
I think since in the integration space we're typically dealing with backend services we're mostly interested in readiness (by default). However, the presence of the parameters will let the final user to configure according to her requirements.
[1] https://kubernetes.io/docs/tasks/configure-pod-container/configure-liveness-readiness-startup-probes/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] mertdotcc commented on pull request #4182: add startup probes into the health trait
Posted by "mertdotcc (via GitHub)" <gi...@apache.org>.
mertdotcc commented on PR #4182:
URL: https://github.com/apache/camel-k/pull/4182#issuecomment-1487141424
@squakez Could you please check again?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] mertdotcc commented on a diff in pull request #4182: add startup probes into the health trait
Posted by "mertdotcc (via GitHub)" <gi...@apache.org>.
mertdotcc commented on code in PR #4182:
URL: https://github.com/apache/camel-k/pull/4182#discussion_r1150846326
##########
pkg/apis/camel/v1/trait/health.go:
##########
@@ -54,4 +54,19 @@ type HealthTrait struct {
ReadinessSuccessThreshold int32 `property:"readiness-success-threshold" json:"readinessSuccessThreshold,omitempty"`
// Minimum consecutive failures for the readiness probe to be considered failed after having succeeded.
ReadinessFailureThreshold int32 `property:"readiness-failure-threshold" json:"readinessFailureThreshold,omitempty"`
+
+ // Configures the startup probe for the integration container (default `true`).
+ StartupProbeEnabled *bool `property:"startup-probe-enabled" json:"startupProbeEnabled,omitempty"`
Review Comment:
Perfect explanation, thank you! Puts things into perspective.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] mertdotcc closed pull request #4182: add startup probes into the health trait
Posted by "mertdotcc (via GitHub)" <gi...@apache.org>.
mertdotcc closed pull request #4182: add startup probes into the health trait
URL: https://github.com/apache/camel-k/pull/4182
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] gansheer commented on a diff in pull request #4182: add startup probes into the health trait
Posted by "gansheer (via GitHub)" <gi...@apache.org>.
gansheer commented on code in PR #4182:
URL: https://github.com/apache/camel-k/pull/4182#discussion_r1151523722
##########
e2e/common/traits/health_test.go:
##########
@@ -29,11 +29,8 @@ import (
"testing"
"time"
- . "github.com/onsi/gomega"
Review Comment:
I think these removals might be the source of the IT's failure. You need to add them again.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] mertdotcc commented on pull request #4182: add startup probes into the health trait
Posted by "mertdotcc (via GitHub)" <gi...@apache.org>.
mertdotcc commented on PR #4182:
URL: https://github.com/apache/camel-k/pull/4182#issuecomment-1487214849
@squakez Could you check it one more time? Not sure about the test tbh.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] mertdotcc commented on a diff in pull request #4182: add startup probes into the health trait
Posted by "mertdotcc (via GitHub)" <gi...@apache.org>.
mertdotcc commented on code in PR #4182:
URL: https://github.com/apache/camel-k/pull/4182#discussion_r1150830799
##########
pkg/apis/camel/v1/trait/health.go:
##########
@@ -54,4 +54,19 @@ type HealthTrait struct {
ReadinessSuccessThreshold int32 `property:"readiness-success-threshold" json:"readinessSuccessThreshold,omitempty"`
// Minimum consecutive failures for the readiness probe to be considered failed after having succeeded.
ReadinessFailureThreshold int32 `property:"readiness-failure-threshold" json:"readinessFailureThreshold,omitempty"`
+
+ // Configures the startup probe for the integration container (default `true`).
+ StartupProbeEnabled *bool `property:"startup-probe-enabled" json:"startupProbeEnabled,omitempty"`
Review Comment:
Sure, I was confused with this anyway. Could you explain why the liveness probe defaults to false but the readiness one defaults to true?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] squakez commented on pull request #4182: add startup probes into the health trait
Posted by "squakez (via GitHub)" <gi...@apache.org>.
squakez commented on PR #4182:
URL: https://github.com/apache/camel-k/pull/4182#issuecomment-1487097102
Nice! The structure is there and looks good. Now you should use those parameters to configure the probe on Kubernetes. I think you can mimic the usage of the readiness and liveness probes to have it working.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] gansheer commented on a diff in pull request #4182: add startup probes into the health trait
Posted by "gansheer (via GitHub)" <gi...@apache.org>.
gansheer commented on code in PR #4182:
URL: https://github.com/apache/camel-k/pull/4182#discussion_r1150849243
##########
pkg/apis/camel/v1/trait/health.go:
##########
@@ -54,4 +54,19 @@ type HealthTrait struct {
ReadinessSuccessThreshold int32 `property:"readiness-success-threshold" json:"readinessSuccessThreshold,omitempty"`
// Minimum consecutive failures for the readiness probe to be considered failed after having succeeded.
ReadinessFailureThreshold int32 `property:"readiness-failure-threshold" json:"readinessFailureThreshold,omitempty"`
+
+ // Configures the startup probe for the integration container (default `true`).
+ StartupProbeEnabled *bool `property:"startup-probe-enabled" json:"startupProbeEnabled,omitempty"`
Review Comment:
It could also be linked to what is written at the beginning of this PR https://github.com/apache/camel-k/pull/2719, it is preferable to have it enabled to use it to reconcile the Integration phase and readiness condition.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] mertdotcc commented on pull request #4182: add startup probes into the health trait
Posted by "mertdotcc (via GitHub)" <gi...@apache.org>.
mertdotcc commented on PR #4182:
URL: https://github.com/apache/camel-k/pull/4182#issuecomment-1487083887
@squakez open to any feedback!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] squakez commented on a diff in pull request #4182: add startup probes into the health trait
Posted by "squakez (via GitHub)" <gi...@apache.org>.
squakez commented on code in PR #4182:
URL: https://github.com/apache/camel-k/pull/4182#discussion_r1150821328
##########
pkg/apis/camel/v1/trait/health.go:
##########
@@ -54,4 +54,19 @@ type HealthTrait struct {
ReadinessSuccessThreshold int32 `property:"readiness-success-threshold" json:"readinessSuccessThreshold,omitempty"`
// Minimum consecutive failures for the readiness probe to be considered failed after having succeeded.
ReadinessFailureThreshold int32 `property:"readiness-failure-threshold" json:"readinessFailureThreshold,omitempty"`
+
+ // Configures the startup probe for the integration container (default `true`).
+ StartupProbeEnabled *bool `property:"startup-probe-enabled" json:"startupProbeEnabled,omitempty"`
Review Comment:
I'd inclined to think this one should default to false
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org