Skip to content

[pytest/Mgmtvrf] fix 'Host Unreachable' error when setup Mgmt vrf configs #1647

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.

Sign up for GitHub

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

Open
irene-pan1202 wants to merge 1 commit into
base: master
Choose a base branch
from
Open

[pytest/Mgmtvrf] fix 'Host Unreachable' error when setup Mgmt vrf configs #1647

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
4 changes: 4 additions & 0 deletions tests/test_mgmtvrf.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,10 @@

logger = logging.getLogger(__name__)

# fixtures
@pytest.fixture(scope="module")
def localhost(testbed_devices):
return testbed_devices['localhost']

@pytest.fixture(scope='module',autouse=True)
def setup_mvrf(duthost, testbed_devices, testbed, localhost):
Copy link
Copy Markdown
Collaborator

@wangxin wangxin May 7, 2020

Choose a reason for hiding this comment

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

@irene-pan1202 I think there is no need to define the localhost fixture again in the script. You can use the localhost object already in the testbed_devices fixture supplied to setup_mvrf(). For example, in setup_mvrf():

    localhost = testbed_devices["localhost"]

BTW, the localhost supplied to setup_mvrf() is redundant and can be removed.

Copy link
Copy Markdown
Contributor Author

@irene-pan1202 irene-pan1202 May 8, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

@wangxin As you say, other test items that require localhost need to use the localhost object in the testbed_devices fixture individually?
But at #1432, localhost = testbed_devices ['localhost'] has been replaced by localhost in test_snmp_fact test case. Which way is better?
Just like test_vrf.py, because many test items need to use localhost, so define the localhost fixture?

Copy link
Copy Markdown
Collaborator

@wangxin wangxin May 9, 2020

Choose a reason for hiding this comment

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

Currently there are two ways to get access to the localhost object. The first one is to get it from testbed_devices fixture. The second one is defined in https://github.com/Azure/sonic-mgmt/blob/master/tests/common/plugins/ansible_fixtures.py
IMO, both of these definitions need improvement.
There was discussion about improving testbed_devices fixture here: #1475 (comment)
The localhost fixture defined in ansible_fixtures.py should be moved to tests/conftest.py.

But anyway, the localhost object is already defined (already defined twice), there is no need to define it again in the test script.

Localhost is not defined before using when setup Management vrf configs before the start of testsuite, it will cause 'Host Unreachable' error

I am not quite sure about your statement of the root cause. The localhost is defined indeed. Did you get a chance to test your fix? According to #1615 (comment), the setup mgmt vrf failed because the console hangs after execution of command sudo config vrf add mgmt.

Expand Down