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): Add support for logging format rfc5424 #3386

Merged
merged 10 commits into from
Dec 1, 2023
Merged

Feat(eos_cli_config_gen): Add support for logging format rfc5424 #3386

merged 10 commits into from
Dec 1, 2023

Conversation

durd
Copy link
Contributor

@durd durd commented Nov 27, 2023

Change Summary

This PR adds support for logging format rfc5424

Related Issue(s)

Fixes #3365

Component(s) name

arista.avd.eos_cli_config_gen

Proposed changes

This adds support for logging format rfc5424.
It adds the templates, documentation and schemas for this.

How to test

I'm not sure how to run tests. I'm using vscode and the dev-container included in the repo.
I'm familiar with Makefiles from *BSD, but not while developing.

See separate comment below.

Checklist

User Checklist

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)

@durd durd requested review from a team as code owners November 27, 2023 21:18
@github-actions github-actions bot added role: eos_cli_config_gen issue related to eos_cli_config_gen role state: Documentation role Updated labels Nov 27, 2023
@durd
Copy link
Contributor Author

durd commented Nov 27, 2023

I found the link to molecule in the PR checkbox comment. Looking through that I couldn't find a test for logging format sequence-numbers, so I figure that this change is as straight forward since I followed sequence-numbers and where it was located.
If there still is a need for a test I'll try to make one with molecule.

@ClausHolbechArista ClausHolbechArista marked this pull request as draft November 28, 2023 14:32
@ClausHolbechArista
Copy link
Contributor

Moving to draft until we have tests in here. You can mark is as "ready for review" when it is ready.

For Molecule, you should add the new key to ansible_collections/arista/avd/molecule/eos_cli_config_gen/inventory/host_vars/logging.yml and run Molecule locally:

cd ansible_collections/arista/avd
molecule converge -s eos_cli_config_gen -- --limit logging

@durd
Copy link
Contributor Author

durd commented Nov 29, 2023

Moving to draft until we have tests in here. You can mark is as "ready for review" when it is ready.

For Molecule, you should add the new key to ansible_collections/arista/avd/molecule/eos_cli_config_gen/inventory/host_vars/logging.yml and run Molecule locally:

cd ansible_collections/arista/avd
molecule converge -s eos_cli_config_gen -- --limit logging
PLAY [Converge] ****************************************************************

TASK [arista.avd.eos_cli_config_gen : Verify Requirements] *********************
AVD version 4.5.0-dev1
Use -v for details.
ok: [logging -> localhost]

TASK [arista.avd.eos_cli_config_gen : Create required output directories if not present] ***
ok: [logging -> localhost] => (item=/workspaces/ansible-avd/ansible_collections/arista/avd/molecule/eos_cli_config_gen/intended/structured_configs)
ok: [logging -> localhost] => (item=/workspaces/ansible-avd/ansible_collections/arista/avd/molecule/eos_cli_config_gen/documentation)
ok: [logging -> localhost] => (item=/workspaces/ansible-avd/ansible_collections/arista/avd/molecule/eos_cli_config_gen/intended/configs)
ok: [logging -> localhost] => (item=/workspaces/ansible-avd/ansible_collections/arista/avd/molecule/eos_cli_config_gen/documentation/devices)

TASK [arista.avd.eos_cli_config_gen : Include device intended structure configuration variables] ***
skipping: [logging]

TASK [arista.avd.eos_cli_config_gen : Generate eos intended configuration] *****
changed: [logging -> localhost]

TASK [arista.avd.eos_cli_config_gen : Generate device documentation] ***********
changed: [logging -> localhost]

PLAY RECAP *********************************************************************
logging                    : ok=4    changed=2    unreachable=0    failed=0    skipped=1    rescued=0    ignored=0

Should I commit the changes? There are a lot of deletes because of --limit logging...

@ClausHolbechArista
Copy link
Contributor

It should not delete anything. It sometimes happens if it is the first time you run it. So try to run without limit and then commit the updates.

@github-actions github-actions bot added the state: CI Updated CI scenario have been updated in the PR label Nov 29, 2023
@durd
Copy link
Contributor Author

durd commented Nov 29, 2023

Thanks, ran the command again without --limit logging and it recreated the cfg-files. Ran it again with --limit to be sure and committed and pushed the changes.

Edit: ah, sorry, I just noticed I committed the template change with the test artefacts. I hope that's ok?

@ClausHolbechArista
Copy link
Contributor

We don't care about your commits in your branch. We will squash them into a single commit using the PR title, once we merge it to the devel branch.

@durd
Copy link
Contributor Author

durd commented Nov 29, 2023

Well, tests passed and the workflow test passed locally earlier once I corrected the capitalization in the schema.
I think I'm ready for review!

Thank you so much for your help! I couldn't have managed the last mile without your help and guidance!

@durd durd marked this pull request as ready for review November 29, 2023 18:19
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. Thank you for this and congratulations on your first contribution to AVD! Looking forward to many more :)

@ClausHolbechArista ClausHolbechArista requested a review from a team November 30, 2023 08:28
@ClausHolbechArista ClausHolbechArista added the one approval This PR has one approval and is only missing one more. label Nov 30, 2023
…tes/documentation/logging.j2

Co-authored-by: Tony Reddy Goda <tgoda@arista.com>
@carlbuchmann carlbuchmann added this to the v4.5.0 milestone Dec 1, 2023
Copy link
Member

@carlbuchmann carlbuchmann left a comment

Choose a reason for hiding this comment

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

Applied review comments from @tgodaA - LGTM

@carlbuchmann carlbuchmann merged commit b0c4aa9 into aristanetworks:devel Dec 1, 2023
39 checks passed
@durd durd deleted the logging-rfc5424 branch December 1, 2023 21:05
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.

Support for syslog format RFC5424
4 participants