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

f8-spring-boot-health-check ignores management context-path #1014

Closed
rasheedamir opened this issue Aug 7, 2017 · 14 comments
Closed

f8-spring-boot-health-check ignores management context-path #1014

rasheedamir opened this issue Aug 7, 2017 · 14 comments
Labels
cat/bug Bug which needs fixing

Comments

@rasheedamir
Copy link

Description

f8-spring-boot-health-check produces probe but ignores the context-path.

It produces:

[INFO] F8: spring-boot-health-check: Adding readiness probe on port 8080, path='/health', scheme='HTTP', with initial delay 10 seconds
[INFO] F8: spring-boot-health-check: Adding liveness probe on port 8080, path='/health', scheme='HTTP', with initial delay 180 seconds

Whereas it must produce:

[INFO] F8: spring-boot-health-check: Adding readiness probe on port 8080, path='/management/health', scheme='HTTP', with initial delay 10 seconds
[INFO] F8: spring-boot-health-check: Adding liveness probe on port 8080, path='/management/health', scheme='HTTP', with initial delay 180 seconds

Info

  • f-m-p version : 3.5.22
  • Maven version (mvn -v) :
Apache Maven 3.5.0 (ff8f5e7444045639af65f6095c62210b5713f426; 2017-04-03T19:39:06Z)
Maven home: /home/jhipster/.m2/wrapper/dists/apache-maven-3.5.0-bin/6ps54u5pnnbbpr6ds9rppcc7iv/apache-maven-3.5.0
Java version: 1.8.0_131, vendor: Oracle Corporation
Java home: /usr/lib/jvm/java-8-openjdk-amd64/jre
Default locale: en_US, platform encoding: ANSI_X3.4-1968
OS name: "linux", version: "4.9.36-moby", arch: "amd64", family: "unix"
  • Kubernetes / OpenShift setup and version :
    Kubernetes 7.3

  • If it's a bug, how to reproduce :
    Just clone the repo; and run the fabric8 maven plugin.

  • Sample project : [GitHub Clone URL]
    As you can see that management.context-path has been defined but its ignored:

https://github.com/stakater-spring-microservice/MovieManager/blob/master/src/main/resources/config/application.yml#L2

@rasheedamir
Copy link
Author

I found out one thing:

I picked up the spring-boot-webmvc from @jstrachan.

Then I added management.context-path and I found out that the probes do have the context-path in it.

But then I moved application.properties to a subdirectory under resources folder and then the probes don't have the context-path in them.

https://github.com/stakater-spring-microservice/spring-boot-webmvc/blob/master/src/main/resources/config/application.properties

So, I think its something related to how application.properties is found in the classpath.

@rhuss any thoughts?

@rhuss
Copy link
Contributor

rhuss commented Aug 8, 2017

I guess this is connected how f-m-p tries to detect the application.yml and it doesn't find it if in a non-'standard' place. @nicolaferraro could you have a look if time permits ?

@rasheedamir
Copy link
Author

that will be awesome! @nicolaferraro plz let me know if we can help to fix it

@nicolaferraro
Copy link
Member

Indeed, the application.properties or application.yml files are taken from the classpath root, that is the default location for spring-boot property files.

I've now seen here that (apart from directory location that we cannot manag in the plugin) the special /config classpath subdirectory can be used.

It should not be difficult to fix it. The lookup mechanism is centralized in the SpringBootUtil class. I don't know what happens when the user puts multiple application.properties and application.yaml or application.yml files in the two locations (users are bad sometimes 😄). It is something we should check in spring-boot.

@rasheedamir it would be awesome if you had some time to check the correct behaviour and fix this issue. Let me know 😉.

@rasheedamir
Copy link
Author

@nicolaferraro let us take a look!

@hrishin
Copy link
Member

hrishin commented Aug 10, 2017

@nicolaferraro @rasheedamir if i'm not wrong program has to lookup the properties file into src/resources directory because these files doesn't exist under target directory during resource goal execution where enrichers get invoked (correct me if its misunderstanding).

Possible solutions

  1. locate files in src directory
  2. handle configurations from enrichers config section only
  3. else any other suggestion?

@hrishin
Copy link
Member

hrishin commented Aug 10, 2017

How about adding path, schema config for springboot-health-check? like wildfly-swarm-health-check has

<enricher>
        <config>
               <spring-boot-health-check>
                       <port></port>
                       <scheme></scheme>
                       <path></path>
               </spring-boot-health-check>
        </config>
</enricher>

Precedence here could be?

  1. properties file -> enricher configuration -> default config
  2. enricher configuration -> properties file -> default config

@nicolaferraro
Copy link
Member

It seems there's already a way to define custom probes by providing a probeconfig resource (see here).

I think we should check if it is working (and maybe include a it test) and document it properly, because custom probe definition should not be related to a particular enricher.

@hrishin
Copy link
Member

hrishin commented Aug 10, 2017

oh nice :)

<resources>
   <readiness>
      <getUrl>/monitor/</getUrl>
   </readiness>
</resources>

Are you talking about this?
If yes then somehow it didint worked for me.

@nicolaferraro
Copy link
Member

Yes, I can't recall when it has been added, but it should be fixed.

I do remember there was a similar issue with a sibling config (<resources><env>) as environment variables were not set correctly in the container (issue here).

The solution was to move the code to a specific enricher, so that it could be applied no matter how the container is created... We can provide a "custom probe enricher" that has priority over the other enrichers if the current way of handling things is problematic.

@nicolaferraro
Copy link
Member

@hrishin I've created another issue for this problem (#1019).

@stale
Copy link

stale bot commented Oct 4, 2018

This issue has been automatically marked as stale because it has not had any activity since 90 days. It will be closed if no further activity occurs within 7 days. Thank you for your contributions!

@stale stale bot added the status/stale Issue/PR considered to be stale label Oct 4, 2018
@rhuss
Copy link
Contributor

rhuss commented Oct 5, 2018

@nicolaferraro do you think this is still an issue ?

@stale stale bot removed the status/stale Issue/PR considered to be stale label Oct 5, 2018
@rhuss rhuss added the cat/bug Bug which needs fixing label Oct 5, 2018
@rohanKanojia
Copy link
Member

Duplicate of #1019

@rohanKanojia rohanKanojia marked this as a duplicate of #1019 Oct 6, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
cat/bug Bug which needs fixing
Projects
None yet
Development

No branches or pull requests

5 participants