Fix for Switch Port Modes and VLAN CLI Enhancement #3108

sabakram · 2024-01-01T07:13:07Z

What I did

This PR is Fixture for PR#2419. The original code PR was reverted due to nonavailability of YANG Model. The code provided by this PR is exactly same as reviewed & approved by reveiwers.
To Fix issues we break the code in three PR as per suggestions:
1. yang models without must condition
2. sonic-config-engine
3. sonic-utilities-fix

How I did it

Code logic added in original PR is added again with new PR to resolve issue

How to verify it

New commands have been added in Command-Reference.md All the syntax and examples have been added there and they can be verified by running the specific command

ridahanif96 · 2024-01-10T16:59:22Z

@qiluo-msft can you please help review this PR!

qiluo-msft · 2024-01-10T18:42:25Z

Could you provide which PR fixed "nonavailability of YANG Model"?

ridahanif96 · 2024-01-11T04:00:57Z

Could you provide which PR fixed "nonavailability of YANG Model"?

Please see YANG PR

ridahanif96 · 2024-01-12T09:37:05Z

@gechiang & @theasianpianist Can you please help review this PR. Thanks!

ridahanif96 · 2024-01-16T15:43:30Z

@gechiang @venkatmahalingam @theasianpianist can you all pls help review this PR. Thanks

config/vlan.py

theasianpianist · 2024-02-02T22:10:43Z

config/vlan.py

 @click.argument('port', metavar='<port>', required=True)
+@click.option('-m', '--multiple', is_flag=True, help="Add Multiple Vlan(s) in Range or in Comma separated list")


same as above

Please note that --multiple flag is required as we did not add our new functionality to the existing command to keep intact the existing VLAN behavior/command.

theasianpianist · 2024-02-02T22:11:03Z

config/vlan.py

@@ -74,57 +99,74 @@ def delete_db_entry(entry_name, db_connector, db_name):


 @vlan.command('del')
-@click.argument('vid', metavar='<vid>', required=True, type=int)
+@click.argument('vid', metavar='<vid>', required=True)
+@click.option('-m', '--multiple', is_flag=True, help="Add Multiple Vlan(s) in Range or in Comma separated list")


same as above

Please note mutliple_vlan_parser will handle vlan(single, comma separated, range) on that list. This vid is exisiting behavior of VLANs. We don't modify existing behavior and added our proposed changes

Why do we need the --multiple flag if the parser can already handle all possible inputs?

Please note that --multiple flag is required as we did not add our new functionality to the existing command to keep intact the existing VLAN behavior/command.

config/vlan.py

prsunny · 2024-02-07T01:39:34Z

@theasianpianist , could you check the reply and signoff if looks good?

ridahanif96 · 2024-02-12T16:42:59Z

@theasianpianist can you please check responses and sign off this PR if you agree.
All code changes provided by this PR is same as being reviewed and approved earlier.

ridahanif96 · 2024-02-14T01:10:54Z

@qiluo-msft can you pls help merge this PR, it's approved by reviewers.

ridahanif96 · 2024-02-16T13:00:40Z

@qiluo-msft Can you please help merge this PR! Thanks

prsunny · 2024-02-24T01:58:22Z

@sabakram , could you please confirm if all the previous CLI commands works exactly as expected and no compatibility issue? We have external scripts using these commands and a new image with this change should not break those old commands. Can you confirm?

sabakram · 2024-02-24T20:04:01Z

@sabakram , could you please confirm if all the previous CLI commands works exactly as expected and no compatibility issue? We have external scripts using these commands and a new image with this change should not break those old commands. Can you confirm?

Yes, it is confirmed that all previous CLI commands works as expected and no compatibility issue. We only provide these new commands and switchport modes. The utilities PR was reverted earlier due to issues in rollback. Now since YANG model has been merged. We have tested with latest master and GCU rollbacked successfully. Please see below running behavior of these commands

ridahanif96 · 2024-02-27T02:16:58Z

@qiluo-msft can you pls help merge this PR, thanks.

wen587 · 2024-03-26T08:11:02Z

config/vlan.py

+            elif not is_port:
+                port_data = config_db.get_entry('PORTCHANNEL',port)
+
+            if "mode" not in port_data: 


Hi @sabakram , I suspect this line break backward compatibility.
It will wrongly treat a port as routed when 'mode' is not present even though it pass the check in
Line 297:

if (is_port and clicommon.is_port_router_interface(db.cfgdb, port)) or \ (not is_port and clicommon.is_pc_router_interface(db.cfgdb, port)): # TODO: MISSING CONSTRAINT IN YANG MODEL ctx.fail("{} is in routed mode!\nUse switchport mode command to change port mode".format(port))

