Skip to content

fix: restart dovecot after package replacement#904

Open
Retengart wants to merge 10 commits intochatmail:mainfrom
Retengart:fix-dovecot
Open

fix: restart dovecot after package replacement#904
Retengart wants to merge 10 commits intochatmail:mainfrom
Retengart:fix-dovecot

Conversation

@Retengart
Copy link
Copy Markdown
Contributor

Summary

  • track Dovecot package replacement separately from config-driven restart logic in cmdeploy
  • explicitly restart Dovecot after package changes even when policy-rc.d blocks package-triggered restarts
  • add unit coverage for package-change restart behavior and an online regression for stale deleted Dovecot binaries

Test Plan

  • venv/bin/pytest cmdeploy/src/cmdeploy/tests/test_dovecot_deployer.py -v
  • venv/bin/pytest cmdeploy/src/cmdeploy/tests/test_cmdeploy.py cmdeploy/src/cmdeploy/tests/test_helpers.py cmdeploy/src/cmdeploy/tests/test_rshell.py -v
  • venv/bin/pytest cmdeploy/src/cmdeploy/tests/online/test_1_basic.py::test_dovecot_main_process_matches_installed_binary -v
  • venv/bin/pytest cmdeploy/src/cmdeploy/tests/online/test_0_login.py::test_login_basic_functioning -v
  • ruff check cmdeploy/src/cmdeploy/dovecot/deployer.py cmdeploy/src/cmdeploy/tests/test_dovecot_deployer.py

@Retengart
Copy link
Copy Markdown
Contributor Author

@missytake , Can you take a look at what's with /home/runner/.ssh/id_ed25519 and restart the deployment?

Copy link
Copy Markdown
Contributor

@j4n j4n left a comment

Choose a reason for hiding this comment

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

Looks good mostly, thank you :)

@Retengart
Copy link
Copy Markdown
Contributor Author

Retengart commented Apr 2, 2026

Merged main into fix-dovecot to resolve the Dovecot deployer conflict. Kept both behaviors in install(): restart intent after package replacement and the new pin-dovecot apt preferences file. Added regression coverage for the pinning path and reran venv/bin/pytest cmdeploy/src/cmdeploy/tests/test_dovecot_deployer.py cmdeploy/src/cmdeploy/tests/test_cmdeploy.py cmdeploy/src/cmdeploy/tests/test_helpers.py cmdeploy/src/cmdeploy/tests/test_rshell.py --import-mode=importlib (19 passed). Ready for re-review.

Copy link
Copy Markdown
Contributor

@j4n j4n left a comment

Choose a reason for hiding this comment

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

looks good, one minor remark still left from the original list.

archive_version = dovecot_deployer.DOVECOT_ARCHIVE_VERSION.replace("+", "%2B")
assert deb == f"/root/dovecot-core_{archive_version}_amd64.deb"
assert changed is True
assert downloads == [
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This test is about the archive-vs-epoch version split. But name (pyinfra unit) and cache_time (calculated twice) are unrelatedand if any of those change, this test breaks with a misleading failure pointing at a version-separation test. Maybe:

assert len(downloads) == 1
assert archive_version in downloads[0]["src"]
assert archive_version in downloads[0]["dest"]
assert dovecot_deployer.DOVECOT_PACKAGE_VERSION not in downloads[0]["src"]
assert dovecot_deployer.DOVECOT_PACKAGE_VERSION not in deb

@j4n j4n temporarily deployed to staging2.testrun.org April 7, 2026 11:49 — with GitHub Actions Inactive
@j4n j4n temporarily deployed to staging-ipv4.testrun.org April 7, 2026 11:49 — with GitHub Actions Inactive
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.

2 participants