Skip to content

Chore: Avoid shell in Docker health check #444

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

Merged
merged 1 commit into from
Feb 15, 2025
Merged

Chore: Avoid shell in Docker health check #444

merged 1 commit into from
Feb 15, 2025

Conversation

scop
Copy link
Contributor

@scop scop commented Feb 14, 2025

Hardly significant, but... why not :)

https://docs.docker.com/reference/dockerfile/#healthcheck

Before (docker image inspect axllent/mailpit | jq ".[0].Config.Healthcheck.Test"):

[
  "CMD-SHELL",
  "/mailpit readyz"
]

After:

[
  "CMD",
  "/mailpit",
  "readyz"
]

@axllent
Copy link
Owner

axllent commented Feb 14, 2025

I fail to understand the benefit of this approach - are you able to explain it to me please?

@scop
Copy link
Contributor Author

scop commented Feb 15, 2025

As it stands, without this change, the healthcheck runs a shell in the container, which runs /mailpit readyz. With the change, the healthcheck runs /mailpit readyz directly, i.e. no need for the shell. Less is more.

@skyscooby
Copy link

I think HEALTHCHECK was moved to an optional extension and not really part of the standard OCI image spec .. not that it hurts anything having it but it's a very Docker specific thing now.

@axllent
Copy link
Owner

axllent commented Feb 15, 2025

Oh right, I understand. Thank you, accepted 👍

@axllent axllent merged commit bc3c057 into axllent:develop Feb 15, 2025
6 checks passed
@scop scop deleted the chore/healthcheck-no-shell branch February 16, 2025 18:25
tmeijn pushed a commit to tmeijn/dotfiles that referenced this pull request Mar 6, 2025
This MR contains the following updates:

| Package | Update | Change |
|---|---|---|
| [axllent/mailpit](https://github.com/axllent/mailpit) | minor | `v1.22.3` -> `v1.23.0` |

MR created with the help of [el-capitano/tools/renovate-bot](https://gitlab.com/el-capitano/tools/renovate-bot).

**Proposed changes to behavior should be submitted there as MRs.**

---

### Release Notes

<details>
<summary>axllent/mailpit (axllent/mailpit)</summary>

### [`v1.23.0`](https://github.com/axllent/mailpit/blob/HEAD/CHANGELOG.md#v1230)

[Compare Source](axllent/mailpit@v1.22.3...v1.23.0)

##### Feature

-   Add configuration to disable SQLite WAL mode for NFS compatibility
-   Add configuration to explicitly disable HTTP compression in web UI/API ([#&#8203;448](axllent/mailpit#448))
-   Add configuration to set message compression level in db (0-3) ([#&#8203;447](axllent/mailpit#447) & [#&#8203;448](axllent/mailpit#448))

##### Chore

-   Update node dependencies
-   Update Go dependencies
-   Minor speed & memory improvements when storing messages
-   Optimize ZSTD encoder for fastest compression of messages ([#&#8203;447](axllent/mailpit#447))
-   Handle BLOB storage for default database differently to rqlite to reduce memory overhead ([#&#8203;447](axllent/mailpit#447))
-   Avoid shell in Docker health check ([#&#8203;444](axllent/mailpit#444))

##### Fix

-   Display the correct STARTTLS or TLS runtime option on startup ([#&#8203;446](axllent/mailpit#446))

##### Testing

-   Add tests for message compression levels

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

♻ **Rebasing**: Whenever MR becomes conflicted, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this MR and you won't be reminded about this update again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this MR, check this box

---

This MR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).
<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzOS4xODIuNCIsInVwZGF0ZWRJblZlciI6IjM5LjE4Mi40IiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6WyJSZW5vdmF0ZSBCb3QiXX0=-->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants