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

Fix more ansible 2.8.7 compability issues #1267

Merged
merged 2 commits into from
Dec 10, 2019
Merged

Fix more ansible 2.8.7 compability issues #1267

merged 2 commits into from
Dec 10, 2019

Conversation

wangxin
Copy link
Collaborator

@wangxin wangxin commented Dec 9, 2019

Description of PR

Summary:
Fixes # (issue)

Issues fixed in this PR:

  • Change 'connection: local' to 'delegate_to: localhost' to support configuring ansible_python_interpreter for specific test servers
  • Add pip_executable configuration option
  • Fix datetime call in kickstart.py
  • Fix tags issue of fanout deploy/config
  • Remove uncesessary tags in vlantb and everflow_testbed testing
  • New ansible parses inventory file as full path, fix issue of finding correct inventory file in pytest_runner.yml
  • Fix issue of creating PTF host in conftest

More details of some of the fixed issues

Change 'connection: local' to 'delegate_to: localhost' to support configuring ansible_python_interpreter for specific test servers

This is half fix half an enhancement. With this fix, we can setup python virtualenv on test servers and configure the test server to use the python binary in virtualenv to run ansible tasks.

  • fix aspect:
    Ansible 2.8.7 uses SSLv3. The apt_key task in files like ansible/roles/vm_set/tasks/docker.yml requires at least python 2.7.9. Unfortunately python version on some linux distributions does not meet the requirement. To upgrade the system python, usually the linux OS need to be upgraded. If the linux OS cannot be upgrade somehow. Then this change is to fix this issue by adding python virtualenv support
  • enhancement aspect:
    If the test server is also used for other purpose. Then there is potential python package conflicts. Python package dependencies of other application running on the test server may conflict with the python packages required by ansible testing. Then this change is to enhance the sonic-mgmt testing.

Caveats:

Add pip_executable configuration option

This is a sister change of the above one. To support virtualenv, we also need to specify pip_executable for specific test server.

New ansible parses inventory file as full path, fix issue of finding correct inventory file in pytest_runner.yml

In Ansible 2.8.7, the inventory file name is parsed to absolute path in runtime. The pytest_runner.yml file needs to be updated accordingly to locate the correct inventory file.

Fix tags issue of fanout deploy/config

In ansible 2.8.7, tags is only inherited to child tasks when "import_tasks" is used. If use tags with "include_tasks", the tags only apply to the include_task task itself. Child tasks will not inherit the tags.
The fanout deploy/config related playbooks use the tags feature and need update accordingly to comply with new ansible behavior.

Fix issue of creating PTF host in conftest

The testbed_devices fixture depends on ansible code to find ptf_host from inventory. The API of ansible 2.8.7 has been changed. The testbed_devices fixture in conftest.py needs to be updated accordingly.

Type of change

  • Bug fix
  • [] Testbed and Framework(new/improvement)
  • [] Test case(new/improvement)

Approach

How did you do it?

How did you verify/test it?

Any platform specific information?

Supported testbed topology if it's a new test case?

Documentation

Issues fixed in this commit:
* Change 'connection: local' to 'delegate_to: localhost' to support configuring ansible_python_interpreter for specific test servers
* Add pip_executable configuration option
* Fix datetime call in kickstart.py
* Fix tags issue of fanout deploy/config
* Remove uncesessary tags in vlantb and everflow_testbed testing
* Fix some 'with_items' and 'with_dict'
* New ansible parses inventory file as full path, fix issue of finding correct inventory file in pytest_runner.yml
* Fix issue of creating PTF host in conftest

Change-Id: I245ff5c4b2480a9b8153c0b896c7a25a448d5956
Signed-off-by: Xin Wang <xinw@mellanox.com>
Copy link
Contributor

@msosyak msosyak left a comment

Choose a reason for hiding this comment

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

Looks good except whitespace changes.

@lguohan

@lguohan
Copy link
Contributor

lguohan commented Dec 10, 2019

LGTM, please revert back those snmp tests to use pytest.

Change-Id: I31f43b19cf8cf6ec77848238d2ed6df0cae025e8
Signed-off-by: Xin Wang <xinw@mellanox.com>
@wangxin
Copy link
Collaborator Author

wangxin commented Dec 10, 2019

@msosyak Thanks for reviewing the code! I didn't add extra white spaces. The editor automatically trimmed ending white spaces while saving the files I touched.

@lguohan Thanks for pointing out the issus! I resolved conflicts using wrong strategy. The SNMP tests have been reverted to pytest now.

@lguohan lguohan merged commit 03bb625 into sonic-net:master Dec 10, 2019
@wangxin wangxin deleted the new-ansible-fix-pr branch March 28, 2020 03:09
wangxin pushed a commit that referenced this pull request Jan 15, 2021
What is the motivation for this PR?
adding test plan for PMON enhancements for chassis

How did you do it?
Based on the code changes in the associated PRs:
Configure and show for platform chassis_modules #1145
CHASSIS_STATE_DB on control-card for chassis state #395
PSUd changes to compute power-budget for Modular chassis #104
Introduce APIs for modular chassis support #124
Common power consumption and supply APIs for modular chassis #136
Thermalctld APIs for recording min and max temp #131
Modular Chassis - Midplane monitoring APIs #148
Modular-Chassis: Show midplane status #1267
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants