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

Jump to bottom

Convert IPv6 addresses to lowercase in apply-patch #2299

Merged
merged 2 commits into from
Aug 11, 2022
Merged

Convert IPv6 addresses to lowercase in apply-patch #2299

merged 2 commits into from
Aug 11, 2022

Conversation

dbarashinvd
Copy link
Collaborator

@dbarashinvd dbarashinvd commented Aug 7, 2022 •
edited by liat-grozovik
Loading

Fixes sonic-net/sonic-buildimage#11622

What I did

Convert IPv6 addresses to lowercase in apply-patch for remove op

How I did it

python regex on 'remove' op in JSON patch input file

How to verify it

manual test of created bug

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)

@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Aug 7, 2022 •
edited
Loading

CLA Signed

The committers listed above are authorized under a signed CLA.

@dbarashinvd
Copy link
Collaborator Author

/azpw run

@mssonicbld
Copy link
Collaborator

/AzurePipelines run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@dbarashinvd
Copy link
Collaborator Author

/azpw run

@mssonicbld
Copy link
Collaborator

/AzurePipelines run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@dbarashinvd dbarashinvd force-pushed the dbarashi_master_ipv6address branch from 81c3c3a to b22d410 Compare August 8, 2022 15:27
@qiluo-msft
Copy link
Contributor

@isabelmsft Please help review this PR.

@qiluo-msft qiluo-msft requested a review from isabelmsft August 8, 2022 20:31
isabelmsft
isabelmsft reviewed Aug 9, 2022
View reviewed changes
config/main.py Outdated Show resolved Hide resolved
@dbarashinvd dbarashinvd force-pushed the dbarashi_master_ipv6address branch from b22d410 to 8f7e4ff Compare August 10, 2022 12:09
@dprital
Copy link
Collaborator

dprital commented Aug 10, 2022

@isabelmsft - comment was addressed. Can you please approve ?

