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

Replace dockerize with built-in healthcheck #1164

Merged
merged 5 commits into from
Feb 4, 2023
Merged

Replace dockerize with built-in healthcheck #1164

merged 5 commits into from
Feb 4, 2023

Conversation

Kailai-Wang
Copy link
Contributor

This PR replaces dockerize with docker healthcheck, reasons:

  • a built-in functionality of docker, thus more standard and doesn't rely on third-party services
  • better control over retrying/intervals
  • better integration with docker view (logs/inspect)

It uses nc to test the availability of the node and curl to test the workers.

@clangenb clangenb added B1-releasenotes C1-low 📌 Does not elevate a release containing this beyond "low priority" E0-breaksnothing A7-somethingelse labels Feb 4, 2023
clangenb
clangenb approved these changes Feb 4, 2023
View reviewed changes
Copy link
Contributor

@clangenb clangenb left a comment

Choose a reason for hiding this comment

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

Nice, looks good! Thanks for the simplifications. The lock file update seems unrelated, but I will accept it for now so that we can merge.

@clangenb clangenb merged commit abbaa70 into integritee-network:master Feb 4, 2023
@Kailai-Wang
Copy link
Contributor Author

Thanks! The Cargo.lock file was updated automatically when I compiled the repo -- I'm used to doing that to double-check it compiles, even though I didn't touch any related files.

@Kailai-Wang Kailai-Wang deleted the use-docker-healthcheck branch February 4, 2023 20:38
m-yahya pushed a commit to olisystems/BEST-Energy that referenced this pull request Feb 17, 2023
@Kailai-Wang @m-yahya
Replace dockerize with built-in healthcheck (integritee-network#1164)
1519e3b 
* replace dockerize with healthcheck

* bump image version

* use container name as hostname

* fix typo
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@clangenb clangenb clangenb approved these changes

Assignees
No one assigned
Labels
A7-somethingelse B1-releasenotes C1-low 📌 Does not elevate a release containing this beyond "low priority" E0-breaksnothing
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@Kailai-Wang @clangenb