Fix vconfig and lconfig commands (#137)

src/Kathara/cli/command/LconfigCommand.py

@@ -2,11 +2,10 @@
 import logging
 from typing import List
 
-from ..ui.utils import alphanumeric
+from ..ui.utils import alphanumeric, cd_mac_address
 from ... import utils
 from ...foundation.cli.command.Command import Command
 from ...manager.Kathara import Kathara
-from ...model.Lab import Lab
 from ...parser.netkit.LabParser import LabParser
 from ...strings import strings, wiki_description
 
@@ -45,9 +44,9 @@ def __init__(self) -> None:
 
         group.add_argument(
             '--add',
-            type=str,
+            type=cd_mac_address,
             dest='to_add',
-            metavar='CD',
+            metavar='CD/MAC',
             nargs='+',
             help='Specify the collision domain to add.'
         )
@@ -66,19 +65,16 @@ def run(self, current_path: str, argv: List[str]) -> None:
 
         lab_path = args['directory'].replace('"', '').replace("'", '') if args['directory'] else current_path
         lab_path = utils.get_absolute_path(lab_path)
-        try:
-            lab = LabParser.parse(lab_path)
-        except (Exception, IOError):
-            lab = Lab(None, path=lab_path)
+
+        lab = LabParser.parse(lab_path)
 
         Kathara.get_instance().update_lab_from_api(lab)
 
         machine_name = args['name']
         device = lab.get_machine(machine_name)
 
         if args['to_add']:
-            for cd_to_add in args['to_add']:
-                cd_name, mac_address = utils.parse_cd_mac_address(cd_to_add)
+            for cd_name, mac_address in args['to_add']:
                 logging.info(
                     f"Adding interface to device `{machine_name}` on collision domain `{cd_name}`" +
                     (f" with MAC Address {mac_address}" if mac_address else "") +

src/Kathara/cli/command/VconfigCommand.py

@@ -2,12 +2,11 @@
 import logging
 from typing import List
 
-from ..ui.utils import alphanumeric
+from ..ui.utils import alphanumeric, cd_mac_address
 from ...foundation.cli.command.Command import Command
 from ...manager.Kathara import Kathara
 from ...model.Lab import Lab
 from ...strings import strings, wiki_description
-from ...utils import parse_cd_mac_address
 
 
 class VconfigCommand(Command):
@@ -38,9 +37,9 @@ def __init__(self) -> None:
 
         group.add_argument(
             '--add',
-            type=str,
+            type=cd_mac_address,
             dest='to_add',
-            metavar='CD',
+            metavar='CD/MAC',
             nargs='+',
             help='Specify the collision domain to add.'
         )
@@ -58,14 +57,14 @@ def run(self, current_path: str, argv: List[str]) -> None:
         args = self.get_args()
 
         lab = Lab("kathara_vlab")
+        Kathara.get_instance().update_lab_from_api(lab)
 
         machine_name = args['name']
-        device = lab.new_machine(machine_name)
+        device = lab.get_machine(machine_name)
         device.api_object = Kathara.get_instance().get_machine_api_object(machine_name, lab_name=lab.name)
 
         if args['to_add']:
-            for cd_to_add in args['to_add']:
-                cd_name, mac_address = parse_cd_mac_address(cd_to_add)
+            for cd_name, mac_address in args['to_add']:
                 logging.info(
                     f"Adding interface to device `{machine_name}` on collision domain `{cd_name}`" +
                     (f" with MAC Address {mac_address}" if mac_address else "") +

src/Kathara/cli/ui/utils.py

@@ -13,6 +13,7 @@
 from ...foundation.manager.stats.IMachineStats import IMachineStats
 from ...setting.Setting import Setting
 from ...trdparty.consolemenu import PromptUtils, Screen
+from ...utils import parse_cd_mac_address
 
 FORBIDDEN_TABLE_COLUMNS = ["container_name"]
 
@@ -168,4 +169,8 @@ def interface_cd_mac(value):
     except ValueError:
         raise argparse.ArgumentTypeError("Invalid interface definition: %s" % value)
 
-    return n, cd, mac
+    return n, cd, mac
+
+
+def cd_mac_address(value):
+    return parse_cd_mac_address(value)

tests/cli/lconfig_command_test.py

@@ -27,7 +27,7 @@ def test_lab():
 @mock.patch("src.Kathara.manager.Kathara.Kathara.get_instance")
 @mock.patch("src.Kathara.manager.docker.DockerManager.DockerManager")
 @mock.patch("src.Kathara.parser.netkit.LabParser.LabParser.parse")
-def test_run_add_link(mock_parse_lab, mock_docker_manager, mock_manager_get_instance, test_lab):
+def test_run_add_interface(mock_parse_lab, mock_docker_manager, mock_manager_get_instance, test_lab):
     test_lab.connect_machine_to_link("pc1", "A")
 
     mock_parse_lab.return_value = test_lab
@@ -46,7 +46,40 @@ def test_run_add_link(mock_parse_lab, mock_docker_manager, mock_manager_get_inst
 @mock.patch("src.Kathara.manager.Kathara.Kathara.get_instance")
 @mock.patch("src.Kathara.manager.docker.DockerManager.DockerManager")
 @mock.patch("src.Kathara.parser.netkit.LabParser.LabParser.parse")
-def test_run_add_link_with_directory(mock_parse_lab, mock_docker_manager, mock_manager_get_instance, test_lab):
+def test_run_add_interface_with_mac_address(mock_parse_lab, mock_docker_manager, mock_manager_get_instance, test_lab):
+    test_lab.connect_machine_to_link("pc1", "A")
+
+    mock_parse_lab.return_value = test_lab
+    mock_manager_get_instance.return_value = mock_docker_manager
+    command = LconfigCommand()
+    command.run('.', ['-n', 'pc1', '--add', 'A/00:00:00:00:00:01'])
+    mock_parse_lab.assert_called_once_with(os.getcwd())
+    mock_docker_manager.update_lab_from_api.assert_called_once_with(test_lab)
+    mock_docker_manager.connect_machine_to_link.assert_called_once_with(
+        test_lab.get_machine('pc1'),
+        test_lab.get_machine('pc1').interfaces[0].link,
+        mac_address='00:00:00:00:00:01'
+    )
+
+
+@mock.patch("src.Kathara.manager.Kathara.Kathara.get_instance")
+@mock.patch("src.Kathara.manager.docker.DockerManager.DockerManager")
+@mock.patch("src.Kathara.parser.netkit.LabParser.LabParser.parse")
+def test_run_add_interface_invalid_interface_definition(mock_parse_lab, mock_docker_manager, mock_manager_get_instance,
+                                                        test_lab):
+    test_lab.connect_machine_to_link("pc1", "A")
+
+    mock_parse_lab.return_value = test_lab
+    mock_manager_get_instance.return_value = mock_docker_manager
+    command = LconfigCommand()
+    with pytest.raises(SyntaxError):
+        command.run('.', ['-n', 'pc1', '--add', 'B/00:/00:00:00:00:01'])
+
+
+@mock.patch("src.Kathara.manager.Kathara.Kathara.get_instance")
+@mock.patch("src.Kathara.manager.docker.DockerManager.DockerManager")
+@mock.patch("src.Kathara.parser.netkit.LabParser.LabParser.parse")
+def test_run_add_interface_with_directory(mock_parse_lab, mock_docker_manager, mock_manager_get_instance, test_lab):
     test_lab.connect_machine_to_link("pc1", "A")
 
     mock_parse_lab.return_value = test_lab
@@ -65,7 +98,7 @@ def test_run_add_link_with_directory(mock_parse_lab, mock_docker_manager, mock_m
 @mock.patch("src.Kathara.manager.Kathara.Kathara.get_instance")
 @mock.patch("src.Kathara.manager.docker.DockerManager.DockerManager")
 @mock.patch("src.Kathara.parser.netkit.LabParser.LabParser.parse")
-def test_run_add_two_links(mock_parse_lab, mock_docker_manager, mock_manager_get_instance, test_lab):
+def test_run_add_two_interfaces(mock_parse_lab, mock_docker_manager, mock_manager_get_instance, test_lab):
     test_lab.connect_machine_to_link("pc1", "A")
     test_lab.connect_machine_to_link("pc1", "B")
 
@@ -90,7 +123,52 @@ def test_run_add_two_links(mock_parse_lab, mock_docker_manager, mock_manager_get
 @mock.patch("src.Kathara.manager.Kathara.Kathara.get_instance")
 @mock.patch("src.Kathara.manager.docker.DockerManager.DockerManager")
 @mock.patch("src.Kathara.parser.netkit.LabParser.LabParser.parse")
-def test_run_remove_link(mock_parse_lab, mock_docker_manager, mock_manager_get_instance, test_lab):
+def test_run_add_two_interfaces_one_mac_address(mock_parse_lab, mock_docker_manager, mock_manager_get_instance,
+                                                test_lab):
+    test_lab.connect_machine_to_link("pc1", "A")
+    test_lab.connect_machine_to_link("pc1", "B")
+
+    mock_parse_lab.return_value = test_lab
+    mock_manager_get_instance.return_value = mock_docker_manager
+    command = LconfigCommand()
+    command.run('.', ['-n', 'pc1', '--add', 'A', 'B/00:00:00:00:00:01'])
+    mock_parse_lab.assert_called_once_with(os.getcwd())
+    mock_docker_manager.update_lab_from_api.assert_called_once_with(test_lab)
+    mock_docker_manager.connect_machine_to_link.assert_any_call(
+        test_lab.get_machine('pc1'),
+        test_lab.get_machine("pc1").interfaces[0].link,
+        mac_address=None
+    )
+    mock_docker_manager.connect_machine_to_link.assert_any_call(
+        test_lab.get_machine('pc1'),
+        test_lab.get_machine("pc1").interfaces[1].link,
+        mac_address='00:00:00:00:00:01'
+    )
+
+
+@mock.patch("src.Kathara.manager.Kathara.Kathara.get_instance")
+@mock.patch("src.Kathara.manager.docker.DockerManager.DockerManager")
+@mock.patch("src.Kathara.parser.netkit.LabParser.LabParser.parse")
+def test_run_add_interface_with_directory(mock_parse_lab, mock_docker_manager, mock_manager_get_instance, test_lab):
+    test_lab.connect_machine_to_link("pc1", "A")
+
+    mock_parse_lab.return_value = test_lab
+    mock_manager_get_instance.return_value = mock_docker_manager
+    command = LconfigCommand()
+    command.run('.', ['-d', os.path.join('/test', 'path'), '-n', 'pc1', '--add', 'A'])
+    mock_parse_lab.assert_called_once_with(os.path.abspath(os.path.join('/test', 'path')))
+    mock_docker_manager.update_lab_from_api.assert_called_once_with(test_lab)
+    mock_docker_manager.connect_machine_to_link.assert_called_once_with(
+        test_lab.get_machine('pc1'),
+        test_lab.get_machine("pc1").interfaces[0].link,
+        mac_address=None
+    )
+
+
+@mock.patch("src.Kathara.manager.Kathara.Kathara.get_instance")
+@mock.patch("src.Kathara.manager.docker.DockerManager.DockerManager")
+@mock.patch("src.Kathara.parser.netkit.LabParser.LabParser.parse")
+def test_run_remove_interface(mock_parse_lab, mock_docker_manager, mock_manager_get_instance, test_lab):
     mock_parse_lab.return_value = test_lab
     mock_manager_get_instance.return_value = mock_docker_manager
     command = LconfigCommand()
@@ -104,8 +182,9 @@ def test_run_remove_link(mock_parse_lab, mock_docker_manager, mock_manager_get_i
 @mock.patch("src.Kathara.manager.Kathara.Kathara.get_instance")
 @mock.patch("src.Kathara.manager.docker.DockerManager.DockerManager")
 @mock.patch("src.Kathara.parser.netkit.LabParser.LabParser.parse")
-def test_run_remove_link_with_directory_absolute_path(mock_parse_lab, mock_docker_manager, mock_manager_get_instance,
-                                                      test_lab):
+def test_run_remove_interface_with_directory_absolute_path(mock_parse_lab, mock_docker_manager,
+                                                           mock_manager_get_instance,
+                                                           test_lab):
     mock_parse_lab.return_value = test_lab
     mock_manager_get_instance.return_value = mock_docker_manager
     command = LconfigCommand()
@@ -119,8 +198,9 @@ def test_run_remove_link_with_directory_absolute_path(mock_parse_lab, mock_docke
 @mock.patch("src.Kathara.manager.Kathara.Kathara.get_instance")
 @mock.patch("src.Kathara.manager.docker.DockerManager.DockerManager")
 @mock.patch("src.Kathara.parser.netkit.LabParser.LabParser.parse")
-def test_run_remove_link_with_directory_relative_path(mock_parse_lab, mock_docker_manager, mock_manager_get_instance,
-                                                      test_lab):
+def test_run_remove_interface_with_directory_relative_path(mock_parse_lab, mock_docker_manager,
+                                                           mock_manager_get_instance,
+                                                           test_lab):
     mock_parse_lab.return_value = test_lab
     mock_manager_get_instance.return_value = mock_docker_manager
     command = LconfigCommand()
@@ -134,7 +214,7 @@ def test_run_remove_link_with_directory_relative_path(mock_parse_lab, mock_docke
 @mock.patch("src.Kathara.manager.Kathara.Kathara.get_instance")
 @mock.patch("src.Kathara.manager.docker.DockerManager.DockerManager")
 @mock.patch("src.Kathara.parser.netkit.LabParser.LabParser.parse")
-def test_run_remove_two_links(mock_parse_lab, mock_docker_manager, mock_manager_get_instance, test_lab):
+def test_run_remove_two_interfaces(mock_parse_lab, mock_docker_manager, mock_manager_get_instance, test_lab):
     mock_parse_lab.return_value = test_lab
     mock_manager_get_instance.return_value = mock_docker_manager
     command = LconfigCommand()

tests/cli/vconfig_command_test.py

@@ -1,30 +1,61 @@
 import sys
 from unittest import mock
 
+import pytest
+
 sys.path.insert(0, './')
 
 from src.Kathara.cli.command.VconfigCommand import VconfigCommand
 from src.Kathara.model.Lab import Lab
 
-
+@mock.patch("src.Kathara.model.Lab.Lab.get_or_new_link")
 @mock.patch("src.Kathara.model.Lab.Lab.get_machine")
 @mock.patch("src.Kathara.manager.Kathara.Kathara.get_instance")
 @mock.patch("src.Kathara.manager.docker.DockerManager.DockerManager")
-def test_run_add_link(mock_docker_manager, mock_manager_get_instance, mock_get_machine):
+def test_run_add_interface(mock_docker_manager, mock_manager_get_instance, mock_get_machine, mock_get_or_new_link):
     lab = Lab('kathara_vlab')
     pc1 = lab.new_machine("pc1")
+    link_a = lab.get_or_new_link("A")
+    mock_get_or_new_link.return_value = link_a
     mock_get_machine.return_value = pc1
     mock_manager_get_instance.return_value = mock_docker_manager
     command = VconfigCommand()
     command.run('.', ['-n', 'pc1', '--add', 'A'])
     mock_docker_manager.get_machine_api_object.assert_called_once_with('pc1', lab_name='kathara_vlab')
-    mock_docker_manager.connect_machine_to_link.assert_called_once()
+    mock_docker_manager.connect_machine_to_link.assert_called_once_with(pc1, link_a, mac_address=None)
+
+
+@mock.patch("src.Kathara.model.Lab.Lab.get_or_new_link")
+@mock.patch("src.Kathara.model.Lab.Lab.get_machine")
+@mock.patch("src.Kathara.manager.Kathara.Kathara.get_instance")
+@mock.patch("src.Kathara.manager.docker.DockerManager.DockerManager")
+def test_run_add_interface_with_mac_address(mock_docker_manager, mock_manager_get_instance, mock_get_machine,
+                                            mock_get_or_new_link):
+    lab = Lab('kathara_vlab')
+    pc1 = lab.new_machine("pc1")
+    link_a = lab.get_or_new_link("A")
+    mock_get_machine.return_value = pc1
+    mock_get_or_new_link.return_value = link_a
+    mock_manager_get_instance.return_value = mock_docker_manager
+    command = VconfigCommand()
+    command.run('.', ['-n', 'pc1', '--add', 'A/00:00:00:00:00:01'])
+    mock_docker_manager.get_machine_api_object.assert_called_once_with('pc1', lab_name='kathara_vlab')
+    mock_docker_manager.connect_machine_to_link.assert_called_once_with(pc1, link_a, mac_address='00:00:00:00:00:01')
+
+
+@mock.patch("src.Kathara.manager.Kathara.Kathara.get_instance")
+@mock.patch("src.Kathara.manager.docker.DockerManager.DockerManager")
+def test_run_add_interface_syntax_error(mock_docker_manager, mock_manager_get_instance):
+    mock_manager_get_instance.return_value = mock_docker_manager
+    command = VconfigCommand()
+    with pytest.raises(SyntaxError):
+        command.run('.', ['-n', 'pc1', '--add', 'A/00/:00:00:00:00:01'])
 
 
 @mock.patch("src.Kathara.model.Lab.Lab.get_machine")
 @mock.patch("src.Kathara.manager.Kathara.Kathara.get_instance")
 @mock.patch("src.Kathara.manager.docker.DockerManager.DockerManager")
-def test_run_remove_link(mock_docker_manager, mock_manager_get_instance, mock_get_machine):
+def test_run_remove_interface(mock_docker_manager, mock_manager_get_instance, mock_get_machine):
     lab = Lab('kathara_vlab')
     pc1 = lab.new_machine("pc1")
     lab.new_link("A")