Hi @wen587 ,
We are already looking into backward compatibility issues for that we have removed all changes in db_migrator where we remove this default mode routed behavior.

Hi @sabakram , I think not only in db_migrator. But also in vlan config commands.
In sonic-mgmt, the test tests/generic_config_updater/test_dhcp_relay.py fail with new vlan config commands.
The Ethernet4 was treated as routed mode whereas it was not before.
Please check below error message.

"results": [{"cmd": "config vlan add 108", "cmd_with_timeout": "", "err_msg": "", "rc": 0, "stderr": "", "stderr_lines": [], "stdout": "", "stdout_lines": [], "timeout": 0}, {"cmd": "config interface ip add Vlan108 192.168.8.1/24", "cmd_with_timeout": "", "err_msg": "", "rc": 0, "stderr": "", "stderr_lines": [], "stdout": "", "stdout_lines": [], "timeout": 0}, {"cmd": "config vlan add 109", "cmd_with_timeout": "", "err_msg": "", "rc": 0, "stderr": "", "stderr_lines": [], "stdout": "", "stdout_lines": [], "timeout": 0}, {"cmd": "config interface ip add Vlan109 192.168.9.1/24", "cmd_with_timeout": "", "err_msg": "", "rc": 0, "stderr": "", "stderr_lines": [], "stdout": "", "stdout_lines": [], "timeout": 0}, {"cmd": "config vlan member add 108 Ethernet4", "cmd_with_timeout": "", "err_msg": "", "rc": 2, "stderr": "Usage: config vlan member add [OPTIONS] port\nTry "config vlan member add -h" for help.\n\nError: Ethernet4 is in routed mode!\nUse switchport mode command to change port mode\n", "stderr_lines": ["Usage: config vlan member add [OPTIONS] port", "Try "config vlan member add -h" for help.", "", "Error: Ethernet4 is in routed mode!", "Use switchport mode command to change port mode"], "stdout": "", "stdout_lines": [], "timeout": 0}, {"cmd": "config vlan member add 109 Ethernet4", "cmd_with_timeout": "", "err_msg": "", "rc": 2, "stderr": "Usage: config vlan member add [OPTIONS] port\nTry "config vlan member add -h" for help.\n\nError: Ethernet4 is in routed mode!\nUse switchport mode command to change port mode\n", "stderr_lines": ["Usage: config vlan member add [OPTIONS] port", "Try "config vlan member add -h" for help.", "", "Error: Ethernet4 is in routed mode!", "Use switchport mode command to change port mode"]

@wen587 Hi,

Okay we also remove this check from vlan config to avoid this failure in GCU

This reverts commit 92220dc.

…3246) Reverts #3108

…#3108)" (sonic-net#3246) Reverts sonic-net#3108

#### What I did This PR is Fixture for #3108 The PR was reverted due to backward compatibility issues. As per new suggestions, removed db migrator changes from this new change along with vlan.py & switchport.py changes To Fix issues as per suggestions removed default mode from YANG model and removed minigraph changes: #### How I did it 1. Removed Db migrator changes from code. 2. Modified Vlan.py & Switchport.py changes #### How to verify it New commands have been added in Command-Reference.md All the syntax and examples have been added there and they can be verified by running the specific command

This PR is Fixture for sonic-net#3108 The PR was reverted due to backward compatibility issues. As per new suggestions, removed db migrator changes from this new change along with vlan.py & switchport.py changes To Fix issues as per suggestions removed default mode from YANG model and removed minigraph changes: 1. Removed Db migrator changes from code. 2. Modified Vlan.py & Switchport.py changes New commands have been added in Command-Reference.md All the syntax and examples have been added there and they can be verified by running the specific command

sabakram and others added 19 commits January 1, 2024 12:10

Fix for switchport mode PR

e36950c

Fix for cli.py

bdddd97

Fix for indentation

643d6a5

Fix for error

1c523b6

Fix

53afe45

Fixing indentation errors

09c2a6f

Fix for error

e7bc924

Fix for failures

3e869e5

Fix for errors

789cfd4

Fix for port version

7bc67d7

Fix for DB migrator versions

afa1476

Fix for db migrator version function

4529899

Fixing versions

b469f1b

Fix for cli.py

2a7bd65

Fix for vlan_test.py

a87039d

Fix for failures

ad3a193

Fix for unexpected characters

7593c2d

Fixing error message

9a3340d

Fix for routed port

5e9a8f0

sabakram marked this pull request as ready for review January 10, 2024 10:30

sabakram changed the title ~~Fix for switchport mode PR~~ Fix for Switch Port Modes and VLAN CLI Enhancement Jan 10, 2024

qiluo-msft requested review from gechiang, theasianpianist and qiluo-msft January 10, 2024 18:42

zhangyanzhao requested a review from venkatmahalingam January 15, 2024 04:59

theasianpianist reviewed Feb 2, 2024

View reviewed changes

config/vlan.py Show resolved Hide resolved

theasianpianist reviewed Feb 2, 2024

View reviewed changes

config/vlan.py Show resolved Hide resolved

theasianpianist reviewed Feb 2, 2024

View reviewed changes

config/vlan.py Show resolved Hide resolved

theasianpianist reviewed Feb 2, 2024

View reviewed changes

config/vlan.py Show resolved Hide resolved

theasianpianist approved these changes Feb 13, 2024

View reviewed changes

prsunny approved these changes Feb 24, 2024

View reviewed changes

prsunny self-requested a review February 24, 2024 01:56

prsunny approved these changes Feb 26, 2024

View reviewed changes

zhangyanzhao merged commit 92220dc into sonic-net:master Mar 1, 2024
5 checks passed

sabakram mentioned this pull request Mar 26, 2024

Fix for Db migrator & Vlan Changes #3243

Closed

wen587 reviewed Mar 26, 2024

View reviewed changes

adyeung mentioned this pull request Mar 26, 2024

Degradation: Failed to add L2 PortChannel to VLAN sonic-net/sonic-buildimage#18392

Open

wen587 added a commit that referenced this pull request Mar 27, 2024

Revert "Fix for Switch Port Modes and VLAN CLI Enhancement (#3108)"

73ec7d9

This reverts commit 92220dc.

wen587 added a commit that referenced this pull request Mar 28, 2024

Revert "Fix for Switch Port Modes and VLAN CLI Enhancement (#3108)"

01ade78

This reverts commit 92220dc.

wen587 mentioned this pull request Mar 28, 2024

Revert "Fix for Switch Port Modes and VLAN CLI Enhancement" #3246

Merged

qiluo-msft pushed a commit that referenced this pull request Mar 28, 2024

Revert "Fix for Switch Port Modes and VLAN CLI Enhancement (#3108)" (#…

bdc5720

…3246) Reverts #3108

sabakram mentioned this pull request Mar 28, 2024

Switchport Mode & CLI Modified Fix #3247

Merged

arfeigin pushed a commit to arfeigin/sonic-utilities that referenced this pull request Apr 2, 2024

Revert "Fix for Switch Port Modes and VLAN CLI Enhancement (sonic-net…

6471d9b

…#3108)" (sonic-net#3246) Reverts sonic-net#3108

sabakram mentioned this pull request May 7, 2024

Switchport Mode Modifications #3309

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix for Switch Port Modes and VLAN CLI Enhancement #3108

Fix for Switch Port Modes and VLAN CLI Enhancement #3108

sabakram commented Jan 1, 2024 •

edited

Loading

ridahanif96 commented Jan 10, 2024

qiluo-msft commented Jan 10, 2024

ridahanif96 commented Jan 11, 2024

ridahanif96 commented Jan 12, 2024

ridahanif96 commented Jan 16, 2024

theasianpianist Feb 2, 2024

ridahanif96 Feb 7, 2024

theasianpianist Feb 2, 2024

ridahanif96 Feb 3, 2024

theasianpianist Feb 7, 2024

ridahanif96 Feb 7, 2024

prsunny commented Feb 7, 2024

ridahanif96 commented Feb 12, 2024

ridahanif96 commented Feb 14, 2024

ridahanif96 commented Feb 16, 2024

prsunny commented Feb 24, 2024

sabakram commented Feb 24, 2024

ridahanif96 commented Feb 27, 2024

wen587 Mar 26, 2024

sabakram Mar 26, 2024

wen587 Mar 26, 2024

sabakram Mar 26, 2024

		@click.argument('port', metavar='<port>', required=True)
		@click.option('-m', '--multiple', is_flag=True, help="Add Multiple Vlan(s) in Range or in Comma separated list")

Fix for Switch Port Modes and VLAN CLI Enhancement #3108

Fix for Switch Port Modes and VLAN CLI Enhancement #3108

Conversation

sabakram commented Jan 1, 2024 • edited Loading

What I did

How I did it

How to verify it

ridahanif96 commented Jan 10, 2024

qiluo-msft commented Jan 10, 2024

ridahanif96 commented Jan 11, 2024

ridahanif96 commented Jan 12, 2024

ridahanif96 commented Jan 16, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

prsunny commented Feb 7, 2024

ridahanif96 commented Feb 12, 2024

ridahanif96 commented Feb 14, 2024

ridahanif96 commented Feb 16, 2024

prsunny commented Feb 24, 2024

sabakram commented Feb 24, 2024

ridahanif96 commented Feb 27, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sabakram commented Jan 1, 2024 •

edited

Loading