draft for dhcp server testcase by wen587 · Pull Request #4485 · sonic-net/sonic-mgmt

wen587 · 2021-10-14T05:26:44Z

Description of PR

Summary: Testcase of dhcp server for generic updater apply-patch
Fixes # (issue)

Type of change

Bug fix
Testbed and Framework(new/improvement)
Test case(new/improvement)

Back port request

201911

Approach

What is the motivation for this PR?

End to End test support for Generic Updater apply-patch
This PR is to verify the usage of 'config apply-patch' works on dhcp server under vlan.

How did you do it?

First we setup vlan for test. Then make some config apply change. And check if the dhcp server is changed as expected.
For code reusability, some vlan setup functions are moved to tests/common/fixtures/duthost_utils.py
Implement a utility file gu_utils.py for future other testcase.

How did you verify/test it?

Run test on sonic-mgmt/tests/generic_config_updater/test_dhcp_relay.py

Any platform specific information?

Supported testbed topology if it's a new test case?

Documentation

tests/generic_config_updater/test_dhcp_relay.py

qiluo-msft · 2021-10-20T08:18:21Z

tests/generic_config_updater/test_dhcp_relay.py

Generate 4 dhcp server for each vlan

Thanks for providing illustrative comments! As we discussed, we need to document of sub-test-case design.

For the init configuration

4 servers as your implementation

0 servers

too many servers that will reach maximal soon. #Closed

Also design for DHCPv6? It is a relative new feature, but we can still design for it.

tests/generic_config_updater/test_dhcp_relay.py

qiluo-msft · 2021-10-20T08:26:47Z

tests/generic_config_updater/test_dhcp_relay.py

add

