Merge pull request #2155 from docker-mailserver/revert-2104-check-for-changes-performance

Revert "check-for-changes: performance improvements + wait for settle"

Makefile

@@ -4,6 +4,8 @@ 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,3 +63,13 @@ 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,6 +154,7 @@ 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,9 +41,6 @@ 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 ")
@@ -53,16 +50,16 @@ do
 
   # get chksum and check it, no need to lock config yet
   _monitored_files_checksums >"${CHKSUM_FILE}.new"
-  CMP_RESULT=$(cmp -- "${CHKSUM_FILE}" "${CHKSUM_FILE}.new")
+  cmp --silent -- "${CHKSUM_FILE}" "${CHKSUM_FILE}.new"
-  if [[ -n "${CMP_RESULT}" ]] # If there is a difference between checksum files
+  # cmp return codes
+  # 0 – files are identical
+  # 1 – files differ
+  # 2 – inaccessible or missing argument
+  if [ $? -eq 1 ]
   then
-    # If the current run's CMP_RESULT is equal to the LAST_CMP_RESULT, changes have settled and we can safely proceed
+    _notify 'inf' "${LOG_DATE} Change detected"
-    # 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
 
@@ -77,13 +74,10 @@ 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+=($!)
           ;;
 
         * )
@@ -94,22 +88,16 @@ 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
@@ -188,19 +176,13 @@ do
       done < <(grep -v "^\s*$\|^\s*\#" /tmp/docker-mailserver/postfix-accounts.cf)
     fi
 
-      if [[ -n ${RELAY_HOST} ]]
+    [[ -n ${RELAY_HOST} ]] && _populate_relayhost_map
-      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 ]]
@@ -235,49 +217,28 @@ 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
 
-      # shellcheck disable=SC2086
+    if find /var/mail -maxdepth 3 -a \( \! -user 5000 -o \! -group 5000 \) | read -r
-      wait ${WAIT_FOR_PIDS[*]}
-
-      supervisorctl restart postfix &
-      wait $!
-      # prevent restart of dovecot when smtp_only=1
-      if [[ ${SMTP_ONLY} -ne 1 ]]
     then
-        supervisorctl restart dovecot &
+      chown -R 5000:5000 /var/mail
-        wait $!
     fi
 
-      LAST_CMP_RESULT=
+    supervisorctl restart postfix
-      CMP_RESULT=
+
+    # prevent restart of dovecot when smtp_only=1
+    [[ ${SMTP_ONLY} -ne 1 ]] && supervisorctl restart dovecot
+  fi
 
   # 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
-wait
+sleep 5
 
 # wait until postfix is dead (triggered by trap)
 while kill -0 "$(< /var/spool/postfix/pid/master.pid)"
 do
-  sleep 1
+  sleep 5
 done

test/bats

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

test/mail_ssl_letsencrypt.bats

@@ -113,11 +113,9 @@ 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 15 docker exec mail_with_relays grep -e domainzero.tld /etc/postfix/relayhost_map
+  run_until_success_or_timeout 10 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 15 docker exec mail_with_relays grep -e domain2.tld /etc/postfix/relayhost_map
+  run_until_success_or_timeout 10 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 "$(pwd)/test/test-files":/tmp/docker-mailserver-test:ro \
+		-v "${CDIR}/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 \