@liat-grozovik liat-grozovik merged commit 28b6ba5 into sonic-net:master Aug 11, 2022
dprital added a commit to dprital/sonic-buildimage that referenced this pull request Aug 11, 2022
@dprital
[submodule] Advance sonic-utilities pointer
6b7fb15 
Update sonic-utilities submodule pointer to include the following:
* Convert IPv6 addresses to lowercase in apply-patch ([sonic-net#2299](sonic-net/sonic-utilities#2299))
* [CLI] Move hostname, mgmt interface/vrf config to hostcfgd ([sonic-net#2173](sonic-net/sonic-utilities#2173))
* [config][muxcable] add support to enable/disable ycable telemetry ([sonic-net#2297](sonic-net/sonic-utilities#2297))

Signed-off-by: dprital <drorp@nvidia.com>
@dprital dprital mentioned this pull request Aug 11, 2022
Merged
7 tasks
yxieca pushed a commit that referenced this pull request Aug 11, 2022
@dbarashinvd @yxieca
Convert IPv6 addresses to lowercase in apply-patch (#2299)
750d1db 
Fixes sonic-net/sonic-buildimage#11622

- What I did
Convert IPv6 addresses to lowercase in apply-patch for remove op

- How I did it
python regex on 'remove' op in JSON patch input file

- How to verify it
Manual test of created bug, Unit test
lguohan pushed a commit to sonic-net/sonic-buildimage that referenced this pull request Aug 11, 2022
@dprital
[submodule] Advance sonic-utilities pointer (#11698)
f601f2f 
Update sonic-utilities submodule pointer to include the following:
* Convert IPv6 addresses to lowercase in apply-patch ([#2299](sonic-net/sonic-utilities#2299))
* [CLI] Move hostname, mgmt interface/vrf config to hostcfgd ([#2173](sonic-net/sonic-utilities#2173))
* [config][muxcable] add support to enable/disable ycable telemetry ([#2297](sonic-net/sonic-utilities#2297))

Signed-off-by: dprital <drorp@nvidia.com>
dprital added a commit to dprital/sonic-buildimage that referenced this pull request Aug 12, 2022
@dprital
[202205][submodule] Advance sonic-utilities pointer
5200fa6 
Update sonic-utilities submodule pointer to include the following:
* Fix test failure in dump table test in 202205 ([sonic-net#2307](sonic-net/sonic-utilities#2307))
* Convert IPv6 addresses to lowercase in apply-patch ([sonic-net#2299](sonic-net/sonic-utilities#2299))
* [config][muxcable] add support to enable/disable ycable telemetry ([sonic-net#2297](sonic-net/sonic-utilities#2297))
* Fix GCU bug when backend service modifying config ([sonic-net#2295](sonic-net/sonic-utilities#2295))
* [intfutil] Check whether the FEC mode is supported on the platform before configuring it to CONFIG_DB ([sonic-net#2223](sonic-net/sonic-utilities#2223))
* Improve the way to check port type of RJ45 port ([sonic-net#2249](sonic-net/sonic-utilities#2249))
* sonic-utils: initial support for link-training ([sonic-net#2071](sonic-net/sonic-utilities#2071))
* Support to enable fips for the command sonic_installer (sonic-net#2154) ([sonic-net#2303](sonic-net/sonic-utilities#2303))

Signed-off-by: dprital <drorp@nvidia.com>
@dprital dprital mentioned this pull request Aug 12, 2022
Closed
7 tasks
preetham-singh pushed a commit to preetham-singh/sonic-utilities that referenced this pull request Nov 21, 2022
@dbarashinvd @preetham-singh
Convert IPv6 addresses to lowercase in apply-patch (sonic-net#2299)
78a09bb 
Fixes sonic-net/sonic-buildimage#11622

- What I did
Convert IPv6 addresses to lowercase in apply-patch for remove op

- How I did it
python regex on 'remove' op in JSON patch input file

- How to verify it
Manual test of created bug, Unit test
@wen587
Copy link
Contributor

wen587 commented Mar 22, 2023

@dbarashinvd
There are some issues when GCU tries to remove the interface IP in tests, such as add rack test.

In the initial config, the INTERFACE's IPv6 was all loaded in uppercase.

    "INTERFACE": {
        "Ethernet68": {},
        "Ethernet68|10.0.0.34/31": {},
        "Ethernet68|FC00::45/126": {},
        "Ethernet72": {},
        "Ethernet72|10.0.0.36/31": {},
        "Ethernet72|FC00::49/126": {},

GCU will never be able to remove that IP because IPv6 was always translated to lowercase due to this PR. It reported error can't remove a non-existent object, thus making GCU fail.

This PR is to deal with this issue: sonic-net/sonic-buildimage#11622.

Although config CLI always translates uppercase to lowercase when adding an IP, user can have two choices to remove that IP: One is to use config CLI to remove that IP no matter uppercase or lowercase. Another way is to use GCU. In order to use GCU, the user has to check the IP format saved in ConfigDB because GCU operation does differentiate between uppercase and lowercase.
@qiluo-msft for vis.

wen587 added a commit to wen587/sonic-utilities that referenced this pull request Mar 27, 2023
@wen587
Revert "Convert IPv6 addresses to lowercase in apply-patch (sonic-net…
8ed64a8 
…#2299)"

This reverts commit 28b6ba5.
@wen587 wen587 mentioned this pull request Mar 27, 2023
Merged
qiluo-msft pushed a commit that referenced this pull request Mar 28, 2023
@wen587
Revert "Convert IPv6 addresses to lowercase in apply-patch (#2299)" (#…
53f611b 
…2758)

This reverts commit 28b6ba5.


There are some issues when GCU tries to remove the interface IP in some tests, such as add rack test.

In the initial config, the INTERFACE's IPv6 was all loaded in uppercase by default.

    "INTERFACE": {
        "Ethernet68": {},
        "Ethernet68|10.0.0.34/31": {},
        "Ethernet68|FC00::45/126": {},
        "Ethernet72": {},
        "Ethernet72|10.0.0.36/31": {},
        "Ethernet72|FC00::49/126": {},
GCU will never be able to remove that IP because IPv6 was always translated to lowercase due to #2299 . It reported the error can't remove a non-existent object, thus making GCU fail.

#2299  is to deal with this issue: sonic-net/sonic-buildimage#11622.

Although config CLI always translates uppercase to lowercase when adding an IP, the user can have two choices to remove that IP: One is to use config CLI to remove that IP no matter uppercase or lowercase. Another way is to use GCU. In order to use GCU, the user has to check the IP format saved in ConfigDB because GCU operation does differentiate between uppercase and lowercase.
#### What I did
Revert #2299
yxieca pushed a commit that referenced this pull request Apr 1, 2023
@wen587 @yxieca
Revert "Convert IPv6 addresses to lowercase in apply-patch (#2299)" (#…
fe224f0 
…2758)

This reverts commit 28b6ba5.


There are some issues when GCU tries to remove the interface IP in some tests, such as add rack test.

In the initial config, the INTERFACE's IPv6 was all loaded in uppercase by default.

    "INTERFACE": {
        "Ethernet68": {},
        "Ethernet68|10.0.0.34/31": {},
        "Ethernet68|FC00::45/126": {},
        "Ethernet72": {},
        "Ethernet72|10.0.0.36/31": {},
        "Ethernet72|FC00::49/126": {},
GCU will never be able to remove that IP because IPv6 was always translated to lowercase due to #2299 . It reported the error can't remove a non-existent object, thus making GCU fail.

#2299  is to deal with this issue: sonic-net/sonic-buildimage#11622.

Although config CLI always translates uppercase to lowercase when adding an IP, the user can have two choices to remove that IP: One is to use config CLI to remove that IP no matter uppercase or lowercase. Another way is to use GCU. In order to use GCU, the user has to check the IP format saved in ConfigDB because GCU operation does differentiate between uppercase and lowercase.
#### What I did
Revert #2299
StormLiangMS pushed a commit that referenced this pull request Apr 8, 2023
@wen587 @StormLiangMS
Revert "Convert IPv6 addresses to lowercase in apply-patch (#2299)" (#…
bab983c 
…2758)

This reverts commit 28b6ba5.


There are some issues when GCU tries to remove the interface IP in some tests, such as add rack test.

In the initial config, the INTERFACE's IPv6 was all loaded in uppercase by default.

    "INTERFACE": {
        "Ethernet68": {},
        "Ethernet68|10.0.0.34/31": {},
        "Ethernet68|FC00::45/126": {},
        "Ethernet72": {},
        "Ethernet72|10.0.0.36/31": {},
        "Ethernet72|FC00::49/126": {},
GCU will never be able to remove that IP because IPv6 was always translated to lowercase due to #2299 . It reported the error can't remove a non-existent object, thus making GCU fail.

#2299  is to deal with this issue: sonic-net/sonic-buildimage#11622.

Although config CLI always translates uppercase to lowercase when adding an IP, the user can have two choices to remove that IP: One is to use config CLI to remove that IP no matter uppercase or lowercase. Another way is to use GCU. In order to use GCU, the user has to check the IP format saved in ConfigDB because GCU operation does differentiate between uppercase and lowercase.
#### What I did
Revert #2299
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@isabelmsft isabelmsft isabelmsft approved these changes

@liat-grozovik liat-grozovik liat-grozovik approved these changes

Assignees
No one assigned
Labels
Included in 202205 Branch Request for 202205 Branch
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Generic Config Updater][IPv6] | Generic Config Updater is case sensitive when dealing with IPv6 addresses
8 participants
@dbarashinvd @mssonicbld @qiluo-msft @dprital @wen587 @liat-grozovik @isabelmsft @yxieca