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_cli_config_gen): Enable or disable specific snmp-server traps #1294

Merged
merged 3 commits into from
Nov 13, 2021

Conversation

tgodaA
Copy link
Contributor

@tgodaA tgodaA commented Oct 28, 2021

Change Summary

This feature supports to enable specific snmp-server traps

Related Issue(s)

Fixes #1152

Component(s) name

arista.avd.eos_cli_config_gen

Proposed changes

Old data model:

snmp_server:
  traps:
    enable: < true | false >

New model:

snmp_server:
  traps:
    # Enable/Disable all snmp-traps
    enable: < true | false >
    # Enable/Disable specific snmp-trap types an their sub_trap types
    snmp_traps:
      - name: < snmp_trap_type | snmp_trap_type snmp_sub_trap_type >
        enabled: < true | false | default --> true >
      - name: < snmp_trap_type | snmp_trap_type snmp_sub_trap_type >

How to test

New molecule test snmp-server-traps was added.

Repository Checklist

  • [ x] My code has been rebased from devel before I start
  • [ x] I have read the CONTRIBUTING document.
  • [ x] My change requires a change to the documentation and documentation have been updated accordingly.
  • [ x] I have updated molecule CI testing accordingly. (check the box if not applicable)

@github-actions github-actions bot added role: eos_cli_config_gen issue related to eos_cli_config_gen role state: CI Updated CI scenario have been updated in the PR state: Documentation role Updated labels Oct 28, 2021
@c-po
Copy link
Contributor

c-po commented Oct 29, 2021

This also fixes #1231

Can I also enable all but disable specific traps?

@tgodaA
Copy link
Contributor Author

tgodaA commented Oct 29, 2021

@c-po, as per your description in #1231, there are some breaking changes. So I made up this model. Please check this molecule example: https://github.com/aristanetworks/ansible-avd/pull/1294/files#diff-503d0c73eb3ff86d35c8d52d8d880568c8ba3779505cda381363abbc83dfa8c6

When we say enable all it will enable everything by default and can't disable specific traps. So the keyword
enable: false should be set or don't define enable: <>. Then define the traps and sub_trap types and the output will be: https://github.com/aristanetworks/ansible-avd/pull/1294/files#diff-c36210703f4675127262674befd7070682a3771d74cbcf5a8e60808305b04202

The molecule example will give you a better idea.

@tgodaA tgodaA changed the title Feat(eos_cli_config_gen): Enable specific snmp-server traps Feat(eos_cli_config_gen): Enable or disable specific snmp-server traps Nov 1, 2021
@tgodaA tgodaA force-pushed the snmp branch 2 times, most recently from 5dfd514 to 0df8739 Compare November 1, 2021 16:44
@tgodaA
Copy link
Contributor Author

tgodaA commented Nov 1, 2021

This new model should enable/disable any traps or all traps.

Copy link
Contributor

@xaviramon xaviramon left a comment

Choose a reason for hiding this comment

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

Check changes to protect wrong use and we can discuss about SNMP trap as a whole.

@tgodaA tgodaA requested a review from xaviramon November 4, 2021 12:08
Copy link
Contributor

@xaviramon xaviramon left a comment

Choose a reason for hiding this comment

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

LGTM!

@ClausHolbechArista ClausHolbechArista modified the milestone: v3.2.0 Nov 11, 2021
@ClausHolbechArista ClausHolbechArista added this to the v3.1.0 milestone Nov 13, 2021
Copy link
Contributor

@ClausHolbechArista ClausHolbechArista left a comment

Choose a reason for hiding this comment

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

LGTM

@ClausHolbechArista ClausHolbechArista merged commit 0e0af70 into aristanetworks:devel Nov 13, 2021
@tgodaA tgodaA deleted the snmp branch November 30, 2021 16:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
role: eos_cli_config_gen issue related to eos_cli_config_gen role state: CI Updated CI scenario have been updated in the PR state: Documentation role Updated
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add support to enable specific traps
4 participants