Skip to content
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.

Sign up for GitHub

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

Jump to bottom

feat(rest): add health API for readiness and liveness probes #894

Merged
merged 2 commits into from
Dec 6, 2024
Merged

feat(rest): add health API for readiness and liveness probes #894

merged 2 commits into from
Dec 6, 2024

Conversation

niladrih
Copy link
Member

@niladrih niladrih commented Dec 3, 2024

No description provided.

@niladrih
Copy link
Member Author

niladrih commented Dec 3, 2024

Should I also add a readiness endpoint?

tiagolobocastro
tiagolobocastro reviewed Dec 3, 2024
View reviewed changes
control-plane/rest/service/src/main.rs Outdated Show resolved Hide resolved
control-plane/rest/service/src/main.rs Outdated Show resolved Hide resolved
@niladrih niladrih force-pushed the healthz branch 2 times, most recently from 2817ab5 to 3f62c14 Compare December 3, 2024 21:29
@niladrih niladrih changed the title feat(rest): add health server feat(rest): add health API for readiness and liveness probes Dec 3, 2024
@niladrih niladrih force-pushed the healthz branch 2 times, most recently from 2802fdd to 8776d97 Compare December 3, 2024 21:36
@niladrih niladrih force-pushed the healthz branch 2 times, most recently from c19a8ff to c8ea837 Compare December 4, 2024 17:52
@niladrih
Copy link
Member Author

niladrih commented Dec 4, 2024

@tiagolobocastro I'll work on a pytest and try to add it into this PR itself.

Abhinandan-Purkait
Abhinandan-Purkait approved these changes Dec 4, 2024
View reviewed changes
Copy link
Member

@Abhinandan-Purkait Abhinandan-Purkait left a comment

Choose a reason for hiding this comment

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

LGTM

@niladrih niladrih marked this pull request as draft December 5, 2024 05:55
@niladrih niladrih force-pushed the healthz branch 3 times, most recently from fe2b033 to 6d037e4 Compare December 6, 2024 06:21
@niladrih niladrih marked this pull request as ready for review December 6, 2024 06:21
@niladrih
Copy link
Member Author

niladrih commented Dec 6, 2024

@tiagolobocastro @Abhinandan-Purkait I've added a pytest for the readiness probe. The liveness probe is trivial and IMO doesn't need tests.

@niladrih
Copy link
Member Author

niladrih commented Dec 6, 2024

bors try

bors-openebs-mayastor bot pushed a commit that referenced this pull request Dec 6, 2024
Try #894:
f61e7cf
@bors-openebs-mayastor
Copy link

try

Build failed:

@bors-openebs-mayastor
Copy link

try

Build failed:

@niladrih
Copy link
Member Author

niladrih commented Dec 6, 2024

bors try

bors-openebs-mayastor bot pushed a commit that referenced this pull request Dec 6, 2024
Try #894:
e96656b
@bors-openebs-mayastor
Copy link

try

Build succeeded:

@niladrih
Copy link
Member Author

niladrih commented Dec 6, 2024

bors merge

bors-openebs-mayastor bot pushed a commit that referenced this pull request Dec 6, 2024
@niladrih
chore(bors): merge pull request #894
c3ae698 
894: feat(rest): add health API for readiness and liveness probes r=niladrih a=niladrih



Co-authored-by: Niladri Halder <[email protected]>
@niladrih
Copy link
Member Author

niladrih commented Dec 6, 2024

bors cancel

@bors-openebs-mayastor
Copy link

Canceled.

@niladrih
Copy link
Member Author

niladrih commented Dec 6, 2024

bors merge

bors-openebs-mayastor bot pushed a commit that referenced this pull request Dec 6, 2024
@niladrih
chore(bors): merge pull request #894
ca10816 
894: feat(rest): add health API for readiness and liveness probes r=niladrih a=niladrih



Co-authored-by: Niladri Halder <[email protected]>
@bors-openebs-mayastor
Copy link

Build failed:

@niladrih
Copy link
Member Author

niladrih commented Dec 6, 2024

@tiagolobocastro -- I've trimmed down on the wait times on this test, however, any lower than this and the test becomes flaky. WDYT?

@tiagolobocastro
Copy link
Contributor

@tiagolobocastro -- I've trimmed down on the wait times on this test, however, any lower than this and the test becomes flaky. WDYT?

I would say try 100's of milliseconds.
Most tests use that sort of magnitude.

If it's still flaky then fine keep as is.

@niladrih
Copy link
Member Author

niladrih commented Dec 6, 2024

@tiagolobocastro -- I've trimmed down on the wait times on this test, however, any lower than this and the test becomes flaky. WDYT?

I would say try 100's of milliseconds.
Most tests use that sort of magnitude.

If it's still flaky then fine keep as is.

I've tried 10s, 100s, quite a few... this is lowest it goes and stays fail free.

@niladrih
Copy link
Member Author

niladrih commented Dec 6, 2024

bors merge

@bors-openebs-mayastor
Copy link

Build succeeded:

@bors-openebs-mayastor bors-openebs-mayastor bot merged commit f6e9145 into develop Dec 6, 2024
4 checks passed
@bors-openebs-mayastor bors-openebs-mayastor bot deleted the healthz branch December 6, 2024 18:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tiagolobocastro tiagolobocastro tiagolobocastro approved these changes

@abhilashshetty04 abhilashshetty04 abhilashshetty04 approved these changes

@Abhinandan-Purkait Abhinandan-Purkait Abhinandan-Purkait approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@niladrih @tiagolobocastro @Abhinandan-Purkait @abhilashshetty04