From e6a813e2fa4acf1d160758e9982a4bd626a59beb Mon Sep 17 00:00:00 2001 From: Renuka Manavalan <47282725+renukamanavalan@users.noreply.github.com> Date: Fri, 16 Apr 2021 13:55:36 -0700 Subject: [PATCH] Kubernetes server configurable using URL 1) Dropped non-required IP update in admin.conf, as all masters use VIP only (#7288) 2) Don't clear VERSION during stop, as it would overwrite new version pending to go. 3) subprocess, get return value from proc and do not imply with presence of data in stderr. --- src/sonic-ctrmgrd/ctrmgr/container | 1 - src/sonic-ctrmgrd/ctrmgr/kube_commands.py | 40 ++++---------- src/sonic-ctrmgrd/tests/common_test.py | 9 +++- src/sonic-ctrmgrd/tests/container_test.py | 10 ++-- src/sonic-ctrmgrd/tests/kube_commands_test.py | 53 ------------------- 5 files changed, 25 insertions(+), 88 deletions(-) diff --git a/src/sonic-ctrmgrd/ctrmgr/container b/src/sonic-ctrmgrd/ctrmgr/container index ff3706a6a7a7..666c967540a7 100755 --- a/src/sonic-ctrmgrd/ctrmgr/container +++ b/src/sonic-ctrmgrd/ctrmgr/container @@ -278,7 +278,6 @@ def container_stop(feature, **kwargs): CURRENT_OWNER: "none", UPD_TIMESTAMP: str(datetime.datetime.now().strftime("%Y-%m-%d %H:%M:%S")), CONTAINER_ID: "", - VERSION: "", SYSTEM_STATE: "down" } if remote_state == "running": diff --git a/src/sonic-ctrmgrd/ctrmgr/kube_commands.py b/src/sonic-ctrmgrd/ctrmgr/kube_commands.py index db8cb348fcb2..5eaad0a82bb2 100755 --- a/src/sonic-ctrmgrd/ctrmgr/kube_commands.py +++ b/src/sonic-ctrmgrd/ctrmgr/kube_commands.py @@ -59,10 +59,7 @@ def _run_command(cmd, timeout=5): (o, e) = proc.communicate(timeout) output = to_str(o) err = to_str(e) - if err: - ret = -1 - else: - ret = proc.returncode + ret = proc.returncode except subprocess.TimeoutExpired as error: proc.kill() output = "" @@ -207,25 +204,11 @@ def _download_file(server, port, insecure): data = r.read() os.write(h, data) os.close(h) + log_debug("Downloaded = {}".format(fname)) - # Ensure the admin.conf has given VIP as server-IP. - update_file = "{}.upd".format(fname) - cmd = r'sed "s/server:.*:{}/server: https:\/\/{}:{}/" {} > {}'.format( - str(port), server, str(port), fname, update_file) - (ret, _, err) = _run_command(cmd) - - log_debug("sed command: ret={}".format(ret)) - if ret != 0: - log_error("sed update of downloaded file failed with ret={}". - format(ret)) - log_debug("sed command failed: ret={}".format(ret)) - return ret - - shutil.copyfile(update_file, KUBE_ADMIN_CONF) + shutil.copyfile(fname, KUBE_ADMIN_CONF) - _run_command("rm -f {} {}".format(fname, update_file)) log_debug("{} downloaded".format(KUBE_ADMIN_CONF)) - return ret def _troubleshoot_tips(): @@ -284,16 +267,15 @@ def _do_join(server, port, insecure): KUBEADM_JOIN_CMD = "kubeadm join --discovery-file {} --node-name {}" err = "" out = "" + ret = 0 try: - ret = _download_file(server, port, insecure) - log_debug("_download ret={}".format(ret)) - if ret == 0: - _do_reset(True) - _run_command("modprobe br_netfilter") - # Copy flannel.conf - _run_command("mkdir -p {}".format(CNI_DIR)) - _run_command("cp {} {}".format(FLANNEL_CONF_FILE, CNI_DIR)) - (ret, _, _) = _run_command("systemctl start kubelet") + _download_file(server, port, insecure) + _do_reset(True) + _run_command("modprobe br_netfilter") + # Copy flannel.conf + _run_command("mkdir -p {}".format(CNI_DIR)) + _run_command("cp {} {}".format(FLANNEL_CONF_FILE, CNI_DIR)) + (ret, _, _) = _run_command("systemctl start kubelet") if ret == 0: (ret, out, err) = _run_command(KUBEADM_JOIN_CMD.format( diff --git a/src/sonic-ctrmgrd/tests/common_test.py b/src/sonic-ctrmgrd/tests/common_test.py index 54c877f11365..6e9763962bde 100755 --- a/src/sonic-ctrmgrd/tests/common_test.py +++ b/src/sonic-ctrmgrd/tests/common_test.py @@ -43,6 +43,13 @@ DO_JOIN = "do_join" # subproc key words + +# List all subprocess commands expected within the test. +# Each call to subproc-side effect (mock_subproc_side_effect) increment index +# Other key words influence how this proc command to be processed +# PROC_RUN having true at that index, implies run it instead of mocking it +# PROC_OUT, ERR, FAIL THROW provide data on how to mock +# PROC_CMD = "subproc_cmd" PROC_RUN = "skip_mock" PROC_FAIL = "proc_fail" @@ -606,7 +613,7 @@ def communicate(self, timeout): err = err_lst[self.index] else: err = "" - self.returncode = 0 + self.returncode = 0 if not err else -1 return (out, err) def kill(self): diff --git a/src/sonic-ctrmgrd/tests/container_test.py b/src/sonic-ctrmgrd/tests/container_test.py index 146d41386de1..cc5f89e8d97f 100755 --- a/src/sonic-ctrmgrd/tests/container_test.py +++ b/src/sonic-ctrmgrd/tests/container_test.py @@ -179,7 +179,8 @@ "remote_state": "none", "system_state": "up", "current_owner": "local", - "container_id": "snmp" + "container_id": "snmp", + "container_version": "20201230.0.15" } } } @@ -192,7 +193,7 @@ "system_state": "down", "current_owner": "none", "container_id": "", - "container_version": "" + "container_version": "20201230.0.15" } }, common_test.KUBE_LABEL_TABLE: { @@ -222,7 +223,8 @@ "container_id": "xxx", "system_state": "up", "current_owner": "kube", - "remote_state": "running" + "remote_state": "running", + "container_version": "20201230.1.15" } } } @@ -235,7 +237,7 @@ "system_state": "down", "current_owner": "none", "container_id": "", - "container_version": "" + "container_version": "20201230.1.15" } }, common_test.KUBE_LABEL_TABLE: { diff --git a/src/sonic-ctrmgrd/tests/kube_commands_test.py b/src/sonic-ctrmgrd/tests/kube_commands_test.py index b76c23e100fe..7c1436e1f6eb 100755 --- a/src/sonic-ctrmgrd/tests/kube_commands_test.py +++ b/src/sonic-ctrmgrd/tests/kube_commands_test.py @@ -103,8 +103,6 @@ common_test.RETVAL: 0, common_test.ARGS: ["10.3.157.24", 6443, True, False], common_test.PROC_CMD: [ - 'sed *', - 'rm -f *', "kubectl --kubeconfig {} --request-timeout 20s drain None \ --ignore-daemonsets".format(KUBE_ADMIN_CONF), "kubectl --kubeconfig {} --request-timeout 20s delete node \ @@ -126,8 +124,6 @@ common_test.RETVAL: 0, common_test.ARGS: ["10.3.157.24", 6443, False, False], common_test.PROC_CMD: [ - 'sed *', - 'rm -f *', "kubectl --kubeconfig {} --request-timeout 20s drain None \ --ignore-daemonsets".format(KUBE_ADMIN_CONF), "kubectl --kubeconfig {} --request-timeout 20s delete node \ @@ -154,55 +150,6 @@ ] }, 3: { - common_test.DESCR: "Regular join: fail file update", - common_test.RETVAL: -1, - common_test.ARGS: ["10.3.157.24", 6443, False, False], - common_test.PROC_CMD: [ - 'sed *', - 'rm -f *', - "kubectl --kubeconfig {} --request-timeout 20s drain None \ ---ignore-daemonsets".format(KUBE_ADMIN_CONF), - "kubectl --kubeconfig {} --request-timeout 20s delete node \ -None".format(KUBE_ADMIN_CONF), - "kubeadm reset -f", - "rm -rf {}".format(CNI_DIR), - "systemctl stop kubelet", - "modprobe br_netfilter", - "mkdir -p {}".format(CNI_DIR), - "cp {} {}".format(FLANNEL_CONF_FILE, CNI_DIR), - "systemctl start kubelet", - "kubeadm join --discovery-file {} --node-name None".format( - KUBE_ADMIN_CONF) - ], - common_test.PROC_RUN: [True, True], - common_test.PROC_FAIL: [True] - }, - 4: { - common_test.DESCR: "Regular join: fail file update", - common_test.RETVAL: -1, - common_test.ARGS: ["10.3.157.24", 6443, False, False], - common_test.PROC_CMD: [ - 'sed *', - 'rm -f *', - "kubectl --kubeconfig {} --request-timeout 20s drain None \ ---ignore-daemonsets".format(KUBE_ADMIN_CONF), - "kubectl --kubeconfig {} --request-timeout 20s delete node \ -None".format(KUBE_ADMIN_CONF), - "kubeadm reset -f", - "rm -rf {}".format(CNI_DIR), - "systemctl stop kubelet", - "modprobe br_netfilter", - "mkdir -p {}".format(CNI_DIR), - "cp {} {}".format(FLANNEL_CONF_FILE, CNI_DIR), - "systemctl start kubelet", - "kubeadm join --discovery-file {} --node-name None".format( - KUBE_ADMIN_CONF) - ], - common_test.PROC_RUN: [True, True], - common_test.PROC_FAIL: [True], - common_test.PROC_THROW: [True] - }, - 5: { common_test.DESCR: "Regular join: fail due to unable to lock", common_test.RETVAL: -1, common_test.ARGS: ["10.3.157.24", 6443, False, False],