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

Improved error handling when not all Interfaces are up #853

Merged
merged 2 commits into from
May 26, 2019

Conversation

andriymoroz-mlnx
Copy link
Contributor

@andriymoroz-mlnx andriymoroz-mlnx commented Apr 2, 2019

Recreated PR
#815 which was merged and reverted

Description of PR

Summary:
Any test, that calls interface.yml, if encountered that some Interfaces/PortChannels are not up, will not just fail but will try to gather interfaces status from Fanout/VMs as well.

Type of change

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

Approach

How did you do it?

  1. Modified interface.yml to include check_testbed_interfaces.yml,
    when 'Verify interfaces are up' step fails.
  2. check_testbed_interfaces.yml will gather interfaces status on:
  • DUT
  • Fanout (for MLNX only, filtered by check_interfaces_status tag)
  • Testbed server (by calling testbed_vm_status.yml playbook)
  • relevant VMs
  1. testbed_vm_status.yml palybook will connect to Testbed server and gather VMs status.
  2. show_int_portchannel_status.j2 will connect to each relevant VM and gather Port-Channel status.

How did you verify/test it?

Any test that call interface.yml, in two cases:

  1. Normal flow, when all interfaces are up.
    In this case nothing is changed.
  2. When Fanout has deliberately its ports down,
    In that cases, the test fails as usual, but gathers Interfaces status from Fanout.
  3. When some relevant VM has its Port-Channel deliberately down.
    In that cases, the test fails as usual, but gathers VMs status from Testbed server, and PortChannel status from VMs.

Any platform specific information?

Current implementation can enter Fanout switch, only if Fanout is MLNX type.
The 'Check Fanout interfaces' step in check_testbed_interfaces.yml calls fanout.yml (actually fanout role) with tag 'check_interfaces_status'
In this case anyone can modify roles/fanout/tasks/main.yml to execute Fanout specific step with
when: peer_hwsku == "<specific_fanout_type>" tags: check_interfaces_status

Example:

 ###################################################################
 # Check Fanout interfaces status                                  #
 ###################################################################
- block:
  - name: Check Fanout interfaces status
    action: apswitch template=roles/fanout/templates/mlnx_interfaces_status.j2
    connection: switch
    register: fanout_interfaces
    args:
      login: "{{ switch_login['MLNX-OS'] }}"

  - debug:
      msg: "{{ fanout_interfaces.stdout.split('\n') }}"

  when: peer_hwsku == "MLNX-OS"
  tags: check_interfaces_status

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

Documentation

@qiluo-msft
Copy link
Contributor

What is changed base on #815?
Is it possible to give incremental commit based on old commit?

@liat-grozovik
Copy link
Collaborator

The original commit had comments by Roman was not able to update it. So he created #815 which was merged and then revered (wrongly as i thought it also requires the first one).
So, Andriy made some order. Recreated #815 which is exactly as this one.

@liat-grozovik
Copy link
Collaborator

this PR was verified on top of sonic-mgmt master today and found to be working just fine.
extra information is added once port channel are down to allow the offline debug.

@liat-grozovik liat-grozovik merged commit 71b6c80 into sonic-net:master May 26, 2019
yxieca pushed a commit that referenced this pull request May 28, 2019
* Improved error handling when not all Interfaces are up

* Fixed PR 853.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants