-
Notifications
You must be signed in to change notification settings - Fork 184
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): Add global logging event storm-control #2994
Conversation
Hello! I believe your schema changes are in the wrong position as both |
ansible_collections/arista/avd/roles/eos_cli_config_gen/schemas/eos_cli_config_gen.schema.yml
Show resolved
Hide resolved
@@ -3922,6 +3922,16 @@ keys: | |||
type: str | |||
valid_values: | |||
- discard | |||
event: | |||
storm_control_discards: | |||
enable: bool |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
enable should include a type
member
enable:
type: bool
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for that inconvenience.
I will correct it tomorrow, since our servers with the development platforms are currently down.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@hamptonmoore would this match the correct syntax?
event:
type: dict
keys:
storm_control_discards:
type: dict
keys:
enable:
type: bool
interval:
type: int
convert_types:
- str
min: 10
max: 65535
description: Logging interval in seconds
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That looks good to me! One thing I'm not sure about is if EOS could add future options to storm-control
in the future and if we should have discards be in a dict inside of there. Any thoughts @carlbuchmann?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@hamptonmoore @carlbuchmann
A comment from my side on this:
Currently EOS storm-control has only the action to "drop/discard" packets exceeding the configured thresholds.
Other vendors also support the action "interface shutdown" when thresholds are exceeded.
So there might be the slightly chance, that "shutdown" could be added in the future and that there would be a syslog message added for this action.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, I agree with @hamptonmoore, so it would be something like this:
event:
type: dict
keys:
storm_control:
type: dict
keys:
discards:
type: dict
keys:
enable:
type: bool
interval:
type: int
convert_types:
- str
min: 10
max: 65535
description: Logging interval in seconds
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@kmueller68 if you need help with schema authoring, we are also available for a live zoom call.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@carlbuchmann thank you very much.
I will prepare things tomorrow and a short "how to do schema" in a zoom session would be very helpful.
Would you be available tomorrow morning EDT?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I will send you an invite!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Change Summary
Add global configuration to enable event logging for storm-control discards.
Related Issue(s)
Fixes #2993
Component(s) name
arista.avd.eos_cli_config_gen
Proposed changes
How to test
renders to:
Checklist
User Checklist
Repository Checklist