Skip to content

docker: Use healthcheck endpoint #8640

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
Merged
merged 1 commit into from
Nov 3, 2022
Merged

docker: Use healthcheck endpoint #8640

merged 1 commit into from
Nov 3, 2022

Conversation

bt90
Copy link
Contributor

@bt90 bt90 commented Nov 3, 2022 •
edited
Loading

Purpose

Fixes #8638

Testing

Tested the cmd against a local instance with HTTP and HTTPS:

curl -fkLsS -m 2 127.0.0.1:8384/rest/noauth/health | grep -o --color=never OK || exit 1

Successful healthcheck:

OK

Failed healthcheck:

curl: (7) Failed to connect to 127.0.0.1 port 8384 after 0 ms: Connection refused

curl parameters:

  • -f Fail fast with no output at all on server errors. Otherwise error pages are passed to grep
  • -k Ignore certificate errors. Important if the UI is using HTTPS with a selfsigned certificate
  • -L Follow redirects. Handles the HTTP -> HTTPS redirect
  • -s Don't output download meter/progress
  • -S Print error messages to stderr
  • -m Abort after x seconds

@bt90 bt90 changed the title Use healthcheck endpoint docker: Use healthcheck endpoint Nov 3, 2022
calmh
calmh approved these changes Nov 3, 2022
View reviewed changes
Copy link
Member

@calmh calmh left a comment

Choose a reason for hiding this comment

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

Looks reasonable

