[ansible] Adding support for multi vlans by tahmed-dev · Pull Request #1353 · sonic-net/sonic-mgmt

tahmed-dev · 2020-01-23T01:21:08Z

Adding Vlan configuration section to existing topology configuration files. The vlan configuration has default default_vlan_cfg attribute which defines which vlan cfg to use. The behavior can be overridden by supplying vlan_cfg flag via cli.

one_vlan_a configuration will create single vlan with the name Vlan1000. This will not break existing test cases. Once existing test cases are able to handle different vlans, this behavior should be removed.

How to verify:

Minigraph generation for a single Vlan; Vlan1000:
ansible-playbook config_sonic_basedon_testbed.yml -i str -l -e testbed_name= --vault-password-file= -e testbed_file= -e local_minigraph=true
or
ansible-playbook config_sonic_basedon_testbed.yml -i str -l -e testbed_name= --vault-password-file= -e testbed_file= -e local_minigraph=true -e vlan_config=one_vlan_a
Minigraph based on config file:
ansible-playbook config_sonic_basedon_testbed.yml -i str -l -e testbed_name= --vault-password-file= -e testbed_file= -e local_minigraph=true -e vlan_config=two_vlan_a
The above configuration file provide example of setting two vlans on T0 DUT.

signed-off-by: Tamer Ahmed [email protected]

lguohan · 2020-01-23T08:34:06Z

can you describe the step to run the test? It is not clear here.

tahmed-dev · 2020-01-23T17:40:14Z

can you describe the step to run the test? It is not clear here.

added paragraph on verification and how to generate minigraphs. I will attach sample files.

tahmed-dev · 2020-01-23T18:10:11Z

Here is sample of the output:

minigraphs.zip

ansible/library/droplet_vlan_cfg.py

ansible/config_sonic_basedon_testbed.yml

ansible/library/droplet_vlan_cfg.py

pavel-shirshov

I'd better to put vlan configuration into topologies.

ansible/library/droplet_vlan_cfg.py

tahmed-dev · 2020-01-23T22:34:49Z

I'd better to put vlan configuration into topologies.

I view the vlan to be orthogonal to the underlying topology. If someone would like to have 2, 3, or 4 vlans with T0, this is down by adding 4 configuration file. These configuration could be combined with different T0 topos.

pavel-shirshov · 2020-01-23T22:44:07Z

@tahmed-dev ok. In this case you need to introduce vlan configurations for all possible port configurations: 32 ports, 64 ports, 116 ports and so on.

tahmed-dev · 2020-01-23T22:52:45Z

@tahmed-dev ok. In this case you need to introduce vlan configurations for all possible port configurations: 32 ports, 64 ports, 116 ports and so on.

I get your point here. I was attempting of having !!python expression in the yaml file, however I dropped it for another iteration. For testing, this could be modeled by some relation such as every 10 interface in a vlan or every nth interface in a vlan. Or we could use j2 to template the dividing interfaces across vlans.

The other point I will debate is it is simpler to split the two and the change is limited this way.

pavel-shirshov · 2020-01-27T20:03:49Z

@tahmed-dev In this case we must put this logic to all tests which are using vlans. The tests need to know how each physical port is used by vlans

ansible/0001-ansible-Adding-support-for-droplet-vlans.patch

signed-off-by: Tamer Ahmed <[email protected]>

Fix subnet for new vlan200

pavel-shirshov

as comments

pavel-shirshov · 2020-01-28T21:31:46Z

ansible/vars/dut_droplet_vlans-1.yml

+   id: 100
+   intfs: [1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12]
+   tag: 100
+   subnets: 192.168.100.0/21


Address should be '.1'
<Prefix>192.168.0.1/21</Prefix>

my bad. good catch. will fix it. Now, I think subnet name is confusing too!

pavel-shirshov · 2020-01-28T21:32:04Z

ansible/vars/dut_droplet_vlans-1.yml

+   id: 200
+   intfs: [13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24]
+   tag: 200
+   subnets: 192.168.200.0/21


Address should be .1

pavel-shirshov · 2020-01-28T21:32:40Z

ansible/vars/dut_droplet_vlans-1.yml

+   id: 100
+   intfs: [1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12]
+   tag: 100
+   subnets: 192.168.100.0/21


Also I'd suggest to rename subnets to prefix

I don't see it is done

this file is part of Option A. will take it out

ansible/vars/dut_droplet_vlans-1.yml

ansible/library/droplet_vlan_cfg.py

ansible/vars/topo_t0.yml

ansible/library/droplet_vlan_config.py

pavel-shirshov

looks good to me

…port [ansible] Adding support for droplet vlans Adding Vlan configuration section to existing topology configuration files. The vlan configuration has default default_vlan_cfg attribute which defines which vlan cfg to use. The behavior can be overridden by supplying vlan_cfg flag via cli. one_vlan_a configuration will create single vlan with the name Vlan1000. This will not break existing test cases. Once existing test cases are able to handle different vlans, this behavior should be removed. How to verify: 1. Minigraph generation for a single Vlan; Vlan1000: ansible-playbook config_sonic_basedon_testbed.yml -i str -l -e testbed_name= --vault-password-file= -e testbed_file= -e local_minigraph=true or ansible-playbook config_sonic_basedon_testbed.yml -i str -l -e testbed_name= --vault-password-file= -e testbed_file= -e local_minigraph=true -e vlan_config=one_vlan_a 2. Minigraph based on config file: ansible-playbook config_sonic_basedon_testbed.yml -i str -l -e testbed_name= --vault-password-file= -e testbed_file= -e local_minigraph=true -e vlan_config=two_vlan_a signed-off-by: Tamer Ahmed <[email protected]>

