Skip to content
This repository has been archived by the owner on Jun 19, 2024. It is now read-only.

Fix #1444: Option to Disable Liveness/readiness checks #1569

Merged
merged 3 commits into from
Mar 13, 2019

Conversation

rohanKanojia
Copy link
Member

@rohanKanojia rohanKanojia commented Mar 12, 2019

Fix #1444

  • Added an option fabric8.disableHealthChecks to disable readiness/liveness checks
  • Added path option in SpringBootHealthCheckEnricher to overwrite path for liveness/readiness

Copy link
Contributor

@devang-gaur devang-gaur left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor nitpicks, otherwise LGTM. @lordofthejars let us know what you think of my comments. I'll approve if you deem it correct.

@@ -225,6 +225,9 @@
@Parameter(property = "fabric8.namespace")
private String namespace;

@Parameter(property = "fabric8.skipHealthChecks", defaultValue = "false")
private Boolean skipHealthChecks;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fabric8.disableProbes might be a better name since Its Health probe and liveness probe.

if not, atleast remove s , just fabric8.skipHealthCheck :)

| If the value is set to `true` then no readiness/liveness checks would be added to any containers.
| `false`

| *fabric8.openshift.deployTimeoutSeconds*
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not much of an issue, but maybe a seperate commit should be there for the 'extra' documentation, not relating to PR or commit description.

+ Added an option fabric8.disableHealthChecks to disable readiness/liveness checks
+ Added path option in SpringBootHealthCheckEnricher to overwrite path for liveness/readiness
@rohanKanojia
Copy link
Member Author

@dev-gaur : I've addressed your comments, going to merge now.

@rohanKanojia rohanKanojia merged commit f832592 into fabric8io:master Mar 13, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants