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

[snmp] Add snmp_facts support for ieee802.1ab MIBs, extend snmp testcase #679

Merged
merged 3 commits into from
Aug 18, 2018

Conversation

mykolaf
Copy link
Contributor

@mykolaf mykolaf commented Aug 14, 2018

Description of PR

Summary:
Fixes # (issue)

  • Extended snmpfacts to gather LLDP-MIB data. (802.1ab)
  • Extended snmp test with a lldp subtest, which verifies that LLDP-MIBs are avalable.
    Subtest logic :
Test checks for ieee802_1ab MIBs:
  - lldpLocalSystemData  1.0.8802.1.1.2.1.3
    - lldpLocPortTable     1.0.8802.1.1.2.1.3.7
    - lldpLocManAddrTable     1.0.8802.1.1.2.1.3.8

  - lldpRemTable  1.0.8802.1.1.2.1.4.1
  - lldpRemManAddrTable  1.0.8802.1.1.2.1.4.2

 For local data check if every OID has value
 For remote values check for availability for
 at least 80% of minigraph neighbors
 (similar to lldp test)

❗WARNING❗ :
make sure submodule is updated before merging this one
sonic-net/sonic-buildimage#1930 (already merged)

Type of change

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

Approach

How did you do it?

  • Created new subtest under test/tasks/snmp/lldp.yml
  • extended library/snmp_facts.py

How did you verify/test it?

Run snmp test

Any platform specific information?

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

Should work on any topology. Tested on t0, t1, t1-lag

Documentation

@qiluo-msft
Copy link
Contributor

qiluo-msft commented Aug 15, 2018

I am interested whether you have did integration test with any management tool. Just curious. #Closed


- name: "Verify {{ item }} is defined"
assert: { that: "{{ snmp_lldp[item] is defined }}
and not {{ snmp_lldp[item] | search('No Such Object currently exists') }}" }
Copy link
Contributor

@qiluo-msft qiluo-msft Aug 15, 2018

Choose a reason for hiding this comment

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

Add one level indentation before 'and not'? #Closed

# Check if lldpLocManAddr is present
- name: "Verify {{ item }} is defined"
assert: { that: "{{ snmp_lldp[item] is defined }}
and not {{ snmp_lldp[item] | search('No Such Object currently exists') }}" }
Copy link
Contributor

@qiluo-msft qiluo-msft Aug 15, 2018

Choose a reason for hiding this comment

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

The same #Closed

- fail:
msg: "lldpLocPortTable data missing for interface {{ item.key }}"
when: "{{ item.value.description | match('^Ethernet') }}
and ( {{ item.value['lldpLocPortNum'] is not defined }}
Copy link
Contributor

@qiluo-msft qiluo-msft Aug 15, 2018

Choose a reason for hiding this comment

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

The same #Closed

with_dict: minigraph_neighbors

- name: Create list of ports with lldpRemTable data
when: "{{ item.value['lldpRemTimeMark'] is defined }}
Copy link
Contributor

@qiluo-msft qiluo-msft Aug 15, 2018

Choose a reason for hiding this comment

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

the same #Closed


- name: Create list of ports with lldpRemManAddr data
when: "{{ item.value['lldpRemManAddrSubtype'] is defined }}
and {{ item.value['lldpRemManAddr'] is defined }}
Copy link
Contributor

@qiluo-msft qiluo-msft Aug 15, 2018

Choose a reason for hiding this comment

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

the same #Closed


- debug:
msg: "Found {{ active_intf | length }} Ifs with lldpRemManAddr data\n
Minigraph contains {{ minigraph_lldp_nei | length }} neighbors"
Copy link
Contributor

@qiluo-msft qiluo-msft Aug 15, 2018

Choose a reason for hiding this comment

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

the same #Closed

msg: "Found {{ active_intf | length }} Ifs with lldpRemManAddr data\n
Minigraph contains {{ minigraph_lldp_nei | length }} neighbors"

- name: Verify lldpRemManAddr is available on most interfaces
Copy link
Contributor

@qiluo-msft qiluo-msft Aug 15, 2018

Choose a reason for hiding this comment

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

Should we test lldpRemManAddr is available on all interfaces? #Closed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Taking into consideration that in lldp test we are satisfied with lldp data awailable on 80% of the interfaces, demanding more of the same data here is questionable.
https://github.com/Azure/sonic-mgmt/blob/master/ansible/roles/test/tasks/lldp.yml#L25
I think the intent was to prevent failures caused by testbed troubles - some VMs being down/not sending lldp...

@mykolaf
Copy link
Contributor Author

mykolaf commented Aug 16, 2018

In progress of integrating with Mellanox NEO. Do you use any management software with your testbed? #Resolved

@qiluo-msft
Copy link
Contributor

We don't have one.


In reply to: 413484821 [](ancestors = 413484821)

@qiluo-msft qiluo-msft merged commit 2faebb7 into sonic-net:master Aug 18, 2018
@qiluo-msft
Copy link
Contributor

It will break the nightly test of old images. Could you add some logic to bypass the test?

qiluo-msft added a commit to qiluo-msft/sonic-mgmt that referenced this pull request Sep 14, 2018
@mykolaf mykolaf deleted the lldp_mib branch February 18, 2020 13:15
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.

3 participants