Skip to content

Commit a44d002

Browse files
ghoooghooo
committed
returning yang validation failures as errors
1 parent 53fbb94 commit a44d002

File tree

4 files changed

+26
-18
lines changed

4 files changed

+26
-18
lines changed

‎generic_config_updater/gu_common.py

+4-4
Original file line numberDiff line numberDiff line change
@@ -105,9 +105,9 @@ def validate_sonic_yang_config(self, sonic_yang_as_json):
105105
sy.loadData(config_db_as_json)
106106

107107
sy.validate_data_tree()
108-
return True
108+
return True, None
109109
except sonic_yang.SonicYangException as ex:
110-
return False
110+
return False, ex
111111

112112
def validate_config_db_config(self, config_db_as_json):
113113
sy = self.create_sonic_yang_with_loaded_models()
@@ -118,9 +118,9 @@ def validate_config_db_config(self, config_db_as_json):
118118
sy.loadData(tmp_config_db_as_json)
119119

120120
sy.validate_data_tree()
121-
return True
121+
return True, None
122122
except sonic_yang.SonicYangException as ex:
123-
return False
123+
return False, ex
124124

125125
def crop_tables_without_yang(self, config_db_as_json):
126126
sy = self.create_sonic_yang_with_loaded_models()

‎generic_config_updater/patch_sorter.py

+8-5
Original file line numberDiff line numberDiff line change
@@ -562,7 +562,8 @@ def __init__(self, config_wrapper):
562562

563563
def validate(self, move, diff):
564564
simulated_config = move.apply(diff.current_config)
565-
return self.config_wrapper.validate_config_db_config(simulated_config)
565+
is_valid, error = self.config_wrapper.validate_config_db_config(simulated_config)
566+
return is_valid
566567

567568
# TODO: Add this validation to YANG models instead
568569
class UniqueLanesMoveValidator:
@@ -1543,8 +1544,9 @@ def sort(self, patch, algorithm=Algorithm.DFS):
15431544

15441545
# Validate target config
15451546
self.logger.log_info("Validating target config according to YANG models.")
1546-
if not(self.config_wrapper.validate_config_db_config(target_config)):
1547-
raise ValueError(f"Given patch is not valid because it will result in an invalid config")
1547+
is_valid, error = self.config_wrapper.validate_config_db_config(target_config)
1548+
if not is_valid:
1549+
raise ValueError(f"Given patch will produce invalid config. Error: {error}")
15481550

15491551
# Generate list of changes to apply
15501552
self.logger.log_info("Sorting patch updates.")
@@ -1731,8 +1733,9 @@ def sort(self, patch, algorithm=Algorithm.DFS):
17311733

17321734
# Validate YANG covered target config
17331735
self.logger.log_info("Validating YANG covered target config according to YANG models.")
1734-
if not(self.config_wrapper.validate_config_db_config(target_config_yang)):
1735-
raise ValueError(f"Given patch is not valid because it will result in an invalid config")
1736+
is_valid, error = self.config_wrapper.validate_config_db_config(target_config_yang)
1737+
if not is_valid:
1738+
raise ValueError(f"Given patch will produce invalid config. Error: {error}")
17361739

17371740
# Generating changes associated with non-YANG covered configs
17381741
self.logger.log_info("Sorting non-YANG covered configs patch updates.")

‎tests/generic_config_updater/gu_common_test.py

+8-4
Original file line numberDiff line numberDiff line change
@@ -144,43 +144,47 @@ def test_validate_sonic_yang_config__valid_config__returns_true(self):
144144
expected = True
145145

146146
# Act
147-
actual = config_wrapper.validate_sonic_yang_config(Files.SONIC_YANG_AS_JSON)
147+
actual, error = config_wrapper.validate_sonic_yang_config(Files.SONIC_YANG_AS_JSON)
148148

149149
# Assert
150150
self.assertEqual(expected, actual)
151+
self.assertIsNone(error)
151152

152153
def test_validate_sonic_yang_config__invvalid_config__returns_false(self):
153154
# Arrange
154155
config_wrapper = gu_common.ConfigWrapper()
155156
expected = False
156157

