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.

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

fix(deployment): simplify probes command to reduce false positives #135

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion charts/docker-mailserver/Chart.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ apiVersion: v2
appVersion: "14.0.0"
description: A fullstack but simple mailserver (smtp, imap, antispam, antivirus, ssl...) using Docker.
name: docker-mailserver
version: 4.0.6
version: 4.0.7
sources:
- https://github.com/docker-mailserver/docker-mailserver-helm
maintainers:
Expand Down
5 changes: 3 additions & 2 deletions charts/docker-mailserver/templates/deployment.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,8 @@ spec:
command:
- /bin/bash
- -c
- supervisorctl status | grep -E "amavis|clamav|cron|dovecot|mailserver|opendkim|opendmarc|postfix|rsyslog" | grep RUNNING
- supervisorctl status cron mailserver postfix rsyslog
# TODO: conditionally? add 'amavis changedetector clamav cron dovecot fail2ban fetchmail mta-sts-daemon opendkim opendmarc postgrey postsrsd rspamd rspamd-redis saslauthd_ldap saslauthd_mysql saslauthd_pam saslauthd_rimap saslauthd_shadow'
Copy link
Member

Choose a reason for hiding this comment

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

If you were wanting to do a broader status check like that, you might as well contribute it upstream so the call here could be a simple command still like dms-healthcheck?

That'd be a shell script that has a list of base features, followed by some logic to append services based on ENV (or probably the env file we write during startup that contains defaults/fallbacks). That way you're not querying status of services that are intentionally not running.

It'd also be simpler to maintain since it'd be in sync with the upstream DMS releases, and those not using helm can also benefit from the simplified healthcheck.

Regarding saslauthd, we only really support ldap and rimap AFAIK, the others were added by the contributor of saslauthd support but we have no test coverage or support offered for such. changedetector presently does not work with LDAP either.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ok, so how would we like to resolve this? I'm find with the PR, but @polarathene sounds like a good suggestion too?

initialDelaySeconds: 10
timeoutSeconds: 5
failureThreshold: 3
Expand All @@ -181,7 +182,7 @@ spec:
command:
- /bin/bash
- -c
- supervisorctl status | grep -E "mailserver|postfix" | grep RUNNING
- supervisorctl status mailserver postfix
initialDelaySeconds: 10
timeoutSeconds: 5
failureThreshold: 3
Expand Down
Loading