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

Feat(eos_validate_state): Add option to exclude endpoint_ports from LLDP neighbour validation #4675

Open
1 task done
bjmeuer opened this issue Oct 31, 2024 · 4 comments · May be fixed by #4684
Open
1 task done

Feat(eos_validate_state): Add option to exclude endpoint_ports from LLDP neighbour validation #4675

bjmeuer opened this issue Oct 31, 2024 · 4 comments · May be fixed by #4684
Assignees
Labels
type: bug Something isn't working type: enhancement New feature or request

Comments

@bjmeuer
Copy link
Contributor

bjmeuer commented Oct 31, 2024

Enhancement summary

if connected_endpoints are configured and the validation role is run it complains about not finding a LLDP neighbor. But as most of the connected endpoints don't send LLDP this is expected.

Which component of AVD is impacted

eos_validate_state

Use case example

In my environment I have connected_endpoints which don't talk LLDP. The validate State will show Fail for these.

example:
['156', 'DC1-LEAF-01A', 'Connectivity', 'VerifyLLDPNeighbors', 'Verifies that the provided LLDP neighbors are connected properly.', 'Local: Ethernet8 - Remote: Server Eth1', 'FAIL', 'No LLDP neighbor(s) on port(s):\n Ethernet8']

Describe the solution you would like

either use the existing option validate_state: in the connected_endpoint model to disable LLDP checks as well or introduce a new option at the same level like validate_lldp:

Describe alternatives you have considered

No response

Additional context

No response

Contributing Guide

  • I agree to follow this project's Code of Conduct
@bjmeuer bjmeuer added the type: enhancement New feature or request label Oct 31, 2024
@ClausHolbechArista
Copy link
Contributor

I would call this a bug, that we don't honor the validate_state key on the endpoint. Thank you for reporting!

@ClausHolbechArista ClausHolbechArista added type: bug Something isn't working and removed type: enhancement New feature or request labels Nov 1, 2024
@bjmeuer
Copy link
Contributor Author

bjmeuer commented Nov 1, 2024

I would call this a bug, that we don't honor the validate_state key on the endpoint. Thank you for reporting!
@ClausHolbechArista

this one is honored, but only for checking the state, something like up/up. This is a separate check from LLDP. For attached servers the state check is fine, but lldp might not be fine.

maybe the model can be changed in that way:

ethernet_interface:
  ...
  validate:
     state: <bool>
     lldp: <bool>

@ClausHolbechArista
Copy link
Contributor

ClausHolbechArista commented Nov 1, 2024

If I understand correctly, we have two things here:

  1. validate_state: false should also disable LLDP tests
  2. Ask for a new option to disable the LLDP test while keeping the remaining validate state checks.

For 2. I think we should add validate_lldp: <bool; default=True>.

EDIT: The description for validate_lldp should make it clear that validate_state: false will also disable LLDP. It would not make sense to validate LLDP for a host that may be down.

@ClausHolbechArista ClausHolbechArista added the type: enhancement New feature or request label Nov 4, 2024
@gmuloc
Copy link
Contributor

gmuloc commented Nov 4, 2024

To whomever picks this up.

Please open two PRs, one to fix the bug (point 1) and then one for the enhancement (point 2).
Only the second PR should close the issue (and be merged after the first one)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug Something isn't working type: enhancement New feature or request
Projects
None yet
4 participants