check-for-changes: performance improvements + wait for settle (#2104)

Makefile

@@ -4,8 +4,6 @@ NAME   ?= mailserver-testing:ci
 VCS_REF = $(shell git rev-parse --short HEAD)
 VCS_VER = $(shell git describe --tags --contains --always)
 
-export CDIR = $(shell pwd)
-
 # -----------------------------------------------
 # --- Generic Build Targets ---------------------
 # -----------------------------------------------

target/bin/addmailuser

@@ -63,13 +63,3 @@ echo "${FULL_EMAIL}|${HASH}" >> "${DATABASE}"
 
 USER="${FULL_EMAIL%@*}"
 DOMAIN="${FULL_EMAIL#*@}"
-
-# Tests fail if the creation of /var/mail/${DOMAIN}/${USER} doesn't happen fast enough after addmailuser executes (check-for-changes.sh race-condition)
-if [[ -e "/tmp/docker-mailserver-config-chksum" ]] # Prevent infinite loop in tests like "checking accounts: user3 should have been added to /tmp/docker-mailserver/postfix-accounts.cf even when that file does not exist"
-then
-  while [[ ! -d "/var/mail/${DOMAIN}/${USER}" ]]
-  do
-    echo "Waiting for dovecot to create /var/mail/${DOMAIN}/${USER}..."
-    sleep 1
-  done
-fi

target/bin/delmailuser

@@ -154,7 +154,6 @@ use 'sudo docker exec mailserver rm -R /var/mail/${DOMAIN}/${USER}'"
     rmdir "/var/mail/${DOMAIN}" 2>/dev/null || true
   else
     echo "Mailbox directory '/var/mail/${DOMAIN}/${USER}' did not exist." >&2
-    ERROR=true
   fi
 
   ${ERROR} && errex 'See the messages above.'

target/scripts/check-for-changes.sh

@@ -41,6 +41,9 @@ PM_ADDRESS="${POSTMASTER_ADDRESS:=postmaster@${DOMAINNAME}}"
 _notify 'inf' "${LOG_DATE} Using postmaster address ${PM_ADDRESS}"
 sleep 10
 
+CMP_RESULT=
+LAST_CMP_RESULT=
+
 while true
 do
   LOG_DATE=$(date +"%Y-%m-%d %H:%M:%S ")
@@ -50,16 +53,16 @@ do
 
   # get chksum and check it, no need to lock config yet
   _monitored_files_checksums >"${CHKSUM_FILE}.new"
-  cmp --silent -- "${CHKSUM_FILE}" "${CHKSUM_FILE}.new"
+  CMP_RESULT=$(cmp -- "${CHKSUM_FILE}" "${CHKSUM_FILE}.new")
-  # cmp return codes
+  if [[ -n "${CMP_RESULT}" ]] # If there is a difference between checksum files
-  # 0 – files are identical
-  # 1 – files differ
-  # 2 – inaccessible or missing argument
-  if [ $? -eq 1 ]
   then
-    _notify 'inf' "${LOG_DATE} Change detected"
+    # If the current run's CMP_RESULT is equal to the LAST_CMP_RESULT, changes have settled and we can safely proceed
+    # This prevents multiple restarts of postfix from happening all at once and causing problems or delays
+    if [[ "${CMP_RESULT}" == "${LAST_CMP_RESULT}" ]]
+    then
+      _notify 'inf' "${LOG_DATE} Changes to checksum files settled"
       CHANGED=$(grep -Fxvf "${CHKSUM_FILE}" "${CHKSUM_FILE}.new" | sed 's/^[^ ]\+  //')
-
+      WAIT_FOR_PIDS=()
       # Bug alert! This overwrites the alias set by start-mailserver.sh
       # Take care that changes in one script are propagated to the other
 
@@ -74,10 +77,13 @@ do
       do
         case "${FILE}" in
           "/etc/letsencrypt/acme.json" )
