Skip to content

[multi vlan] Fix Ansible condition when evaluating vlan config for T0 variants#1377

Merged
tahmed-dev merged 3 commits intosonic-net:masterfrom
stephenxs:fix-vlan-parameter
Feb 14, 2020
Merged

[multi vlan] Fix Ansible condition when evaluating vlan config for T0 variants#1377
tahmed-dev merged 3 commits intosonic-net:masterfrom
stephenxs:fix-vlan-parameter

Conversation

@stephenxs
Copy link
Contributor

Description of PR

Summary:
Fixes # (issue)
Only compute vlan parameters for t0 series topo (t0, t0-xx).
PR [ansible] Adding support for multi vlans #1353 introduces some vlan features but only supported on t0 series topo.
However, it tries to call this module on other topos like t1, which causes the following issue:

fatal: [mtbc-sonic-03-2700 -> localhost]: FAILED! => {
"changed": false, 
"msg": "Traceback (most recent call last):\n
File \"/tmp/ansible_vlan_config_payload_h5ngrh/__main__.py\", line 46, in main\n 
vlan_config = m_args['vm_topo_config']['DUT']['vlan_configs']['default_vlan_config']\n
KeyError: 'vlan_configs'\n"
}

This PR add logic of checking the topology and calling the module on t0 series only.

Type of change

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

Approach

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

@liat-grozovik
Copy link
Collaborator

@tahmed-dev please check the PR as it fixes an issue observed following some changes provided by you.
To me the fix seems to be correct but the problem is that i am not sure if this was missed or by intention and the fix need to be different.
NOTE: who ever try to run non t0 topo is currently blocked as of that.

@stephenxs stephenxs marked this pull request as ready for review February 11, 2020 07:30
@lguohan lguohan requested a review from tahmed-dev February 11, 2020 15:45
@tahmed-dev
Copy link
Contributor

tahmed-dev commented Feb 11, 2020

@tahmed-dev please check the PR as it fixes an issue observed following some changes provided by you.
To me the fix seems to be correct but the problem is that i am not sure if this was missed or by intention and the fix need to be different.
NOTE: who ever try to run non t0 topo is currently blocked as of that.

Thanks @liat-grozovik Every t0 variant device is a ToR device. Current practice is to identify the device type by checking the dut_type from the topology file. this could be seen in:

https://github.com/Azure/sonic-mgmt/blob/master/ansible/vars/topo_t0.yml#L77 and
https://github.com/Azure/sonic-mgmt/blob/master/ansible/vars/topo_t1.yml#L135

Which topo was used to produce this issue? Is it in public repo?

Copy link
Contributor

@tahmed-dev tahmed-dev left a comment

Choose a reason for hiding this comment

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

Every t0 variant is a ToR device and so the check for 'tor' in vm_topo_config['dut_type'] suffices.

Which topo file was used to generate this error? is it missing dut_type field?

remove redundant condition checking
@stephenxs
Copy link
Contributor Author

Hi @tahmed-dev,
I saw the error in t1 and t1-lag. I suspect it can be reproduced in any non-t0 topo.

@stephenxs
Copy link
Contributor Author

This previous format works too, the issue was with the extra double quotation

when: ('host_interfaces' in vm_topo_config) and ('tor' in vm_topo_config['dut_type'] | lower)

Please fix this one to get going and I will take care of the rest.

Understood.
I will do it in this way.

@stephenxs stephenxs requested a review from tahmed-dev February 14, 2020 03:14
@tahmed-dev tahmed-dev changed the title Only compute vlan parameters for t0 series topo when generate minigraph [multi vlan] Fix Ansible condition when evaluating vlan config for T0 variants Feb 14, 2020
@tahmed-dev tahmed-dev merged commit 15f514f into sonic-net:master Feb 14, 2020
@stephenxs stephenxs deleted the fix-vlan-parameter branch February 15, 2020 00:28
@stephenxs stephenxs restored the fix-vlan-parameter branch July 9, 2021 16:30
@stephenxs stephenxs deleted the fix-vlan-parameter branch July 23, 2021 09:48
kazinator-arista pushed a commit to kazinator-arista/sonic-mgmt that referenced this pull request Mar 4, 2026
* c2fb282 2021-01-29 | [ecnconfig] Allow ecn unit test to run without sudo (sonic-net#1390) [Neetha John]
* 6cc635b 2021-01-29 | [sonic-installer] Add information to syslog (sonic-net#1369) [Dmytro]
* 7a8024a 2021-01-27 | Prevent user from adding more then a single untagged VLAN to an interface (sonic-net#1382) [Eran Dahan]
* 41e62c6 2021-01-26 | [pcieutil] Add 'pcie-aer' sub-command to display AER stats (sonic-net#1169) [Arun Saravanan Balachandran]
* 47f412b 2021-01-26 | Improve robustness of consutil plugin loading (sonic-net#1353) [Samuel Angebault]
* 64aa1b8 2021-01-25 | [show] Fix warnings, related to gearbox, while show commands execution (sonic-net#1343) [maksymbelei95]
* ff226d0 2021-01-25 | Prevent configuring IP interface on a port which is a member of VLAN (sonic-net#1374) [Eran Dahan]
* f1522b9 2021-01-21 | [config_mgmt.py]: Set leaf-list to empty list while port breakout. (sonic-net#1268) [Praveen Chaudhary]
* 99c05d5 2021-01-21 | add vlan_intf_object only if there are ipv4 or ipv6 mappings (sonic-net#1377) [Sumukha Tumkur Vani]
* b082684 2021-01-21 | [ecn] Add tests for ecnconfig command (sonic-net#1372) [Neetha John]
* 23e0920 2021-01-21 | [sfpshow] Enhance QSFP-DD DOM information (sonic-net#1207) [shlomibitton]
* f4edba1 2021-01-20 | [ecnconfig] handle backend port names when extracting port I/F ID from the port name (sonic-net#1361) [Mahesh Maddikayala]

Signed-off-by: Danny Allen <[email protected]>
kazinator-arista pushed a commit to kazinator-arista/sonic-mgmt that referenced this pull request Mar 4, 2026
d324eaec945081f8718468b39a8cf12dae965fd5 (HEAD -> 201911, origin/201911) [PFCWD] Fix 'start' pfcwd command (sonic-net#1345)
235c61cccbbbb1f948f53b561c98888681b7071a [ecnconfig] handle backend port names when extracting port I/F ID from the port name (sonic-net#1361)
7f5c3b497148fdd8e710131c5ac3f9f0a5f2cddf Drop explict 3 seconds pause between two object updates/deletes. (sonic-net#1359)
12c899207917751eac719916be69c0078671963d add vlan_intf_object only if there are ipv4 or ipv6 mappings (sonic-net#1377)
52ce2c32bf4e267d043a739641f5eefba3f3910f Add  subcommand description to interfaces counters (sonic-net#1373)
Signed-off-by: Abhishek Dosi <[email protected]>
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.

3 participants