search

tumblr / collins

groovy kind of love
tumblr.github.com/collins
Apache License 2.0
572 stars 99 forks source link

make vlan-id attribute optional in lldp #523

Closed byxorna closed 7 years ago

byxorna commented 7 years ago

Fixes #466

This codifies the behavior of some odd switches (some variants of Arista) where the vlan-id attribute does not get set in lldpctl output, despite the interface definitely being configured with a VLAN. Support this case and don't reject an lldp report just because the asset is in a topology collins doesnt like.

@michaeljs1990 this is an upstream fix for the hacks we have in Genesis to post-process the LLDP report to fix malformed reports at induction time.

@roymarantz @michaeljs1990 @qx-xp @gtorre @defect

defect commented 7 years ago

Those aristas man... Looks good, but what do you think of adding this as a config parameter? The way we do with requireVlanName right now: https://github.com/tumblr/collins/blob/master/conf/reference/lldp_reference.conf#L4

byxorna commented 7 years ago

@defect yea, I can totally do that. Ill update with that fix in a few

byxorna commented 7 years ago

Tests pass, except for the lame collins-state gem which i need to fix. Landing this and accompanying #530 docs