From 1eaa1ef4df01899f52e487093a89dee92f927243 Mon Sep 17 00:00:00 2001 From: Fuming Zhang Date: Wed, 22 Jun 2022 16:14:36 +0800 Subject: [PATCH 1/4] fix check-acr test case --- .../cli/command_modules/acs/acs_client.py | 3 ++ .../azure/cli/command_modules/acs/custom.py | 21 ++++---- .../acs/tests/latest/test_aks_commands.py | 48 +++++++++++++++---- 3 files changed, 54 insertions(+), 18 deletions(-) diff --git a/src/azure-cli/azure/cli/command_modules/acs/acs_client.py b/src/azure-cli/azure/cli/command_modules/acs/acs_client.py index 53d35a8baed..0319a5c19ea 100644 --- a/src/azure-cli/azure/cli/command_modules/acs/acs_client.py +++ b/src/azure-cli/azure/cli/command_modules/acs/acs_client.py @@ -18,6 +18,9 @@ from knack.prompting import prompt_pass from knack.util import CLIError +# TODO: this file is deprecated, will remove this after container service commands (acs) are removed during +# the next breaking change window. + def _load_key(key_filename): pkey = None diff --git a/src/azure-cli/azure/cli/command_modules/acs/custom.py b/src/azure-cli/azure/cli/command_modules/acs/custom.py index 82ca5b458ed..9ba5188b5a0 100644 --- a/src/azure-cli/azure/cli/command_modules/acs/custom.py +++ b/src/azure-cli/azure/cli/command_modules/acs/custom.py @@ -2351,8 +2351,11 @@ def aks_update_credentials(cmd, client, resource_group_name, name, def aks_check_acr(cmd, client, resource_group_name, name, acr, node_name=None): + # normailze acr name if not acr.endswith(CONST_ACR_DOMAIN_NAME): acr = acr + CONST_ACR_DOMAIN_NAME + + # check binary kubectl if not which("kubectl"): raise ValidationError("Can not find kubectl executable in PATH") @@ -2363,10 +2366,16 @@ def aks_check_acr(cmd, client, resource_group_name, name, acr, node_name=None): cmd, client, resource_group_name, name, admin=False, path=browse_path ) + # test hook configs + test_hook_data = get_cmd_test_hook_data("test_aks_create_attach_acr.hook") + return_output = test_hook_data.get("returnOutput", False) + custom_kubectl_path = test_hook_data.get("customKubectlPath", None) + kubectl_binary_path = "kubectl" if not custom_kubectl_path else custom_kubectl_path + # Get kubectl minor version kubectl_minor_version = -1 try: - cmd = f"kubectl version -o json --kubeconfig {browse_path}" + cmd = f"{kubectl_binary_path} version -o json --kubeconfig {browse_path}" output = subprocess.Popen(cmd, shell=True, stdout=subprocess.PIPE) jsonS, _ = output.communicate() kubectl_version = json.loads(jsonS) @@ -2436,7 +2445,7 @@ def aks_check_acr(cmd, client, resource_group_name, name, acr, node_name=None): try: cmd = [ - "kubectl", + kubectl_binary_path, "run", "--kubeconfig", browse_path, @@ -2464,12 +2473,8 @@ def aks_check_acr(cmd, client, resource_group_name, name, acr, node_name=None): raise AzureInternalError("Failed to check the ACR: {} Command output: {}".format(err, err.output)) if output: print(output) - # only return the output in test case "test_aks_create_attach_acr" - test_hook_data = get_cmd_test_hook_data("test_aks_create_attach_acr.hook") - if test_hook_data: - test_configs = test_hook_data.get("configs", None) - if test_configs and test_configs.get("returnOutput", False): - return_msg = output + if return_output: + return_msg = output else: raise AzureInternalError("Failed to check the ACR.") finally: diff --git a/src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_aks_commands.py b/src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_aks_commands.py index 5236b7f16ff..260c510cdf1 100644 --- a/src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_aks_commands.py +++ b/src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_aks_commands.py @@ -5301,6 +5301,31 @@ def test_aks_create_network_cidr(self, resource_group, resource_group_location): self.cmd( 'aks delete -g {resource_group} -n {name} --yes --no-wait', checks=[self.is_empty()]) + def prepare_cli_tools(self, remove_files, ctl_version="latest", ctl_base_url="", login_version="latest", login_base_url=""): + ctl_fd, ctl_temp_file = tempfile.mkstemp() + login_fd, login_temp_file = tempfile.mkstemp() + install_cmd = ( + f"aks install-cli --client-version={ctl_version} --install-location={ctl_temp_file} " + f"--base-src-url={ctl_base_url} --kubelogin-version={login_version} " + f"--kubelogin-install-location={login_temp_file} --kubelogin-base-src-url={login_base_url}" + ) + + # install kubectl & kubelogin + try: + subprocess.call(["az"] + install_cmd.split(" ")) + self.assertGreater(os.path.getsize(ctl_temp_file), 0) + self.assertGreater(os.path.getsize(login_temp_file), 0) + except subprocess.CalledProcessError as err: + raise CLIInternalError("Failed to install kubectl with error: '{}'!".format(err)) + finally: + os.close(ctl_fd) + os.close(login_fd) + if remove_files: + os.remove(ctl_temp_file) + os.remove(login_temp_file) + return None, None + return ctl_temp_file, login_temp_file + # live only due to dependency `_add_role_assignment` is not mocked @live_only() @AllowLargeResponse() @@ -5333,18 +5358,15 @@ def test_aks_create_attach_acr(self, resource_group, resource_group_location, sp self.check('servicePrincipalProfile.clientId', sp_name) ]) - # install kubectl - try: - subprocess.call(["az", "aks", "install-cli"]) - except subprocess.CalledProcessError as err: - raise CLIInternalError("Failed to install kubectl with error: '{}'!".format(err)) + show_cmd = "aks show --resource-group={resource_group} --name={name}" + k8s_version = self.cmd(show_cmd).get_output_in_json().get("kubernetesVersion") + kubectl_path, kubelogin_path = self.prepare_cli_tools(remove_files=False, ctl_version=k8s_version) # create test hook file hook_file_path = get_test_data_file_path("test_aks_create_attach_acr.hook") test_hook_data = { - "configs": { - "returnOutput": True, - } + "returnOutput": True, + "customKubectlPath": kubectl_path } with open(hook_file_path, "w") as f: json.dump(test_hook_data, f) @@ -5363,7 +5385,7 @@ def test_aks_create_attach_acr(self, resource_group, resource_group_location, sp finally: os.close(fd) # get node name - k_get_node_cmd = ["kubectl", "get", "node", "-o", "name", "--kubeconfig", browse_path] + k_get_node_cmd = [kubectl_path, "get", "node", "-o", "name", "--kubeconfig", browse_path] k_get_node_output = subprocess.check_output( k_get_node_cmd, universal_newlines=True, @@ -5386,10 +5408,16 @@ def test_aks_create_attach_acr(self, resource_group, resource_group_location, sp ) # clean up test hook file even if test failed finally: + # remove test hook if os.path.exists(hook_file_path): - # delete file os.remove(hook_file_path) + # remove binaries + if os.path.exists(kubectl_path): + os.remove(kubectl_path) + if os.path.exists(kubelogin_path): + os.remove(kubelogin_path) + # delete cluster self.cmd( 'aks delete -g {resource_group} -n {name} --yes --no-wait', checks=[self.is_empty()]) From a9041001469c77b44f35c9fe914d73e39111db50 Mon Sep 17 00:00:00 2001 From: Fuming Zhang Date: Wed, 22 Jun 2022 16:33:13 +0800 Subject: [PATCH 2/4] update test hook --- .../azure/cli/command_modules/acs/custom.py | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/src/azure-cli/azure/cli/command_modules/acs/custom.py b/src/azure-cli/azure/cli/command_modules/acs/custom.py index 9ba5188b5a0..c429bbcc9eb 100644 --- a/src/azure-cli/azure/cli/command_modules/acs/custom.py +++ b/src/azure-cli/azure/cli/command_modules/acs/custom.py @@ -115,7 +115,10 @@ def get_cmd_test_hook_data(filename): test_hook_file_path = os.path.join(curr_dir, 'tests/latest/data', filename) if os.path.exists(test_hook_file_path): with open(test_hook_file_path, "r") as f: - hook_data = json.load(f) + try: + hook_data = json.load(f) + except Exception as ex: + logger.warning("failed to load test hook from %s, error: %s", test_hook_file_path, str(ex)) return hook_data @@ -2351,12 +2354,18 @@ def aks_update_credentials(cmd, client, resource_group_name, name, def aks_check_acr(cmd, client, resource_group_name, name, acr, node_name=None): + # test hook configs + test_hook_data = get_cmd_test_hook_data("test_aks_create_attach_acr.hook") + return_output = test_hook_data.get("returnOutput", False) + custom_kubectl_path = test_hook_data.get("customKubectlPath", None) + kubectl_binary_path = "kubectl" if not custom_kubectl_path else custom_kubectl_path + # normailze acr name if not acr.endswith(CONST_ACR_DOMAIN_NAME): acr = acr + CONST_ACR_DOMAIN_NAME # check binary kubectl - if not which("kubectl"): + if not which("kubectl") and not custom_kubectl_path: raise ValidationError("Can not find kubectl executable in PATH") return_msg = None @@ -2366,12 +2375,6 @@ def aks_check_acr(cmd, client, resource_group_name, name, acr, node_name=None): cmd, client, resource_group_name, name, admin=False, path=browse_path ) - # test hook configs - test_hook_data = get_cmd_test_hook_data("test_aks_create_attach_acr.hook") - return_output = test_hook_data.get("returnOutput", False) - custom_kubectl_path = test_hook_data.get("customKubectlPath", None) - kubectl_binary_path = "kubectl" if not custom_kubectl_path else custom_kubectl_path - # Get kubectl minor version kubectl_minor_version = -1 try: From 6f31e4af27d98938159aee6a177ce5909208e59f Mon Sep 17 00:00:00 2001 From: Fuming Zhang Date: Thu, 23 Jun 2022 15:37:36 +0800 Subject: [PATCH 3/4] add sleep --- src/azure-cli/azure/cli/command_modules/acs/custom.py | 11 ++++++++--- .../acs/tests/latest/test_aks_commands.py | 3 ++- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/src/azure-cli/azure/cli/command_modules/acs/custom.py b/src/azure-cli/azure/cli/command_modules/acs/custom.py index c429bbcc9eb..33a9532414e 100644 --- a/src/azure-cli/azure/cli/command_modules/acs/custom.py +++ b/src/azure-cli/azure/cli/command_modules/acs/custom.py @@ -110,7 +110,7 @@ def get_cmd_test_hook_data(filename): - hook_data = None + hook_data = {} curr_dir = os.path.dirname(os.path.realpath(__file__)) test_hook_file_path = os.path.join(curr_dir, 'tests/latest/data', filename) if os.path.exists(test_hook_file_path): @@ -2358,6 +2358,7 @@ def aks_check_acr(cmd, client, resource_group_name, name, acr, node_name=None): test_hook_data = get_cmd_test_hook_data("test_aks_create_attach_acr.hook") return_output = test_hook_data.get("returnOutput", False) custom_kubectl_path = test_hook_data.get("customKubectlPath", None) + sleep_time = test_hook_data.get("sleepTime", 0) kubectl_binary_path = "kubectl" if not custom_kubectl_path else custom_kubectl_path # normailze acr name @@ -2445,6 +2446,10 @@ def aks_check_acr(cmd, client, resource_group_name, name, acr, node_name=None): } } overrides["spec"]["affinity"] = affinity + # fix test + if sleep_time: + overrides["spec"]["containers"][0]["command"] = ["/bin/sh", "-c"] + overrides["spec"]["containers"][0]["args"] = [f"sleep {sleep_time}; /canipull -v6 {acr}"] try: cmd = [ @@ -2473,13 +2478,13 @@ def aks_check_acr(cmd, client, resource_group_name, name, acr, node_name=None): stderr=subprocess.STDOUT, ) except subprocess.CalledProcessError as err: - raise AzureInternalError("Failed to check the ACR: {} Command output: {}".format(err, err.output)) + raise AzureInternalError(f"failed to check ACR '{acr}', error details: {err.output}") if output: print(output) if return_output: return_msg = output else: - raise AzureInternalError("Failed to check the ACR.") + raise AzureInternalError(f"failed to check the ACR '{acr}', no command output, please retry the command") finally: os.close(fd) return return_msg diff --git a/src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_aks_commands.py b/src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_aks_commands.py index 260c510cdf1..6541723bdc6 100644 --- a/src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_aks_commands.py +++ b/src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_aks_commands.py @@ -5366,7 +5366,8 @@ def test_aks_create_attach_acr(self, resource_group, resource_group_location, sp hook_file_path = get_test_data_file_path("test_aks_create_attach_acr.hook") test_hook_data = { "returnOutput": True, - "customKubectlPath": kubectl_path + "customKubectlPath": kubectl_path, + "sleepTime": "10", } with open(hook_file_path, "w") as f: json.dump(test_hook_data, f) From cf69d2b74b1222a0f6e6cb2b9bae63b77d9cf5d9 Mon Sep 17 00:00:00 2001 From: Fuming Zhang Date: Fri, 24 Jun 2022 13:37:49 +0800 Subject: [PATCH 4/4] fix lint --- src/azure-cli/azure/cli/command_modules/acs/custom.py | 1 + 1 file changed, 1 insertion(+) diff --git a/src/azure-cli/azure/cli/command_modules/acs/custom.py b/src/azure-cli/azure/cli/command_modules/acs/custom.py index 33a9532414e..6ef89bd4dbf 100644 --- a/src/azure-cli/azure/cli/command_modules/acs/custom.py +++ b/src/azure-cli/azure/cli/command_modules/acs/custom.py @@ -109,6 +109,7 @@ # pylint: disable=unused-argument +# pylint: disable=broad-except def get_cmd_test_hook_data(filename): hook_data = {} curr_dir = os.path.dirname(os.path.realpath(__file__))