-
Notifications
You must be signed in to change notification settings - Fork 727
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
[pytest] Convert interface naming mode test to pytest #1702
[pytest] Convert interface naming mode test to pytest #1702
Conversation
can you modify the sonic-mgmt build to include that patch in the sonic-mgmt docker? |
sonic-buildimage/PR4681 has been raised to include the pytest-ansible patch in docker-sonic-mgmt. |
minigraph_neighbors = setup['minigraph_facts']['minigraph_neighbors'] | ||
|
||
lldp_table = dutHostGuest.shell('SONIC_CLI_IFACE_MODE={} show lldp table'.format(ifmode))['stdout'] | ||
logger.debug('lldp_table:\n{}'.format(lldp_table)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This log message will only be output if logging level is DEBUG. When logging level is DEBUG, the previous call to the shell module would log the module execution result too. It looks like duplicated logs. Maybe you can consider raising the logging level to info here. Similar comments for other logger.debug calls later in the script.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The logging level has been changed from DEBUG to INFO.
logger.info('Reverting the port alias name in redis db to the actual values') | ||
for item in default_interfaces: | ||
port_alias_old = port_alias_facts['port_name_map'][item] | ||
duthost.command('redis-cli -n 4 HSET "PORT|{}" alias {}'.format(item, port_alias_old)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The original ansible version of this testing reboot the DUT to recover after the testing is done. Is setting port alias to old names enough to recover or do we need to perform a reboot or config reload here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All the configurations changes done as part of the testing are reverted by the script itself. So reboot or config reload is not required.
Since the corresponding ansible testing has been converted to pytest, could you please update the ansible/roles/test/tasks/iface_naming_mode.yml to call this pytest script? And rest of the unused ansible playbooks can be removed too. |
…ansible playbooks
@wangxin , the "ansible/roles/test/tasks/iface_naming_mode.yml" has been modified to call the pytest script and the other playbooks have been removed. |
Description of PR
Note: Requires pytest-ansible/PR46 changes to support 'become' and 'become_user'.
Type of change
How did you verify/test it?
Logs: IFACE_pytest_t1_topo_logs.txt, IFACE_pytest_t0_topo_logs.txt