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): Global logging keys for congestion-drops, link-status, and repeat-messages #4493

Merged
merged 15 commits into from
Sep 23, 2024

Conversation

nathanmusser
Copy link
Contributor

Change Summary

Added new logging keys to disable repeat-messages, change the logging interval for the congestion-drops event, and set the global command for the link-status logging event.

Related Issue(s)

N/A

Component(s) name

arista.avd.eos_cli_config_gen

Proposed changes

Added the following data model:

logging:
  repeat_messages: <bool>
  event:
    congestion_drops:
      interval: <int>
    link_status:
      global: <bool>

And added the associated j2 templates to render the associated commands:

no logging repeat-messages
logging event congestion-drops interval 600
logging event link-status global

How to test

Used the example dual-dc-l3ls topology and added the various commands to the FABRIC.yml group_var. Tested removing them and adding them and setting invalid values to make sure behavior is as expected. Commands are only rendered when the new model is present, etc..

Checklist

User Checklist

  • N/A

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)

Note: I'm still new to contributing here, but for the repo checklist item referring to documentation. My understanding is that for these simple knobs the documentation is generated from the schema. AFAIK there's nowhere else I need to add documentation? Should I be checking this box in those cases? Or leaving it unchecked since there is no documentation beyond what is auto-generated that needs to be modified?

Also I know I didn't open an issue for this first. It felt like a simple enough PR that there wouldn't be enough discussion to warrant an issue. Feel free to correct that assumption and I can create one as well as do so going forward for all PRs.

@nathanmusser nathanmusser requested review from a team as code owners September 19, 2024 03:41
Copy link

Review docs on Read the Docs

To test this pull request:

# Create virtual environment for this testing below the current directory
python -m venv test-avd-pr-4493
# Activate the virtual environment
source test-avd-pr-4493/bin/activate
# Install all requirements including PyAVD
pip install "pyavd[ansible] @ git+https://github.com/nathanmusser/avd.git@logging_dev#subdirectory=python-avd" --force
# Point Ansible collections path to the Python virtual environment
export ANSIBLE_COLLECTIONS_PATH=$VIRTUAL_ENV/ansible_collections
# Install Ansible collection
ansible-galaxy collection install git+https://github.com/nathanmusser/avd.git#/ansible_collections/arista/avd/,logging_dev --force
# Optional: Install AVD examples
cd test-avd-pr-4493
ansible-playbook arista.avd.install_examples

@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 Sep 19, 2024
@gmuloc gmuloc added the one approval This PR has one approval and is only missing one more. label Sep 23, 2024
Copy link

Copy link
Contributor

@Shivani-gslab Shivani-gslab left a comment

Choose a reason for hiding this comment

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

LGTM!

@gmuloc gmuloc merged commit 5758359 into aristanetworks:devel Sep 23, 2024
43 checks passed
@nathanmusser nathanmusser deleted the logging_dev branch September 26, 2024 14:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
one approval This PR has one approval and is only missing one more. rn: Feat(eos_cli_config_gen) 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.

5 participants