From d1fac6f5b3a79cd945f83a6b8a1ad3e53e197b24 Mon Sep 17 00:00:00 2001 From: Joachim Wiberg Date: Thu, 11 Feb 2021 07:58:06 +0100 Subject: [PATCH] Fix #143: redesign condition subsystem Change service conditions from the non-obvious to , utilizing the unique name of the service instead of the weird composition of paths from /run and PID file names. Hopefully making it more clear that one service's pidfile is another service's `` condition. Note: this is an incompatible change to the condition system! The Finit major version will be stepped to indicate this. Signed-off-by: Joachim Wiberg --- ChangeLog.md | 6 ++-- src/cond-w.c | 90 +++++++++++++++------------------------------------- src/conf.c | 9 +++--- 3 files changed, 34 insertions(+), 71 deletions(-) diff --git a/ChangeLog.md b/ChangeLog.md index 2ebd4d07..bd6430f0 100644 --- a/ChangeLog.md +++ b/ChangeLog.md @@ -21,9 +21,9 @@ Major bug fix release. New features include cgroups and a new progress! * Incompatible `configure` script changes, i.e., you must give proper path arguments to the script, no more guessing just GNU defaults. There are examples in the documentation and the `contrib/` section -* Change service PID file conditions from `` to ``. - This will hopefully make it more clear that one service's 'pid:!foo' - pidfile is another service's condition +* Change service conditions from the non-obvious `` to + ``. This to hopefully make it clear that one service's + 'pid:!foo' pidfile is another service's `` condition * Major refactor of Finit's `main()` function to be able to start the event loop earlier. This also facilitated factoring out functionality previously hard-coded in Finit, e.g., starting the bundled watchdogd, diff --git a/src/cond-w.c b/src/cond-w.c index df2e135b..d748e10a 100644 --- a/src/cond-w.c +++ b/src/cond-w.c @@ -32,84 +32,46 @@ #include "service.h" /* - * The service condition name is constructed from the 'pid/' prefix, - * the dirname of the svc->cmd, e.g., '/usr/bin/teamd' => 'usr/bin', and - * the service's pid:filename, including an optional subdirectory, - * without the .pid extension. + * The service condition name is constructed from the 'pid/' prefix and + * the unique NAME:ID tuple that identify each process in Finit. Here + * are a few examples: * - * The following example uses the team (aggregate) service: + * The Linux aggregate helper teamd creates PID files in a subdirectory, + * /run/teamd/lag1.pid: * - * service pid:!/run/teamd/a1.pid /usr/bin/teamd -f /etc/teamd/a1.conf + * service teamd -f /etc/teamd/lag1.conf -- Aggregate lag1 * - * => 'pid/' + 'usr/bin' + 'teamd/a1' => pid/usr/bin/teamd/a1 + * => 'pid/' + 'teamd' + '' = condition * - * The next example uses the dbus-daemon: + * When you add a second aggrate you need to tell Finit it's a different + * instance usugin the :ID syntax: * - * service pid:!/run/dbus/pid /usr/bin/dbus-daemon + * service :lag2 teamd -f /etc/teamd/lag2.conf -- Aggregate lag2 * - * => 'pid/' + 'usr/bin' + 'dbus' => pid/usr/bin/dbus + * => 'pid/' + 'teamd' + ':lag2' = condition * - * The last example uses lxc-start to start container foo: + * The next example is for the dbus-daemon. It also use a subdirectory, + * /run/dbus/pid: * - * service pid:!/run/lxc/foo.pid lxc-start -n foo -F -p /run/lxc/foo.pid -- Container foo + * service dbus-daemon -- DBus daemon * - * => 'pid/' + '' + 'lxc/foo' => pid/lxc/foo + * => 'pid/' + 'dbus-daemon' = condition * - * Note: previously the condition ws called 'svc/..', the conf.c parser - * automatically translates to the 'pid/..' prefix and warns. + * The last example uses lxc-start to start container foo, again in a + * subdirectory, /run/lxc/foo.pid. We set an ID to be user-friendly to + * ourselves and override the name usually derivedd using the basename + * of the path: + * + * service name:lxc :foo lxc-start -n foo -F -p /run/lxc/foo.pid -- Container foo + * + * => 'pid/' + 'lxc' + ':foo' = condition */ char *mkcond(svc_t *svc, char *buf, size_t len) { - char path[256]; - char *pidfile; - char *ptr, *nm; + char ident[sizeof(svc->name) + sizeof(svc->id) + 2]; - strlcpy(path, svc->cmd, sizeof(path)); - ptr = rindex(path, '/'); - if (ptr) - *ptr++ = 0; - else - path[0] = 0; - - /* Figure out default name used when registering service */ - if (ptr) - nm = ptr; - else - nm = svc->cmd; - - pidfile = pid_file(svc); - ptr = strstr(pidfile, "run"); - if (ptr) - ptr += 3; - else - ptr = rindex(pidfile, '/'); - - /* Custom name:foo declaration found => pid/foo instead of /pid/bin/path/pidfile-.pid */ - if (strcmp(nm, svc->name)) { - snprintf(buf, len, "pid/%s", svc->name); - _d("Composed condition from svc->name %s => %s", svc->name, buf); - } else { - snprintf(buf, len, "pid%s%s%s", path[0] != 0 && path[0] != '/' ? "/" : "", path, ptr); - _d("Composed condition from cmd %s (path %s) and pidfile %s => %s", svc->cmd, path, ptr, buf); - } - - /* Case: /var/run/dbus/pid */ - ptr = strstr(buf, "/pid"); - if (ptr && !strcmp(ptr, "/pid")) - *ptr = 0; - - /* Case /var/run/teamd/a1.pid */ - ptr = strstr(buf, ".pid"); - if (ptr && !strcmp(ptr, ".pid")) - *ptr = 0; - - /* Always append /ID if service is declared with :ID */ - if (svc->id[0]) { - strlcat(buf, "/", len); - strlcat(buf, svc->id, len); - } - - _d("Creating condition => %s", buf); + snprintf(buf, len, "pid/%s", svc_ident(svc, ident, sizeof(ident))); + _d("Created condition => %s", buf); return buf; } diff --git a/src/conf.c b/src/conf.c index ff6d430a..51d0bea0 100644 --- a/src/conf.c +++ b/src/conf.c @@ -237,10 +237,11 @@ void conf_parse_cond(svc_t *svc, char *cond) } if (!strncmp(ptr, "svc/", 4)) { - snprintf(svc->cond, sizeof(svc->cond), "pid/%s", &ptr[4]); - logit(LOG_INFO, "Migrating cond syntax of %s: %s -> %s", svc->cmd, ptr, svc->cond); - } else - strlcpy(svc->cond, ptr, sizeof(svc->cond)); + logit(LOG_ERR, "Unsupported cond syntax for %s: <%s", svc->cmd, ptr); + return; + } + + strlcpy(svc->cond, ptr, sizeof(svc->cond)); } struct rlimit_name {