Skip to content

GCU vlan interface test#4834

Merged
wen587 merged 4 commits intosonic-net:masterfrom
wen587:gu_vlan
Dec 24, 2021
Merged

GCU vlan interface test#4834
wen587 merged 4 commits intosonic-net:masterfrom
wen587:gu_vlan

Conversation

@wen587
Copy link
Contributor

@wen587 wen587 commented Dec 10, 2021

Description of PR

Summary: Testcase of vlan interface 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 vlan interface

How did you do it?

Make some config apply change based on predefined t0 topo. And check if the vlaninterface is changed as expected.

How did you verify/test it?

Run test of sonic-mgmt/tests/generic_config_updater/test_vlan_interface.py on KVM

Any platform specific information?

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

Documentation

@wen587 wen587 requested a review from qiluo-msft December 10, 2021 03:43
@wen587
Copy link
Contributor Author

wen587 commented Dec 10, 2021

Incremental change of MTU and admin_status is not supported. Do we need to remove that in sonic-vlan.yang?

@wen587 wen587 marked this pull request as ready for review December 22, 2021 08:08
@wen587 wen587 requested a review from a team as a code owner December 22, 2021 08:08
@qiluo-msft
Copy link
Contributor

qiluo-msft commented Dec 23, 2021

Summary: Testcase of loopback interface for generic updater apply-patch

But the PR title says vlan interface


In reply to: 1000107821

fe80::5054:ff:feda:c6af%Vlan1000/64 N/A N/A
"""
address_family = "ip" if is_ipv4 else "ipv6"
output = duthost.shell("show {} interfaces | grep {} || true".format(address_family, intf_name))
Copy link
Contributor

@qiluo-msft qiluo-msft Dec 23, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

true

Any chance the command will fail if you don't add true ?

If no, remove it and also need to check return code. #Closed

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If grep no output, it will return a failure with error code 1. If there is a scenario we grep a non-existed variable, we can check if output is empty instead of fail.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Catching return code 1 is better to ignore any return code. The whole command could fail with other reasons such as some container stopped (so failed before first pipe), and they will lead to different return code.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please see here #4835 (comment)
Seems we can only capture the error of the last pipe command.

The whole command could fail with other reasons such as some container stopped (so failed before first pipe), and they will lead to different return code.

Even though the command fail before first pipe, the return code is only related to the last pipe.

@wen587 wen587 merged commit 0a87f88 into sonic-net:master Dec 24, 2021
AntonHryshchuk pushed a commit to AntonHryshchuk/sonic-mgmt that referenced this pull request Jan 4, 2022
…ric config updater (sonic-net#4834)

Summary:
Testcase of VLAN interface for generic updater apply-patch.
Move some functions to gu_utils.

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 VLAN interface

How did you do it?
Make some config apply change based on predefined t0 topo. And check if the VLAN interface is changed as expected.

How did you verify/test it?
Run test of sonic-mgmt/tests/generic_config_updater/test_vlan_interface.py on KVM
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants