diff --git a/generic_config_updater/gu_common.py b/generic_config_updater/gu_common.py index 0fa1e66f92..356bff3435 100644 --- a/generic_config_updater/gu_common.py +++ b/generic_config_updater/gu_common.py @@ -105,9 +105,9 @@ def validate_sonic_yang_config(self, sonic_yang_as_json): sy.loadData(config_db_as_json) sy.validate_data_tree() - return True + return True, None except sonic_yang.SonicYangException as ex: - return False + return False, ex def validate_config_db_config(self, config_db_as_json): sy = self.create_sonic_yang_with_loaded_models() @@ -118,9 +118,9 @@ def validate_config_db_config(self, config_db_as_json): sy.loadData(tmp_config_db_as_json) sy.validate_data_tree() - return True + return True, None except sonic_yang.SonicYangException as ex: - return False + return False, ex def crop_tables_without_yang(self, config_db_as_json): sy = self.create_sonic_yang_with_loaded_models() @@ -151,7 +151,8 @@ def remove_empty_tables(self, config): def create_sonic_yang_with_loaded_models(self): # sonic_yang_with_loaded_models will only be initialized once the first time this method is called if self.sonic_yang_with_loaded_models is None: - loaded_models_sy = sonic_yang.SonicYang(self.yang_dir) + sonic_yang_print_log_enabled = genericUpdaterLogging.get_verbose() + loaded_models_sy = sonic_yang.SonicYang(self.yang_dir, print_log_enabled=sonic_yang_print_log_enabled) loaded_models_sy.loadYangModel() # This call takes a long time (100s of ms) because it reads files from disk self.sonic_yang_with_loaded_models = loaded_models_sy @@ -829,6 +830,9 @@ def __init__(self): def set_verbose(self, verbose): self._verbose = verbose + def get_verbose(self): + return self._verbose + def get_logger(self, title, print_all_to_console=False): return TitledLogger(SYSLOG_IDENTIFIER, title, self._verbose, print_all_to_console) diff --git a/generic_config_updater/patch_sorter.py b/generic_config_updater/patch_sorter.py index 97db21b24e..f23b347bde 100644 --- a/generic_config_updater/patch_sorter.py +++ b/generic_config_updater/patch_sorter.py @@ -562,7 +562,8 @@ def __init__(self, config_wrapper): def validate(self, move, diff): simulated_config = move.apply(diff.current_config) - return self.config_wrapper.validate_config_db_config(simulated_config) + is_valid, error = self.config_wrapper.validate_config_db_config(simulated_config) + return is_valid # TODO: Add this validation to YANG models instead class UniqueLanesMoveValidator: @@ -1543,8 +1544,9 @@ def sort(self, patch, algorithm=Algorithm.DFS): # Validate target config self.logger.log_info("Validating target config according to YANG models.") - if not(self.config_wrapper.validate_config_db_config(target_config)): - raise ValueError(f"Given patch is not valid because it will result in an invalid config") + is_valid, error = self.config_wrapper.validate_config_db_config(target_config) + if not is_valid: + raise ValueError(f"Given patch will produce invalid config. Error: {error}") # Generate list of changes to apply self.logger.log_info("Sorting patch updates.") @@ -1731,8 +1733,9 @@ def sort(self, patch, algorithm=Algorithm.DFS): # Validate YANG covered target config self.logger.log_info("Validating YANG covered target config according to YANG models.") - if not(self.config_wrapper.validate_config_db_config(target_config_yang)): - raise ValueError(f"Given patch is not valid because it will result in an invalid config") + is_valid, error = self.config_wrapper.validate_config_db_config(target_config_yang) + if not is_valid: + raise ValueError(f"Given patch will produce invalid config. Error: {error}") # Generating changes associated with non-YANG covered configs self.logger.log_info("Sorting non-YANG covered configs patch updates.") diff --git a/tests/generic_config_updater/gu_common_test.py b/tests/generic_config_updater/gu_common_test.py index 2b95bdecc7..e3a8a3bde0 100644 --- a/tests/generic_config_updater/gu_common_test.py +++ b/tests/generic_config_updater/gu_common_test.py @@ -144,10 +144,11 @@ def test_validate_sonic_yang_config__valid_config__returns_true(self): expected = True # Act - actual = config_wrapper.validate_sonic_yang_config(Files.SONIC_YANG_AS_JSON) + actual, error = config_wrapper.validate_sonic_yang_config(Files.SONIC_YANG_AS_JSON) # Assert self.assertEqual(expected, actual) + self.assertIsNone(error) def test_validate_sonic_yang_config__invvalid_config__returns_false(self): # Arrange @@ -155,10 +156,11 @@ def test_validate_sonic_yang_config__invvalid_config__returns_false(self): expected = False # Act - actual = config_wrapper.validate_sonic_yang_config(Files.SONIC_YANG_AS_JSON_INVALID) + actual, error = config_wrapper.validate_sonic_yang_config(Files.SONIC_YANG_AS_JSON_INVALID) # Assert self.assertEqual(expected, actual) + self.assertIsNotNone(error) def test_validate_config_db_config__valid_config__returns_true(self): # Arrange @@ -166,10 +168,11 @@ def test_validate_config_db_config__valid_config__returns_true(self): expected = True # Act - actual = config_wrapper.validate_config_db_config(Files.CONFIG_DB_AS_JSON) + actual, error = config_wrapper.validate_config_db_config(Files.CONFIG_DB_AS_JSON) # Assert self.assertEqual(expected, actual) + self.assertIsNone(error) def test_validate_config_db_config__invalid_config__returns_false(self): # Arrange @@ -177,10 +180,11 @@ def test_validate_config_db_config__invalid_config__returns_false(self): expected = False # Act - actual = config_wrapper.validate_config_db_config(Files.CONFIG_DB_AS_JSON_INVALID) + actual, error = config_wrapper.validate_config_db_config(Files.CONFIG_DB_AS_JSON_INVALID) # Assert self.assertEqual(expected, actual) + self.assertIsNotNone(error) def test_crop_tables_without_yang__returns_cropped_config_db_as_json(self): # Arrange diff --git a/tests/generic_config_updater/patch_sorter_test.py b/tests/generic_config_updater/patch_sorter_test.py index 79f2b5a76b..4cb8fa7019 100644 --- a/tests/generic_config_updater/patch_sorter_test.py +++ b/tests/generic_config_updater/patch_sorter_test.py @@ -925,7 +925,7 @@ def test_validate__invalid_config_db_after_applying_move__failure(self): # Arrange config_wrapper = Mock() config_wrapper.validate_config_db_config.side_effect = \ - create_side_effect_dict({(str(self.any_simulated_config),): False}) + create_side_effect_dict({(str(self.any_simulated_config),): (False, None)}) validator = ps.FullConfigMoveValidator(config_wrapper) # Act and assert @@ -935,7 +935,7 @@ def test_validate__valid_config_db_after_applying_move__success(self): # Arrange config_wrapper = Mock() config_wrapper.validate_config_db_config.side_effect = \ - create_side_effect_dict({(str(self.any_simulated_config),): True}) + create_side_effect_dict({(str(self.any_simulated_config),): (True, None)}) validator = ps.FullConfigMoveValidator(config_wrapper) # Act and assert @@ -3100,7 +3100,8 @@ def run_single_success_case(self, data, skip_exact_change_list_match): simulated_config = current_config for change in actual_changes: simulated_config = change.apply(simulated_config) - self.assertTrue(self.config_wrapper.validate_config_db_config(simulated_config)) + is_valid, error = self.config_wrapper.validate_config_db_config(simulated_config) + self.assertTrue(is_valid, f"Change will produce invalid config. Error: {error}") self.assertEqual(target_config, simulated_config) def test_patch_sorter_failure(self): @@ -3424,7 +3425,7 @@ def __create_patch_sorter(self, (str(any_target_config),): (any_target_config_yang, any_target_config_non_yang)}) config_wrapper.validate_config_db_config.side_effect = \ - create_side_effect_dict({(str(any_target_config_yang),): valid_yang_covered_config}) + create_side_effect_dict({(str(any_target_config_yang),): (valid_yang_covered_config, None)}) patch_wrapper.generate_patch.side_effect = \ create_side_effect_dict( @@ -3517,7 +3518,7 @@ def __create_patch_sorter(self, config_wrapper.validate_config_db_config.side_effect = \ create_side_effect_dict( - {(str(any_target_config),): valid_config_db}) + {(str(any_target_config),): (valid_config_db, None)}) inner_patch_sorter.sort.side_effect = \