Skip to content

Commit

Permalink
[config/config_mgmt.py]: Fix dpb issue with upper case mac in
Browse files Browse the repository at this point in the history
device_metadata.

libYang converts ietf yang types to lower case internally,which
creates false config diff for us while DPB.
This PR fixes the issue by not precessing false diff.

Related issue"
sonic-net/sonic-buildimage#9478

Signed-off-by: Praveen Chaudhary <[email protected]>
  • Loading branch information
Praveen Chaudhary committed Apr 5, 2022
1 parent 6bd54d0 commit 53b19bd
Show file tree
Hide file tree
Showing 2 changed files with 93 additions and 0 deletions.
21 changes: 21 additions & 0 deletions config/config_mgmt.py
Original file line number Diff line number Diff line change
Expand Up @@ -872,6 +872,27 @@ def _recurCreateConfig(diff, inp, outp, config):
# we do not allow updates right now
if isinstance(diff, list) and isinstance(outp, dict):
return changed
'''
libYang converts ietf yang types to lower case internally, which
creates false config diff for us while DPB.
Example:
For DEVICE_METADATA['localhost']['mac'] type is yang:mac-address.
Libyang converts from 'XX:XX:XX:E4:B3:DD' -> 'xx:xx:xx:e4:b3:dd'
so args for this functions will be:
diff = DEVICE_METADATA['localhost']['mac']
where DEVICE_METADATA': {'localhost': {'mac': ['XX:XX:XX:E4:B3:DD', 'xx:xx:xx:e4:b3:dd']}}}
Note: above dict is representation of diff in config given by diffJson
library.
out = 'XX:XX:XX:e4:b3:dd'
inp = 'xx:xx:xx:E4:B3:DD'
With below check, we will avoid processing of such config diff for DPB.
'''
if isinstance(diff, list) and isinstance(outp, str) and \
inp.lower() == outp.lower():
return changed

idx = -1
for key in diff:
Expand Down
72 changes: 72 additions & 0 deletions tests/config_mgmt_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,78 @@ def test_search_keys(self):
len(out['ACL_TABLE'][k]) == 1
return

def test_upper_case_mac_fix(self):
'''
Issue:
https://github.com/Azure/sonic-buildimage/issues/9478
LibYang converts ietf yang types to lower case internally,which
creates false config diff for us while DPB.
This tests is to verify the fix done in config_mgmt.py to resolve this
issue.
Example:
For DEVICE_METADATA['localhost']['mac'] type is yang:mac-address.
Libyang converts from 'XX:XX:XX:E4:B3:DD' -> 'xx:xx:xx:e4:b3:dd'
'''
curConfig = deepcopy(configDbJson)
# Keep only PORT part to skip dependencies.
curConfig = {'PORT': curConfig['PORT']}
# add DEVICE_METADATA Config
curConfig['DEVICE_METADATA'] = {
"localhost": {
"bgp_asn": "65100",
"default_bgp_status": "up",
"default_pfcwd_status": "disable",
"docker_routing_config_mode": "split",
"hostname": "sonic",
"hwsku": "Seastone-DX010",
"mac": "00:11:22:BB:CC:DD",
"platform": "x86_64-cel_seastone-r0",
"type": "LeafRouter"
}
}
cmdpb = self.config_mgmt_dpb(curConfig)
# create ARGS
dPorts, pJson = self.generate_args(portIdx=0, laneIdx=65, \
curMode='4x25G', newMode='2x50G')

# use side effect to mock _createConfigToLoad but with call to same
# function
cmdpb._createConfigToLoad = mock.MagicMock(side_effect=cmdpb._createConfigToLoad)

# Try to breakout and see if writeConfigDB is called thrice
deps, ret = cmdpb.breakOutPort(delPorts=dPorts, portJson=pJson, \
force=True, loadDefConfig=False)

'''
assert call to _createConfigToLoad has
DEVICE_METADATA': {'localhost': {'mac': ['XX:XX:XX:E4:B3:DD',
'xx:xx:xx:e4:b3:dd']}} in diff
'''
(args, kwargs) = cmdpb._createConfigToLoad.call_args_list[0]
print(args)
# in case of tuple get first arg, which is diff
if type(args) == tuple:
args = args[0]
assert args['DEVICE_METADATA']['localhost']['mac'] == \
['00:11:22:BB:CC:DD', '00:11:22:bb:cc:dd']

# verify correct function call to writeConfigDB
assert cmdpb.writeConfigDB.call_count == 3
print(cmdpb.writeConfigDB.call_args_list[0])
# args is populated if call is done as writeConfigDB(a, b), kwargs is
# populated if call is done as writeConfigDB(A=a, B=b)
(args, kwargs) = cmdpb.writeConfigDB.call_args_list[0]
print(args)
# in case of tuple also, we should have only one element writeConfigDB
if type(args) == tuple:
args = args[0]
# Make sure no DEVICE_METADATA in the args to func
assert "DEVICE_METADATA" not in args

return

@pytest.mark.skip(reason="not stable")
def test_break_out(self):
# prepare default config
Expand Down

0 comments on commit 53b19bd

Please sign in to comment.