@calmh calmh merged commit 1e9bf17 into syncthing:main Nov 3, 2022
@bt90 bt90 mentioned this pull request Nov 3, 2022
Merged
calmh added a commit to imsodin/syncthing that referenced this pull request Nov 8, 2022
@calmh
Merge branch 'main' into fix-8556
db49472 
* main: (36 commits)
  lib/protocol: Ignore inode time when xattr&ownership is ignored (fixes syncthing#8654) (syncthing#8655)
  lib/fs: Try to remove read only Windows files (fixes syncthing#3744) (syncthing#8650)
  gui: Add copy to clipboard, share by email, and share by SMS buttons to device IDs (fixes syncthing#2771, ref syncthing#3868) (syncthing#7984)
  gui, man, authors: Update docs, translations, and contributors
  build: Add GitHub actions build for Windows (syncthing#8627)
  gui: Fix connection type icon width (fixes syncthing#8592) (syncthing#8644)
  gui: Adjust connection type icon size scaling and alignment (syncthing#8645)
  docker: Use healthcheck endpoint (syncthing#8640)
  lib/connections: Use adaptive write size for rate limited connections (fixes syncthing#8630) (syncthing#8631)
  gui: Mark devices that haven't connected for a long time (fixes syncthing#7703) (syncthing#8530)
  gui: Fix rescan interval when add encrypted folder with watch for changes enabled (fixes syncthing#8570) (syncthing#8571)
  gui: Always show Out of Sync Items for remote devices (syncthing#8632)
  lib/fs: Let xattr test avoid non-test attributes (fixes syncthing#8601) (syncthing#8628)
  build: Add GitHub actions build for Windows
  gui, man, authors: Update docs, translations, and contributors
  gui: Display folder and device count number (syncthing#8615)
  gui, man, authors: Update docs, translations, and contributors
  lib/model, lib/protocol: Fix file comparisons (fixes syncthing#8594) (syncthing#8603)
  lib/scanner: More sensible debug output (syncthing#8596)
  gui: Allow automatic device ID selection on WebKit browsers (ref syncthing#8544) (syncthing#8597)
  ...
@calmh calmh added this to the v1.22.2 milestone Nov 9, 2022
@Tupsi
Copy link

Tupsi commented Nov 16, 2022

can you make the port number configurable? I changed my gui port to the std 443 in the settings and because I use macvlan as a network driver I can not change the ports through -p/ports. I am currently using this for a healthcheck in my docker compose which seems to work, so I might as well just change the test: line to what you have here. Should work as well, right?

    healthcheck:
      test: nc -z 127.0.0.1 443 || exit 1
#      test: curl -fkLsS -m 2 127.0.0.1:443/rest/noauth/health | grep -o --color=never OK || exit 1
      timeout: 1s
      interval: 1m0s
      retries: 1

@bt90
Copy link
Contributor Author

bt90 commented Nov 16, 2022

That should work.

@calmh would the STGUIADDRESS be appropriate to extract the port from?

@calmh
Copy link
Member

calmh commented Nov 16, 2022

I guess

calmh added a commit to calmh/syncthing that referenced this pull request Nov 22, 2022
@calmh
Merge branch 'main' into linuxbuild
4ee1015 
* main: (23 commits)
  lib/fs: Optimize WindowsInvalidFilename (syncthing#8687)
  gui, man, authors: Update docs, translations, and contributors
  cmd/syncthing: Use main logger in generate subcommand (fixes syncthing#8682) (syncthing#8685)
  build: Update all dependencies (fixes syncthing#8679) (syncthing#8680)
  gui, man, authors: Update docs, translations, and contributors
  lib/model: Correctly set xattrs on temp files (fixes syncthing#8667) (syncthing#8670)
  gui: Automatically dismiss authentication reminder when in LDAP mode (fixes syncthing#8661) (syncthing#8663)
  lib/model: Correctly handle xattrs on directories (fixes syncthing#8657) (syncthing#8658)
  lib/protocol: Ignore inode time when xattr&ownership is ignored (fixes syncthing#8654) (syncthing#8655)
  lib/fs: Try to remove read only Windows files (fixes syncthing#3744) (syncthing#8650)
  gui: Add copy to clipboard, share by email, and share by SMS buttons to device IDs (fixes syncthing#2771, ref syncthing#3868) (syncthing#7984)
  gui, man, authors: Update docs, translations, and contributors
  build: Add GitHub actions build for Windows (syncthing#8627)
  gui: Fix connection type icon width (fixes syncthing#8592) (syncthing#8644)
  gui: Adjust connection type icon size scaling and alignment (syncthing#8645)
  docker: Use healthcheck endpoint (syncthing#8640)
  lib/connections: Use adaptive write size for rate limited connections (fixes syncthing#8630) (syncthing#8631)
  gui: Mark devices that haven't connected for a long time (fixes syncthing#7703) (syncthing#8530)
  gui: Fix rescan interval when add encrypted folder with watch for changes enabled (fixes syncthing#8570) (syncthing#8571)
  gui: Always show Out of Sync Items for remote devices (syncthing#8632)
  ...
@Tupsi
Copy link

Tupsi commented Dec 6, 2022

This is tagged for 1.22.2 but is not in the current 1.22.2 image on docker hub as I mentioned #8705

calmh added a commit that referenced this pull request Dec 6, 2022
@calmh
docker: Update healthcheck cmd for buildx (ref #8640) (#8706)
d4b668c
crazygolem added a commit to crazygolem/mustash-compose that referenced this pull request Aug 18, 2023
@crazygolem
Sync syncthing dockerfile with upstream
798d85e 
Ported changes from upstream that were missed in past upgrades:

- Add env var to control capabilities (1.22.0 [8552])
- Use healthcheck endpoint (1.22.3 [8640])

[8552]: syncthing/syncthing#8552
[8640]: syncthing/syncthing#8640
@st-review st-review added the frozen-due-to-age Issues closed and untouched for a long time, together with being locked for discussion label Nov 4, 2023
@syncthing syncthing locked and limited conversation to collaborators Nov 4, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@calmh calmh calmh approved these changes

Assignees
No one assigned
Labels
frozen-due-to-age Issues closed and untouched for a long time, together with being locked for discussion
Projects
None yet
Milestone
v1.22.2
Development

Successfully merging this pull request may close these issues.

Use healtcheck endpoint for docker
4 participants
@bt90 @Tupsi @calmh @st-review