+            {
               for CERTDOMAIN in ${SSL_DOMAIN} ${HOSTNAME} ${DOMAINNAME}
               do
                 _extract_certs_from_acme "${CERTDOMAIN}" && break
               done
+            } &
+            WAIT_FOR_PIDS+=($!)
             ;;
 
           * )
@@ -88,16 +94,22 @@ do
       done
 
       # regenerate postix aliases
+      {
         echo "root: ${PM_ADDRESS}" >/etc/aliases
         if [[ -f /tmp/docker-mailserver/postfix-aliases.cf ]]
         then
           cat /tmp/docker-mailserver/postfix-aliases.cf >>/etc/aliases
         fi
         postalias /etc/aliases
+      } &
+      WAIT_FOR_PIDS+=($!)
 
       # regenerate postfix accounts
+      {
         : >/etc/postfix/vmailbox
         : >/etc/dovecot/userdb
+      } &
+      WAIT_FOR_PIDS+=($!)
 
       if [[ -f /tmp/docker-mailserver/postfix-accounts.cf ]] && [[ ${ENABLE_LDAP} -ne 1 ]]
       then
@@ -176,13 +188,19 @@ do
         done < <(grep -v "^\s*$\|^\s*\#" /tmp/docker-mailserver/postfix-accounts.cf)
       fi
 
-    [[ -n ${RELAY_HOST} ]] && _populate_relayhost_map
+      if [[ -n ${RELAY_HOST} ]]
-
+      then
+        _populate_relayhost_map &
+        WAIT_FOR_PIDS+=($!)
+      fi
 
       if [[ -f /etc/postfix/sasl_passwd ]]
       then
+        {
           chown root:root /etc/postfix/sasl_passwd
           chmod 0600 /etc/postfix/sasl_passwd
+        } &
+        WAIT_FOR_PIDS+=($!)
       fi
 
       if [[ -f postfix-virtual.cf ]]
@@ -217,28 +235,49 @@ s/$/ regexp:\/etc\/postfix\/regexp/
         fi
       fi
 
+      # shellcheck disable=SC2044
+      for USER_MAIL_LOCATION in $(find /var/mail -maxdepth 3 -a \( \! -user 5000 -o \! -group 5000 \)); do
+        chown -R 5000:5000 "${USER_MAIL_LOCATION}" &
+        WAIT_FOR_PIDS+=($!)
+      done
+
+      # relies on if [[ -f postfix-virtual.cf ]] block
       if [[ -f /tmp/vhost.tmp ]]
       then
         sort < /tmp/vhost.tmp | uniq >/etc/postfix/vhost
         rm /tmp/vhost.tmp
       fi
 
-    if find /var/mail -maxdepth 3 -a \( \! -user 5000 -o \! -group 5000 \) | read -r
+      # shellcheck disable=SC2086
-    then
+      wait ${WAIT_FOR_PIDS[*]}
-      chown -R 5000:5000 /var/mail
-    fi
-
-    supervisorctl restart postfix
 
+      supervisorctl restart postfix &
+      wait $!
       # prevent restart of dovecot when smtp_only=1
-    [[ ${SMTP_ONLY} -ne 1 ]] && supervisorctl restart dovecot
+      if [[ ${SMTP_ONLY} -ne 1 ]]
+      then
+        supervisorctl restart dovecot &
+        wait $!
       fi
 
+      LAST_CMP_RESULT=
+      CMP_RESULT=
+
       # mark changes as applied
       mv "${CHKSUM_FILE}.new" "${CHKSUM_FILE}"
+
+    else # The files differ...
+      _notify 'inf' "${LOG_DATE} Changes to checksum files detected... Ensuring changes have settled before proceeding..."
+      LAST_CMP_RESULT="${CMP_RESULT}"
+      sleep 5 # The longer the sleep here, the more time there is for users to make changes before a full restart
+    fi
+
+  else # Checksum files don't differ
+    LAST_CMP_RESULT=
+  fi
+
   remove_lock "${SCRIPT_NAME}"
 
-  sleep 1
 done
 
 exit 0

target/scripts/postfix-wrapper.sh

