From c076f0a7706ce0f09e6ac3831b1dc6f3ff03fec3 Mon Sep 17 00:00:00 2001 From: Tobias Waldekranz Date: Mon, 9 Dec 2024 10:18:46 +0100 Subject: [PATCH 1/3] confd: dagger: Reduce memory usage When reconfiguring the system many times (seen during regression testing), the RAM usage of keeping all generations around starts to add up. A tmpfs file, no matter how small, will always consume at least one full page (4k). So one generation of tiny setup scripts and log files can end up locking a 1 MB (!) of memory. Therefore: Once we're done with a generation, archive it in a compressed tarball, which typically fits inside a single 4k page. Limit the total number of archives to 10. This should give full visibility into the history of a system in all normal situations, while still allowing hammering systems with reconfig cycles in testing scenarios. --- src/confd/bin/dagger | 24 +++++++++++++++++++++++- src/confd/src/dagger.c | 12 ++++++++++++ 2 files changed, 35 insertions(+), 1 deletion(-) diff --git a/src/confd/bin/dagger b/src/confd/bin/dagger index e1fa0fff6..7f7de0ebe 100755 --- a/src/confd/bin/dagger +++ b/src/confd/bin/dagger @@ -104,7 +104,9 @@ do_abandon() [ "$next" ] && [ -d "$basedir/$next" ] \ || abort "Next generation does not exist" - mv "$basedir/$next" "$basedir/$next-ABANDONED-$(date +%F-%T)" + tar caf "$basedir/$next-ABANDONED-$(date +%F-%t).tar.gz" "$basedir/$next" + rm -rf "$basedir/$next" + rm "$basedir/next" inform info "Abandoned generation $next" } @@ -128,6 +130,19 @@ do_evolve() mv "$basedir/next" "$basedir/current" inform info "Evolved to generation $next" + + if [ "$current" ]; then + (cd "$basedir" && tar caf "$current.tar.gz" "$current") + rm -rf "$basedir/$current" + fi +} + +do_prune() +{ + local keep=${1:-10} + + cd "$basedir" + rm -f old=$(ls -rv *.tar.gz | tail "+$((keep + 1))") } usage() @@ -158,6 +173,10 @@ Commands: exec Run the specified action of the current generation. + prune [] + Remove all but the last , or 10 by default, + archived generations. + help Show this message. @@ -201,6 +220,9 @@ case $cmd in "exec") do_exec "$@" ;; + "prune") + do_prune "$@" + ;; *) usage && exit 1 diff --git a/src/confd/src/dagger.c b/src/confd/src/dagger.c index 36213b8ad..de98decf3 100644 --- a/src/confd/src/dagger.c +++ b/src/confd/src/dagger.c @@ -114,11 +114,23 @@ int dagger_evolve(struct dagger *d) return exitcode; } +static int dagger_prune(struct dagger *d) +{ + int exitcode; + + exitcode = systemf("dagger -C %s prune", d->path); + DEBUG("dagger(%d->%d): prune: exitcode=%d\n", + d->current, d->next, exitcode); + + return exitcode; +} + int dagger_evolve_or_abandon(struct dagger *d) { int exitcode, err; exitcode = dagger_evolve(d); + dagger_prune(d); if (!exitcode) return 0; From 28ae8fca425a186f29d09176f04d2fa446a39bc8 Mon Sep 17 00:00:00 2001 From: Tobias Waldekranz Date: Mon, 9 Dec 2024 13:34:22 +0100 Subject: [PATCH 2/3] confd: dagger: Get rid of some code duplication --- src/confd/bin/dagger | 44 ++++++++++++++++++++++++++++---------------- 1 file changed, 28 insertions(+), 16 deletions(-) diff --git a/src/confd/bin/dagger b/src/confd/bin/dagger index 7f7de0ebe..a72c87f9b 100755 --- a/src/confd/bin/dagger +++ b/src/confd/bin/dagger @@ -84,19 +84,25 @@ EOF done } -do_exec() +maybe_get_current() +{ + echo $([ -f "$basedir/current" ] && cat "$basedir/current" || true) + [ ! "$current" ] || [ -d "$basedir/$current" ] \ + || abort "Current generation does not exist" +} + +get_current() { - local action="$1" local current= - current=$([ -f "$basedir/current" ] && cat "$basedir/current") + current=$(maybe_get_current) [ "$current" ] && [ -d "$basedir/$current" ] \ || abort "Current generation does not exist" - action_exec "$1" "$basedir/$current" + echo "$current" } -do_abandon() +get_next() { local next= @@ -104,7 +110,21 @@ do_abandon() [ "$next" ] && [ -d "$basedir/$next" ] \ || abort "Next generation does not exist" - tar caf "$basedir/$next-ABANDONED-$(date +%F-%t).tar.gz" "$basedir/$next" + echo "$next" +} + +do_exec() +{ + local action="$1" + + action_exec "$1" "$basedir/$(get_current)" +} + +do_abandon() +{ + local next=$(get_next) + + (cd "$basedir" && tar caf "$next-ABANDONED-$(date +%F-%T.tar.gz)" "$next") rm -rf "$basedir/$next" rm "$basedir/next" @@ -113,16 +133,8 @@ do_abandon() do_evolve() { - local current= - local next= - - current=$([ -f "$basedir/current" ] && cat "$basedir/current" || true) - [ ! "$current" ] || [ -d "$basedir/$current" ] \ - || abort "Current generation does not exist" - - next=$([ -f "$basedir/next" ] && cat "$basedir/next" || true) - [ "$next" ] && [ -d "$basedir/$next" ] \ - || abort "Next generation does not exist" + local current=$(maybe_get_current) + local next=$(get_next) [ "$current" ] && action_exec exit "$basedir/$current" From 429c4f15735c34631f13a5033e6aa49ba87d5a31 Mon Sep 17 00:00:00 2001 From: Tobias Waldekranz Date: Tue, 10 Dec 2024 09:46:20 +0100 Subject: [PATCH 3/3] confd: dagger: Always increment generation number Previously, the generation number was only increased for successful evolutions. This led to a confusing state in /run/net where multiple -ABANDONED- directories could exist, with the same value of N. Move to a scheme where the generation increments every time we evolve, even if the evolution fails. If multiple consecutive evolutions fail, we can now unabiguously determine the order in which they were executed. --- src/confd/bin/dagger | 13 ++++++++----- src/confd/src/ietf-interfaces.c | 5 +++++ 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/src/confd/bin/dagger b/src/confd/bin/dagger index a72c87f9b..f5356d72d 100755 --- a/src/confd/bin/dagger +++ b/src/confd/bin/dagger @@ -124,10 +124,9 @@ do_abandon() { local next=$(get_next) - (cd "$basedir" && tar caf "$next-ABANDONED-$(date +%F-%T.tar.gz)" "$next") - rm -rf "$basedir/$next" + date +%F-%T >"$basedir/$next/ABANDONED" - rm "$basedir/next" + mv "$basedir/next" "$basedir/current" inform info "Abandoned generation $next" } @@ -135,6 +134,7 @@ do_evolve() { local current=$(maybe_get_current) local next=$(get_next) + local ar= [ "$current" ] && action_exec exit "$basedir/$current" @@ -144,8 +144,11 @@ do_evolve() inform info "Evolved to generation $next" if [ "$current" ]; then - (cd "$basedir" && tar caf "$current.tar.gz" "$current") - rm -rf "$basedir/$current" + ar="$current.tar.gz" + [ -f "$basedir/$current/ABANDONED" ] && ar="$current-ABANDONED.tar.gz" + + (cd "$basedir" && tar caf "$ar.tar.gz" "$current") + rm -rf "$basedir/$current" fi } diff --git a/src/confd/src/ietf-interfaces.c b/src/confd/src/ietf-interfaces.c index a54ccd038..ec914b94c 100644 --- a/src/confd/src/ietf-interfaces.c +++ b/src/confd/src/ietf-interfaces.c @@ -304,6 +304,11 @@ static int netdag_exit_reload(struct dagger *net) { FILE *initctl; + if (systemf("runlevel >/dev/null 2>&1")) + /* If we are still bootstrapping, there is nothing to + * reload. */ + return 0; + /* We may end up writing this file multiple times, e.g. if * multiple services are disabled in the same config cycle, * but since the contents of the file are static it doesn't