From 021767c903f032a2db193cbe5e5e25b1ca385c20 Mon Sep 17 00:00:00 2001 From: Joachim Wiberg Date: Fri, 15 Nov 2024 08:51:48 +0100 Subject: [PATCH] confd: refactor and refine container creation - Reduce the amount of queues: 3 -> 1 - Simplify post hook - Refine execd The resulting simplification of infix_containers_post_hook(), and touching execd, also ensure container environment variable changes are propagated. Fixes #822 Signed-off-by: Joachim Wiberg --- board/common/rootfs/usr/sbin/container | 2 +- package/execd/execd.conf | 2 +- package/execd/tmpfiles.conf | 1 - src/confd/src/infix-containers.c | 169 +++++++++---------------- src/execd/execd.c | 60 ++++----- 5 files changed, 83 insertions(+), 151 deletions(-) diff --git a/board/common/rootfs/usr/sbin/container b/board/common/rootfs/usr/sbin/container index bd3eebb2..79e50504 100755 --- a/board/common/rootfs/usr/sbin/container +++ b/board/common/rootfs/usr/sbin/container @@ -627,7 +627,7 @@ case $cmd in ;; upgrade) # Start script used to initially create container - script=/run/containers/active/S01-${1}.sh + script=/run/containers/${1}.sh # Find container image img=$(podman inspect "$1" | jq -r .[].ImageName) diff --git a/package/execd/execd.conf b/package/execd/execd.conf index 1a8da3b8..0a6e2e6c 100644 --- a/package/execd/execd.conf +++ b/package/execd/execd.conf @@ -1,4 +1,4 @@ task log:prio:local1.notice,tag:container-flush \ [S] container flush -- Cleaning up lingering containers service log:prio:local1.err,tag:container \ - [2345] execd /run/containers/queue /run/containers/active -- Container job runner + [2345] execd /run/containers/queue -- Container job runner diff --git a/package/execd/tmpfiles.conf b/package/execd/tmpfiles.conf index 6ee94af2..0d97bce6 100644 --- a/package/execd/tmpfiles.conf +++ b/package/execd/tmpfiles.conf @@ -3,6 +3,5 @@ d /run/containers/files 0700 - - d /var/lib/containers/oci 0755 - - d /run/containers/inbox 0700 - - d /run/containers/queue 0700 - - -d /run/containers/active 0700 - - d /run/cni 0755 - - L+ /var/lib/cni - - - - /run/cni diff --git a/src/confd/src/infix-containers.c b/src/confd/src/infix-containers.c index d6b85692..5db409fd 100644 --- a/src/confd/src/infix-containers.c +++ b/src/confd/src/infix-containers.c @@ -13,13 +13,16 @@ #include #include "core.h" + #define ARPING_MSEC 1000 +#define LOGGER "logger -t container -p local1.notice" + #define MODULE "infix-containers" #define CFG_XPATH "/infix-containers:containers" -#define INBOX_QUEUE "/run/containers/inbox" -#define JOB_QUEUE "/run/containers/queue" -#define ACTIVE_QUEUE "/run/containers/active" -#define LOGGER "logger -t container -p local1.notice" + +#define _PATH_CONT "/run/containers" +#define _PATH_INBOX _PATH_CONT "/INBOX" +#define _PATH_QUEUE _PATH_CONT "/queue" static int add(const char *name, struct lyd_node *cif) @@ -27,11 +30,13 @@ static int add(const char *name, struct lyd_node *cif) const char *image = lydx_get_cattr(cif, "image"); const char *restart_policy, *string; struct lyd_node *node, *nets, *caps; + char script[strlen(name) + 5]; FILE *fp, *ap; - fp = fopenf("w", "%s/S01-%s.sh", INBOX_QUEUE, name); + snprintf(script, sizeof(script), "%s.sh", name); + fp = fopenf("w", "%s/%s", _PATH_CONT, script); if (!fp) { - ERRNO("Failed adding job S01-%s.sh to job queue" INBOX_QUEUE, name); + ERRNO("Failed creating container script %s/%s", _PATH_CONT, script); return SR_ERR_SYS; } @@ -176,37 +181,45 @@ static int add(const char *name, struct lyd_node *cif) fprintf(fp, " %s", string); fprintf(fp, "\n"); + + if (lydx_is_enabled(cif, "manual")) + fprintf(fp, "initctl -bnq cond set container:%s\n", name); + fchmod(fileno(fp), 0700); fclose(fp); systemf("initctl -bnq enable container@%s.conf", name); + /* + * All start scripts must wait for the rest of confd to complete + * before being enqueued to execd, so we postpone it using this + * "inbox" to the post hook. + */ + writesf(script, "a", "%s", _PATH_INBOX); + return 0; } static int del(const char *name) { - const char *queue[] = { - JOB_QUEUE, - INBOX_QUEUE, - ACTIVE_QUEUE, - }; + char fn[strlen(_PATH_QUEUE) + strlen(name) + 10]; FILE *fp; /* Remove any pending download/create job first */ - for (size_t i = 0; i < NELEMS(queue); i++) { - char fn[strlen(queue[i]) + strlen(name) + 5]; + snprintf(fn, sizeof(fn), "%s/S01-%s.sh", _PATH_QUEUE, name); + erase(fn); - snprintf(fn, sizeof(fn), "%s/%s.sh", queue[i], name); - erase(fn); - } + /* Remove container script itself */ + snprintf(fn, sizeof(fn), "%s/%s.sh", _PATH_CONT, name); + erase(fn); /* Disable service and schedule for deletion. */ systemf("initctl -bnq disable container@%s.conf", name); - fp = fopenf("w", "%s/K01-%s.sh", INBOX_QUEUE, name); + snprintf(fn, sizeof(fn), "%s/K01-%s.sh", _PATH_CONT, name); + fp = fopen(fn, "w"); if (!fp) { - ERRNO("Failed adding job 00-delete-%s.sh to job queue" INBOX_QUEUE, name); + ERRNO("Failed creating container stop script %s", fn); return SR_ERR_SYS; } @@ -215,6 +228,9 @@ static int del(const char *name) fchmod(fileno(fp), 0700); fclose(fp); + /* Enqueue kill job immediately on execd */ + movefile(fn, _PATH_QUEUE); + return SR_ERR_OK; } @@ -330,109 +346,42 @@ static int oci_load(sr_session_ctx_t *session, uint32_t sub_id, const char *xpat return SR_ERR_OK; } -static int is_active(sr_session_ctx_t *session, const char *name) -{ - return srx_enabled(session, CFG_XPATH "/container[name='%s']/enabled", name); -} - -static int is_manual(sr_session_ctx_t *session, const char *name) -{ - return srx_enabled(session, CFG_XPATH "/container[name='%s']/manual", name); -} - -/* - * When container configurations are not saved to startup-config and the - * user reboot the system (or lose power) we will have lingering active - * containers cached on persistent storage. - * - * This function runs every time a configuration is applied to clean up - * any lingering active jobs to prevent false matches in the cmp magic - * in the below post-hook. - */ -static void cleanup(sr_session_ctx_t *session, struct confd *confd) -{ - struct dirent *d; - DIR *dir; - - dir = opendir(ACTIVE_QUEUE); - if (!dir) - return; - - while ((d = readdir(dir))) { - char name[strlen(ACTIVE_QUEUE) + strlen(d->d_name) + 2]; - char *ptr; - - if (d->d_name[0] == '.') - continue; - - strlcpy(name, d->d_name, sizeof(name)); - ptr = strstr(name, ".sh"); - if (!ptr) - continue; /* odd, non-script file? */ - *ptr = 0; - - if (strncmp(name, "S01-", 4)) - continue; /* odd, not start script? */ - - if (is_active(session, &name[4])) - continue; - - /* Not found in running-config, remove stale cache. */ - snprintf(name, sizeof(name), "%s/%s", ACTIVE_QUEUE, d->d_name); - if (erase(name)) - ERRNO("Failed removing stale container job %s", name); - } - - closedir(dir); -} - /* * Containers depend on a lot of other system resources being properly * set up, e.g., networking, which is run by dagger. So we need to wait - * for all that before we can launch new, or modified, containers. + * for all that before we can launch new, or modified, containers. This + * post hook runs as (one of) the last actions on a config change/boot. */ void infix_containers_post_hook(sr_session_ctx_t *session, struct confd *confd) { - struct dirent *d; - DIR *dir; + char script[256]; + FILE *fp; - cleanup(session, confd); + fp = fopen(_PATH_INBOX, "r"); + if (!fp) + return; /* nothing to do today */ - dir = opendir(INBOX_QUEUE); - if (!dir) { - ERROR("Cannot open %s to launch scripts.", INBOX_QUEUE); - return; + while (fgets(script, sizeof(script), fp)) { + char link[strlen(_PATH_QUEUE) + strlen(script) + 10]; + char path[strlen(script) + 10]; + + chomp(script); + + /* + * Enqueue start job on execd, use a symlink since we + * want to be able to reuse the script for manual image + * uprgade (and debugging) purposes. + */ + snprintf(link, sizeof(link), "%s/S01-%s", _PATH_QUEUE, script); + snprintf(path, sizeof(path), "../%s", script); + if (symlink(path, link) && errno != EEXIST) + ERRNO("Creating symlink %s -> %s", link, path); } - while ((d = readdir(dir))) { - char next[strlen(INBOX_QUEUE) + strlen(d->d_name) + 2]; - char name[strlen(d->d_name) + 1]; - char *ptr; + fclose(fp); + erase(_PATH_INBOX); - if (d->d_name[0] == '.') - continue; - - snprintf(next, sizeof(next), "%s/%s", INBOX_QUEUE, d->d_name); - - strlcpy(name, d->d_name, sizeof(name)); - ptr = strstr(name, ".sh"); - if (ptr) { - char *nm = NULL; - - *ptr = 0; - if (!strncmp(name, "S01-", 4)) - nm = &name[4]; - - if (nm && !is_manual(session, nm)) - systemf("initctl -bnq cond set container:%s", nm); - } - - if (movefile(next, JOB_QUEUE)) - ERRNO("Failed moving %s to job queue %s", next, JOB_QUEUE); - } - - closedir(dir); - systemf("container volume prune -f >/dev/null 2>&1"); + systemf("initctl -bnq touch execd"); } int infix_containers_init(struct confd *confd) diff --git a/src/execd/execd.c b/src/execd/execd.c index 03a23f36..1c7c03a9 100644 --- a/src/execd/execd.c +++ b/src/execd/execd.c @@ -30,9 +30,8 @@ static int logmask = LOG_UPTO(LOG_NOTICE); static char buffer[BUFSIZ]; -static char *done; -static void run_job(const char *path, char *file, int archive) +static void run_job(const char *path, char *file) { char cmd[strlen(path) + strlen(file) + 2]; int rc; @@ -57,11 +56,7 @@ static void run_job(const char *path, char *file, int archive) return; } - dbg("job %s in %s done %p, archive: %d", file, path, done, archive); - if (done && archive) - movefile(cmd, done); - else - erase(cmd); + erase(cmd); } /* @@ -69,7 +64,7 @@ static void run_job(const char *path, char *file, int archive) * a type '*' just to figure out if a job should be archived in * the done directory. */ -static int should_run(const char *name, int type, int *archive) +static int should_run(const char *name, int type) { if (!name || strlen(name) < 3) return 0; @@ -78,32 +73,25 @@ static int should_run(const char *name, int type, int *archive) if (type == '*') { switch (name[0]) { case 'K': - *archive = 0; - return 1; case 'S': - *archive = 1; return 1; default: - errx("unsupported '%s', scripts must start with S or K", name); - return 0; + goto done; } } switch (type) { case 'K': - *archive = 0; - break; case 'S': - *archive = 1; break; default: return 0; } - dbg("name:%s type:'%c' archive:%d => run:%d", name, type, *archive, type == name[0]); + dbg("name:%s type:'%c' => run:%d", name, type, type == name[0]); return type == name[0]; } - +done: errx("unsupported script %s, must follow pattern SNN/KNN", name); return 0; } @@ -111,7 +99,6 @@ static int should_run(const char *name, int type, int *archive) static void run_dir(const char *path, int type) { struct dirent **namelist; - int archive = 0; int n, i; n = scandir(path, &namelist, NULL, alphasort); @@ -126,8 +113,8 @@ static void run_dir(const char *path, int type) if (d->d_type == DT_DIR) continue; - if (should_run(d->d_name, type, &archive)) - run_job(path, d->d_name, archive); + if (should_run(d->d_name, type)) + run_job(path, d->d_name); free(d); } @@ -139,7 +126,7 @@ static void run_dir(const char *path, int type) * Call stop/cleanup jobs first, may use same container name or * resources as replacement container start scripts use. */ -static void run_queue(char *path) +static void run_queue(const char *path) { run_dir(path, 'K'); run_dir(path, 'S'); @@ -174,14 +161,13 @@ static void inotify_cb(uev_t *w, void *arg, int _) for (char *p = buffer; p < buffer + bytes;) { struct inotify_event *event = (struct inotify_event *)p; char *name = event->name; - int archive; if (event->mask & (IN_CLOSE_WRITE | IN_ATTRIB | IN_MOVED_TO)) { dbg("Got inotify event %s 0x%04x", name, event->mask); - if (!should_run(name, '*', &archive)) + if (!should_run(name, '*')) continue; - run_job(arg, name, archive); + run_job(arg, name); } p += sizeof(struct inotify_event) + event->len; @@ -225,13 +211,13 @@ int logmask_from_str(const char *str) static int usage(char *arg0, int rc) { printf("Usage:\n" - " %s [-dh] [-l LVL] JOBDIR\n" + " %s [-dh] [-l LVL] QUEUE\n" "Options:\n" " -d Log to stderr as well\n" " -h This help text\n" " -l LVL Set log level: none, err, warn, notice*, info, debug\n" "\n" - "Runs jobs from JOBDIR, re-runs failing jobs on route changes or SIGHUP.\n" + "Runs jobs from QUEUE, re-runs failing jobs on route changes or SIGHUP.\n" "Use SIGUSR1 to toggle debug messages at runtime.\n", arg0); return rc; @@ -246,7 +232,7 @@ int main(int argc, char *argv[]) uev_t sighup_watcher; int logopt = LOG_PID; int wd, sd, fd, c; - char *jobdir; + char *queue; uev_ctx_t ctx; int rc = 0; @@ -272,13 +258,11 @@ int main(int argc, char *argv[]) if (optind >= argc) return usage(argv[0], 1); - jobdir = argv[optind++]; - if (optind < argc) - done = argv[optind]; + queue = argv[optind]; - if (access(jobdir, X_OK)) { + if (access(queue, X_OK)) { fprintf(stderr, "Cannot find job directory %s, errno %d: %s\n", - jobdir, errno, strerror(errno)); + queue, errno, strerror(errno)); return 1; } @@ -299,7 +283,7 @@ int main(int argc, char *argv[]) return 1; } - wd = inotify_add_watch(fd, jobdir, IN_CLOSE_WRITE | IN_ATTRIB | IN_MOVED_TO); + wd = inotify_add_watch(fd, queue, IN_CLOSE_WRITE | IN_ATTRIB | IN_MOVED_TO); if (wd == -1) { err("inotify_add_watch"); close(fd); @@ -323,7 +307,7 @@ int main(int argc, char *argv[]) } uev_init(&ctx); - if (uev_signal_init(&ctx, &sighup_watcher, signal_cb, jobdir, SIGHUP) == -1) { + if (uev_signal_init(&ctx, &sighup_watcher, signal_cb, queue, SIGHUP) == -1) { err("uev_signal_init (sighup)"); rc = 1; goto done; @@ -334,19 +318,19 @@ int main(int argc, char *argv[]) goto done; } - if (uev_io_init(&ctx, &inotify_watcher, inotify_cb, jobdir, fd, UEV_READ) == -1) { + if (uev_io_init(&ctx, &inotify_watcher, inotify_cb, queue, fd, UEV_READ) == -1) { err("uev_io_init (inotify)"); rc = 1; goto done; } - if (uev_io_init(&ctx, &netlink_watcher, netlink_cb, jobdir, sd, UEV_READ) == -1) { + if (uev_io_init(&ctx, &netlink_watcher, netlink_cb, queue, sd, UEV_READ) == -1) { err("uev_io_init (netlink)"); rc = 1; goto done; } - run_queue(jobdir); + run_queue(queue); if (uev_run(&ctx, 0) == -1) { err("uev_run"); rc = 1;