Skip to content

Prevent race condition on supervisorctl reload#2343

Merged
casperklein merged 2 commits intodocker-mailserver:masterfrom
casperklein:supervisorctl
Dec 29, 2021
Merged

Prevent race condition on supervisorctl reload#2343
casperklein merged 2 commits intodocker-mailserver:masterfrom
casperklein:supervisorctl

Conversation

@casperklein
Copy link
Copy Markdown
Member

@casperklein casperklein commented Dec 25, 2021 •
edited
Loading

Description

When setting SUPERVISOR_LOGLEVEL e.g. to info, supervisord will be restarted (don't get fooled by the term 'reload'):

docker-mailserver/target/scripts/startup/setup-stack.sh

Lines 12 to 23 in 701037d

function _setup_supervisor
{
if ! grep -q "loglevel = ${SUPERVISOR_LOGLEVEL}" /etc/supervisor/supervisord.conf
then
case "${SUPERVISOR_LOGLEVEL}" in
'critical' | 'error' | 'info' | 'debug' )
sed -i -E \
"s|(loglevel).*|\1 = ${SUPERVISOR_LOGLEVEL}|g" \
/etc/supervisor/supervisord.conf
supervisorctl reload
;;

The restart however, can take some time. While this time period, start-mailserver.sh continues to run until terminated by supervisord. This can lead to errors, if files are modified (by our numerous sed-operations).

When supervisord is completely restarted, also start-mailserver.sh is started again. Files that already have been modified by the first run, will now be reported as errors.

This is how a problematic log looks like (empty lines added for readability):

[ TASKLOG ]  Welcome to docker-mailserver 10.4.0
[ TASKLOG ]  Initializing setup
[ TASKLOG ]  Checking configuration
[ TASKLOG ]  Configuring mail server
Restarted supervisord

[ WARNING ]  No DKIM key provided. Check the documentation on how to get your keys.
[ WARNING ]  Spam messages WILL NOT BE DELIVERED, you will NOT be notified of ANY message bounced. Please define SPAMASSASSIN_SPAM_TO_INBOX explicitly.
[ WARNING ]  Clamav is disabled. You can enable it with 'ENABLE_CLAMAV=1'

2021-12-25 22:50:36,774 INFO Included extra file "/etc/supervisor/conf.d/saslauth.conf" during parsing
2021-12-25 22:50:36,774 INFO Included extra file "/etc/supervisor/conf.d/supervisor-app.conf" during parsing
2021-12-25 22:50:36,774 INFO Set uid to user 0 succeeded
2021-12-25 22:50:36,774 INFO RPC interface 'supervisor' initialized
2021-12-25 22:50:36,775 INFO supervisord started with pid 7
2021-12-25 22:50:37,779 INFO spawned: 'mailserver' with pid 2008

[ TASKLOG ]  Welcome to docker-mailserver 10.4.0
2021-12-25 22:50:37,802 INFO success: mailserver entered RUNNING state, process has stayed up for > than 0 seconds (startsecs)
[ TASKLOG ]  Initializing setup
[ TASKLOG ]  Checking configuration
[ TASKLOG ]  Configuring mail server
[ WARNING ]  No DKIM key provided. Check the documentation on how to get your keys.
Nameservers 127.0.0.11
Error: sed -i -r -e s|^(MinProtocol).*|\1 = TLSv1| -e s|^(CipherString).*|\1 = DEFAULT@SECLEVEL=1| /usr/lib/ssl/openssl.cnf
Error: sed -i -r s|^(smtpd_tls_chain_files =).*|\1 /etc/dms/tls/key /etc/dms/tls/cert| /etc/postfix/main.cf
Error: sed -i -r -e s|^(ssl_key =).*|\1 </etc/dms/tls/key| -e s|^(ssl_cert =).*|\1 </etc/dms/tls/cert| /etc/dovecot/conf.d/10-ssl.conf

1st part: start-mailserver.sh is running and restarts supervisord
2nd part: start-mailserver.sh keeps running for a bit
3rd/4th part: start-mailserver.sh started again (see the 'Welcome to docker-mailserver 10.4.0' message). Notice the sed errors, because sed was run a second time.

To prevent this, start-mailserver.sh should exit after calling supervisorctl reload.

After applying this fix, the log looks like this:

[ TASKLOG ]  Welcome to docker-mailserver 10.4.0
[ TASKLOG ]  Initializing setup
[ TASKLOG ]  Checking configuration
[ TASKLOG ]  Configuring mail server
Restarted supervisord

2021-12-25 23:37:51,922 INFO Included extra file "/etc/supervisor/conf.d/saslauth.conf" during parsing
2021-12-25 23:37:51,922 INFO Included extra file "/etc/supervisor/conf.d/supervisor-app.conf" during parsing
2021-12-25 23:37:51,922 INFO Set uid to user 0 succeeded
2021-12-25 23:37:51,923 INFO RPC interface 'supervisor' initialized
2021-12-25 23:37:51,923 INFO supervisord started with pid 7
2021-12-25 23:37:52,926 INFO spawned: 'mailserver' with pid 27
[ TASKLOG ]  Welcome to docker-mailserver 10.4.0
2021-12-25 23:37:52,946 INFO success: mailserver entered RUNNING state, process has stayed up for > than 0 seconds (startsecs)
[ TASKLOG ]  Initializing setup
[ TASKLOG ]  Checking configuration
[ TASKLOG ]  Configuring mail server
[ WARNING ]  No DKIM key provided. Check the documentation on how to get your keys.
[ WARNING ]  Spam messages WILL NOT BE DELIVERED, you will NOT be notified of ANY message bounced. Please define SPAMASSASSIN_SPAM_TO_INBOX explicitly.
[ WARNING ]  Clamav is disabled. You can enable it with 'ENABLE_CLAMAV=1'
[ TASKLOG ]  Applying user patches

Fixes #

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Improvement (non-breaking change that does improve existing functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation (README.md or the documentation under docs/)
  • If necessary I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

@casperklein casperklein self-assigned this Dec 25, 2021
@casperklein casperklein requested a review from a team December 25, 2021 23:47
@casperklein casperklein marked this pull request as ready for review December 26, 2021 00:39
polarathene
polarathene approved these changes Dec 28, 2021
View reviewed changes
Copy link
Copy Markdown
Member

@polarathene polarathene left a comment

Choose a reason for hiding this comment

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

Good catch! 👍

@georglauterbach
Copy link
Copy Markdown
Member

georglauterbach commented Dec 29, 2021

The more I think about it, and the more PRs out there are merged - especially considering the issue in #2348 - v10.5 seems more appropriate that v10.4.1. I'll add the correct milestone and this can be merged.

@georglauterbach georglauterbach added this to the v10.5.0 milestone Dec 29, 2021
@casperklein casperklein merged commit f7465a5 into docker-mailserver:master Dec 29, 2021
@casperklein casperklein deleted the supervisorctl branch December 29, 2021 16:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@polarathene polarathene polarathene approved these changes

@wernerfred wernerfred wernerfred approved these changes

@georglauterbach georglauterbach georglauterbach approved these changes

Assignees

@casperklein casperklein

Labels

area/scripts

Projects

None yet

Milestone

v10.5.0

Development

Successfully merging this pull request may close these issues.

4 participants

@casperklein @georglauterbach @polarathene @wernerfred