[ansible] Adding support for droplet vlans Adding Vlan configuration section to existing topology configuration files. The vlan configuration has default default_vlan_cfg attribute which defines which vlan cfg to use. The behavior can be overridden by supplying vlan_cfg flag via cli. one_vlan_a configuration will create single vlan with the name Vlan1000. This will not break existing test cases. Once existing test cases are able to handle different vlans, this behavior should be removed. How to verify: 1. Minigraph generation for a single Vlan; Vlan1000: ansible-playbook config_sonic_basedon_testbed.yml -i str -l -e testbed_name= --vault-password-file= -e testbed_file= -e local_minigraph=true or ansible-playbook config_sonic_basedon_testbed.yml -i str -l -e testbed_name= --vault-password-file= -e testbed_file= -e local_minigraph=true -e vlan_config=one_vlan_a 2. Minigraph based on config file: ansible-playbook config_sonic_basedon_testbed.yml -i str -l -e testbed_name= --vault-password-file= -e testbed_file= -e local_minigraph=true -e vlan_config=two_vlan_a signed-off-by: Tamer Ahmed <[email protected]>

* 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]>

tahmed-dev requested review from jleveque, lguohan, neethajohn and pavel-shirshov January 23, 2020 01:21

jleveque suggested changes Jan 23, 2020

View reviewed changes

pavel-shirshov reviewed Jan 23, 2020

View reviewed changes

jleveque suggested changes Jan 23, 2020

View reviewed changes

ansible/library/droplet_vlan_cfg.py Outdated Show resolved Hide resolved

tahmed-dev requested a review from jleveque January 27, 2020 17:26

jleveque approved these changes Jan 27, 2020

View reviewed changes

jleveque reviewed Jan 28, 2020

View reviewed changes

ansible/0001-ansible-Adding-support-for-droplet-vlans.patch Outdated Show resolved Hide resolved

tahmed-dev added 5 commits January 28, 2020 10:05

[ansible] Adding support for droplet vlans

ab78b30

signed-off-by: Tamer Ahmed <[email protected]>

Fix bug with IPInterface for new vlans.

7a9fdae

Fix subnet for new vlan200

Addressing code comments

09b9398

PR comments

78ab5d2

Option 2: making vlan part of topo files

48f23a8

tahmed-dev force-pushed the taahme/add-vlan-support branch from f63fcf5 to 48f23a8 Compare January 28, 2020 18:06

tahmed-dev requested review from jleveque and pavel-shirshov January 28, 2020 20:39

pavel-shirshov suggested changes Jan 28, 2020

View reviewed changes

pavel-shirshov reviewed Jan 28, 2020

View reviewed changes

ansible/vars/topo_t0.yml Show resolved Hide resolved

PR comments

6574f1e

pavel-shirshov reviewed Jan 29, 2020

View reviewed changes

ansible/library/droplet_vlan_config.py Outdated Show resolved Hide resolved

PR Comments

50dd4aa

Removing separate vlan file

16687cb

pavel-shirshov approved these changes Jan 29, 2020

View reviewed changes

lguohan approved these changes Jan 29, 2020

View reviewed changes

tahmed-dev changed the title ~~[ansible] Adding support for droplet vlans~~ [ansible] Adding support for multi vlans Jan 29, 2020

Offline comments

8dfaae9

jleveque approved these changes Jan 29, 2020

View reviewed changes

tahmed-dev merged this pull request into sonic-net:master Jan 29, 2020

stephenxs mentioned this pull request Feb 11, 2020

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

Merged

1 task

Conversation

tahmed-dev commented Jan 23, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lguohan commented Jan 23, 2020

Uh oh!

tahmed-dev commented Jan 23, 2020

Uh oh!

tahmed-dev commented Jan 23, 2020

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

pavel-shirshov left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

tahmed-dev commented Jan 23, 2020

Uh oh!

pavel-shirshov commented Jan 23, 2020

Uh oh!

tahmed-dev commented Jan 23, 2020

Uh oh!

pavel-shirshov commented Jan 27, 2020

Uh oh!

Uh oh!

pavel-shirshov left a comment

Choose a reason for hiding this comment

Uh oh!

pavel-shirshov Jan 28, 2020

Choose a reason for hiding this comment

Uh oh!

tahmed-dev Jan 28, 2020 • 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!

pavel-shirshov Jan 28, 2020

Choose a reason for hiding this comment

Uh oh!

tahmed-dev Jan 28, 2020

Choose a reason for hiding this comment

Uh oh!

pavel-shirshov Jan 28, 2020

Choose a reason for hiding this comment

Uh oh!

tahmed-dev Jan 29, 2020

Choose a reason for hiding this comment

Uh oh!

pavel-shirshov Jan 29, 2020

Choose a reason for hiding this comment

Uh oh!

tahmed-dev Jan 29, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

pavel-shirshov left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

tahmed-dev commented Jan 23, 2020 •

edited

Loading

tahmed-dev Jan 28, 2020 •

edited

Loading