@@ -20,11 +20,11 @@ trap "service postfix stop" SIGINT
 trap "service postfix stop" SIGTERM
 trap "service postfix reload" SIGHUP
 
-service postfix start
+service postfix start &
-sleep 5
+wait
 
 # wait until postfix is dead (triggered by trap)
 while kill -0 "$(< /var/spool/postfix/pid/master.pid)"
 do
-  sleep 5
+  sleep 1
 done

target/supervisor/conf.d/supervisor-app.conf

@@ -108,7 +108,7 @@ command=/usr/bin/fetchmail -f /etc/fetchmailrc -v --nodetach --daemon %(ENV_FETC
 
 [program:postfix]
 startsecs=0
-stopwaitsecs=55
+stopwaitsecs=20
 autostart=false
 autorestart=true
 stdout_logfile=/var/log/supervisor/%(program_name)s.log

test/bats

@@ -1 +1 @@
-Subproject commit 54e965fa9d269c2b3ff9036d81f32bac3df0edea
+Subproject commit 9f27e3c47d70ad5aa99e4674593155652c195344

test/mail_ssl_letsencrypt.bats

@@ -113,9 +113,11 @@ function teardown_file() {
   cp "$(private_config_path mail_lets_acme_json)/letsencrypt/acme-changed.json" "$(private_config_path mail_lets_acme_json)/acme.json"
   sleep 11
   run docker exec mail_lets_acme_json /bin/bash -c "supervisorctl tail changedetector"
+  assert_output --partial "Changes to checksum files detected"
+  sleep 6
+  run docker exec mail_lets_acme_json /bin/bash -c "supervisorctl tail changedetector"
   assert_output --partial "postfix: stopped"
   assert_output --partial "postfix: started"
-  assert_output --partial "Change detected"
 
   run docker exec mail_lets_acme_json /bin/bash -c "cat /etc/letsencrypt/live/mail.my-domain.com/key.pem"
   assert_output "$(cat "$(private_config_path mail_lets_acme_json)/letsencrypt/changed/key.pem")"

test/mail_with_relays.bats

@@ -51,7 +51,7 @@ function teardown_file() {
   run docker exec mail_with_relays grep -e domainzero.tld /etc/postfix/relayhost_map
   assert_output ''
   run ./setup.sh -c mail_with_relays email add user0@domainzero.tld password123
-  run_until_success_or_timeout 10 docker exec mail_with_relays grep -e domainzero.tld /etc/postfix/relayhost_map
+  run_until_success_or_timeout 15 docker exec mail_with_relays grep -e domainzero.tld /etc/postfix/relayhost_map
   assert_output -e '^@domainzero.tld[[:space:]]+\[default.relay.com\]:2525$'
 }
 
@@ -59,7 +59,7 @@ function teardown_file() {
   run docker exec mail_with_relays grep -e domain2.tld /etc/postfix/relayhost_map
   assert_output ''
   run ./setup.sh -c mail_with_relays alias add user2@domain2.tld user2@domaintwo.tld
-  run_until_success_or_timeout 10 docker exec mail_with_relays grep -e domain2.tld /etc/postfix/relayhost_map
+  run_until_success_or_timeout 15 docker exec mail_with_relays grep -e domain2.tld /etc/postfix/relayhost_map
   assert_output -e '^@domain2.tld[[:space:]]+\[default.relay.com\]:2525$'
 }
 

test/open_dkim.bats

@@ -21,7 +21,7 @@ function setup_file
     --name "${CONTAINER_NAME}" \
 		--cap-add=SYS_PTRACE \
 		-v "${PRIVATE_CONFIG}":/tmp/docker-mailserver \
-		-v "${CDIR}/test/test-files":/tmp/docker-mailserver-test:ro \
+		-v "$(pwd)/test/test-files":/tmp/docker-mailserver-test:ro \
 		-e DEFAULT_RELAY_HOST=default.relay.host.invalid:25 \
 		-e PERMIT_DOCKER=host \
 		-e DMS_DEBUG=0 \