From d6d7180e57d8af0221a690d25269ff49d3a7f89c Mon Sep 17 00:00:00 2001 From: CaioCorrea Date: Fri, 18 Aug 2023 16:41:09 -0300 Subject: [PATCH] Improves compatibility with VirtualBox 7.0 Fixes an issue with overwriting portfowarding rules. Closes-Bug: 2031930 Task: 48624 Adds a new argument: --y Automatically answers yes to all prompts improving the autonomy of the installation Task: 48626 Story: 2005051 Change-Id: I426be7486c59f2163c32227da83924b61e40ea30 --- virtualbox/pybox/Parser.py | 10 ++-- .../pybox/helper/tests/test_install_vbox.py | 43 +++++++++++---- virtualbox/pybox/helper/vboxmanage.py | 4 +- virtualbox/pybox/install_vbox.py | 52 +++++++++++++------ virtualbox/pybox/tests/test_install_vbox.py | 22 ++++---- 5 files changed, 88 insertions(+), 43 deletions(-) diff --git a/virtualbox/pybox/Parser.py b/virtualbox/pybox/Parser.py index 8229fdf..1fc3c04 100644 --- a/virtualbox/pybox/Parser.py +++ b/virtualbox/pybox/Parser.py @@ -314,6 +314,11 @@ def parse_networking(parser: ArgumentParser): """, type=str, default='32000') + parser.add_argument("--y", "--yes-to-all", help= + """ + Automatically answers all yes/no prompts with yes. + """, + action='store_true') def parse_custom_scripts(parser: ArgumentParser): @@ -380,11 +385,6 @@ def parse_other(parser: ArgumentParser): Base directory to store logs. """, type=str) - parser.add_argument("--force-delete-lab", help= - """ - Don't ask for confirmation when deleting a lab. - """, - action='store_true') parser.add_argument("--snapshot", help= """ Take snapshot at different stages when the lab is installed. diff --git a/virtualbox/pybox/helper/tests/test_install_vbox.py b/virtualbox/pybox/helper/tests/test_install_vbox.py index b5bdf68..dad8148 100644 --- a/virtualbox/pybox/helper/tests/test_install_vbox.py +++ b/virtualbox/pybox/helper/tests/test_install_vbox.py @@ -40,20 +40,20 @@ class CreateportforwardTestCase(unittest.TestCase): # Assert mock_addport.assert_called_once_with('TestVM', '8080', '10.10.10.1', '80', 'NatNetwork') + @patch("install_vbox.V_BOX_OPTIONS") @patch("helper.vboxmanage.vboxmanage_getrulename") @patch("helper.vboxmanage.vboxmanage_addportforward") @patch("helper.vboxmanage.vboxmanage_deleteportforward", return_value=None) @patch('utils.install_log.LOG.info') - @patch("builtins.input") - def test_rewrite_rule(self, mock_input, mock_log, mock_deleteport, mock_addport, mock_getrule): + def test_rewrite_rule_yes_to_all(self, mock_log, mock_deleteport, mock_addport, mock_getrule, mock_v_box_options): """ - Test create_port_forward method that fails to create rule and the user input 'y' to rewrite the existing rule + Test create_port_forward method that fails to create rule and the installer was called with --y (yes to all) as an argument """ # Setup - mock_input.return_value = 'y' mock_addport.side_effect = [False, True] mock_getrule.return_value = "Rule1" + mock_v_box_options.y = True # Run result = create_port_forward(self.hostname, self.network, self.local_port, self.guest_port, self.guest_ip) @@ -61,27 +61,53 @@ class CreateportforwardTestCase(unittest.TestCase): # Assert mock_log.assert_any_call( "Trying to create a port-forwarding rule with port: %s, but it is already in use with rule name: %s", "8080", "Rule1") - mock_log.assert_any_call("Rewrite rule? (y/n)") + mock_log.assert_any_call("Rewriting portforwarding rule...") + mock_getrule.assert_called_once_with('NatNetwork', '8080') + mock_deleteport.assert_called_once_with('Rule1', 'NatNetwork') + mock_addport.assert_called_with('TestVM', '8080', '10.10.10.1', '80', 'NatNetwork') + self.assertIsNone(result) + + @patch("install_vbox.yes_no_prompt") + @patch("helper.vboxmanage.vboxmanage_getrulename") + @patch("helper.vboxmanage.vboxmanage_addportforward") + @patch("helper.vboxmanage.vboxmanage_deleteportforward", return_value=None) + @patch('utils.install_log.LOG.info') + def test_rewrite_rule(self, mock_log, mock_deleteport, mock_addport, mock_getrule, mock_y_n_prompt): + """ + Test create_port_forward method that fails to create rule and the user input 'y' to rewrite the existing rule + """ + + # Setup + mock_addport.side_effect = [False, True] + mock_getrule.return_value = "Rule1" + mock_y_n_prompt.return_value = True + + # Run + result = create_port_forward(self.hostname, self.network, self.local_port, self.guest_port, self.guest_ip) + + # Assert + mock_log.assert_any_call( + "Trying to create a port-forwarding rule with port: %s, but it is already in use with rule name: %s", "8080", "Rule1") mock_getrule.assert_called_once_with('NatNetwork', '8080') mock_deleteport.assert_called_once_with('Rule1', 'NatNetwork') mock_addport.assert_called_with('TestVM', '8080', '10.10.10.1', '80', 'NatNetwork') self.assertIsNone(result) + @patch("install_vbox.yes_no_prompt") @patch("helper.vboxmanage.vboxmanage_getrulename") @patch("helper.vboxmanage.vboxmanage_addportforward") @patch('utils.install_log.LOG.info') - @patch("builtins.input") - def test_dont_rewrite_rule(self, mock_input, mock_log, mock_addport, mock_getrule): + def test_dont_rewrite_rule(self, mock_log, mock_addport, mock_getrule,mock_y_n_prompt): """ Test create_port_forward method that fails to create rule and the user input 'n' to rewrite the existing rule """ # Setup - mock_input.return_value = 'n' mock_addport.return_value = False mock_getrule.return_value = "Rule1" + mock_y_n_prompt.return_value = False # Run result = create_port_forward(self.hostname, self.network, self.local_port, self.guest_port, self.guest_ip) @@ -89,7 +115,6 @@ class CreateportforwardTestCase(unittest.TestCase): # Assert mock_log.assert_any_call( "Trying to create a port-forwarding rule with port: %s, but it is already in use with rule name: %s", "8080", "Rule1") - mock_log.assert_any_call("Rewrite rule? (y/n)") mock_log.assert_any_call("Ignoring the creation of the port-forward rule and continuing installation!") mock_getrule.assert_called_once_with('NatNetwork', '8080') diff --git a/virtualbox/pybox/helper/vboxmanage.py b/virtualbox/pybox/helper/vboxmanage.py index f773680..54e9b74 100644 --- a/virtualbox/pybox/helper/vboxmanage.py +++ b/virtualbox/pybox/helper/vboxmanage.py @@ -811,9 +811,9 @@ def vboxmanage_getrulename(network, local_port): """ # List information about all nat networks in VirtualBox - cmd = ["vboxmanage", "list", "natnets"] + cmd = ["vboxmanage", "list", "natnets", "--long"] result = subprocess.check_output(cmd, stderr=subprocess.STDOUT) - natpattern = r"NetworkName:(.*?)loopback mappings \(ipv4\)" + natpattern = r"Name:(.*?)loopback mappings \(ipv4\)" natnetworks = re.findall(natpattern,result.decode(),re.DOTALL) # Get the rule name of the given local port in the given natnetwork diff --git a/virtualbox/pybox/install_vbox.py b/virtualbox/pybox/install_vbox.py index de0eda3..91fd376 100755 --- a/virtualbox/pybox/install_vbox.py +++ b/virtualbox/pybox/install_vbox.py @@ -238,7 +238,7 @@ def install_controller_0(cont0_stream, menu_select_dict, network_dict): ) -def delete_lab(labname, force=False): +def delete_lab(labname): """ This allows for the deletion of an existing lab. """ @@ -246,17 +246,14 @@ def delete_lab(labname, force=False): node_list = vboxmanage.get_all_vms(labname, option="vms") if len(node_list) != 0: - if not force: - LOG.info("This will delete lab %s with vms: %s", labname, node_list) - LOG.info("Continue? (y/N)") - while True: - choice = input().lower() - if choice == 'y': - break - LOG.info("Aborting!") - sys.exit(1) - LOG.info("#### Deleting lab %s.", labname) - LOG.info("VMs in lab: %s.", node_list) + LOG.info("This will delete lab %s with vms: %s", labname, node_list) + #LOG.info("Continue? (y/N)") + if yes_no_prompt("Delete lab?"): + LOG.info("#### Deleting lab %s.", labname) + LOG.info("VMs in lab: %s.", node_list) + else: + LOG.info("Aborting!") + sys.exit(1) vboxmanage.vboxmanage_controlvms(node_list, "poweroff") time.sleep(2) vboxmanage.vboxmanage_deletevms(node_list) @@ -276,6 +273,25 @@ def get_disk_sizes(comma_list): return sizes +def yes_no_prompt(message): + """ + Creates a yes/no prompt to be answered by user. + Uses forced yes-to-all parameter + + Args: + message (str): Message to be displayed + + Returns: + Answer to the prompt(bool) + """ + if V_BOX_OPTIONS.y == True: + return True + LOG.info("%s (y/n)",message) + choice = input().lower() + if choice == 'y': + return True + return False + def create_port_forward(hostname, network, local_port, guest_port, guest_ip): """ Create a port forwarding rule for a NAT network in VirtualBox. @@ -307,15 +323,15 @@ def create_port_forward(hostname, network, local_port, guest_port, guest_ip): local_port, rule_name) - LOG.info("Rewrite rule? (y/n)") - choice = input().lower() - if choice == 'y': + if yes_no_prompt("Rewrite rule?"): + LOG.info("Rewriting portforwarding rule...") vboxmanage.vboxmanage_deleteportforward(rule_name, network) vboxmanage.vboxmanage_addportforward( hostname, local_port, guest_ip, guest_port, network ) else: LOG.info("Ignoring the creation of the port-forward rule and continuing installation!") + # pylint: disable=too-many-locals, too-many-branches, too-many-statements @@ -1019,7 +1035,7 @@ def stage_create_lab(): using `vboxoptions`. """ - delete_lab(V_BOX_OPTIONS.labname, V_BOX_OPTIONS.force_delete_lab) + delete_lab(V_BOX_OPTIONS.labname) create_lab(V_BOX_OPTIONS) @@ -2174,10 +2190,14 @@ def load_config(): if V_BOX_OPTIONS.vboxnet_ip is None: V_BOX_OPTIONS.vboxnet_ip = OAM_CONFIG[0]['ip'] + if V_BOX_OPTIONS.y is None: + V_BOX_OPTIONS.y=False + if V_BOX_OPTIONS.hostiocache: V_BOX_OPTIONS.hostiocache = 'on' else: V_BOX_OPTIONS.hostiocache = 'off' + if V_BOX_OPTIONS.lab_setup_conf is None: V_BOX_OPTIONS.lab_setup_conf = {"~/lab_setup.conf"} else: diff --git a/virtualbox/pybox/tests/test_install_vbox.py b/virtualbox/pybox/tests/test_install_vbox.py index 876d51a..b92cf3a 100644 --- a/virtualbox/pybox/tests/test_install_vbox.py +++ b/virtualbox/pybox/tests/test_install_vbox.py @@ -319,12 +319,12 @@ class DeleteLabTestCase(unittest.TestCase): mock_labname = "test_lab" mock_node_list = ["vm1", "vm2", "vm3"] + @patch("install_vbox.yes_no_prompt", return_value = True) @patch("install_vbox.vboxmanage") @patch("install_vbox.LOG") @patch("install_vbox.time") - @patch("install_vbox.input", return_value="y") def test_delete_lab_not_force( - self, mock_input, mock_time, mock_log, mock_vboxmanage + self, mock_time, mock_log, mock_vboxmanage, mock_prompt ): """ Test delete_lab with force=False and user input 'y' @@ -334,14 +334,13 @@ class DeleteLabTestCase(unittest.TestCase): mock_vboxmanage.get_all_vms.return_value = self.mock_node_list # Run - install_vbox.delete_lab(self.mock_labname, force=False) + install_vbox.delete_lab(self.mock_labname) # Assert mock_vboxmanage.get_all_vms.assert_called_once_with(self.mock_labname, option="vms") - mock_input.assert_called_once_with() mock_log.info.assert_has_calls([ call("This will delete lab %s with vms: %s", self.mock_labname, self.mock_node_list), - call("Continue? (y/N)"), + #call("Continue? (y/N)"), call("#### Deleting lab %s.", self.mock_labname), call("VMs in lab: %s.", self.mock_node_list), ]) @@ -349,11 +348,11 @@ class DeleteLabTestCase(unittest.TestCase): mock_time.sleep.assert_called_once_with(2) mock_vboxmanage.vboxmanage_deletevms.assert_called_once_with(self.mock_node_list) + @patch("install_vbox.yes_no_prompt", return_value = False) @patch("install_vbox.LOG") @patch("install_vbox.vboxmanage") - @patch("install_vbox.input", return_value="n") def test_delete_lab_not_force_abort( - self, mock_input, mock_vboxmanage, mock_log + self, mock_vboxmanage, mock_log, mock_prompt ): """ Test delete_lab with force=False and user input 'n' @@ -361,20 +360,21 @@ class DeleteLabTestCase(unittest.TestCase): # Setup mock_vboxmanage.get_all_vms.return_value = self.mock_node_list + mock_vboxmanage.y = False # Run with self.assertRaises(SystemExit): - install_vbox.delete_lab(self.mock_labname, force=False) + install_vbox.delete_lab(self.mock_labname) # Assert - mock_input.assert_called_once_with() mock_log.info.assert_called_with("Aborting!") + @patch("install_vbox.yes_no_prompt", return_value = True) @patch("install_vbox.vboxmanage") @patch("install_vbox.LOG") @patch("install_vbox.time") def test_delete_lab_force( - self, mock_time, mock_log, mock_vboxmanage + self, mock_time, mock_log, mock_vboxmanage, mock_prompt ): """ Test delete_lab with force=True @@ -384,7 +384,7 @@ class DeleteLabTestCase(unittest.TestCase): mock_vboxmanage.get_all_vms.return_value = self.mock_node_list # Run - install_vbox.delete_lab(self.mock_labname, force=True) + install_vbox.delete_lab(self.mock_labname) # Assert mock_vboxmanage.get_all_vms.assert_called_once_with(self.mock_labname, option="vms")