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

[vstest] VS test failure fix after fabric port orch PR merge #1811

Merged
merged 3 commits into from
Dec 7, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 4 additions & 16 deletions tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,8 @@
# a dynamic number of ports. GitHub Issue: Azure/sonic-swss#1384.
NUM_PORTS = 32

# FIXME: Voq asics will have 16 fabric ports created (defined in Azure/sonic-buildimage#6185).
# Right now, we set FABRIC_NUM_PORTS to 0, and change to 16 when PR#6185 merges. PR#6185 can't
# be merged before this PR. Otherwise it will cause swss voq test failures.
FABRIC_NUM_PORTS = 0
# Voq asics will have 16 fabric ports created (defined in Azure/sonic-buildimage#7629).
FABRIC_NUM_PORTS = 16

def ensure_system(cmd):
rc, output = subprocess.getstatusoutput(cmd)
Expand Down Expand Up @@ -526,22 +524,12 @@ def _polling_function():

# Verify that all ports have been created
asic_db = self.get_asic_db()

# Verify that we have "at least" NUM_PORTS + FABRIC_NUM_PORTS, rather exact number.
# Right now, FABRIC_NUM_PORTS = 0. So it essentially waits for at least NUM_PORTS.
# This will allow us to merge Azure/sonic-buildimage#6185 that creates 16 fabric ports.
# When PR#6185 merges, FABRIC_NUM_PORTS should be 16, and so this verification (at least
# NUM_PORTS) still holds.
# Will update FABRIC_NUM_PORTS to 16, and revert back to wait exact NUM_PORTS + FABRIC_NUM_PORTS
# when PR#6185 merges.
wait_at_least_n_keys = True

asic_db.wait_for_n_keys("ASIC_STATE:SAI_OBJECT_TYPE_PORT", num_ports + 1, wait_at_least_n_keys) # +1 CPU Port
asic_db.wait_for_n_keys("ASIC_STATE:SAI_OBJECT_TYPE_PORT", num_ports + 1) # +1 CPU Port

# Verify that fabric ports are monitored in STATE_DB
if metadata.get('switch_type', 'npu') in ['voq', 'fabric']:
self.get_state_db()
self.state_db.wait_for_n_keys("FABRIC_PORT_TABLE", FABRIC_NUM_PORTS, wait_at_least_n_keys)
self.state_db.wait_for_n_keys("FABRIC_PORT_TABLE", FABRIC_NUM_PORTS)

def net_cleanup(self) -> None:
"""Clean up network, remove extra links."""
Expand Down
6 changes: 0 additions & 6 deletions tests/test_virtual_chassis.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import pytest
from swsscommon import swsscommon
from dvslib.dvs_database import DVSDatabase
import ast
Expand Down Expand Up @@ -136,7 +135,6 @@ def test_voq_switch(self, vct):
spcfg = ast.literal_eval(value)
assert spcfg['count'] == sp_count, "Number of systems ports configured is invalid"

@pytest.mark.skip(reason="This test is not stable enough")
Copy link
Contributor

Choose a reason for hiding this comment

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

Can I have the context for this?

@pytest.mark.skip(reason="This test is not stable enough")

Why was it added? Was it added after sonic-net/sonic-buildimage#7629 merged? Did sonic-net/sonic-buildimage#7629 cause test instability?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Those 3 tests consistently failed. So PR #1807 was raised to skip these tests.
These tests failed not because of sonic-net/sonic-buildimage#7629 but because of sonic-net/sonic-buildimage#7857. I noticed that once the problem created by sonic-net/sonic-buildimage#7857 is fixed, the port count mismatch problem occurred, which was due to sonic-net/sonic-buildimage#7629

Copy link
Contributor

Choose a reason for hiding this comment

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

Cool, thanks.

def test_chassis_app_db_sync(self, vct):
"""Test chassis app db syncing.
Expand Down Expand Up @@ -213,7 +211,6 @@ def test_chassis_system_interface(self, vct):
# Remote system ports's switch id should not match local switch id
assert spcfginfo["attached_switch_id"] != lc_switch_id, "RIF system port with wrong switch_id"

@pytest.mark.skip(reason="This test is not stable enough")
def test_chassis_system_neigh(self, vct):
"""Test neigh record create/delete and syncing to chassis app db.
Expand Down Expand Up @@ -470,7 +467,6 @@ def test_chassis_system_neigh(self, vct):
# Cleanup inband if configuration
self.del_inbandif_port(vct, inband_port)

@pytest.mark.skip(reason="This test is not stable enough")
def test_chassis_system_lag(self, vct):
"""Test PortChannel in VOQ based chassis systems.
Expand Down Expand Up @@ -607,7 +603,6 @@ def test_chassis_system_lag(self, vct):

break

@pytest.mark.skip(reason="This test is not stable enough")
def test_chassis_system_lag_id_allocator_table_full(self, vct):
"""Test lag id allocator table full.
Expand Down Expand Up @@ -685,7 +680,6 @@ def test_chassis_system_lag_id_allocator_table_full(self, vct):

break

@pytest.mark.skip(reason="This test is not stable enough")
def test_chassis_system_lag_id_allocator_del_id(self, vct):
"""Test lag id allocator's release id and re-use id processing.
Expand Down