157158
# Act
158-
actual = config_wrapper.validate_sonic_yang_config(Files.SONIC_YANG_AS_JSON_INVALID)
159+
actual, error = config_wrapper.validate_sonic_yang_config(Files.SONIC_YANG_AS_JSON_INVALID)
159160

160161
# Assert
161162
self.assertEqual(expected, actual)
163+
self.assertIsNotNone(error)
162164

163165
def test_validate_config_db_config__valid_config__returns_true(self):
164166
# Arrange
165167
config_wrapper = gu_common.ConfigWrapper()
166168
expected = True
167169

168170
# Act
169-
actual = config_wrapper.validate_config_db_config(Files.CONFIG_DB_AS_JSON)
171+
actual, error = config_wrapper.validate_config_db_config(Files.CONFIG_DB_AS_JSON)
170172

171173
# Assert
172174
self.assertEqual(expected, actual)
175+
self.assertIsNone(error)
173176

174177
def test_validate_config_db_config__invalid_config__returns_false(self):
175178
# Arrange
176179
config_wrapper = gu_common.ConfigWrapper()
177180
expected = False
178181

179182
# Act
180-
actual = config_wrapper.validate_config_db_config(Files.CONFIG_DB_AS_JSON_INVALID)
183+
actual, error = config_wrapper.validate_config_db_config(Files.CONFIG_DB_AS_JSON_INVALID)
181184

182185
# Assert
183186
self.assertEqual(expected, actual)
187+
self.assertIsNotNone(error)
184188

185189
def test_crop_tables_without_yang__returns_cropped_config_db_as_json(self):
186190
# Arrange

‎tests/generic_config_updater/patch_sorter_test.py

+6-5
Original file line numberDiff line numberDiff line change
@@ -925,7 +925,7 @@ def test_validate__invalid_config_db_after_applying_move__failure(self):
925925
# Arrange
926926
config_wrapper = Mock()
927927
config_wrapper.validate_config_db_config.side_effect = \
928-
create_side_effect_dict({(str(self.any_simulated_config),): False})
928+
create_side_effect_dict({(str(self.any_simulated_config),): (False, None)})
929929
validator = ps.FullConfigMoveValidator(config_wrapper)
930930

931931
# Act and assert
@@ -935,7 +935,7 @@ def test_validate__valid_config_db_after_applying_move__success(self):
935935
# Arrange
936936
config_wrapper = Mock()
937937
config_wrapper.validate_config_db_config.side_effect = \
938-
create_side_effect_dict({(str(self.any_simulated_config),): True})
938+
create_side_effect_dict({(str(self.any_simulated_config),): (True, None)})
939939
validator = ps.FullConfigMoveValidator(config_wrapper)
940940

941941
# Act and assert
@@ -3102,7 +3102,8 @@ def run_single_success_case(self, data, skip_exact_change_list_match):
31023102
simulated_config = current_config
31033103
for change in actual_changes:
31043104
simulated_config = change.apply(simulated_config)
3105-
self.assertTrue(self.config_wrapper.validate_config_db_config(simulated_config))
3105+
is_valid, error = self.config_wrapper.validate_config_db_config(simulated_config)
3106+
self.assertTrue(is_valid, f"Change will produce invalid config. Error: {error}")
31063107
self.assertEqual(target_config, simulated_config)
31073108

31083109
def test_patch_sorter_failure(self):
@@ -3426,7 +3427,7 @@ def __create_patch_sorter(self,
34263427
(str(any_target_config),): (any_target_config_yang, any_target_config_non_yang)})
34273428

34283429
config_wrapper.validate_config_db_config.side_effect = \
3429-
create_side_effect_dict({(str(any_target_config_yang),): valid_yang_covered_config})
3430+
create_side_effect_dict({(str(any_target_config_yang),): (valid_yang_covered_config, None)})
34303431

34313432
patch_wrapper.generate_patch.side_effect = \
34323433
create_side_effect_dict(
@@ -3519,7 +3520,7 @@ def __create_patch_sorter(self,
35193520

35203521
config_wrapper.validate_config_db_config.side_effect = \
35213522
create_side_effect_dict(
3522-
{(str(any_target_config),): valid_config_db})
3523+
{(str(any_target_config),): (valid_config_db, None)})
35233524

35243525

35253526
inner_patch_sorter.sort.side_effect = \

0 commit comments

Comments
 (0)