-
Notifications
You must be signed in to change notification settings - Fork 442
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat (jkube-kit/quarkus): Add startup probe in QuarkusHealthCheckEnricher (#1468) #1524
Conversation
Eclipse JKube CI ReportStarted new GH workflow run for #1524 (2022-06-09T15:47:50Z) ⚙️ JKube E2E Tests (2469448145)
|
Codecov Report
@@ Coverage Diff @@
## master #1524 +/- ##
============================================
+ Coverage 51.44% 51.46% +0.01%
- Complexity 3824 3828 +4
============================================
Files 459 459
Lines 20700 20707 +7
Branches 2816 2816
============================================
+ Hits 10649 10656 +7
Misses 8935 8935
Partials 1116 1116
Continue to review full report at Codecov.
|
Could you please update quarkus health check enricher documentation as well? |
Sure |
b34f2c6
to
38b3de1
Compare
Startup probe endpoint was only added to MicroProfile starting on 3.1. We need to check which versions (if not all) of Quarkus are based on a new version of MicroProfile to make sure that the enricher is only activated for these versions. |
38b3de1
to
fb7b5e2
Compare
jkube-kit/jkube-kit-quarkus/src/test/java/org/eclipse/jkube/quarkus/QuarkusUtilsTest.java
Show resolved
Hide resolved
fb7b5e2
to
f413280
Compare
…cher (eclipse-jkube#1468) Signed-off-by: Anurag Singh Rajawat <anuragsinghrajawat22@gmail.com>
…probes when Quarkus version >= 2.1 Looks like Quarkus only seems to support these new `/q/health/started` endpoints since `2.1.0.Final`. Add a check to only add startup probes in projects using Quarkus 2.1.x and later. Signed-off-by: Rohan Kumar <rohaan@redhat.com>
f413280
to
fe21e30
Compare
Kudos, SonarCloud Quality Gate passed! |
private static final int QUARKUS_MAJOR_VERSION_SINCE_STARTUP_CHANGE = 2; | ||
private static final int QUARKUS_MINOR_VERSION_SINCE_STARTUP_CHANGE = 1; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you please provide information on why we're checking for Quarkus 2.1 instead of Quarkus 2.0
AFAIK Smallrye health was updated to 3.0 in this PR quarkusio/quarkus#16333 which was released along with Quarkus 2.0
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just tried again on a simple Quarkus project with SmallRYE dependency. These were my observations:
- On 2.0.0.Final
/q/health/started
gives 404 - On 2.1.0.Final
q/health/started
gives 200 OK
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems that there was some work done on Quarkus in between the Smallrye dependency update and the startup probe wiring.
#1524 (comment) - @rohanKanojia:
|
Description
Fix: #1468
Type of change
test, version modification, documentation, etc.)
Checklist