-
Notifications
You must be signed in to change notification settings - Fork 102
add health check to dockerfiles #4346
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
base: master
Are you sure you want to change the base?
Conversation
90528f9
to
ea90b05
Compare
@@ -6,3 +6,4 @@ ADD cloud-sync-service /cloud-sync-service/ | |||
|
|||
CMD ["/cloud-sync-service/cloud-sync-service"] | |||
|
|||
HEALTHCHECK --interval=30s --timeout=5s --retries=3 CMD grep -q css_start.sh /proc/1/cmdline || exit 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.
[general] put HEALTHCHECK before CMD.
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.
OK. I'll update this for all images later.
@@ -6,3 +6,4 @@ ADD cloud-sync-service /cloud-sync-service/ | |||
|
|||
CMD ["/cloud-sync-service/cloud-sync-service"] | |||
|
|||
HEALTHCHECK --interval=30s --timeout=5s --retries=3 CMD grep -q css_start.sh /proc/1/cmdline || exit 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.
in some images you use grep ... /proc/1/cmdline
, in other pgrep
- could this be unified?
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.
Some images don't have pgrep. Nor do they have ps so I cannot use ps to get the process name. That is why I am using grep ... /proc/1/cmdline for these images
Signed-off-by: Wenyang Cao <[email protected]>
Signed-off-by: Wenyang Cao <[email protected]>
ea90b05
to
a7f2e7d
Compare
@omordyk @wenyang-cao Can we go ahead and cancel this PR ? Health checks are provided elsewhere |
Pull Request Template
Description
One of the important security triads is availability. Adding HEALTHCHECK instruction to your container image ensures that the docker engine periodically checks the running container instances against that instruction to ensure that the instances are still working.
Fixes # (issue)
Type of change
Please delete options that are not relevant.
How Has This Been Tested?
Build and run the images (except the ones for k8s, since k8s has its own liveness probe and I cannot really see if health check is working in this case, but adding health check to k8s images is still required for twistlick) locally to verify the health check is working.
Additional Context (Please include any Screenshots/gifs if relevant)
...
Checklist: