Skip to content

Commit bf8a6ba

Browse files
committed
[config/config_mgmt.py]: Fix dpb issue with upper case mac in (#2066)
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 #### What I did fixes issue: sonic-net/sonic-buildimage#9478 #### How I did it 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 function _recurCreateConfig in this case 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' I add a check to avoid processing of such config diff for DPB. #### How to verify it Added a unit test. Build time.
1 parent 0e9b7e0 commit bf8a6ba

File tree

2 files changed

+93
-0
lines changed

2 files changed

+93
-0
lines changed

config/config_mgmt.py

+21
Original file line numberDiff line numberDiff line change
@@ -872,6 +872,27 @@ def _recurCreateConfig(diff, inp, outp, config):
872872
# we do not allow updates right now
873873
if isinstance(diff, list) and isinstance(outp, dict):
874874
return changed
875+
'''
876+
libYang converts ietf yang types to lower case internally, which
877+
creates false config diff for us while DPB.
878+
879+
Example:
880+
For DEVICE_METADATA['localhost']['mac'] type is yang:mac-address.
881+
Libyang converts from 'XX:XX:XX:E4:B3:DD' -> 'xx:xx:xx:e4:b3:dd'
882+
so args for this functions will be:
883+
884+
diff = DEVICE_METADATA['localhost']['mac']
885+
where DEVICE_METADATA': {'localhost': {'mac': ['XX:XX:XX:E4:B3:DD', 'xx:xx:xx:e4:b3:dd']}}}
886+
Note: above dict is representation of diff in config given by diffJson
887+
library.
888+
out = 'XX:XX:XX:e4:b3:dd'
889+
inp = 'xx:xx:xx:E4:B3:DD'
890+
891+
With below check, we will avoid processing of such config diff for DPB.
892+
'''
893+
if isinstance(diff, list) and isinstance(outp, str) and \
894+
inp.lower() == outp.lower():
895+
return changed
875896

876897
idx = -1
877898
for key in diff:

tests/config_mgmt_test.py

+72
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,78 @@ def test_search_keys(self):
5757
len(out['ACL_TABLE'][k]) == 1
5858
return
5959

60+
def test_upper_case_mac_fix(self):
61+
'''
62+
Issue:
63+
https://github.com/Azure/sonic-buildimage/issues/9478
64+
65+
LibYang converts ietf yang types to lower case internally,which
66+
creates false config diff for us while DPB.
67+
This tests is to verify the fix done in config_mgmt.py to resolve this
68+
issue.
69+
70+
Example:
71+
For DEVICE_METADATA['localhost']['mac'] type is yang:mac-address.
72+
Libyang converts from 'XX:XX:XX:E4:B3:DD' -> 'xx:xx:xx:e4:b3:dd'
73+
'''
74+
curConfig = deepcopy(configDbJson)
75+
# Keep only PORT part to skip dependencies.
76+
curConfig = {'PORT': curConfig['PORT']}
77+
# add DEVICE_METADATA Config
78+
curConfig['DEVICE_METADATA'] = {
79+
"localhost": {
80+
"bgp_asn": "65100",
81+
"default_bgp_status": "up",
82+
"default_pfcwd_status": "disable",
83+
"docker_routing_config_mode": "split",
84+
"hostname": "sonic",
85+
"hwsku": "Seastone-DX010",
86+
"mac": "00:11:22:BB:CC:DD",
87+
"platform": "x86_64-cel_seastone-r0",
88+
"type": "LeafRouter"
89+
}
90+
}
91+
cmdpb = self.config_mgmt_dpb(curConfig)
92+
# create ARGS
93+
dPorts, pJson = self.generate_args(portIdx=0, laneIdx=65, \
94+
curMode='4x25G', newMode='2x50G')
95+
96+
# use side effect to mock _createConfigToLoad but with call to same
97+
# function
98+
cmdpb._createConfigToLoad = mock.MagicMock(side_effect=cmdpb._createConfigToLoad)
99+
100+
# Try to breakout and see if writeConfigDB is called thrice
101+
deps, ret = cmdpb.breakOutPort(delPorts=dPorts, portJson=pJson, \
102+
force=True, loadDefConfig=False)
103+
104+
'''
105+
assert call to _createConfigToLoad has
106+
DEVICE_METADATA': {'localhost': {'mac': ['XX:XX:XX:E4:B3:DD',
107+
'xx:xx:xx:e4:b3:dd']}} in diff
108+
'''
109+
(args, kwargs) = cmdpb._createConfigToLoad.call_args_list[0]
110+
print(args)
111+
# in case of tuple get first arg, which is diff
112+
if type(args) == tuple:
113+
args = args[0]
114+
assert args['DEVICE_METADATA']['localhost']['mac'] == \
115+
['00:11:22:BB:CC:DD', '00:11:22:bb:cc:dd']
116+
117+
# verify correct function call to writeConfigDB
118+
assert cmdpb.writeConfigDB.call_count == 3
119+
print(cmdpb.writeConfigDB.call_args_list[0])
120+
# args is populated if call is done as writeConfigDB(a, b), kwargs is
121+
# populated if call is done as writeConfigDB(A=a, B=b)
122+
(args, kwargs) = cmdpb.writeConfigDB.call_args_list[0]
123+
print(args)
124+
# in case of tuple also, we should have only one element writeConfigDB
125+
if type(args) == tuple:
126+
args = args[0]
127+
# Make sure no DEVICE_METADATA in the args to func
128+
assert "DEVICE_METADATA" not in args
129+
130+
return
131+
60132
@pytest.mark.skip(reason="not stable")
61133
def test_break_out(self):
62134
# prepare default config

0 commit comments

Comments
 (0)