Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[GCU] Handling non-compliant leaf-list with string values #2174

Merged
merged 3 commits into from
May 20, 2022
Merged

[GCU] Handling non-compliant leaf-list with string values #2174

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f2f5196
[GCU] Handling non-compliant leaf-list with string values
ghooo May 18, 2022
def7e14
checking list type
ghooo May 18, 2022
5fdb1c7
fixing typo
ghooo May 18, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions generic_config_updater/gu_common.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -750,6 +750,19 @@ def _get_path_tokens_from_leaf(self, model, token_index, xpath_tokens, config):
# leaf_list_name = match.group(1)
leaf_list_value = match.group(1)
list_config = config[leaf_list_name]
# Workaround for those fields who is defined as leaf-list in YANG model but have string value in config DB
# No need to lookup the item index in ConfigDb since the list is represented as a string, return path to string immediately
# Example:
# xpath: /sonic-buffer-port-egress-profile-list:sonic-buffer-port-egress-profile-list/BUFFER_PORT_EGRESS_PROFILE_LIST/BUFFER_PORT_EGRESS_PROFILE_LIST_LIST[port='Ethernet9']/profile_list[.='egress_lossy_profile']
# path: /BUFFER_PORT_EGRESS_PROFILE_LIST/Ethernet9/profile_list
if isinstance(list_config, str):
return [leaf_list_name]

if not isinstance(list_config, list):
raise ValueError(f"list_config is expected to be of type list or string. Found {type(list_config)}.\n " + \
f"model: {model}\n token_index: {token_index}\n " + \
f"xpath_tokens: {xpath_tokens}\n config: {config}")

list_idx = list_config.index(leaf_list_value)
Copy link
Contributor

@qiluo-msft qiluo-msft May 18, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

list_config

Do you want to add else if block? What is the expected type? #Closed

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The return type is a list of tokens (strings). I don't like adding else to an if that has return. I prefer to avoid indentation when possible.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree. Then add a check of type "list"? and list_config is a misleading name since it could be str.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add description to PR. I think I should enrich the documentation in the code as well adding more examples for each return.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated

return [leaf_list_name, list_idx]

Expand Down
42 changes: 42 additions & 0 deletions tests/generic_config_updater/files/config_db_with_profile_list.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
{
"BUFFER_POOL": {
"egress_lossless_pool": {
"mode": "static",
"size": "33004032",
"xoff": "196608",
"type": "egress"
},
"egress_lossy_pool": {
"size": "12766208",
"type": "egress",
"mode": "dynamic"
}
},
"BUFFER_PROFILE": {
"egress_lossless_profile": {
"pool":"egress_lossless_pool",
"size":"1518",
"dynamic_th":"3"
},
"egress_lossy_profile": {
"pool":"egress_lossy_pool",
"size":"1518",
"dynamic_th":"3"
}
},
"BUFFER_PORT_EGRESS_PROFILE_LIST": {
"Ethernet9": {
"profile_list": "egress_lossless_profile,egress_lossy_profile"
}
},
"PORT": {
"Ethernet9": {
"alias": "Eth3/2",
"lanes": "74",
"description": "",
"speed": "11100",
"tpid": "0x8100",
"admin_status": "up"
}
}
}
22 changes: 22 additions & 0 deletions tests/generic_config_updater/gu_common_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -779,6 +779,19 @@ def test_find_ref_paths__does_not_remove_tables_without_yang(self):
# Assert
self.assertEqual(expected_config, config)

def test_find_ref_paths__ref_path_is_leaflist_in_yang_but_string_in_config_db__path_to_string_returned(self):
# Arrange
path = "/BUFFER_PROFILE/egress_lossless_profile"
expected = [
"/BUFFER_PORT_EGRESS_PROFILE_LIST/Ethernet9/profile_list",
]

# Act
actual = self.path_addressing.find_ref_paths(path, Files.CONFIG_DB_WITH_PROFILE_LIST)

# Assert
self.assertEqual(expected, actual)

def test_convert_path_to_xpath(self):
def check(path, xpath, config=None):
if not config:
Expand Down Expand Up @@ -839,6 +852,9 @@ def check(path, xpath, config=None):
check(path="/LLDP/GLOBAL/mode",
xpath="/sonic-lldp:sonic-lldp/LLDP/GLOBAL/mode",
config=Files.CONFIG_DB_WITH_LLDP)
check(path="/BUFFER_PORT_EGRESS_PROFILE_LIST/Ethernet9/profile_list",
xpath="/sonic-buffer-port-egress-profile-list:sonic-buffer-port-egress-profile-list/BUFFER_PORT_EGRESS_PROFILE_LIST/BUFFER_PORT_EGRESS_PROFILE_LIST_LIST[port='Ethernet9']/profile_list",
config=Files.CONFIG_DB_WITH_PROFILE_LIST)

def test_convert_xpath_to_path(self):
def check(xpath, path, config=None):
Expand Down Expand Up @@ -914,6 +930,12 @@ def check(xpath, path, config=None):
check(xpath="/sonic-lldp:sonic-lldp/LLDP/GLOBAL/mode",
path="/LLDP/GLOBAL/mode",
config=Files.CONFIG_DB_WITH_LLDP)
check(xpath="/sonic-buffer-port-egress-profile-list:sonic-buffer-port-egress-profile-list/BUFFER_PORT_EGRESS_PROFILE_LIST/BUFFER_PORT_EGRESS_PROFILE_LIST_LIST[port='Ethernet9']/profile_list",
path="/BUFFER_PORT_EGRESS_PROFILE_LIST/Ethernet9/profile_list",
config=Files.CONFIG_DB_WITH_PROFILE_LIST)
check(xpath="/sonic-buffer-port-egress-profile-list:sonic-buffer-port-egress-profile-list/BUFFER_PORT_EGRESS_PROFILE_LIST/BUFFER_PORT_EGRESS_PROFILE_LIST_LIST[port='Ethernet9']/profile_list[.='egress_lossy_profile']",
path="/BUFFER_PORT_EGRESS_PROFILE_LIST/Ethernet9/profile_list",
config=Files.CONFIG_DB_WITH_PROFILE_LIST)

def test_has_path(self):
def check(config, path, expected):
Expand Down