From 147ba56736483a19c48320f72ec3009aa2edca6f Mon Sep 17 00:00:00 2001 From: Daniel Waugh Date: Wed, 11 Feb 2026 11:44:33 -0800 Subject: [PATCH] Fix VLAN 1 default state handling to prevent false removal failures Fixes #140 - Updated verify_interface_has_no_vlan_assigned() to treat VLAN 1 as default state - VLAN 1 is now considered equivalent to 'no VLANs assigned' during cleanup - Handles both explicit (switchport trunk allowed vlan 1) and implicit VLAN 1 configs - Prevents '[FAIL] VLAN(s) ALL removal failed' errors on subsequent reservations - Added comprehensive unit tests covering various VLAN 1 configuration scenarios This fix resolves the issue where some Cisco switches show 'switchport trunk allowed vlan 1' in their default configuration state, causing VLAN connectivity setup to fail after teardown, while other switches don't show it (implied). --- .../add_remove_vlan_actions.py | 30 +++++- .../command_actions/test_vlan_actions.py | 98 +++++++++++++++++++ 2 files changed, 125 insertions(+), 3 deletions(-) diff --git a/cloudshell/networking/cisco/command_actions/add_remove_vlan_actions.py b/cloudshell/networking/cisco/command_actions/add_remove_vlan_actions.py index 11d55af..3e1f6d2 100644 --- a/cloudshell/networking/cisco/command_actions/add_remove_vlan_actions.py +++ b/cloudshell/networking/cisco/command_actions/add_remove_vlan_actions.py @@ -90,12 +90,36 @@ def verify_interface_has_no_vlan_assigned(current_config): :return: True or False """ success = True - if re.search( + vlan_match = re.search( r"switchport.*vlan", current_config, re.IGNORECASE, - ): - success = False + ) + if vlan_match: + # Check if the only VLAN configured is VLAN 1 (default state) + # VLAN 1 is the default VLAN on Cisco switches and should be treated + # as "no VLANs assigned" for cleanup purposes + # Matches patterns like: + # - "switchport trunk allowed vlan 1" + # - "switchport access vlan 1" + # - "switchport mode access" + "switchport access vlan 1" + vlan_only_1 = re.search( + r"switchport\s+.*?vlan\s+1\s*$", + current_config, + re.MULTILINE | re.IGNORECASE, + ) + # Also check for other VLAN configurations that would indicate + # non-default VLANs are present (VLANs other than 1, or VLAN ranges/lists) + other_vlans = re.search( + r"switchport.*vlan\s+(?!1\s*$)\d+|switchport.*vlan\s+1[,-]", + current_config, + re.IGNORECASE | re.MULTILINE, + ) + # If only VLAN 1 is found and no other VLANs, consider it as "no VLANs" + if vlan_only_1 and not other_vlans: + success = True + else: + success = False return success def create_vlan(self, vlan_range, action_map=None, error_map=None): diff --git a/tests/networking/cisco/command_actions/test_vlan_actions.py b/tests/networking/cisco/command_actions/test_vlan_actions.py index fa7dd51..a468d4e 100644 --- a/tests/networking/cisco/command_actions/test_vlan_actions.py +++ b/tests/networking/cisco/command_actions/test_vlan_actions.py @@ -67,6 +67,104 @@ def test_verify_interface_has_any_vlan(self): """ self.assertFalse(self._handler.verify_interface_configured(current_config)) + def test_verify_interface_has_no_vlan_assigned_with_default_vlan_1_trunk(self): + """Test that VLAN 1 on trunk is considered as no VLANs assigned (default state).""" + current_config = """Building configuration... + + Current configuration : 144 bytes + ! + interface GigabitEthernet110/1/0/6 + description KG-255X-06-PT + switchport + switchport trunk allowed vlan 1 + switchport mode dynamic auto + end + """ + # VLAN 1 is the default state, should be considered as "no VLANs assigned" + self.assertTrue( + self._handler.verify_interface_has_no_vlan_assigned(current_config) + ) + + def test_verify_interface_has_no_vlan_assigned_with_default_vlan_1_access(self): + """Test that VLAN 1 on access port is considered as no VLANs assigned (default state).""" + current_config = """Building configuration... + + Current configuration : 100 bytes + ! + interface GigabitEthernet1/0/4 + switchport + switchport access vlan 1 + end + """ + # VLAN 1 is the default state, should be considered as "no VLANs assigned" + self.assertTrue( + self._handler.verify_interface_has_no_vlan_assigned(current_config) + ) + + def test_verify_interface_has_no_vlan_assigned_without_explicit_vlan(self): + """Test that interface without explicit VLAN config is considered as no VLANs assigned.""" + current_config = """Building configuration... + + Current configuration : 85 bytes + ! + interface TenGigabitEthernet1/5/10 + description SPIRENT Port 2/10 + switchport + end + """ + # No explicit VLAN, should be considered as "no VLANs assigned" + self.assertTrue( + self._handler.verify_interface_has_no_vlan_assigned(current_config) + ) + + def test_verify_interface_has_no_vlan_assigned_with_non_default_vlan(self): + """Test that non-default VLAN is detected as VLANs assigned.""" + current_config = """Building configuration... + + Current configuration : 100 bytes + ! + interface GigabitEthernet1/0/1 + switchport + switchport trunk allowed vlan 10 + end + """ + # VLAN 10 is not the default, should be considered as "VLANs assigned" + self.assertFalse( + self._handler.verify_interface_has_no_vlan_assigned(current_config) + ) + + def test_verify_interface_has_no_vlan_assigned_with_multiple_vlans(self): + """Test that multiple VLANs including VLAN 1 is detected as VLANs assigned.""" + current_config = """Building configuration... + + Current configuration : 110 bytes + ! + interface GigabitEthernet1/0/2 + switchport + switchport trunk allowed vlan 1,10,20 + end + """ + # Multiple VLANs, should be considered as "VLANs assigned" + self.assertFalse( + self._handler.verify_interface_has_no_vlan_assigned(current_config) + ) + + def test_verify_interface_has_no_vlan_assigned_with_vlan_range(self): + """Test that VLAN range is detected as VLANs assigned.""" + current_config = """Building configuration... + + Current configuration : 105 bytes + ! + interface GigabitEthernet1/0/3 + switchport + switchport trunk allowed vlan 10-20 + end + """ + # VLAN range, should be considered as "VLANs assigned" + self.assertFalse( + self._handler.verify_interface_has_no_vlan_assigned(current_config) + ) + @patch( "cloudshell.networking.cisco.command_actions.add_remove_vlan_actions" ".CommandTemplateExecutor"