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.

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

[config/config_mgmt.py]: Fix dpb issue with upper case mac in device_metadata. #2066

Merged
merged 1 commit into from
Apr 8, 2022

Conversation

praveen-li
Copy link

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 pchaudhary@linkedin.com

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.

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)

@praveen-li praveen-li force-pushed the fix_upper_case_mac_dpb branch 3 times, most recently from 9768648 to 8fcfda7 Compare February 15, 2022 17:38
@praveen-li
Copy link
Author

@qiluo-msft @zhangyanzhao

Look like a issue while running VS test in Pipeline. Any POC who can help to resolve it.

ImportError while loading conftest '/agent/_work/1/sonic-swss/tests/conftest.py'.
conftest.py:18: in <module>
    from swsscommon import swsscommon
/usr/lib/python3/dist-packages/swsscommon/swsscommon.py:13: in <module>
    from . import _swsscommon
E   ImportError: libnl-nf-3.so.200: cannot open shared object file: No such file or directory

@praveen-li
Copy link
Author

/azpw run Azure.sonic-utilities

@mssonicbld
Copy link
Collaborator

/AzurePipelines run Azure.sonic-utilities

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@praveen-li
Copy link
Author

Had a look at test failures, the test are not DPB related. Need to re-run Pipeline.

@praveen-li
Copy link
Author

/azpw run Azure.sonic-utilities

@mssonicbld
Copy link
Collaborator

/AzurePipelines run Azure.sonic-utilities

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@praveen-li
Copy link
Author

New Test

@praveen-li
Copy link
Author

/azpw run Azure.sonic-utilities

@mssonicbld
Copy link
Collaborator

/AzurePipelines run Azure.sonic-utilities

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

dgsudharsan
dgsudharsan previously approved these changes Mar 16, 2022
@dgsudharsan
Copy link
Collaborator

@qiluo-msft Can you please review these changes?

qiluo-msft
qiluo-msft previously approved these changes Mar 18, 2022
@dgsudharsan
Copy link
Collaborator

@praveen-li Can you please sync to the latest master? The vs-tests shouldn't run there and it should unblock this PR

@zhangyanzhao
Copy link
Collaborator

Need @praveen-li help to fix the build failure.

@praveen-li
Copy link
Author

/azpw run Azure.sonic-utilities

@mssonicbld
Copy link
Collaborator

/AzurePipelines run Azure.sonic-utilities

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@dgsudharsan
Copy link
Collaborator

@praveen-li Can you please sync to latest sonic-utilities? The pipeline will not have vstests

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 <pchaudhary@linkedin.com>
@praveen-li praveen-li dismissed stale reviews from qiluo-msft and dgsudharsan via 53b19bd April 5, 2022 00:16
@praveen-li
Copy link
Author

praveen-li commented Apr 5, 2022 via email

@dgsudharsan
Copy link
Collaborator

@qiluo-msft The pipelines pass now. Can you please help in merging this?

@qiluo-msft qiluo-msft merged commit 52ca324 into sonic-net:master Apr 8, 2022
@qiluo-msft
Copy link
Contributor

Did this PR fix "Related issue"?

@dgsudharsan
Copy link
Collaborator

Did this PR fix "Related issue"?

Yes it did

judyjoseph pushed a commit that referenced this pull request Apr 11, 2022
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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants