Merge branch 'bugfix/fix-exec-when-containerd-restart' into q/133.0

Author: bert-e

CHANGELOG.md

@@ -30,6 +30,12 @@
   and Loki image version to [3.6.5](https://github.com/grafana/loki/releases/tag/v3.6.5)
   (PR[#4792](https://github.com/scality/metalk8s/pull/4792))
 
+### Bug Fixes
+
+- Fix a bug where part of the upgrade process would silently be skipped
+  if the containerd socket is lost (crictl exec would exit with code 0)
+  (PR[#4802](https://github.com/scality/metalk8s/pull/4802))
+
 ## Release 132.0.2 (in development)
 
 ## Release 132.0.1

salt/_modules/cri.py

@@ -99,48 +99,6 @@ def pull_image(image):
     return ret
 
 
-def execute(name, command, *args):
-    """
-    Run a command in a container.
-
-    .. note::
-
-       This uses the :command:`crictl` command, which should be configured
-       correctly on the system, e.g. in :file:`/etc/crictl.yaml`.
-
-    name
-        Name of the target container
-    command
-        Command to run
-    args
-        Command parameters
-    """
-    log.info('Retrieving ID of container "%s"', name)
-    out = __salt__["cmd.run_all"](
-        f'crictl ps -q --label io.kubernetes.container.name="{name}"'
-    )
-
-    if out["retcode"] != 0:
-        log.error('Failed to find container "%s"', name)
-        return None
-
-    container_id = out["stdout"]
-    if not container_id:
-        log.error('Container "%s" does not exists', name)
-        return None
-
-    cmd_opts = f"{command} {' '.join(args)}"
-
-    log.info('Executing command "%s"', cmd_opts)
-    out = __salt__["cmd.run_all"](f"crictl exec {container_id} {cmd_opts}")
-
-    if out["retcode"] != 0:
-        log.error('Failed run command "%s"', cmd_opts)
-        return None
-
-    return out["stdout"]
-
-
 def wait_container(name, state, timeout=60, delay=5):
     """
     Wait for a container to be in given state.

salt/tests/unit/modules/test_cri.py

@@ -157,40 +157,6 @@ def test_pull_image(self, retcode, stdout, result):
             self.assertEqual(cri.pull_image("my-images"), result)
             mock_cmd.assert_called_once_with('crictl pull "my-images"')
 
-    @parameterized.expand(
-        [
-            (0, "292c3b07b", 0, "All ok", "All ok"),
-            (1, "292c3b07b", 0, "All ok", None),
-            (0, "", 0, "All ok", None),
-            (0, "292c3b07b", 1, "All not ok", None),
-            (0, "292c3b07b", 0, "", ""),
-        ]
-    )
-    def test_execute(self, ret_ps, stdout_ps, ret_exec, stdout_exec, result):
-        """
-        Tests the return of `execute` function
-        """
-        cmd_ps = utils.cmd_output(retcode=ret_ps, stdout=stdout_ps)
-        cmd_exec = utils.cmd_output(retcode=ret_exec, stdout=stdout_exec)
-
-        def _cmd_run_all_mock(cmd):
-            if "crictl ps" in cmd:
-                return cmd_ps
-            elif "crictl exec" in cmd:
-                return cmd_exec
-            return None
-
-        mock_cmd = MagicMock(side_effect=_cmd_run_all_mock)
-        with patch.dict(cri.__salt__, {"cmd.run_all": mock_cmd}):
-            self.assertEqual(cri.execute("my_cont", "my command"), result)
-            mock_cmd.assert_any_call(
-                'crictl ps -q --label io.kubernetes.container.name="my_cont"'
-            )
-            if ret_ps == 0 and stdout_ps:
-                mock_cmd.assert_called_with(
-                    "crictl exec {} my command ".format(stdout_ps)
-                )
-
     @parameterized.expand(
         [
             # Success: Found one container

scripts/backup.sh.in

@@ -127,7 +127,7 @@ backup_etcd() {
     # etcd 3.4+ defaults to API v3, so ETCDCTL_API=3 is no longer required.
     # In distroless images, the root filesystem is read-only, so we save the
     # snapshot to /var/lib/etcd which is a writable mounted volume.
-    crictl exec -i "$etcd_container" \
+    "${EXEC_CONTAINER_COMMAND[@]}" "$etcd_container" \
         etcdctl \
         --endpoints=https://127.0.0.1:2379 \
         --cert=/etc/kubernetes/pki/etcd/salt-master-etcd-client.crt \
@@ -167,7 +167,7 @@ EOF
 }
 
 replicate_archives() {
-    salt_master_exec=(crictl exec -i "$(get_salt_container)")
+    salt_master_exec=("${EXEC_CONTAINER_COMMAND[@]}" "$(get_salt_container)")
 
     "${salt_master_exec[@]}" salt-run --state-output=mixed state.orchestrate \
         metalk8s.orchestrate.backup.replication \

scripts/bootstrap.sh.in

@@ -95,7 +95,7 @@ orchestrate_bootstrap() {
             pillarenv=metalk8s-@@VERSION \
             pillar="${pillar[*]}"
 
-    SALT_MASTER_CALL=(crictl exec -i "$(get_salt_container)")
+    SALT_MASTER_CALL=("${EXEC_CONTAINER_COMMAND[@]}" "$(get_salt_container)")
 
     run "Syncing Utility modules on Salt master" \
         "${SALT_MASTER_CALL[@]}" salt-run --state-output=mixed saltutil.sync_utils \

scripts/common.sh.in

@@ -15,6 +15,11 @@ SYSTEMCTL=${SYSTEMCTL:-$(command -v systemctl)}
 PYTHON=${PYTHON:-$(command -v python3 || command -v python || true)}
 LOGFILE_MAX_ROTATIONS=10
 
+# NOTE: We do not use kubectl nor crictl to execute commands in containers,
+# since some commands may restart kube-apiserver and/or containerd and/or kubelet.
+# That's why we use runc directly to execute commands in containers.
+EXEC_CONTAINER_COMMAND=(runc --root=/run/containerd/runc/k8s.io exec)
+
 
 determine_os() {
     # We rely on /etc/os-release to discover the OS because its present on all
@@ -355,7 +360,7 @@ get_salt_minion_ids() {
 
     (
         set -o pipefail
-        retry 5 10 crictl exec -i "$salt_container" \
+        retry 5 10 "${EXEC_CONTAINER_COMMAND[@]}" "$salt_container" \
             salt \* grains.get id --out txt | \
             cut -d ' ' -f 2
     )

scripts/downgrade.sh.in

@@ -89,7 +89,7 @@ cleanup() {
 trap cleanup EXIT
 
 precheck_downgrade () {
-    SALT_MASTER_CALL=(crictl exec -i "$(get_salt_container)")
+    SALT_MASTER_CALL=("${EXEC_CONTAINER_COMMAND[@]}" "$(get_salt_container)")
     "${SALT_MASTER_CALL[@]}" salt-run saltutil.sync_all \
         saltenv="$SALTENV"
     # Due to a bug in salt do not "raises" when we run from CLI
@@ -102,7 +102,7 @@ precheck_downgrade () {
 }
 
 launch_pre_downgrade () {
-    SALT_MASTER_CALL=(crictl exec -i "$(get_salt_container)")
+    SALT_MASTER_CALL=("${EXEC_CONTAINER_COMMAND[@]}" "$(get_salt_container)")
     "${SALT_MASTER_CALL[@]}" salt-run saltutil.sync_all \
         saltenv="$SALTENV"
 
@@ -112,14 +112,14 @@ launch_pre_downgrade () {
 }
 
 launch_post_downgrade () {
-    SALT_MASTER_CALL=(crictl exec -i "$(get_salt_container)")
+    SALT_MASTER_CALL=("${EXEC_CONTAINER_COMMAND[@]}" "$(get_salt_container)")
     "${SALT_MASTER_CALL[@]}" salt-run state.orchestrate \
         metalk8s.orchestrate.downgrade.post \
         saltenv="$SALTENV"
 }
 
 launch_downgrade () {
-    SALT_MASTER_CALL=(crictl exec -i "$(get_salt_container)")
+    SALT_MASTER_CALL=("${EXEC_CONTAINER_COMMAND[@]}" "$(get_salt_container)")
     # NOTE: We want to use `$SALTENV` version for the runners
     "${SALT_MASTER_CALL[@]}" salt-run saltutil.sync_all \
         extmod_whitelist="{'runners': []}" \
@@ -146,7 +146,7 @@ downgrade_bootstrap () {
     repo_endpoint="$($SALT_CALL pillar.get metalk8s:endpoints:repositories --out txt \
         | cut -d' ' -f2- )"
 
-    SALT_MASTER_CALL=(crictl exec -i "$(get_salt_container)")
+    SALT_MASTER_CALL=("${EXEC_CONTAINER_COMMAND[@]}" "$(get_salt_container)")
     "${SALT_MASTER_CALL[@]}" salt-run state.orchestrate \
         metalk8s.orchestrate.bootstrap.pre-downgrade \
         saltenv="$SALTENV" || return 1
@@ -159,7 +159,7 @@ downgrade_bootstrap () {
 
 # patch the kube-system namespace annotation with <destination-version> input
 patch_kubesystem_namespace() {
-    SALT_MASTER_CALL=(crictl exec -i "$(get_salt_container)")
+    SALT_MASTER_CALL=("${EXEC_CONTAINER_COMMAND[@]}" "$(get_salt_container)")
 
     "${SALT_MASTER_CALL[@]}" salt-run saltutil.sync_all \
         saltenv="$SALTENV"

scripts/restore.sh.in

@@ -182,7 +182,7 @@ configure_salt_master() {
         saltenv=metalk8s-@@VERSION \
         pillar="${pillar[*]}"
 
-    SALT_MASTER_CALL=(crictl exec -i "$(get_salt_container)")
+    SALT_MASTER_CALL=("${EXEC_CONTAINER_COMMAND[@]}" "$(get_salt_container)")
 
     "${SALT_MASTER_CALL[@]}" salt-run saltutil.sync_all \
         saltenv=metalk8s-@@VERSION
@@ -237,7 +237,7 @@ push_cas() {
 }
 
 mark_control_plane() {
-    SALT_MASTER_CALL=(crictl exec -i "$(get_salt_container)")
+    SALT_MASTER_CALL=("${EXEC_CONTAINER_COMMAND[@]}" "$(get_salt_container)")
 
     local -r bootstrap_id=$(
         ${SALT_CALL} --local --out txt grains.get id \
@@ -252,7 +252,7 @@ mark_control_plane() {
 }
 
 reconfigure_nodes() {
-    SALT_MASTER_CALL=(crictl exec -i "$(get_salt_container)")
+    SALT_MASTER_CALL=("${EXEC_CONTAINER_COMMAND[@]}" "$(get_salt_container)")
     local -r non_bootstrap=$(
         ${SALT_CALL} --out=txt slsutil.renderer \
         string="{{ pillar.metalk8s.nodes.keys() | difference(salt.metalk8s.minions_by_role('bootstrap')) | join(',') }}" \
@@ -290,7 +290,7 @@ reconfigure_nodes() {
 }
 
 highstate_bootstrap() {
-    SALT_MASTER_CALL=(crictl exec -i "$(get_salt_container)")
+    SALT_MASTER_CALL=("${EXEC_CONTAINER_COMMAND[@]}" "$(get_salt_container)")
     local -r bootstrap_id=$(
         ${SALT_CALL} --local --out txt grains.get id \
         | awk '/^local\: /{ print $2 }'
@@ -306,7 +306,7 @@ highstate_bootstrap() {
 }
 
 reconfigure_k8s_obj() {
-    SALT_MASTER_CALL=(crictl exec -i "$(get_salt_container)")
+    SALT_MASTER_CALL=("${EXEC_CONTAINER_COMMAND[@]}" "$(get_salt_container)")
 
     "${SALT_MASTER_CALL[@]}" salt-run --state-output=mixed state.orchestrate \
         metalk8s.addons.nginx-ingress-control-plane.deployed \

scripts/solutions.sh

@@ -241,7 +241,7 @@ salt_minion_exec() {
 salt_master_exec() {
     SALTENV=${SALTENV:-$(get_salt_env)}
     SALT_MASTER_CONTAINER_ID=${SALT_MASTER_CONTAINER_ID:-$(get_salt_container)}
-    crictl exec -i "$SALT_MASTER_CONTAINER_ID" "$@" saltenv="$SALTENV"
+    "${EXEC_CONTAINER_COMMAND[@]}" "$SALT_MASTER_CONTAINER_ID" "$@" saltenv="$SALTENV"
 }
 
 activate_solution() {

scripts/upgrade.sh.in

@@ -80,7 +80,7 @@ upgrade_bootstrap () {
     "$SALT_CALL" saltutil.sync_all saltenv="$SALTENV"
     "$SALT_CALL" saltutil.refresh_grains
 
-    SALT_MASTER_CALL=(crictl exec -i "$(get_salt_container)")
+    SALT_MASTER_CALL=("${EXEC_CONTAINER_COMMAND[@]}" "$(get_salt_container)")
     "${SALT_MASTER_CALL[@]}" salt-run state.orchestrate \
         metalk8s.orchestrate.bootstrap.pre-upgrade \
         saltenv="$SALTENV"
@@ -93,7 +93,7 @@ upgrade_bootstrap () {
 }
 
 launch_pre_upgrade () {
-    SALT_MASTER_CALL=(crictl exec -i "$(get_salt_container)")
+    SALT_MASTER_CALL=("${EXEC_CONTAINER_COMMAND[@]}" "$(get_salt_container)")
     "${SALT_MASTER_CALL[@]}" salt-run saltutil.sync_all \
         saltenv="$SALTENV"
 
@@ -103,14 +103,14 @@ launch_pre_upgrade () {
 }
 
 launch_post_upgrade () {
-    SALT_MASTER_CALL=(crictl exec -i "$(get_salt_container)")
+    SALT_MASTER_CALL=("${EXEC_CONTAINER_COMMAND[@]}" "$(get_salt_container)")
     "${SALT_MASTER_CALL[@]}" salt-run state.orchestrate \
         metalk8s.orchestrate.upgrade.post \
         saltenv="$SALTENV"
 }
 
 upgrade_etcd () {
-    SALT_MASTER_CALL=(crictl exec -i "$(get_salt_container)")
+    SALT_MASTER_CALL=("${EXEC_CONTAINER_COMMAND[@]}" "$(get_salt_container)")
     "${SALT_MASTER_CALL[@]}" salt-run saltutil.sync_all \
         saltenv="$SALTENV"
 
@@ -125,7 +125,7 @@ upgrade_etcd () {
 }
 
 upgrade_apiservers () {
-    SALT_MASTER_CALL=(crictl exec -i "$(get_salt_container)")
+    SALT_MASTER_CALL=("${EXEC_CONTAINER_COMMAND[@]}" "$(get_salt_container)")
     "${SALT_MASTER_CALL[@]}" salt-run state.orchestrate \
         metalk8s.orchestrate.apiserver saltenv="$SALTENV"
 }
@@ -166,14 +166,14 @@ upgrade_local_engines () {
 }
 
 upgrade_nodes () {
-    SALT_MASTER_CALL=(crictl exec -i "$(get_salt_container)")
+    SALT_MASTER_CALL=("${EXEC_CONTAINER_COMMAND[@]}" "$(get_salt_container)")
     "${SALT_MASTER_CALL[@]}" salt-run state.orchestrate \
         metalk8s.orchestrate.upgrade saltenv="$SALTENV" \
         pillar="{'orchestrate': {'drain_timeout': $DRAIN_TIMEOUT}}"
 }
 
 precheck_upgrade() {
-    SALT_MASTER_CALL=(crictl exec -i "$(get_salt_container)")
+    SALT_MASTER_CALL=("${EXEC_CONTAINER_COMMAND[@]}" "$(get_salt_container)")
     "${SALT_MASTER_CALL[@]}" salt-run saltutil.sync_all \
         saltenv="$SALTENV"
     # Due to a bug in salt do not "raises" when we run from CLI
@@ -186,7 +186,7 @@ precheck_upgrade() {
 
 # patch the kube-system namespace annotation with <destination-version> input
 patch_kubesystem_namespace() {
-    SALT_MASTER_CALL=(crictl exec -i "$(get_salt_container)")
+    SALT_MASTER_CALL=("${EXEC_CONTAINER_COMMAND[@]}" "$(get_salt_container)")
 
     "${SALT_MASTER_CALL[@]}" salt-run saltutil.sync_all \
         saltenv="$SALTENV"