From 3f0fff6660e209c2c45fea7017c1d05f7bad49ae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luk=C3=A1=C5=A1=20Doktor?= Date: Thu, 23 Jan 2025 16:19:42 +0100 Subject: [PATCH 1/4] kata-deploy: Avoid removing containerd config when backup exists MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit let's rely on move to override the main configuration file to avoid situation where we are interrupted after the removal but before the move. Signed-off-by: Lukáš Doktor --- tools/packaging/kata-deploy/scripts/kata-deploy.sh | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tools/packaging/kata-deploy/scripts/kata-deploy.sh b/tools/packaging/kata-deploy/scripts/kata-deploy.sh index 75d9709a7a..6c08f26fbe 100755 --- a/tools/packaging/kata-deploy/scripts/kata-deploy.sh +++ b/tools/packaging/kata-deploy/scripts/kata-deploy.sh @@ -720,9 +720,10 @@ function cleanup_containerd() { return fi - rm -f $containerd_conf_file if [ -f "$containerd_conf_file_backup" ]; then - mv "$containerd_conf_file_backup" "$containerd_conf_file" + mv -f "$containerd_conf_file_backup" "$containerd_conf_file" + else + rm -f $containerd_conf_file fi } From 047ddfaac1fc1f8e0964932ec846445d8b96a2bb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luk=C3=A1=C5=A1=20Doktor?= Date: Thu, 23 Jan 2025 16:29:30 +0100 Subject: [PATCH 2/4] kata-deploy: Modify imports after they are defined MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit in case we modify the imports and get interrupted the containerd will be left broken. Let's first create the to-be-imported files first and then modify the containerd configuration to be more resilient against re-execution/termination. Signed-off-by: Lukáš Doktor --- tools/packaging/kata-deploy/scripts/kata-deploy.sh | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/tools/packaging/kata-deploy/scripts/kata-deploy.sh b/tools/packaging/kata-deploy/scripts/kata-deploy.sh index 6c08f26fbe..55980f184f 100755 --- a/tools/packaging/kata-deploy/scripts/kata-deploy.sh +++ b/tools/packaging/kata-deploy/scripts/kata-deploy.sh @@ -655,13 +655,14 @@ function configure_containerd() { cp -n "$containerd_conf_file" "$containerd_conf_file_backup" fi + for shim in "${shims[@]}"; do + configure_containerd_runtime "$1" $shim + done + if [ $use_containerd_drop_in_conf_file = "true" ]; then tomlq -i -t $(printf '.imports|=.+["%s"]' ${containerd_drop_in_conf_file}) ${containerd_conf_file} fi - for shim in "${shims[@]}"; do - configure_containerd_runtime "$1" $shim - done } function remove_artifacts() { From 8bf3582e830e855442b564a39202fa17848c6b3f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luk=C3=A1=C5=A1=20Doktor?= Date: Thu, 23 Jan 2025 16:32:37 +0100 Subject: [PATCH 3/4] kata-deploy: Quotate the shims variable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit to ensure it's just a single parameter. Signed-off-by: Lukáš Doktor --- tools/packaging/kata-deploy/scripts/kata-deploy.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/packaging/kata-deploy/scripts/kata-deploy.sh b/tools/packaging/kata-deploy/scripts/kata-deploy.sh index 55980f184f..4005182c5b 100755 --- a/tools/packaging/kata-deploy/scripts/kata-deploy.sh +++ b/tools/packaging/kata-deploy/scripts/kata-deploy.sh @@ -656,7 +656,7 @@ function configure_containerd() { fi for shim in "${shims[@]}"; do - configure_containerd_runtime "$1" $shim + configure_containerd_runtime "$1" "$shim" done if [ $use_containerd_drop_in_conf_file = "true" ]; then From f47d643b82fd391e77e5d2de3c47b8bd8ee5ef19 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luk=C3=A1=C5=A1=20Doktor?= Date: Thu, 23 Jan 2025 17:51:43 +0100 Subject: [PATCH 4/4] kata-deploy: Modify containerd config atomically MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit the kata-deploy pod can be restarted/interrupted at any time which might lead to partially modified files. Let's use a loop to configure containerd to minimize the probability of partial modification. Note this is still not perfect as there is a short non-atomic part, but given the order of changes it shouldn't result in partially modified files. Signed-off-by: Lukáš Doktor --- .../kata-deploy/scripts/kata-deploy.sh | 38 ++++++++++++++----- 1 file changed, 28 insertions(+), 10 deletions(-) diff --git a/tools/packaging/kata-deploy/scripts/kata-deploy.sh b/tools/packaging/kata-deploy/scripts/kata-deploy.sh index 4005182c5b..68ed175c4f 100755 --- a/tools/packaging/kata-deploy/scripts/kata-deploy.sh +++ b/tools/packaging/kata-deploy/scripts/kata-deploy.sh @@ -592,14 +592,14 @@ function configure_containerd_runtime() { local runtime="kata-${adjusted_shim_to_multi_install}" local configuration="configuration-${shim}" local pluginid=cri - local configuration_file="${containerd_conf_file}" + local configuration_file="$3" # Properly set the configuration file in case drop-in files are supported if [ $use_containerd_drop_in_conf_file = "true" ]; then configuration_file="/host${containerd_drop_in_conf_file}" fi - local containerd_root_conf_file="$containerd_conf_file" + local containerd_root_conf_file="$configuration_file" if [[ "$1" =~ ^(k0s-worker|k0s-controller)$ ]]; then containerd_root_conf_file="/etc/containerd/containerd.toml" fi @@ -649,15 +649,33 @@ function configure_containerd() { mkdir -p /etc/containerd/ - if [ $use_containerd_drop_in_conf_file = "false" ] && [ -f "$containerd_conf_file" ]; then - # only backup in case drop-in files are not supported, and when doing the backup - # only do it if a backup doesn't already exist (don't override original) - cp -n "$containerd_conf_file" "$containerd_conf_file_backup" - fi + # To avoid problems when this pod gets re-created let's be a bit paranoiac + # and try to configure/backup the most recent configuration more-less + # atomically. This shouldn't be that needed for drop-in configuration + # but shouldn't harm either. + local tmp_config_file=$(mktemp) + ( for i in {1..10}; do + local pre_config="$(cat "$containerd_conf_file")" + echo "$pre_config" > "$tmp_config_file" - for shim in "${shims[@]}"; do - configure_containerd_runtime "$1" "$shim" - done + for shim in "${shims[@]}"; do + configure_containerd_runtime "$1" "$shim" "$tmp_config_file" + done + + if [ "$(cat "$containerd_conf_file")" == "$pre_config" ]; then + # Nobody modified the containerd file, write our changes in + # least dangerous way (no atomicity of this section...) + if [ $use_containerd_drop_in_conf_file = "false" ] && [ -f "$containerd_conf_file" ]; then + # only backup in case drop-in files are not supported, and when doing the backup + # only do it if a backup doesn't already exist (don't override original) + cp -n "$containerd_conf_file" "$containerd_conf_file_backup" + fi + mv -f "$tmp_config_file" "$containerd_conf_file" + exit 0 + fi + sleep $(($RANDOM / 1000)) + done ) || die "Failed to configure containerd in 10 iterations, is someone else modifying ${containerd_conf_file}?" + rm -f "$tmp_config_file" if [ $use_containerd_drop_in_conf_file = "true" ]; then tomlq -i -t $(printf '.imports|=.+["%s"]' ${containerd_drop_in_conf_file}) ${containerd_conf_file}