libre-server / rolekit

'rolekit' is a daemon for Linux systems providing a stable D-BUS interface to manage the deployment of ​Server Roles.
19 stars 7 forks source link

Refactor systemd unit file creation to use python-systemdunit #17

Open sgallagher opened 8 years ago

sgallagher commented 8 years ago

I've created a variant of python-configparser to create and manipulate systemd unit files. We should use this in rolekit.

https://github.com/sgallagher/systemdunitparser

sgallagher commented 8 years ago

Copying from https://fedorahosted.org/rolekit/ticket/45

Right now, all roles are required to return a dictionary of a form like:

target = {'Role': 'databaseserver',
              'Instance': self.get_name(),
              'Description': "Database Server Role - %s" % self.get_name(),
              'BindsTo': ['postgresql.service'],
              'Requires': ['postgresql.service'],
              'RequiredBy': ['postgresql.service'],
              'After': ['syslog.target', 'network.target']}

Since this is systemd-specific in any case, we should actually construct a unit file object using the new library python3-systemdunitparser (a wrapper around python3-configparser) and return that object instead. It would be best to create helper routines in rolekit to accomplish this (creating the target units and other helpers).

alxgrtnstrngl commented 8 years ago

What kind of naming convention should this unit file have? I was thinking it could be something like databaseserver-rolekit.service and then it's placed in /lib/systemd/system/databaseserver-rolekit.service. Or would the /usr/lib/systemd/system/databaseserver-rolekit.server be a better location?

sgallagher commented 8 years ago

The unit files already have a naming convention: /etc/systemd/system/role-<roletype>-<roleinstance>.target

See the create_target() in src/rolekit/server/rolebase.py

alxgrtnstrngl commented 8 years ago

It seems that in rolekit.server.rolebase that RoleDeploymentValues class methods to_unitfile and write both already use SystemdUnitParser in creation of the single target unit so I don't know what more is needed here.

Also, I've noticed that rolekit.server.io.systemd that the SystemdContainerServiceUnit needs refactoring to use SystemdUnitParser:

        with open(path, "w") as f:
            f.write("[Unit]\n")
            f.write("Description=%s\n" % self.desc)
            f.write("Requires=docker.service\n")
            f.write("After=docker.service\n\n")
            f.write("[Service]\n")
            f.write("Restart=always\n")
            f.write("ExecStart=%s\n" % docker_run)
            f.write("ExecStop=/usr/bin/docker stop -t 5 {0} ; /usr/bin/docker rm -f {0}\n\n".format(self.container_name))
            f.write("[Install]\n")
            f.write("WantedBy=multi-user.target\n")

Is SystemdContainerServiceUnit still used at all?

After looking at these two items there are two alternatives I'm considering to close this issue out:

  1. refactor SystemdContainerServiceUnit to use SystemdUnitParser and leave everything else as is.
  2. create a new helper class in rolekit.server.io.systemd that provides a standard way for creating and writing out unit files that will eliminate the bolierplate found in various areas.

Which alternative should I pursue?

alxgrtnstrngl commented 8 years ago

Decided that the first alternative was the least invasive and went ahead and wrote a small patch to address. Please see alxgrtnstrngl/rolekit@904b5ad5c356824715049da4317a05d3dc4835a7 for review and let me know when you want me to create a pull request. Also created a test for this and it creates the following unit file 'testcontainer.service':

[Unit]
Description=this test container
Requires=docker.service
After=docker.service

[Service]
Restart=always
ExecStart=/usr/bin/docker run --name=testcontainer --env var_y=value_y --env var_x=value_x -p 8080 -p 443 testimage
ExecStop=/usr/bin/docker stop -t 5 testcontainer ; /usr/bin/docker rm -f testcontainer

[Install]
WantedBy=multi-user.target
sgallagher commented 8 years ago

Alex, that patch looks pretty good (aside from the leftover comments). Clean it up and submit it for a pull request, please.