Feat(eos_cli_config_gen): Add support for logging event storm-control and link-status in interfaces

[Shivani-gslab]

Change Summary

Add support for logging event storm-control and link-status in interfaces

Related Issue(s)

Fixes #1953

1. Global settings mentioned in the issue are already implemented in our existing schema and template.
2. As mentioned in the Note: when configuring logging event storm-control discards use-global under an interface, it is the default and it does not show up in the config for 4.27.1F on 7050X3., So not applying any changes to the template/schema for use-global inside interfaces
3. Adding the settings for logging event storm-control and link-status if it is missing inside interfaces.

Component(s) name

arista.avd.eos_cli_config_gen

Proposed changes

Adding logging event storm-control discards to port-channel interfaces.
Adding logging event link-status to vlan interfaces.

How to test

interface Ethernet4
   logging event storm-control discards use-global

interface Port-Channel4
   logging event storm-control discards use-global

interface Vlan3
   logging event link-status

Checklist

User Checklist

• Verify the command ordering on EOS

Repository Checklist

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

[ankudinov]

There are a few things I'd like to optimise in this PR:

1. Please add the missing molecule tests for port-channel settings. As right now we are only testing SVI settings.
2. It's not very clear what part of the Feat(eos_cli_config_gen): add support for logging event storm-control discards * #1953 this PR is addressing. Could you please add some comments on that? I see that the issue mentions global settings as well and there are no other PRs associated with it.
3. Suggesting to re-run molecule is not a valid way of testing in my opinion. I'd appreciate if you specify exact settings you tested in the lab so that I can check them, modify and retest them in my test inventory. Every PR should have a good description of the data model implemented for reviewers to pick it quick. If there was no lab test - please take schema and convert it to an example to be tested.
4. The PR covers SVis (not mentioned in the issue) and port-channels only. While according to the Feat(eos_cli_config_gen): add support for logging event storm-control discards * #1953 ethernet ports must be part of the PR as well.

[github-actions]

This pull request has conflicts, please resolve those before we can evaluate the pull request.

[github-actions]

Conflicts have been resolved. A maintainer will review the pull request shortly.

[Shivani-gslab]

There are a few things I'd like to optimise in this PR:

1. Please add the missing molecule tests for port-channel settings. As right now we are only testing SVI settings.
2. It's not very clear what part of the Feat(eos_cli_config_gen): add support for logging event storm-control discards * #1953 this PR is addressing. Could you please add some comments on that? I see that the issue mentions global settings as well and there are no other PRs associated with it.
3. Suggesting to re-run molecule is not a valid way of testing in my opinion. I'd appreciate if you specify exact settings you tested in the lab so that I can check them, modify and retest them in my test inventory. Every PR should have a good description of the data model implemented for reviewers to pick it quick. If there was no lab test - please take schema and convert it to an example to be tested.
4. The PR covers SVis (not mentioned in the issue) and port-channels only. While according to the Feat(eos_cli_config_gen): add support for logging event storm-control discards * #1953 ethernet ports must be part of the PR as well.

Thanks for your feedback @ankudinov.

1. Added the molecule test for port-channel interface.
2. I have updated the PR description for the issue fix.
3. Added the commands to run in PR description.
4. As ethernet interface already had the storm_control discards, I have added in port-channel and for Vlan interface I could see only link-status on EOS CLI (which is included in this PR)

Please review again. Your comments have been addressed.

[ankudinov]

LGTM, but please add a short version of the following info in the schema description:
"NOTE: when configuring logging event storm-control discards use-global under an interface, it is the default and it does not show up in the config for 4.27.1F on 7050X3."
This is mentioned in the original version, but not anywhere in our docs and can be quite confusing for the users.
I'll approve once that is addressed.

[ClausHolbechArista]

LGTM, but please add a short version of the following info in the schema description: "NOTE: when configuring logging event storm-control discards use-global under an interface, it is the default and it does not show up in the config for 4.27.1F on 7050X3." This is mentioned in the original version, but not anywhere in our docs and can be quite confusing for the users. I'll approve once that is addressed.

@ankudinov please comment in the code/schema next time. Preferably with a ```suggestion block These global comments cannot be "resolved".

I don't agree on this comment you asked to add. The use-global is not mentioned anywhere in the data model we are implementing here, so there is no need to mention that. You could instead say Overrides global logging event settings.

[ankudinov]

LGTM. Thx for resolving!

stale

[carlbuchmann]

LGTM, but please add a short version of the following info in the schema description: "NOTE: when configuring logging event storm-control discards use-global under an interface, it is the default and it does not show up in the config for 4.27.1F on 7050X3." This is mentioned in the original version, but not anywhere in our docs and can be quite confusing for the users. I'll approve once that is addressed.

@ankudinov please comment in the code/schema next time. Preferably with a ```suggestion block These global comments cannot be "resolved".

I don't agree on this comment you asked to add. The use-global is not mentioned anywhere in the data model we are implementing here, so there is no need to mention that. You could instead say Overrides global logging event settings.

I agree with @ClausHolbechArista that this comment is confusing, and we should remove it!

[carlbuchmann]

LGTM