Comments (14)
yeah, not a problem from my end. I'll share my investigation findings before implementing anything.
from jkube.
So then, I guess it would be better to completely replace the global endpoint with two separate specific endpoints for liveness and readiness.
We should give precedence to specific endpoints when we detect user has explicitly configured them. Otherwise, we can stick to current behavior.
from jkube.
@manusa / @rohanKanojia, I'm sorry I haven't been able to get to this for a while, I've been cramped up with work from my day job. I'll try to propose a solution for this by the end of next week. I hope that's fine.
from jkube.
Hi @manusa / @rohanKanojia, I'm really sorry, I haven't been able to give this issue a considerable amount of time. Though, I do have a plan to implement this through by adding two additional constants to Config
enum for liveness probe path
& readiness probe path
and replacing any instances of PATH
with a conditional statement. I still need some time to test this idea and make it as simple and readable as possible.
from jkube.
@rohanKanojia : Oh, okay. I thought of creating the two separate Config
fields for LIVENESS_PROBE_PATH
& READINESS_PROBE_PATH
in addition to the PATH
already present and then add a new field to SpringBootConfiguration class for management.health.probes.enabled
, but it would indeed be much easier to add a suffix based on the value of springBootConfiguration.managementHealthProbesEnabled
as the base path already matches both of the specific parts.
Thanks for your insight. I'll try to implement this in code and test the result this week and update.
from jkube.
@manusa / @rohanKanojia : Wanted to give both of you an update, I've implemented the changes and now, I'm working on Unit Tests for the same.
from jkube.
@manusa / @rohanKanojia : I am willing to pick this up if it is available for contributors. Kindly assign this to me.
from jkube.
@l3002 : We need to figure out what property gets set whenever probes are enabled, also is the behavior the same when Spring boot configuration is provided in the form of application.yml
instead of application.properties
.
Are you okay with investigating this part first and then sharing your findings?
from jkube.
Hi @manusa / @rohanKanojia: After doing a bit research on the topic, I'm unable to understand the point of changing the endpoints with the explicit actuator endpoint. Correct me if I'm wrong that we require LivenessStateHealthIndicator
and ReadinessStateHealthIndicator
from the endpoint to check the liveness and readiness of the application while it is deployed on kubernetes. If that's the case then, I don't think we require explicit endpoints for retrieving this data from actuators as per below:
Both the global endpoint /actuator/health
and explicit endpoints /acutator/health/liveness
& /acutator/health/readiness
can be used to retrieve the data. So, I'm not sure if this is required.
But If this still holds some importance then I guess we just need to set the value for PATH
in config with regards to the project configuration in SpringBootHealthCheckEnricher$Config
enum:
from jkube.
Both the global endpoint
/actuator/health
and explicit endpoints/acutator/health/liveness
&/acutator/health/readiness
can be used to retrieve the data. So, I'm not sure if this is required.
Path to documentation https://docs.spring.io/spring-boot/docs/current/reference/html/actuator.html#actuator.endpoints.kubernetes-probes
We don't know if the user has customized the health checks with specific checks for readiness and liveness (which are different concepts, Kubernetes acts differently upon failure of the probe -liveness failure triggers restart, readiness failure isolates the pod for a period until healthy.).
It's important that we respect this and that we define the probe as per the specific endpoints instead of the global endpoint.
from jkube.
Okay, I understand it now. So then, I guess it would be better to completely replace the global endpoint with two separate specific endpoints for liveness and readiness. Would that be fine?
from jkube.
@l3002 : polite ping, Did you get any time to revisit this issue? If not, shall I unassign you from this issue?
from jkube.
@rohanKanojia: I'm still working on this, I'm trying to figure out, All the places where the changes are needed, after changing the Config
Enum. Though, I haven't been able to give this issue much time since past couple of weeks, I'm still working on it and possibly will be able to close this by 2nd week of the next month.
As this is a part of enhancement, I'm trying to be as cautious as possible. Just don't want to accidently break anything, this is a pretty big issue for me.
from jkube.
I'm trying to figure out, All the places where the changes are needed, after changing the Config Enum.
I think my original motivation behind creating this issue was to auto detect this configuration. I didn't mean to add a new configuration option for this. Let me try to explain step by step what needs to be done:
- Add a
boolean
field in SpringBootConfiguration class namedmanagementHealthProbesEnabled
. This would be storing value of detectedmanagement.health.probes.enabled
property. You would need to add an equivalent builder method call for this new field here:
- SpringBootHealthCheckEnricher would read the value of
springBootConfiguration.isManagementHealthProbesEnabled()
to decide whether to go with generic health check path/actuator/health
or specifics/actuator/health/liveness
//actuator/health/readiness
- Right now
buildProbe
has no information about what kind of probe it's building, maybe we can add anotherString
field calledactuatorPathSuffix
that will base passed as/readiness
and/liveness
fromgetReadinessProbe()
andgetLivenessProbe()
respectively. WhenspringBootConfiguration.isManagementHealthProbesEnabled()
would evaluate totrue
, we will append these suffixes toactuatorBasePath
if there is no explicitConfig.PATH
provided. - Add tests verifying above mentioned changes work as expected
from jkube.
Related Issues (20)
- [OpenShift Maven Plugin] : Move duplicated methods in Mojos to a common interface HOT 2
- JavaExecGenerator does not honor %t setting HOT 4
- How can I use the org.eclipse.jkube.kubernetes plugin in Gradle Kotlin DSL? HOT 6
- DockerImageWatcherRestartContainerTest : Replace AssertJ's deprecated `asList()` DSL method with `asInstanceOf(InstanceOfAssertFactories.list(type.class))` HOT 1
- WildflyJARHealthCheckEnricherTest : Replace AssertJ's deprecated `asList()` DSL method with `asInstanceOf(InstanceOfAssertFactories.list(type.class))` HOT 1
- SpringBootWatcherIntegrationTest is failing on windows HOT 1
- BuildService : Replace outdated method for joining strings with inbuilt `String.join` HOT 8
- OpenShiftResourceMojoTest is failing on Windows due to use of `\n` as line separator HOT 4
- Remove workaround from KubernetesMockServerUtil once issue is fixed in KubernetesMockServer
- Replace deprecated method from Updatable interface HOT 2
- Incompatible descriptor's return type: ProcessorConfig HOT 4
- BuildOptions 'buildArgs.size() > 0' can be replaced with '!buildArgs.isEmpty()' HOT 1
- BuildService 'nocache.length() == 0' can be replaced with 'nocache.isEmpty()' HOT 1
- CommandLine 'toProcess.length() == 0' can be replaced with 'toProcess.isEmpty()'
- DockerFileBuilderTest 'line.trim().length() == 0' can be replaced with 'line.trim().isEmpty()' HOT 1
- GoTimeUtil 'duration.length() == 0' can be replaced with 'duration.isEmpty()' HOT 3
- ImageChangeTriggerEnricher 'containerToImageMap.size() != 0' can be replaced with '!containerToImageMap.isEmpty()' HOT 5
- Remove duplicate dependency from `quickstarts/maven/micronaut4` `pom.xml` HOT 1
- `KubernetesHelperTest.exportKubernetesClientConfigToFile_worksWithKubernetesMockServer` is failing on windows HOT 5
- fix failing unit tests in `gradle-plugin/openshift` module HOT 1
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from jkube.