diff --git a/azurelinuxagent/common/osutil/default.py b/azurelinuxagent/common/osutil/default.py index f992185fd9..39d70b428c 100644 --- a/azurelinuxagent/common/osutil/default.py +++ b/azurelinuxagent/common/osutil/default.py @@ -554,7 +554,12 @@ def set_selinux_context(self, path, con): # pylint: disable=R1710 if not os.path.exists(path): logger.error("Path does not exist: {0}".format(path)) return 1 - return shellutil.run('chcon ' + con + ' ' + path) + + try: + shellutil.run_command(['chcon', con, path], log_error=True) + except shellutil.CommandError as cmd_err: + return cmd_err.returncode + return 0 def conf_sshd(self, disable_password): option = "no" if disable_password else "yes" @@ -630,9 +635,20 @@ def umount_dvd(self, chk_err=True, mount_point=None): def eject_dvd(self, chk_err=True): dvd = self.get_dvd_device() - retcode = shellutil.run("eject {0}".format(dvd)) - if chk_err and retcode != 0: - raise OSUtilError("Failed to eject dvd: ret={0}".format(retcode)) + try: + shellutil.run_command(["eject", dvd]) + except shellutil.CommandError as cmd_err: + if chk_err: + + msg = """Failed to eject dvd: ret={0} + [stdout] + {1} + + [stderr] + {2} + """.format(cmd_err.returncode, cmd_err.stdout, cmd_err.stderr) + + raise OSUtilError(msg) def try_load_atapiix_mod(self): try: @@ -669,15 +685,22 @@ def is_atapiix_mod_loaded(self, max_retry=1): return False def mount(self, device, mount_point, option="", chk_err=True): - cmd = "mount {0} {1} {2}".format(option, device, mount_point) - retcode, err = shellutil.run_get_output(cmd, chk_err) - if retcode != 0: - detail = "[{0}] returned {1}: {2}".format(cmd, retcode, err) - err = detail - return retcode, err + cmd = ["mount", option, device, mount_point] + try: + output = shellutil.run_command(cmd, log_error=chk_err) + except shellutil.CommandError as cmd_err: + detail = "[{0}] returned {1}:\n stdout: {2}\n\nstderr: {3}".format(cmd, cmd_err.returncode, + cmd_err.stdout, cmd_err.stderr) + return cmd_err.returncode, detail + + return 0, output def umount(self, mount_point, chk_err=True): - return shellutil.run("umount {0}".format(mount_point), chk_err=chk_err) + try: + shellutil.run_command(["umount", mount_point], log_error=chk_err) + except shellutil.CommandError as cmd_err: + return cmd_err.returncode + return 0 def allow_dhcp_broadcast(self): # Open DHCP port if iptables is enabled. @@ -1112,15 +1135,26 @@ def set_dhcp_hostname(self, hostname): def restart_if(self, ifname, retries=3, wait=5): retry_limit=retries+1 for attempt in range(1, retry_limit): - return_code=shellutil.run("ifdown {0} && ifup {0}".format(ifname), expected_errors=[1] if attempt < retries else []) - if return_code == 0: + try: + shellutil.run_command(["ifdown", ifname]) + shellutil.run_command(["ifup", ifname]) return - logger.warn("failed to restart {0}: return code {1}".format(ifname, return_code)) - if attempt < retry_limit: - logger.info("retrying in {0} seconds".format(wait)) - time.sleep(wait) - else: - logger.warn("exceeded restart retries") + except shellutil.CommandError as cmd_err: + + msg = """failed to restart {0}: returncode={1} + [stdout] + {2} + + [stderr] + {3} + """.format(ifname, cmd_err.returncode, cmd_err.stdout, cmd_err.stderr) + logger.warn(msg) + + if attempt < retry_limit: + logger.info("retrying in {0} seconds".format(wait)) + time.sleep(wait) + else: + logger.warn("exceeded restart retries") def publish_hostname(self, hostname): self.set_dhcp_hostname(hostname) diff --git a/azurelinuxagent/common/osutil/ubuntu.py b/azurelinuxagent/common/osutil/ubuntu.py index eaf4b7ffe7..0ed8ad6939 100644 --- a/azurelinuxagent/common/osutil/ubuntu.py +++ b/azurelinuxagent/common/osutil/ubuntu.py @@ -39,10 +39,18 @@ def start_network(self): return shellutil.run("service networking start", chk_err=False) def stop_agent_service(self): - return shellutil.run("service {0} stop".format(self.service_name), chk_err=False) + try: + shellutil.run_command(["service", self.service_name, "stop"]) + except shellutil.CommandError as cmd_err: + return cmd_err.returncode + return 0 def start_agent_service(self): - return shellutil.run("service {0} start".format(self.service_name), chk_err=False) + try: + shellutil.run_command(["service", self.service_name, "start"]) + except shellutil.CommandError as cmd_err: + return cmd_err.returncode + return 0 def remove_rules_files(self, rules_files=""): pass @@ -119,15 +127,17 @@ def restart_if(self, ifname, retries=3, wait=5): """ retry_limit=retries+1 for attempt in range(1, retry_limit): - return_code=shellutil.run("ip link set {0} down && ip link set {0} up".format(ifname)) - if return_code == 0: - return - logger.warn("failed to restart {0}: return code {1}".format(ifname, return_code)) - if attempt < retry_limit: - logger.info("retrying in {0} seconds".format(wait)) - time.sleep(wait) - else: - logger.warn("exceeded restart retries") + try: + shellutil.run_command(["ip", "link", "set", ifname, "down"]) + shellutil.run_command(["ip", "link", "set", ifname, "up"]) + + except shellutil.CommandError as cmd_err: + logger.warn("failed to restart {0}: return code {1}".format(ifname, cmd_err.returncode)) + if attempt < retry_limit: + logger.info("retrying in {0} seconds".format(wait)) + time.sleep(wait) + else: + logger.warn("exceeded restart retries") class UbuntuSnappyOSUtil(Ubuntu14OSUtil): diff --git a/azurelinuxagent/common/utils/cryptutil.py b/azurelinuxagent/common/utils/cryptutil.py index 77b2520152..a5909631d5 100644 --- a/azurelinuxagent/common/utils/cryptutil.py +++ b/azurelinuxagent/common/utils/cryptutil.py @@ -41,12 +41,19 @@ def gen_transport_cert(self, prv_file, crt_file): """ Create ssl certificate for https communication with endpoint server. """ - cmd = ("{0} req -x509 -nodes -subj /CN=LinuxTransport -days 730 " - "-newkey rsa:2048 -keyout {1} " - "-out {2}").format(self.openssl_cmd, prv_file, crt_file) - rc = shellutil.run(cmd) # pylint: disable=C0103 - if rc != 0: - logger.error("Failed to create {0} and {1} certificates".format(prv_file, crt_file)) + cmd = [self.openssl_cmd, "req", "-x509", "-nodes", "-subj", "/CN=LinuxTransport", + "-days", "730", "-newkey", "rsa:2048", "-keyout", prv_file, "-out", crt_file] + try: + shellutil.run_command(cmd) + except shellutil.CommandError as cmd_err: + msg = """Failed to create {0} and {1} certificates. + [stdout] + {2} + + [stderr] + {3} + """.format(prv_file, crt_file, cmd_err.stdout, cmd_err.stderr) + logger.error(msg) def get_pubkey_from_prv(self, file_name): if not os.path.exists(file_name): # pylint: disable=R1720 @@ -79,18 +86,39 @@ def decrypt_p7m(self, p7m_file, trans_prv_file, trans_cert_file, pem_file): elif not os.path.exists(trans_prv_file): raise IOError(errno.ENOENT, "File not found", trans_prv_file) else: - cmd = ("{0} cms -decrypt -in {1} -inkey {2} -recip {3} " - "| {4} pkcs12 -nodes -password pass: -out {5}" - "").format(self.openssl_cmd, p7m_file, trans_prv_file, - trans_cert_file, self.openssl_cmd, pem_file) - shellutil.run(cmd) - rc = shellutil.run(cmd) # pylint: disable=C0103 - if rc != 0: - logger.error("Failed to decrypt {0}".format(p7m_file)) + first_cmd = [self.openssl_cmd, "cms", "-decrypt", "-in", p7m_file, "-inkey", + trans_prv_file, "-recip", trans_cert_file] + second_cmd = [self.openssl_cmd, "pkcs12", "-nodes", "-password", "pass:", + "-out", pem_file] + + first_proc = subprocess.Popen(first_cmd, stdout=subprocess.PIPE) + second_proc = subprocess.Popen(second_cmd, stdin=first_proc.stdout, stdout=subprocess.PIPE) + stdout, stderr = second_proc.communicate() + + if first_proc.returncode != 0 or second_proc.returncode != 0: + stdout = ustr(stdout, encoding='utf-8', errors="backslashreplace") if stdout else "" + stderr = ustr(stderr, encoding='utf-8', errors="backslashreplace") if stderr else "" + + msg = """Failed to decrypt {0} + [stdout] + {1} + + [stderr] + {2} + """.format(p7m_file, stdout, stderr) + logger.error(msg) + def crt_to_ssh(self, input_file, output_file): - shellutil.run("ssh-keygen -i -m PKCS8 -f {0} >> {1}".format(input_file, - output_file)) + with open(output_file, "a") as file_out: + keygen_proc = subprocess.Popen(["ssh-keygen", "-i", "-m", "PKCS8", "-f", input_file]) + stdout, _ = keygen_proc.communicate() + + if keygen_proc.returncode != 0: + return + + file_out.write(stdout) + def asn1_to_ssh(self, pubkey): lines = pubkey.split("\n") diff --git a/tests/common/osutil/test_default.py b/tests/common/osutil/test_default.py index d7dee8bd81..62a1d945a1 100644 --- a/tests/common/osutil/test_default.py +++ b/tests/common/osutil/test_default.py @@ -56,15 +56,22 @@ def test_restart(self): # setup retries = 3 ifname = 'dummy' - with patch.object(shellutil, "run") as run_patch: - run_patch.return_value = 1 + with patch.object(shellutil, "run_command") as run_patch: + run_patch.side_effect = shellutil.CommandError("ifupdown dummy", 1, "", "") # execute osutil.DefaultOSUtil.restart_if(osutil.DefaultOSUtil(), ifname=ifname, retries=retries, wait=0) # assert self.assertEqual(run_patch.call_count, retries) - self.assertEqual(run_patch.call_args_list[0][0][0], 'ifdown {0} && ifup {0}'.format(ifname)) + + cmd_queue = list(args[0] for (args, _) in run_patch.call_args_list) + while cmd_queue: + self.assertEqual(cmd_queue.pop(0), ["ifdown", ifname]) + # We don't expect the following command to be called because 'dummy' does + # not exist. + self.assertNotEqual(cmd_queue[0] if cmd_queue else None, ["ifup", ifname]) + def test_get_dvd_device_success(self): with patch.object(os, 'listdir', return_value=['cpu', 'cdrom0']): @@ -85,8 +92,8 @@ def test_mount_dvd_success(self, _): 'get_dvd_device', return_value='/dev/cdrom'): with patch.object(shellutil, - 'run_get_output', - return_value=(0, msg)) as patch_run: # pylint: disable=unused-variable + 'run_command', + return_value=msg) as patch_run: # pylint: disable=unused-variable with patch.object(os, 'makedirs'): try: osutil.DefaultOSUtil().mount_dvd() @@ -99,16 +106,15 @@ def test_mount_dvd_failure(self, _): with patch.object(osutil.DefaultOSUtil, 'get_dvd_device', return_value='/dev/cdrom'): - with patch.object(shellutil, - 'run_get_output', - return_value=(1, msg)) as patch_run: + with patch.object(shellutil, 'run_command', + side_effect=shellutil.CommandError("mount dvd", 1, "", msg)) as patch_run: with patch.object(os, 'makedirs'): try: osutil.DefaultOSUtil().mount_dvd() self.fail('OSUtilError was not raised') except OSUtilError as ose: self.assertTrue(msg in ustr(ose)) - self.assertTrue(patch_run.call_count == 6) + self.assertEqual(patch_run.call_count, 5) def test_empty_proc_net_route(self): routing_table = "" diff --git a/tests/common/test_cgroupapi.py b/tests/common/test_cgroupapi.py index 3755221344..a5461fedd4 100644 --- a/tests/common/test_cgroupapi.py +++ b/tests/common/test_cgroupapi.py @@ -173,19 +173,19 @@ def _get_mount_commands(mock): mount_commands = '' for call_args in mock.call_args_list: args, kwargs = call_args # pylint: disable=unused-variable - mount_commands += ';' + args[0] + mount_commands += ';' + " ".join(args[0]) return mount_commands def test_mount_cgroups_should_mount_the_cpu_and_memory_controllers(self): # the mount command requires root privileges; make it a no op and check only for file existence - original_run_get_output = shellutil.run_get_output + original_run_command = shellutil.run_command - def mock_run_get_output(cmd, *args, **kwargs): - if cmd.startswith('mount '): - return 0, None - return original_run_get_output(cmd, *args, **kwargs) + def mock_run_command(cmd, *args, **kwargs): + if cmd[0] == 'mount': + return None + return original_run_command(cmd, *args, **kwargs) - with patch("azurelinuxagent.common.osutil.default.shellutil.run_get_output", side_effect=mock_run_get_output) as patch_run_get_output: + with patch("azurelinuxagent.common.osutil.default.shellutil.run_command", side_effect=mock_run_command) as patch_run_command: FileSystemCgroupsApi.mount_cgroups() # the directories for the controllers should have been created @@ -194,7 +194,7 @@ def mock_run_get_output(cmd, *args, **kwargs): self.assertTrue(os.path.exists(directory), "A directory for controller {0} was not created".format(controller)) # the cgroup filesystem and the cpu and memory controllers should have been mounted - mount_commands = MountCgroupsTestCase._get_mount_commands(patch_run_get_output) + mount_commands = MountCgroupsTestCase._get_mount_commands(patch_run_command) self.assertRegex(mount_commands, ';mount.* cgroup_root ', 'The cgroups file system was not mounted') self.assertRegex(mount_commands, ';mount.* cpu,cpuacct ', 'The cpu controller was not mounted') @@ -203,17 +203,17 @@ def mock_run_get_output(cmd, *args, **kwargs): def test_mount_cgroups_should_not_mount_the_cgroups_file_system_when_it_already_exists(self): os.mkdir(self.cgroups_file_system_root) - original_run_get_output = shellutil.run_get_output + original_run_command = shellutil.run_command - def mock_run_get_output(cmd, *args, **kwargs): - if cmd.startswith('mount '): - return 0, None - return original_run_get_output(cmd, *args, **kwargs) + def mock_run_command(cmd, *args, **kwargs): + if cmd[0] == 'mount': + return None + return original_run_command(cmd, *args, **kwargs) - with patch("azurelinuxagent.common.osutil.default.shellutil.run_get_output", side_effect=mock_run_get_output) as patch_run_get_output: + with patch("azurelinuxagent.common.osutil.default.shellutil.run_command", side_effect=mock_run_command) as patch_run_command: FileSystemCgroupsApi.mount_cgroups() - mount_commands = MountCgroupsTestCase._get_mount_commands(patch_run_get_output) + mount_commands = MountCgroupsTestCase._get_mount_commands(patch_run_command) self.assertNotIn('cgroup_root', mount_commands, 'The cgroups file system should not have been mounted') self.assertRegex(mount_commands, ';mount.* cpu,cpuacct ', 'The cpu controller was not mounted') @@ -224,38 +224,38 @@ def test_mount_cgroups_should_not_mount_cgroup_controllers_when_they_already_exi os.mkdir(os.path.join(self.cgroups_file_system_root, 'cpu,cpuacct')) os.mkdir(os.path.join(self.cgroups_file_system_root, 'memory')) - original_run_get_output = shellutil.run_get_output + original_run_command = shellutil.run_command - def mock_run_get_output(cmd, *args, **kwargs): - if cmd.startswith('mount '): - return 0, None - return original_run_get_output(cmd, *args, **kwargs) + def mock_run_command(cmd, *args, **kwargs): + if cmd[0] == 'mount': + return None + return original_run_command(cmd, *args, **kwargs) - with patch("azurelinuxagent.common.osutil.default.shellutil.run_get_output", side_effect=mock_run_get_output) as patch_run_get_output: + with patch("azurelinuxagent.common.osutil.default.shellutil.run_command", side_effect=mock_run_command) as patch_run_command: FileSystemCgroupsApi.mount_cgroups() - mount_commands = MountCgroupsTestCase._get_mount_commands(patch_run_get_output) + mount_commands = MountCgroupsTestCase._get_mount_commands(patch_run_command) self.assertNotIn('cgroup_root', mount_commands, 'The cgroups file system should not have been mounted') self.assertNotIn('cpu,cpuacct', mount_commands, 'The cpu controller should not have been mounted') self.assertNotIn('memory', mount_commands, 'The memory controller should not have been mounted') def test_mount_cgroups_should_handle_errors_when_mounting_an_individual_controller(self): - original_run_get_output = shellutil.run_get_output + original_run_command = shellutil.run_command - def mock_run_get_output(cmd, *args, **kwargs): - if cmd.startswith('mount '): - if 'memory' in cmd: + def mock_run_command(cmd, *args, **kwargs): + if cmd[0] == 'mount': + if 'memory' in " ".join(cmd): raise Exception('A test exception mounting the memory controller') - return 0, None - return original_run_get_output(cmd, *args, **kwargs) + return None + return original_run_command(cmd, *args, **kwargs) - with patch("azurelinuxagent.common.osutil.default.shellutil.run_get_output", side_effect=mock_run_get_output) as patch_run_get_output: + with patch("azurelinuxagent.common.osutil.default.shellutil.run_command", side_effect=mock_run_command) as patch_run_command: with patch("azurelinuxagent.common.cgroupconfigurator.logger.warn") as mock_logger_warn: FileSystemCgroupsApi.mount_cgroups() # the cgroup filesystem and the cpu controller should still have been mounted - mount_commands = MountCgroupsTestCase._get_mount_commands(patch_run_get_output) + mount_commands = MountCgroupsTestCase._get_mount_commands(patch_run_command) self.assertRegex(mount_commands, ';mount.* cgroup_root ', 'The cgroups file system was not mounted') self.assertRegex(mount_commands, ';mount.* cpu,cpuacct ', 'The cpu controller was not mounted') @@ -265,52 +265,52 @@ def mock_run_get_output(cmd, *args, **kwargs): self.assertIn('A test exception mounting the memory controller', args) def test_mount_cgroups_should_raise_when_the_cgroups_filesystem_fails_to_mount(self): - original_run_get_output = shellutil.run_get_output + original_run_command = shellutil.run_command - def mock_run_get_output(cmd, *args, **kwargs): - if cmd.startswith('mount '): - if 'cgroup_root' in cmd: + def mock_run_command(cmd, *args, **kwargs): + if cmd[0] == 'mount': + if 'cgroup_root' in ",".join(cmd): raise Exception('A test exception mounting the cgroups file system') - return 0, None - return original_run_get_output(cmd, *args, **kwargs) + return None + return original_run_command(cmd, *args, **kwargs) - with patch("azurelinuxagent.common.osutil.default.shellutil.run_get_output", side_effect=mock_run_get_output) as patch_run_get_output: + with patch("azurelinuxagent.common.osutil.default.shellutil.run_command", side_effect=mock_run_command) as patch_run_command: with self.assertRaises(Exception) as context_manager: FileSystemCgroupsApi.mount_cgroups() self.assertRegex(str(context_manager.exception), 'A test exception mounting the cgroups file system') - mount_commands = MountCgroupsTestCase._get_mount_commands(patch_run_get_output) + mount_commands = MountCgroupsTestCase._get_mount_commands(patch_run_command) self.assertNotIn('memory', mount_commands, 'The memory controller should not have been mounted') self.assertNotIn('cpu', mount_commands, 'The cpu controller should not have been mounted') def test_mount_cgroups_should_raise_when_all_controllers_fail_to_mount(self): - original_run_get_output = shellutil.run_get_output + original_run_command = shellutil.run_command - def mock_run_get_output(cmd, *args, **kwargs): - if cmd.startswith('mount '): - if 'memory' in cmd or 'cpu,cpuacct' in cmd: + def mock_run_command(cmd, *args, **kwargs): + if cmd[0] == 'mount': + if 'memory' in "".join(cmd) or 'cpu,cpuacct' in "".join(cmd): raise Exception('A test exception mounting a cgroup controller') - return 0, None - return original_run_get_output(cmd, *args, **kwargs) + return None + return original_run_command(cmd, *args, **kwargs) - with patch("azurelinuxagent.common.osutil.default.shellutil.run_get_output", side_effect=mock_run_get_output): + with patch("azurelinuxagent.common.osutil.default.shellutil.run_command", side_effect=mock_run_command): with self.assertRaises(Exception) as context_manager: FileSystemCgroupsApi.mount_cgroups() self.assertRegex(str(context_manager.exception), 'A test exception mounting a cgroup controller') def test_mount_cgroups_should_not_create_symbolic_links_when_the_cpu_controller_fails_to_mount(self): - original_run_get_output = shellutil.run_get_output + original_run_command = shellutil.run_command - def mock_run_get_output(cmd, *args, **kwargs): - if cmd.startswith('mount '): - if 'cpu,cpuacct' in cmd: + def mock_run_command(cmd, *args, **kwargs): + if cmd[0] == 'mount': + if 'cpu,cpuacct' in "".join(cmd): raise Exception('A test exception mounting the cpu controller') - return 0, None - return original_run_get_output(cmd, *args, **kwargs) + return None + return original_run_command(cmd, *args, **kwargs) - with patch("azurelinuxagent.common.osutil.default.shellutil.run_get_output", side_effect=mock_run_get_output): + with patch("azurelinuxagent.common.osutil.default.shellutil.run_command", side_effect=mock_run_command): with patch("azurelinuxagent.common.osutil.default.os.symlink") as patch_symlink: FileSystemCgroupsApi.mount_cgroups() @@ -518,14 +518,14 @@ def test_create_extension_cgroups_root_should_create_extensions_root_slice(self) SystemdCgroupsApi().create_extension_cgroups_root() unit_name = SystemdCgroupsApi()._get_extensions_slice_root_name() # pylint: disable=protected-access - _, status = shellutil.run_get_output("systemctl status {0}".format(unit_name)) + status = shellutil.run_command(["systemctl", "status", unit_name]) self.assertIn("Loaded: loaded", status) self.assertIn("Active: active", status) - shellutil.run_get_output("systemctl stop {0}".format(unit_name)) - shellutil.run_get_output("systemctl disable {0}".format(unit_name)) + shellutil.run_command(["systemctl", "stop", unit_name ]) + shellutil.run_command(["systemctl", "disable", unit_name]) os.remove("/etc/systemd/system/{0}".format(unit_name)) - shellutil.run_get_output("systemctl daemon-reload") + shellutil.run_command(["systemctl", "daemon-reload"]) def test_get_processes_in_cgroup_should_return_the_processes_within_the_cgroup(self): with mock_cgroup_commands():