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

chore: add docker healthcheck to SIPI image (INFRA-130) #2359

Merged
merged 4 commits into from Jan 5, 2023
Merged

chore: add docker healthcheck to SIPI image (INFRA-130) #2359

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
8ea899d
add docker healthcheck for SIPI
SamuelBoerlin Jan 2, 2023
90eb5cd
Change healthcheck curl address to localhost
SamuelBoerlin Jan 3, 2023
c5b5039
Merge branch 'main' into wip/INFRA-130-sipi-docker-healthcheck
SamuelBoerlin Jan 3, 2023
a104602
Merge branch 'main' into wip/INFRA-130-sipi-docker-healthcheck
SamuelBoerlin Jan 5, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 4 additions & 0 deletions build.sbt
View file
Open in desktop
Expand Up @@ -110,6 +110,10 @@ lazy val sipi: Project = Project(id = "sipi", base = file("sipi"))
Universal / mappings ++= {
directory("sipi/scripts")
},
dockerCommands += Cmd(
"""HEALTHCHECK --interval=15s --timeout=5s --retries=3 --start-period=30s \
|CMD bash /sipi/scripts/healthcheck.sh || exit 1""".stripMargin
),
// use filterNot to return all items that do NOT meet the criteria
dockerCommands := dockerCommands.value.filterNot {

Expand Down
3 changes: 3 additions & 0 deletions sipi/scripts/healthcheck.sh
View file
Open in desktop
@@ -0,0 +1,3 @@
#!/bin/bash

curl -sS --fail 'http://localhost:1024/server/test.html'
Copy link

Choose a reason for hiding this comment

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

We usually use 0.0.0.0 instead of localhost, so I think it would be better to stay consistent here as well, although it doesn't make a difference here.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I guess this would not work, as 0.0.0.0 is not a synonym for localhost (127.0.0.1 mostly is), you can try it out on your local machine:

~❯ ping localhost
PING localhost (127.0.0.1): 56 data bytes
64 bytes from 127.0.0.1: icmp_seq=0 ttl=64 time=0.108 ms
^C
--- localhost ping statistics ---
1 packets transmitted, 1 packets received, 0.0% packet loss
round-trip min/avg/max/stddev = 0.108/0.108/0.108/0.000 ms
~❯ ping 127.0.0.1
PING 127.0.0.1 (127.0.0.1): 56 data bytes
64 bytes from 127.0.0.1: icmp_seq=0 ttl=64 time=0.106 ms
64 bytes from 127.0.0.1: icmp_seq=1 ttl=64 time=0.346 ms
^C
--- 127.0.0.1 ping statistics ---
2 packets transmitted, 2 packets received, 0.0% packet loss
round-trip min/avg/max/stddev = 0.106/0.226/0.346/0.120 ms
~❯ ping 0.0.0.0
PING 0.0.0.0 (0.0.0.0): 56 data bytes
ping: sendto: Socket is not connected
Request timeout for icmp_seq 0
ping: sendto: Socket is not connected
Request timeout for icmp_seq 1
^C
--- 0.0.0.0 ping statistics ---
3 packets transmitted, 0 packets received, 100.0% packet loss

Copy link

Choose a reason for hiding this comment

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

I've just tried it locally and for me it works (when the stack is running).
@SamuelBoerlin , feel free to ignore my comment. The only reason I think consistency would be nice is when we want to change 0.0.0.0 to something else - then we would find all occurrences. But maybe this is not relevant for this case.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes I had it as 0.0.0.0 first and it worked in this specific case, but to me it's sort of unclear what happens when that address is used as destination. On some system it seems to behave like localhost, on some not so much. So I think it's better to use localhost or 127.0.0.1.