diff --git a/config/main.py b/config/main.py index 7ab13bc863..2a64f315ca 100644 --- a/config/main.py +++ b/config/main.py @@ -2078,18 +2078,17 @@ def remove_portchannel(ctx, portchannel_name): if is_portchannel_present_in_db(db, portchannel_name) is False: ctx.fail("{} is not present.".format(portchannel_name)) - # Dont let to remove port channel if vlan membership exists - for k,v in db.get_table('VLAN_MEMBER'): - if v == portchannel_name: - ctx.fail("{} has vlan {} configured, remove vlan membership to proceed".format(portchannel_name, str(k))) + # Dont let to remove port channel if vlan membership exists + for k,v in db.get_table('VLAN_MEMBER'): # TODO: MISSING CONSTRAINT IN YANG MODEL + if v == portchannel_name: + ctx.fail("{} has vlan {} configured, remove vlan membership to proceed".format(portchannel_name, str(k))) - if len([(k, v) for k, v in db.get_table('PORTCHANNEL_MEMBER') if k == portchannel_name]) != 0: - click.echo("Error: Portchannel {} contains members. Remove members before deleting Portchannel!".format(portchannel_name)) - else: - try: - db.set_entry('PORTCHANNEL', portchannel_name, None) - except JsonPatchConflict: - ctx.fail("{} is not present.".format(portchannel_name)) + if len([(k, v) for k, v in db.get_table('PORTCHANNEL_MEMBER') if k == portchannel_name]) != 0: # TODO: MISSING CONSTRAINT IN YANG MODEL + click.echo("Error: Portchannel {} contains members. Remove members before deleting Portchannel!".format(portchannel_name)) + try: + db.set_entry('PORTCHANNEL', portchannel_name, None) + except JsonPatchConflict: + ctx.fail("{} is not present.".format(portchannel_name)) @portchannel.group(cls=clicommon.AbbreviationGroup, name='member') @click.pass_context @@ -6161,9 +6160,9 @@ def add_loopback(ctx, loopback_name): ctx.fail("{} is invalid, name should have prefix '{}' and suffix '{}' " .format(loopback_name, CFG_LOOPBACK_PREFIX, CFG_LOOPBACK_NO)) - lo_intfs = [k for k, v in config_db.get_table('LOOPBACK_INTERFACE').items() if type(k) != tuple] - if loopback_name in lo_intfs: - ctx.fail("{} already exists".format(loopback_name)) + lo_intfs = [k for k, v in config_db.get_table('LOOPBACK_INTERFACE').items() if type(k) != tuple] + if loopback_name in lo_intfs: + ctx.fail("{} already exists".format(loopback_name)) # TODO: MISSING CONSTRAINT IN YANG VALIDATION try: config_db.set_entry('LOOPBACK_INTERFACE', loopback_name, {"NULL" : "NULL"}) diff --git a/config/validated_config_db_connector.py b/config/validated_config_db_connector.py index 9e5d5f2f12..434e9aca9d 100644 --- a/config/validated_config_db_connector.py +++ b/config/validated_config_db_connector.py @@ -36,10 +36,12 @@ def validated_set_entry(table, key, value): gcu_patch = jsonpatch.JsonPatch(gcu_json_input) format = ConfigFormat.CONFIGDB.name config_format = ConfigFormat[format.upper()] + + config_db = ConfigDBConnector() + config_db.connect() try: GenericUpdater().apply_patch(patch=gcu_patch, config_format=config_format, verbose=False, dry_run=False, ignore_non_yang_tables=False, ignore_paths=None) except EmptyTableError: - config_db = ConfigDBConnector() - config_db.connect() + print(config_db) config_db.set_entry(table, key, None) diff --git a/tests/config_test.py b/tests/config_test.py index a9f4982548..627be68400 100644 --- a/tests/config_test.py +++ b/tests/config_test.py @@ -15,6 +15,7 @@ from sonic_py_common import device_info from utilities_common.db import Db from utilities_common.general import load_module_from_source +from mock import patch from generic_config_updater.generic_updater import ConfigFormat @@ -862,6 +863,7 @@ def test_apply_patch__only_required_params__default_values_used_for_optional_par result = self.runner.invoke(config.config.commands["apply-patch"], [self.any_path], catch_exceptions=False) # Assert + print(result.output) self.assertEqual(expected_exit_code, result.exit_code) self.assertTrue(expected_output in result.output) mock_generic_updater.apply_patch.assert_called_once() @@ -988,6 +990,7 @@ def test_replace__only_required_params__default_values_used_for_optional_params( # Assert self.assertEqual(expected_exit_code, result.exit_code) + print(result.output) self.assertTrue(expected_output in result.output) mock_generic_updater.replace.assert_called_once() mock_generic_updater.replace.assert_has_calls([expected_call_with_default_values]) @@ -1614,3 +1617,87 @@ def test_hostname_add(self, db_conn_patch, get_cmd_module): @classmethod def teardown_class(cls): print("TEARDOWN") + + +class TestConfigLoopback(object): + @classmethod + def setup_class(cls): + print("SETUP") + import config.main + importlib.reload(config.main) + + @patch("config.validated_config_db_connector.validated_set_entry", mock.Mock(side_effect=ValueError)) + def test_add_loopback_with_invalid_name_yang_validation(self): + config.ADHOC_VALIDATION = False + runner = CliRunner() + db = Db() + obj = {'db':db.cfgdb} + + result = runner.invoke(config.config.commands["loopback"].commands["add"], ["Loopbax1"], obj=obj) + print(result.exit_code) + print(result.output) + assert result.exit_code != 0 + assert "Error: Loopbax1 is invalid, name should have prefix 'Loopback' and suffix '<0-999>'" in result.output + + def test_add_loopback_with_invalid_name_adhoc_validation(self): + config.ADHOC_VALIDATION = True + runner = CliRunner() + db = Db() + obj = {'db':db.cfgdb} + + result = runner.invoke(config.config.commands["loopback"].commands["add"], ["Loopbax1"], obj=obj) + print(result.exit_code) + print(result.output) + assert result.exit_code != 0 + assert "Error: Loopbax1 is invalid, name should have prefix 'Loopback' and suffix'<0-999>'" in result.output + + def test_del_nonexistent_loopback_adhoc_validation(self): + config.ADHOC_VALIDATION = True + runner = CliRunner() + db = Db() + obj = {'db':db.cfgdb} + + result = runner.invoke(config.config.commands["loopback"].commands["del"], ["Loopback12"], obj=obj) + print(result.exit_code) + print(result.output) + assert result.exit_code != 0 + assert "Loopback12 does not exist" in result.output + + def test_del_nonexistent_loopback_adhoc_validation(self): + config.ADHOC_VALIDATION = True + runner = CliRunner() + db = Db() + obj = {'db':db.cfgdb} + + result = runner.invoke(config.config.commands["loopback"].commands["del"], ["Loopbax1"], obj=obj) + print(result.exit_code) + print(result.output) + assert result.exit_code != 0 + assert "Loopbax1 is invalid, name should have prefix 'Loopback' and suffix '<0-999>'" in result.output + + @patch("config.validated_config_db_connector.validated_set_entry", mock.Mock(return_value=True)) + def test_add_loopback_yang_validation(self): + config.ADHOC_VALIDATION = False + runner = CliRunner() + db = Db() + obj = {'db':db.cfgdb} + + result = runner.invoke(config.config.commands["loopback"].commands["add"], ["Loopback12"], obj=obj) + print(result.exit_code) + print(result.output) + assert result.exit_code == 0 + + def test_add_loopback_adhoc_validation(self): + config.ADHOC_VALIDATION = True + runner = CliRunner() + db = Db() + obj = {'db':db.cfgdb} + + result = runner.invoke(config.config.commands["loopback"].commands["add"], ["Loopback12"], obj=obj) + print(result.exit_code) + print(result.output) + assert result.exit_code == 0 + + @classmethod + def teardown_class(cls): + print("TEARDOWN") diff --git a/tests/portchannel_test.py b/tests/portchannel_test.py index d763cba393..289095766b 100644 --- a/tests/portchannel_test.py +++ b/tests/portchannel_test.py @@ -30,7 +30,6 @@ def test_add_portchannel_with_invalid_name_yang_validation(self): print(result.output) assert result.exit_code != 0 assert "Error: PortChan005 is invalid!, name should have prefix 'PortChannel' and suffix '<0-9999>'" in result.output - def test_add_portchannel_with_invalid_name_adhoc_validation(self): config.ADHOC_VALIDATION = True @@ -45,6 +44,20 @@ def test_add_portchannel_with_invalid_name_adhoc_validation(self): assert result.exit_code != 0 assert "Error: PortChan005 is invalid!, name should have prefix 'PortChannel' and suffix '<0-9999>'" in result.output + @patch("config.validated_config_db_connector.validated_set_entry", mock.Mock(side_effect=JsonPatchConflict)) + def test_delete_nonexistent_portchannel_yang_validation(self): + config.ADHOC_VALIDATION = False + runner = CliRunner() + db = Db() + obj = {'db':db.cfgdb} + + # delete a portchannel with invalid name + result = runner.invoke(config.config.commands["portchannel"].commands["del"], ["PortChan005"], obj=obj) + print(result.exit_code) + print(result.output) + assert result.exit_code != 0 + assert "PortChan005 is not present" in result.output + def test_delete_portchannel_with_invalid_name_adhoc_validation(self): config.ADHOC_VALIDATION = True runner = CliRunner() diff --git a/tests/validated_config_db_connector_test.py b/tests/validated_config_db_connector_test.py index 0c287bc447..244f789dc6 100644 --- a/tests/validated_config_db_connector_test.py +++ b/tests/validated_config_db_connector_test.py @@ -23,7 +23,7 @@ class TestValidatedConfigDBConnector(TestCase): Test Class for validated_config_db_connector.py ''' - def test_validated_config_db_connector(self): + def test_validated_config_db_connector_empty_table(self): mock_generic_updater = mock.Mock() mock_generic_updater.apply_patch = mock.Mock(side_effect=EmptyTableError) with mock.patch('validated_config_db_connector.GenericUpdater', return_value=mock_generic_updater): @@ -32,5 +32,3 @@ def test_validated_config_db_connector(self): with mock.patch('validated_config_db_connector.ConfigDBConnector', return_value=db.cfgdb): validated_config_db_connector.validated_set_entry(SAMPLE_TABLE, SAMPLE_KEY, SAMPLE_VALUE_EMPTY) assert db.cfgdb.get_entry(SAMPLE_TABLE, SAMPLE_KEY) == {} - -