Skip to content

Commit

Permalink
[GCU] Handling non-compliant leaf-list with string values (#2174)
Browse files Browse the repository at this point in the history
#### What I did
Fixes #2170

Converting xpath to path:
- There is only 1 case where the path is referring to the list itself. Example:
```
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",
```
- There is no other case as we the list is just a single string, and we cannot refer to individual elements in string in ConfigDb.

Converting path to xpath, there are 2 cases:
- Xpath is pointing to leaf-list as a whole, return path of whole 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",
path="/BUFFER_PORT_EGRESS_PROFILE_LIST/Ethernet9/profile_list",
```
- Xpath is pointing to an element of the leaf-list, return path of whole list as well since it is not possible to point to specific element -- here we are pointing to the element `egress_lossy_profile`
```
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",
```

#### How I did it
Only a single change if the xpath is pointing to a specific element, just return whole list token from ConfigDb format.

Please note this solution is future proof, if ConfigDb changes how a list is saved as a string or json list, we don't care.

#### How to verify it
unit-test

#### Previous command output (if the output of a command-line utility has changed)

#### New command output (if the output of a command-line utility has changed)
  • Loading branch information
ghooo authored May 20, 2022
1 parent 675c7b6 commit 4fc09b1
Show file tree
Hide file tree
Showing 3 changed files with 77 additions and 0 deletions.
13 changes: 13 additions & 0 deletions generic_config_updater/gu_common.py
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)
return [leaf_list_name, list_idx]

Expand Down
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
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

0 comments on commit 4fc09b1

Please sign in to comment.