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

[ VRF ] Returned t0 configuration for run vrf cases #2580

Merged
merged 3 commits into from
Nov 30, 2020

Conversation

SavchukRomanLv
Copy link
Contributor

Signed-off-by: Roman Savchuk romanx.savchuk@intel.com

Description of PR

As described in #2538, when run vrf test suite "AnsibleUndefinedVariable: 'dict object' has no attribute 'podset_number'" appears. The reason is that these attributes were deleted from topo_t0.yml by commit #1233

Summary:
Return back needed configuration for VRF test cases. Fixes #2468

Type of change

  • [+ ] Bug fix
  • Testbed and Framework(new/improvement)
  • Test case(new/improvement)

Approach

What is the motivation for this PR?

Want to have ability to run full VRF test suite

How did you do it?

Returned required configuration of t0 for VRF TC.

How did you verify/test it?

Run cases which requires t0 changes:
vrf/test_vrf.py::TestVrfIsolation::test_neigh_isolate_vrf1_from_vrf2 PASSED vrf/test_vrf.py::TestVrfIsolation::test_neigh_isolate_vrf2_from_vrf1 PASSED vrf/test_vrf.py::TestVrfIsolation::test_fib_isolate_vrf1_from_vrf2 PASSED vrf/test_vrf.py::TestVrfIsolation::test_fib_isolate_vrf2_from_vrf1 PASSED

Any platform specific information?

SONiC Software Version: SONiC.master.35-dirty-20201112.031542
Distribution: Debian 10.6
Kernel: 4.19.0-9-2-amd64
Build commit: 6c362a08
Build date: Thu Nov 12 11:49:55 UTC 2020
Platform: x86_64-accton_wedge100bf_32x-r0
HwSKU: montara
ASIC: barefoot

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

Documentation

Signed-off-by: Roman Savchuk <romanx.savchuk@intel.com>
@SavchukRomanLv
Copy link
Contributor Author

retest vsimage please

@NazarTkachuk
Copy link
Contributor

Pls review @wangxin @yxieca

@wangxin wangxin requested a review from lguohan November 26, 2020 03:01
@wangxin
Copy link
Collaborator

wangxin commented Nov 26, 2020

Currently these variables are defined in the fib plugin for announcing routes. If define them again in topo file, there are two sources of these information. However, I feel that topo file is a better place for these definitions. The fib plugin can try to get values from topo file. If no value is in topo file, then the fib plugin can fallback to use default value. So, more changes are needed to make it complete.

  • Update the fib plugin
  • Add the definitions to other topo files as well.

Can you make those changes as well?

Added missed params to all topo files

Signed-off-by: Roman Savchuk <romanx.savchuk@intel.com>
@SavchukRomanLv
Copy link
Contributor Author

SavchukRomanLv commented Nov 26, 2020

Currently these variables are defined in the fib plugin for announcing routes. If define them again in topo file, there are two sources of these information. However, I feel that topo file is a better place for these definitions. The fib plugin can try to get values from topo file. If no value is in topo file, then the fib plugin can fallback to use default value. So, more changes are needed to make it complete.

  • Update the fib plugin
  • Add the definitions to other topo files as well.

Can you make those changes as well?

@wangxin make changes in latest commit.
Test it on two topo:
announced 25613 routes on t0, fib TC passed
announced 82877 routes on t1, arp TC passed

@lgtm-com
Copy link

lgtm-com bot commented Nov 26, 2020

This pull request introduces 4 alerts when merging 511e16f into 9df3f25 - view on LGTM.com

new alerts:

  • 4 for Wrong number of arguments in a call

Added missed topo config for t1-lag.yml

Signed-off-by: Roman Savchuk <romanx.savchuk@intel.com>
@wangxin wangxin merged commit d8ad002 into sonic-net:master Nov 30, 2020
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.

[Vrf] test_vrf fails due to missing attributes in topo_t0.yml
3 participants