← Back to team overview

launchpad-reviewers team mailing list archive

[Merge] ~cjwatson/lp-mailman:charm into lp-mailman:master

 

Colin Watson has proposed merging ~cjwatson/lp-mailman:charm into lp-mailman:master.

Commit message:
Add a basic charm

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/lp-mailman/+git/lp-mailman/+merge/456439

This is incomplete.  It has two major known problems:

 * lp-mailman currently wants to write its configuration on startup.  This is difficult to disentangle, and it conflicts with the way our charms usually work where we deploy code as root and start it as a non-root user.  I made an effort to only write configuration if it differs from the current contents, but this is fragile and I'm not sure it completely works yet.  It may be worth exploring other approaches too.

 * I haven't done anything with email configuration yet.  Both inbound and outbound email are of course vital for lp-mailman.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/lp-mailman:charm into lp-mailman:master.
diff --git a/charm/.gitignore b/charm/.gitignore
new file mode 100644
index 0000000..f795aeb
--- /dev/null
+++ b/charm/.gitignore
@@ -0,0 +1 @@
+*.charm
diff --git a/charm/lp-mailman/README.md b/charm/lp-mailman/README.md
new file mode 100644
index 0000000..fd1a7bb
--- /dev/null
+++ b/charm/lp-mailman/README.md
@@ -0,0 +1,7 @@
+# Launchpad mailing list manager
+
+This charm runs a service that operates Launchpad mailing lists.
+
+You will need the following relation:
+
+    juju relate lp-mailman rabbitmq-server
diff --git a/charm/lp-mailman/charmcraft.yaml b/charm/lp-mailman/charmcraft.yaml
new file mode 100644
index 0000000..5e4bb85
--- /dev/null
+++ b/charm/lp-mailman/charmcraft.yaml
@@ -0,0 +1,75 @@
+type: charm
+bases:
+  - build-on:
+    - name: ubuntu
+      channel: "18.04"
+      architectures: [amd64]
+    run-on:
+    - name: ubuntu
+      channel: "18.04"
+      architectures: [amd64]
+parts:
+  charm-wheels:
+    source: https://git.launchpad.net/~ubuntuone-hackers/ols-charm-deps/+git/wheels
+    source-commit: "7bcd79fa4fca485eaf15ee9e2ee16f3ab902678d"
+    source-submodules: []
+    source-type: git
+    plugin: dump
+    organize:
+      "*": charm-wheels/
+    stage:
+      - charm-wheels
+      # XXX cjwatson 2023-09-26: This is horrible, and if you can find a
+      # better approach that works then please replace this.  Ubuntu 18.04's
+      # Python is incompatible with newer versions of pip and setuptools,
+      # but the charm build mechanism tries to upgrade pip and setuptools in
+      # a way that causes the old version of pip that we started with to
+      # select an overly-new version and then complain that it's
+      # incompatible.  Removing these wheels ensures that it doesn't try,
+      # though of course this is fragile and may break when we switch this
+      # part to a newer source-commit.
+      - "-charm-wheels/pip-22.0.4-py3-none-any.whl"
+      - "-charm-wheels/setuptools-62.1.0-py3-none-any.whl"
+      - "-charm-wheels/setuptools-65.3.0-py3-none-any.whl"
+    prime:
+      - "-charm-wheels"
+  ols-layers:
+    source: https://git.launchpad.net/ols-charm-deps
+    source-commit: "9c59a9804f1f40e2a74be7dac9bf18a655a7864f"
+    source-submodules: []
+    source-type: git
+    plugin: dump
+    organize:
+      "*": layers/
+    stage:
+      - layers
+    prime:
+      - "-layers"
+  launchpad-layers:
+    after:
+      - ols-layers
+    source: https://git.launchpad.net/launchpad-layers
+    source-commit: "8b7d25a29b8297d491abe6a0c6b69f34e39dddba"
+    source-submodules: []
+    source-type: git
+    plugin: dump
+    organize:
+      launchpad-payload: layers/layer/launchpad-payload
+    stage:
+      - layers
+    prime:
+      - "-layers"
+  charm:
+    after:
+      - charm-wheels
+      - launchpad-layers
+    source: .
+    plugin: reactive
+    build-snaps: [charm]
+    build-packages: [python3-dev]
+    build-environment:
+      - CHARM_LAYERS_DIR: $CRAFT_STAGE/layers/layer
+      - CHARM_INTERFACES_DIR: $CRAFT_STAGE/layers/interface
+      - PIP_NO_INDEX: "true"
+      - PIP_FIND_LINKS: $CRAFT_STAGE/charm-wheels
+    reactive-charm-build-arguments: [--binary-wheels-from-source]
diff --git a/charm/lp-mailman/config.yaml b/charm/lp-mailman/config.yaml
new file mode 100644
index 0000000..e9badd8
--- /dev/null
+++ b/charm/lp-mailman/config.yaml
@@ -0,0 +1,66 @@
+options:
+  active:
+    type: boolean
+    default: true
+    description: If true, run the lp-mailman service.
+  bounce_address:
+    type: string
+    description: Envelope sender address for outgoing email.
+    default: "noreply@xxxxxxxxxxxxxx"
+  domain:
+    type: string
+    description: Domain name for this instance.
+    default: "launchpad.test"
+  domain_lists:
+    type: string
+    description: Domain name for this instance's mailing list service.
+    default: "lists.launchpad.test"
+  domain_xmlrpc_private:
+    type: string
+    description: Domain name for this instance's private XML-RPC service.
+    default: "xmlrpc-private.launchpad.test"
+  log_hosts_allow:
+    type: string
+    description: >
+      Hosts that should be allowed to rsync logs.  Note that this relies on
+      the nrpe subordinate charm to set up /etc/rsyncd.conf properly.
+    default: ""
+  mailman_shared_secret:
+    type: string
+    description: >
+      Shared secret used to pre-approve Launchpad-generated messages to
+      Launchpad mailing lists.
+    default: ""
+  oops_prefix:
+    type: string
+    description: A prefix for OOPS codes for this instance.
+    default: "TEST"
+  port_xmlrpc:
+    type: int
+    description: Port for the XML-RPC application server.
+    default: 8087
+  rabbitmq_broker_urls:
+    type: string
+    description: >
+      Space-separated list of RabbitMQ broker URLs.  If unset, rely on the
+      rabbitmq relation instead.
+    default: ""
+  rabbitmq_user:
+    type: string
+    description: Username to use with RabbitMQ.
+    default: "test"
+  site_list_owner:
+    type: string
+    description: >
+      Site list owner email address and password, separated by a colon.
+      (This isn't actually used for anything significant because we
+      administer lists via Launchpad instead, but Mailman requires us to
+      provide something here.  As a result, providing a default isn't too
+      risky.)
+    default: "listowner@xxxxxxxxxxxxxx:secret"
+  site_message:
+    type: string
+    description: >
+      If provided, this message will appear in the site status bar at the
+      top of every page.
+    default: ""
diff --git a/charm/lp-mailman/layer.yaml b/charm/lp-mailman/layer.yaml
new file mode 100644
index 0000000..1e8d8fa
--- /dev/null
+++ b/charm/lp-mailman/layer.yaml
@@ -0,0 +1,23 @@
+includes:
+  - interface:rabbitmq
+  - layer:launchpad-payload
+options:
+  apt:
+    packages:
+      - python
+      - python-dev
+      - virtualenv
+  ols:
+    service_name: lp-mailman
+    config_filename: service.conf
+    user: launchpad
+    tarball_payload: true
+    symlink_switch_payload: true
+    python_bin: python2.7
+  launchpad-payload:
+    # Stub out create_virtualenv by giving it a target that does nothing,
+    # because buildmailman.py does some odd monkey-patching of the source
+    # tree which has to be run with the correct configuration, so we need to
+    # do that later.
+    build_target: setup.py
+repo: https://git.launchpad.net/lp-mailman
diff --git a/charm/lp-mailman/lib/charms/lp_mailman.py b/charm/lp-mailman/lib/charms/lp_mailman.py
new file mode 100644
index 0000000..d285bbf
--- /dev/null
+++ b/charm/lp-mailman/lib/charms/lp_mailman.py
@@ -0,0 +1,102 @@
+# Copyright 2023 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+import os.path
+import subprocess
+from urllib.parse import urlparse
+
+from charmhelpers.core import hookenv, host, templating
+from charms.launchpad.payload import get_service_config as get_payload_config
+from charms.reactive import endpoint_from_flag
+from ols import base
+
+
+def oopses_dir():
+    return os.path.join(base.base_dir(), "oopses")
+
+
+def secrets_dir():
+    return os.path.join(base.base_dir(), "secrets")
+
+
+def var_dir():
+    return os.path.join(base.base_dir(), "var")
+
+
+def change_shell(user, shell):
+    if (
+        subprocess.run(
+            ["getent", "passwd", user],
+            stdout=subprocess.PIPE,
+            check=True,
+            universal_newlines=True,
+        )
+        .stdout.splitlines()[0]
+        .split(":")[6]
+        != shell
+    ):
+        subprocess.run(["chsh", "-s", shell, user], check=True)
+
+
+def ensure_lp_directories():
+    for dirpath in oopses_dir(), var_dir():
+        host.mkdir(dirpath, group=base.user(), perms=0o775)
+    host.mkdir(secrets_dir(), group=base.user(), perms=0o750)
+
+
+def _get_first_rabbitmq_hostname(rabbitmq):
+    for conversation in rabbitmq.conversations():
+        for relation_id in conversation.relation_ids:
+            for unit in hookenv.related_units(relation_id):
+                return hookenv.relation_get(
+                    "private-address", unit, relation_id
+                )
+
+
+def get_service_config():
+    config = get_payload_config()
+    config.update(
+        {
+            "oopses_dir": oopses_dir(),
+            "secrets_dir": secrets_dir(),
+            "var_dir": var_dir(),
+        }
+    )
+
+    # oops-datedir2amqp is used in crontabs and needs broken-out RabbitMQ
+    # credentials.  This isn't ideal because it doesn't support high
+    # availability, but we can tolerate that for fallback OOPS publishing
+    # for now.
+    if config["rabbitmq_broker_urls"]:
+        rabbitmq_url = urlparse(config["rabbitmq_broker_urls"].split()[0])
+        config.update(
+            {
+                "rabbitmq_host": rabbitmq_url.hostname,
+                "rabbitmq_password": rabbitmq_url.password,
+                "rabbitmq_username": rabbitmq_url.username,
+                "rabbitmq_vhost": rabbitmq_url.path.lstrip("/"),
+            }
+        )
+    else:
+        rabbitmq = endpoint_from_flag("rabbitmq.available")
+        hostname = _get_first_rabbitmq_hostname(rabbitmq)
+        if hostname is not None:
+            config.update(
+                {
+                    "rabbitmq_host": hostname,
+                    "rabbitmq_password": rabbitmq.password(),
+                    "rabbitmq_username": rabbitmq.username(),
+                    "rabbitmq_vhost": rabbitmq.vhost(),
+                }
+            )
+
+    return config
+
+
+def configure_rsync(config, template, name):
+    hookenv.log("Writing rsync configuration.")
+    rsync_path = os.path.join("/etc/rsync-juju.d", name)
+    if config["log_hosts_allow"]:
+        templating.render(template, rsync_path, config, perms=0o644)
+    elif os.path.exists(rsync_path):
+        os.unlink(rsync_path)
diff --git a/charm/lp-mailman/metadata.yaml b/charm/lp-mailman/metadata.yaml
new file mode 100644
index 0000000..4823b1c
--- /dev/null
+++ b/charm/lp-mailman/metadata.yaml
@@ -0,0 +1,18 @@
+name: lp-mailman
+display-name: lp-mailman
+summary: Launchpad mailing list manager
+maintainer: Launchpad Developers <launchpad-dev@xxxxxxxxxxxxxxxxxxx>
+description: |
+  Launchpad is an open source suite of tools that help people and teams
+  to work together on software projects.
+
+  This charm integrates Mailman with Launchpad to operate mailing lists.
+tags:
+  # https://juju.is/docs/charm-metadata#heading--charm-store-fields
+  - network
+series:
+  - bionic
+subordinate: false
+requires:
+  rabbitmq:
+    interface: rabbitmq
diff --git a/charm/lp-mailman/reactive/lp-mailman.py b/charm/lp-mailman/reactive/lp-mailman.py
new file mode 100644
index 0000000..e6230fb
--- /dev/null
+++ b/charm/lp-mailman/reactive/lp-mailman.py
@@ -0,0 +1,171 @@
+# Copyright 2023 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+import subprocess
+
+from charmhelpers.core import hookenv, host, templating
+from charms.launchpad.payload import (
+    config_file_path,
+    configure_cron,
+    configure_lazr,
+)
+from charms.lp_mailman import (
+    change_shell,
+    configure_rsync,
+    ensure_lp_directories,
+    get_service_config,
+)
+from charms.reactive import (
+    clear_flag,
+    endpoint_from_flag,
+    helpers,
+    hook,
+    set_flag,
+    when,
+    when_any,
+    when_none,
+    when_not,
+    when_not_all,
+)
+from ols import base
+
+
+@when("rabbitmq.connected")
+def prepare_rabbitmq():
+    rabbitmq = endpoint_from_flag("rabbitmq.connected")
+    config = hookenv.config()
+    rabbitmq.request_access(config["rabbitmq_user"], config["domain"])
+
+
+def get_rabbitmq_uris(rabbitmq):
+    config = hookenv.config()
+    if config["rabbitmq_broker_urls"]:
+        yield from config["rabbitmq_broker_urls"].split()
+    else:
+        for conversation in rabbitmq.conversations():
+            for relation_id in conversation.relation_ids:
+                for unit in hookenv.related_units(relation_id):
+                    hostname = hookenv.relation_get(
+                        "private-address", unit, relation_id
+                    )
+                    vhost = rabbitmq.vhost()
+                    username = rabbitmq.username()
+                    password = rabbitmq.password()
+                    yield f"amqp://{username}:{password}@{hostname}/{vhost}"
+
+
+@when_any("rabbitmq.available", "config.set.rabbitmq_broker_urls")
+def rabbitmq_available():
+    set_flag("lp-mailman.rabbitmq.available")
+
+
+@when_none("rabbitmq.available", "config.set.rabbitmq_broker_urls")
+def rabbitmq_unavailable():
+    clear_flag("lp-mailman.rabbitmq.available")
+
+
+def configure_service(config):
+    hookenv.log("Writing systemd service.")
+    templating.render(
+        "lp-mailman.service.j2",
+        "/lib/systemd/system/lp-mailman.service",
+        config,
+    )
+    subprocess.run(["systemctl", "daemon-reload"], check=True)
+
+
+def configure_logrotate(config):
+    hookenv.log("Writing logrotate configuration.")
+    templating.render(
+        "logrotate.conf.j2", "/etc/logrotate.d/lp-mailman", config, perms=0o644
+    )
+
+
+def config_files():
+    files = []
+    files.append(config_file_path("lp-mailman/mailman-lazr.conf"))
+    files.append(config_file_path("lp-mailman-secrets-lazr.conf", secret=True))
+    return files
+
+
+@when("launchpad.payload.configured", "lp-mailman.rabbitmq.available")
+@when_not("service.configured")
+def configure():
+    rabbitmq = endpoint_from_flag("rabbitmq.available")
+    # Interactive use shouldn't be frequent, but it's still needed
+    # sometimes, so make it less annoying.
+    change_shell(base.user(), "/bin/bash")
+    ensure_lp_directories()
+    config = get_service_config()
+    config["rabbitmq_broker_urls"] = sorted(get_rabbitmq_uris(rabbitmq))
+    configure_lazr(
+        config, "lp-mailman-lazr.conf", "lp-mailman/mailman-lazr.conf"
+    )
+    configure_lazr(
+        config,
+        "lp-mailman-secrets-lazr.conf",
+        "lp-mailman-secrets-lazr.conf",
+        secret=True,
+    )
+    # Normally we'd be careful to ensure that the code tree is owned by root
+    # while the service runs as a non-root user so that it can't write to
+    # its own code, but that's currently very difficult to arrange here: the
+    # build procedure writes to `lib/mailman/`, but it also populates
+    # `config.mailman.build_var_dir` and the service needs to be able to
+    # write to that at run-time.  We'll just have to put up with the code
+    # being owned by the service user for now.
+    subprocess.run(
+        ["chown", "-R", "%s:", base.user(), base.code_dir()], check=True
+    )
+    # Build the tree now that we have the correct configuration in place.
+    subprocess.run(
+        [
+            "make",
+            "compile",
+            "PYTHON=%s" % base.python_bin(),
+            "LPCONFIG=lp-mailman",
+        ],
+        cwd=base.code_dir(),
+        check=True,
+    )
+    configure_rsync(config, "lp-mailman-rsync.conf", "010-lp-mailman.conf")
+    configure_service(config)
+    configure_logrotate(config)
+    configure_cron(config, "crontab.j2")
+
+    if config["active"]:
+        if helpers.any_file_changed(
+            [
+                base.version_info_path(),
+                "/lib/systemd/system/lp-mailman.service",
+            ]
+            + config_files()
+        ):
+            hookenv.log("Config files changed; restarting lp-mailman")
+            host.service_restart("lp-mailman.service")
+        host.service_resume("lp-mailman.service")
+    else:
+        host.service_pause("lp-mailman.service")
+
+    set_flag("service.configured")
+
+
+@when("service.configured")
+@when_not_all("launchpad.payload.configured", "lp-mailman.rabbitmq.available")
+def deconfigure():
+    clear_flag("service.configured")
+
+
+@when("service.configured")
+def check_is_running():
+    hookenv.status_set("active", "Ready")
+
+
+@when("config.changed")
+def config_changed():
+    clear_flag("service.configured")
+
+
+@hook("{requires:rabbitmq}-relation-changed")
+def rabbitmq_relation_changed(*args):
+    clear_flag("service.configured")
diff --git a/charm/lp-mailman/templates/crontab.j2 b/charm/lp-mailman/templates/crontab.j2
new file mode 100644
index 0000000..53b2ed3
--- /dev/null
+++ b/charm/lp-mailman/templates/crontab.j2
@@ -0,0 +1,9 @@
+TZ=UTC
+MAILFROM={{ bounce_address }}
+MAILTO={{ cron_mailto }}
+LPCONFIG=lp-mailman
+
+# Catch up with publishing OOPSes that were temporarily spooled to disk due
+# to RabbitMQ being unavailable.
+*/15 * * * * {{ code_dir }}/bin/datedir2amqp --exchange oopses --host {{ rabbitmq_host }} --username {{ rabbitmq_username }} --password {{ rabbitmq_password }} --vhost {{ rabbitmq_vhost }} --repo {{ oopses_dir }} --key ""
+
diff --git a/charm/lp-mailman/templates/logrotate.conf.j2 b/charm/lp-mailman/templates/logrotate.conf.j2
new file mode 100644
index 0000000..044297e
--- /dev/null
+++ b/charm/lp-mailman/templates/logrotate.conf.j2
@@ -0,0 +1,16 @@
+{{ logs_dir }}/*.log
+{
+    rotate 90
+    daily
+    dateext
+    delaycompress
+    compress
+    notifempty
+    missingok
+    create 0644 {{ user }} {{ user }}
+    sharedscripts
+    postrotate
+        systemctl restart lp-mailman.service
+    endscript
+}
+
diff --git a/charm/lp-mailman/templates/lp-mailman-lazr.conf b/charm/lp-mailman/templates/lp-mailman-lazr.conf
new file mode 100644
index 0000000..cdccf13
--- /dev/null
+++ b/charm/lp-mailman/templates/lp-mailman-lazr.conf
@@ -0,0 +1,35 @@
+# Public configuration data.  The contents of this file may be freely shared
+# with developers if needed for debugging.
+
+# A schema's sections, keys, and values are automatically inherited, except
+# for '.optional' sections.  Update this config to override key values.
+# Values are strings, except for numbers that look like ints.  The tokens
+# true, false, and none are treated as True, False, and None.
+
+[meta]
+extends: ../../lib/lp/services/config/schema-lazr.conf
+
+[canonical]
+pid_dir: {{ var_dir }}
+
+[error_reports]
+error_dir: {{ oopses_dir }}
+oops_prefix: {{ oops_prefix }}
+
+[launchpad]
+config_overlay_dir: {{ secrets_dir }}
+site_message: {{ site_message }}
+
+[mailman]
+archive_url_template: https://{{ domain_lists }}/$team_name
+build: True
+build_host_name: {{ domain_lists }}
+build_prefix: lib/mailman
+build_user_group: {{ user }}:{{ user }}
+build_var_dir: {{ var_dir }}/mailman
+launch: True
+list_owner_header_template: https://{{ domain }}/~$team_name
+list_subscription_headers: https://{{ domain }}/~$team_name
+subscription_batch_size: 5
+xmlrpc_url: http://{{ domain_xmlrpc_private }}:{{ port_xmlrpc }}/mailinglists
+
diff --git a/charm/lp-mailman/templates/lp-mailman-rsync.conf b/charm/lp-mailman/templates/lp-mailman-rsync.conf
new file mode 100644
index 0000000..2921300
--- /dev/null
+++ b/charm/lp-mailman/templates/lp-mailman-rsync.conf
@@ -0,0 +1,8 @@
+
+[lp-logs]
+  path = {{ logs_dir }}
+  comment = LP Logs
+  list = false
+  read only = true
+  hosts allow = {{ log_hosts_allow }}
+
diff --git a/charm/lp-mailman/templates/lp-mailman-secrets-lazr.conf b/charm/lp-mailman/templates/lp-mailman-secrets-lazr.conf
new file mode 100644
index 0000000..fbb668f
--- /dev/null
+++ b/charm/lp-mailman/templates/lp-mailman-secrets-lazr.conf
@@ -0,0 +1,19 @@
+# Secret configuration data.  This is stored in an overlay directory, mainly
+# to avoid accidental information leaks from the public configuration file.
+# Entries in this file should not be shared with developers, although the
+# structure of the file is not secret, only configuration values.
+
+# A schema's sections, keys, and values are automatically inherited, except
+# for '.optional' sections.  Update this config to override key values.
+# Values are strings, except for numbers that look like ints.  The tokens
+# true, false, and none are treated as True, False, and None.
+
+{% from "macros.j2" import opt -%}
+
+[mailman]
+build_site_list_owner: {{ site_list_owner }}
+{{- opt("shared_secret", mailman_shared_secret) }}
+
+[rabbitmq]
+broker_urls: {{ rabbitmq_broker_urls|join(" ") }}
+
diff --git a/charm/lp-mailman/templates/lp-mailman.service.j2 b/charm/lp-mailman/templates/lp-mailman.service.j2
new file mode 100644
index 0000000..869273a
--- /dev/null
+++ b/charm/lp-mailman/templates/lp-mailman.service.j2
@@ -0,0 +1,16 @@
+[Unit]
+Description=Launchpad mailing list manager
+After=network.target
+ConditionPathExists=!{{ code_dir }}/maintenance.txt
+
+[Service]
+User={{ user }}
+Group={{ user }}
+WorkingDirectory={{ code_dir }}
+Environment=LPCONFIG=lp-mailman
+ExecStart={{ code_dir }}/bin/run -i ${LPCONFIG}
+Restart=on-failure
+
+[Install]
+WantedBy=multi-user.target
+
diff --git a/lib/lp/services/mailman/monkeypatches/__init__.py b/lib/lp/services/mailman/monkeypatches/__init__.py
index 0311737..23760b0 100644
--- a/lib/lp/services/mailman/monkeypatches/__init__.py
+++ b/lib/lp/services/mailman/monkeypatches/__init__.py
@@ -9,6 +9,25 @@ import shutil
 from lazr.config import as_host_port
 
 
+def write_if_different(path, contents):
+    """Write `contents` to `path` if they differ from the current contents.
+
+    This allows us to monkey-patch Mailman in such a way that changing the
+    configuration works gracefully in development (where
+    `lib/mailman/Mailman/` will normally be writeable by the user running
+    Mailman) while not breaking in Juju-deployed environments (where it
+    normally won't): we just need to be careful to run `make compile` with
+    the correct configuration first as a user that can write to the source
+    tree.
+    """
+    if os.path.exists(path):
+        with open(path) as f:
+            previous_contents = f.read()
+    if contents != previous_contents:
+        with open(path, "w") as f:
+            f.write(contents)
+
+
 def monkey_patch(mailman_path, config):
     """Monkey-patch an installed Mailman 2.1 tree.
 
@@ -49,54 +68,44 @@ def monkey_patch(mailman_path, config):
     owner_address, owner_password = configure_siteowner(
         config.mailman.build_site_list_owner)
     config_path_in = os.path.join(os.path.dirname(__file__), 'mm_cfg.py.in')
-    config_file_in = open(config_path_in)
-    try:
+    with open(config_path_in) as config_file_in:
         config_template = config_file_in.read()
-    finally:
-        config_file_in.close()
     config_path_out = os.path.join(mailman_path, 'Mailman', 'mm_cfg.py')
-    config_file_out = open(config_path_out, 'w')
-    try:
-        print >> config_file_out, config_template % dict(
-            launchpad_top=launchpad_top,
-            smtp_host=host,
-            smtp_port=port,
-            smtp_max_rcpts=config.mailman.smtp_max_rcpts,
-            smtp_max_sesions_per_connection=(
-                config.mailman.smtp_max_sesions_per_connection),
-            xmlrpc_url=config.mailman.xmlrpc_url,
-            xmlrpc_sleeptime=config.mailman.xmlrpc_runner_sleep,
-            xmlrpc_timeout=config.mailman.xmlrpc_timeout,
-            xmlrpc_subscription_batch_size=(
-                config.mailman.subscription_batch_size),
-            site_list_owner=owner_address,
-            list_help_header=config.mailman.list_help_header,
-            list_subscription_headers=(
-                config.mailman.list_subscription_headers),
-            archive_url_template=config.mailman.archive_url_template,
-            list_owner_header_template=(
-                config.mailman.list_owner_header_template),
-            footer=footer,
-            var_dir=config.mailman.build_var_dir,
-            shared_secret=config.mailman.shared_secret,
-            soft_max_size=config.mailman.soft_max_size,
-            hard_max_size=config.mailman.hard_max_size,
-            register_bounces_every=config.mailman.register_bounces_every,
-            )
-    finally:
-        config_file_out.close()
+    config_path_contents = config_template % dict(
+        launchpad_top=launchpad_top,
+        smtp_host=host,
+        smtp_port=port,
+        smtp_max_rcpts=config.mailman.smtp_max_rcpts,
+        smtp_max_sesions_per_connection=(
+            config.mailman.smtp_max_sesions_per_connection),
+        xmlrpc_url=config.mailman.xmlrpc_url,
+        xmlrpc_sleeptime=config.mailman.xmlrpc_runner_sleep,
+        xmlrpc_timeout=config.mailman.xmlrpc_timeout,
+        xmlrpc_subscription_batch_size=(
+            config.mailman.subscription_batch_size),
+        site_list_owner=owner_address,
+        list_help_header=config.mailman.list_help_header,
+        list_subscription_headers=(
+            config.mailman.list_subscription_headers),
+        archive_url_template=config.mailman.archive_url_template,
+        list_owner_header_template=(
+            config.mailman.list_owner_header_template),
+        footer=footer,
+        var_dir=config.mailman.build_var_dir,
+        shared_secret=config.mailman.shared_secret,
+        soft_max_size=config.mailman.soft_max_size,
+        hard_max_size=config.mailman.hard_max_size,
+        register_bounces_every=config.mailman.register_bounces_every,
+        )
+    write_if_different(config_path_out, config_path_contents)
     # Mailman's qrunner system requires runner modules to live in the
     # Mailman.Queue package.  Set things up so that there's a hook module in
     # there for the XMLRPCRunner.
     runner_path = os.path.join(mailman_path,
                                'Mailman', 'Queue', 'XMLRPCRunner.py')
-    runner_file = open(runner_path, 'w')
-    try:
-        print >> runner_file, (
-            'from lp.services.mailman.monkeypatches.xmlrpcrunner '
-            'import *')
-    finally:
-        runner_file.close()
+    write_if_different(
+        runner_path,
+        'from lp.services.mailman.monkeypatches.xmlrpcrunner import *\n')
     # Install our handler wrapper modules so that Mailman can find them.  Most
     # of the actual code of the handler comes from our monkey patches modules.
     for mm_name, lp_name in (('LaunchpadMember', 'lphandler'),
@@ -107,13 +116,9 @@ def monkey_patch(mailman_path, config):
                              ):
         handler_path = os.path.join(
             mailman_path, 'Mailman', 'Handlers', mm_name + '.py')
-        handler_file = open(handler_path, 'w')
-        try:
-            package = 'lp.services.mailman.monkeypatches'
-            module = package + '.' + lp_name
-            print >> handler_file, 'from', module, 'import *'
-        finally:
-            handler_file.close()
+        package = 'lp.services.mailman.monkeypatches'
+        module = package + '.' + lp_name
+        write_if_different(handler_path, 'from %s import *\n' % module)
 
     here = os.path.dirname(__file__)
     # Install the MHonArc control file.