apache / camel-k

Apache Camel K is a lightweight integration platform, born on Kubernetes, with serverless superpowers
https://camel.apache.org/camel-k
Apache License 2.0
868 stars 348 forks source link

Make healt trait as default #5024

Open squakez opened 10 months ago

squakez commented 10 months ago

Considering the issue reported in #4977 and the improvements done in the monitoring, I wonder if we should enable by default the health trait. Right now, it's disabled by default, but I think we should change the behavior in order to have a more reliable experience when starting up an Integration which should be able to check at least the readiness probe before turning the Integration as running.

squakez commented 10 months ago

@lburgazzoli wdyt?

gansheer commented 10 months ago

That looks like a good idea but I wonder how would we work the default probe path on non-quarkus runtime ? As it is activating by default the health trait mean having readiness and liveness active by default. For now the default probe path are hard-coded here : https://github.com/apache/camel-k/blob/cae9e899510656f71fecc0131cc8a0aaf6e8fcfe/pkg/trait/health.go#L34-L36

squakez commented 10 months ago

In theory we should be safe as those endpoints should be exposed by Camel regardless the runtime used. I'd expect their availability also on non Quarkus runtimes.

lburgazzoli commented 10 months ago

@squakez I think it makes a lot of sense @gansheer you are right, as today camel-k assumes the runtime is quarkus hence the health endpoint path is hard-coded to the quarkus default one so once we add support for an additional runtime, then we have to make those path runtime dependant.

As a side note, we should start supporeting/sugin the quarkus/spring-boot management port for non "business" traffic

squakez commented 6 months ago

Enabling this trait by default (with readiness probe only) would have the following side effects [1]:

[1] https://github.com/apache/camel-k/actions/runs/8849672619?pr=5096

squakez commented 6 months ago
* It seems that the health is affecting the execution of cron (or the test which result in error). I don't have yet a clear understanding of what's happening.

The problem with the cron is the fact that our test is very quick to execute, therefore, the Pod is finished before reaching the Ready state. Maybe it makes sense that our tests have some artificial delay in order to simulate a workload that is above the few millis required to print a text to output.

cron-yaml-28574444-trd9t            0/1     Pending     0          0s
cron-yaml-28574444-trd9t            0/1     ContainerCreating   0          0s
cron-yaml-28574444-trd9t            0/1     Running             0          1s
cron-yaml-28574444-trd9t            0/1     Completed           0          3s
squakez commented 6 months ago

We must reopen this because it turned out the change would be a breaking compatibility one. More details in #5516.

github-actions[bot] commented 2 months ago

This issue has been automatically marked as stale due to 90 days of inactivity. It will be closed if no further activity occurs within 15 days. If you think that’s incorrect or the issue should never stale, please simply write any comment. Thanks for your contributions!

squakez commented 3 weeks ago

With the recent works done, the previous condition may no longer apply. I've marked this as a best effort to verify against 2.6.0. If not, we should move again into 3.x milestone bucket.