"""Test add and del dhcp servers cases

There are some corner cases for "add":

reach maximal servers

add an existing one
#Pending

How about "1. reach maximal servers" ? Is there an maximal number by design or by implementation?

Will ask see if there is the design

Low priority test case.

tests/generic_config_updater/test_dhcp_relay.py

lgtm-com · 2021-10-28T02:37:28Z

This pull request introduces 1 alert when merging afafa900a7a868f79351e46ddfae78e8e3f57012 into 66c554d - view on LGTM.com

new alerts:

1 for Unused import

lgtm-com · 2021-10-29T03:07:45Z

This pull request introduces 1 alert when merging d2573c2af20287f83e2c823465d2e00888ba1cd8 into 058a790 - view on LGTM.com

new alerts:

1 for Unused import

tests/vlan/test_vlan.py

tests/common/fixtures/duthost_utils.py

tests/generic_config_updater/dhcp_utils.py

tests/generic_config_updater/test_dhcp_relay.py

tests/generic_config_updater/dhcp_utils.py

tests/generic_config_updater/test_dhcp_relay.py

tests/generic_config_updater/dhcp_utils.py

tests/generic_config_updater/test_dhcp_relay.py

lgtm-com · 2021-11-01T12:58:31Z

This pull request introduces 1 alert when merging 4faac70e9f33b576182c395291ee3a4cdb54ad17 into 7f777f4 - view on LGTM.com

new alerts:

1 for Unused import

lgtm-com · 2021-11-02T04:05:16Z

This pull request introduces 1 alert when merging 6c4554c3eef2f8f20f0ccfb406c37a252fbb8c09 into 106b4bb - view on LGTM.com

new alerts:

1 for Unused import

tests/generic_config_updater/gu_utils.py

lgtm-com · 2021-11-11T04:11:18Z

This pull request introduces 1 alert when merging 323eeacd9caef436785a3e27c6a6ed590245a257 into 8d65f7c - view on LGTM.com

new alerts:

1 for Unused import

qiluo-msft · 2021-11-11T23:12:01Z

tests/common/fixtures/duthost_utils.py

utils_vlan_intfs_dict_orig

It's not easy to understand the function just from the name. They are implemented to be reused. Could you add some function level comments, explaining the purpose, parameter, and return values. #Closed

Also applicable to other new functions in this file.

qiluo-msft · 2021-11-11T23:13:22Z

tests/generic_config_updater/gu_utils.py

output = duthost.shell(cmds)

# output = duthost.shell(cmds)

Remove commented code. #Closed

qiluo-msft · 2021-11-11T23:20:13Z

tests/generic_config_updater/gu_utils.py

duthost.shell

duthost.shell("sudo systemctl reset-failed {}.service".format(container_name))

Always check return code, in reality, we see systemctl fails. #Closed

qiluo-msft · 2021-11-11T23:22:53Z

tests/generic_config_updater/test_dhcp_relay.py

first_avai_vlan_port

avai -> available?

qiluo-msft · 2021-11-11T23:29:00Z

tests/generic_config_updater/test_dhcp_relay.py

vlan_intfs_list[1]

Is it possible that the value is too large and the IP is not valid? Check before use it this way? #Closed

No. It is restricted by utils_vlan_intfs_dict_add which in range(0, 255)

qiluo-msft · 2021-11-11T23:30:37Z

tests/generic_config_updater/test_dhcp_relay.py

3

also a magic number #Closed

Here I use magic number due that if the index change, the json will look much different. Here I choose to remove the last element for simplicity but also check the correctness. If I choose to remove other index such as 1. The json_patch will look like this:

[ { "op": "remove", "path": "/VLAN/Vlan108/dhcp_servers/3" }, { "op": "replace", "path": "/VLAN/Vlan108/dhcp_servers/2", "value": "192.0.108.4" }, { "op": "replace", "path": "/VLAN/Vlan108/dhcp_servers/1", "value": "192.0.108.3" } ]

The vlan dhcp server is not simple key->value pair. It looks like a key->indexed value

2. Reuse code from vlan/test_vlan.py 3. resolve comment

…lelist

…config updater(sonic-net#4485) Summary: Testcase of dhcp server under vlan for generic updater apply-patch Approach: What is the motivation for this PR? This is a requirement from E2E test support for Generic Updater apply-patch. This PR is to verify the usage of 'config apply-patch' works on dhcp server under vlan. How did you do it? First we setup vlan for test. Then make some config apply change. And check if the dhcp server is changed as expected. How did you verify/test it? Run test on sonic-mgmt/tests/generic_config_updater/test_dhcp_relay.py

wen587 requested review from isabelmsft and qiluo-msft October 14, 2021 07:52

qiluo-msft reviewed Oct 20, 2021

View reviewed changes

tests/generic_config_updater/test_dhcp_relay.py Outdated Show resolved Hide resolved

qiluo-msft reviewed Oct 20, 2021

View reviewed changes

tests/generic_config_updater/test_dhcp_relay.py Outdated Show resolved Hide resolved

qiluo-msft reviewed Oct 20, 2021

View reviewed changes

tests/generic_config_updater/test_dhcp_relay.py Outdated Show resolved Hide resolved

qiluo-msft reviewed Oct 20, 2021

View reviewed changes

tests/generic_config_updater/test_dhcp_relay.py Outdated Show resolved Hide resolved

qiluo-msft reviewed Oct 20, 2021

View reviewed changes

tests/generic_config_updater/test_dhcp_relay.py Outdated Show resolved Hide resolved

qiluo-msft reviewed Oct 20, 2021

View reviewed changes

tests/generic_config_updater/test_dhcp_relay.py Outdated Show resolved Hide resolved

wen587 force-pushed the gu_dhcp branch from f58bb48 to afafa90 Compare October 28, 2021 02:05

wen587 requested a review from ganglyu October 29, 2021 02:40

qiluo-msft reviewed Oct 29, 2021

View reviewed changes

tests/vlan/test_vlan.py Outdated Show resolved Hide resolved

qiluo-msft reviewed Oct 29, 2021

View reviewed changes

tests/common/fixtures/duthost_utils.py Outdated Show resolved Hide resolved

qiluo-msft reviewed Oct 29, 2021

View reviewed changes

tests/generic_config_updater/dhcp_utils.py Outdated Show resolved Hide resolved

qiluo-msft reviewed Oct 29, 2021

View reviewed changes

tests/generic_config_updater/dhcp_utils.py Outdated Show resolved Hide resolved

qiluo-msft reviewed Oct 29, 2021

View reviewed changes

tests/generic_config_updater/test_dhcp_relay.py Outdated Show resolved Hide resolved

ganglyu reviewed Oct 29, 2021

View reviewed changes

qiluo-msft reviewed Oct 29, 2021

View reviewed changes

tests/generic_config_updater/test_dhcp_relay.py Outdated Show resolved Hide resolved

qiluo-msft requested review from ghooo and renukamanavalan October 29, 2021 08:27

qiluo-msft reviewed Oct 29, 2021

View reviewed changes

tests/generic_config_updater/dhcp_utils.py Outdated Show resolved Hide resolved

qiluo-msft reviewed Oct 29, 2021

View reviewed changes

tests/generic_config_updater/test_dhcp_relay.py Outdated Show resolved Hide resolved

wen587 force-pushed the gu_dhcp branch from 4faac70 to 6c4554c Compare November 2, 2021 03:35

qiluo-msft reviewed Nov 9, 2021

View reviewed changes

tests/generic_config_updater/gu_utils.py Outdated Show resolved Hide resolved

qiluo-msft mentioned this pull request Nov 10, 2021

Add incremental QOS config tests #4580

Merged

4 tasks

wen587 force-pushed the gu_dhcp branch from e1c7237 to 323eeac Compare November 11, 2021 03:40

qiluo-msft reviewed Nov 11, 2021

View reviewed changes

wen587 added 10 commits November 12, 2021 02:43

draft for dhcp server testcase

7b979ae

1. Add clean_setup and default_setup for testing

80c551f

2. Reuse code from vlan/test_vlan.py 3. resolve comment

change naming in arp test

aa90d91

1. make utils general 2. polling instead of sleep 3.mktemp instead fi…

1015fc0

…lelist

rebase and resolve build issue

c08fcd5

1. resolve comment; 2. add start-limit-hit handle

5e71d5c

resolve lgtm issue

e71312b

check return code before check output stdout

b88ac51

fix minor issue

029cbb1

1. check return code; 2. add comment for util function

8181b43

wen587 force-pushed the gu_dhcp branch from 323c97d to 8181b43 Compare November 12, 2021 07:24

qiluo-msft approved these changes Nov 12, 2021

View reviewed changes

wen587 marked this pull request as ready for review November 12, 2021 23:54

wen587 requested a review from a team as a code owner November 12, 2021 23:54

wen587 merged commit 0bcb76e into sonic-net:master Nov 13, 2021

ghooo mentioned this pull request Jun 26, 2022

SONiC Generic Update and Rollback - HLD sonic-net/SONiC#736

Merged

Conversation

wen587 commented Oct 14, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description of PR

Type of change

Back port request

Approach

What is the motivation for this PR?

How did you do it?

How did you verify/test it?

Any platform specific information?

Supported testbed topology if it's a new test case?

Documentation

Uh oh!

Uh oh!

qiluo-msft Oct 20, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

qiluo-msft Oct 20, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

"""Test add and del dhcp servers cases

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

lgtm-com bot commented Oct 28, 2021

Uh oh!

lgtm-com bot commented Oct 29, 2021

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

lgtm-com bot commented Nov 1, 2021

Uh oh!

lgtm-com bot commented Nov 2, 2021

Uh oh!

Uh oh!

lgtm-com bot commented Nov 11, 2021

Uh oh!

qiluo-msft Nov 11, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

qiluo-msft Nov 11, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

output = duthost.shell(cmds)

# output = duthost.shell(cmds)

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wen587 commented Oct 14, 2021 •

edited

Loading

qiluo-msft Oct 20, 2021 •

edited

Loading

qiluo-msft Oct 20, 2021 •

edited

Loading

qiluo-msft Nov 11, 2021 •

edited

Loading

qiluo-msft Nov 11, 2021 •

edited

Loading

qiluo-msft Nov 11, 2021 •

edited

Loading

qiluo-msft Nov 11, 2021 •

edited

Loading

qiluo-msft Nov 11, 2021 •

edited

Loading