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.

Sign up for GitHub

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

Jump to bottom

Feat(eos_cli_config_gen): add system l1 support #3221

Merged
merged 8 commits into from Oct 13, 2023
Merged

Feat(eos_cli_config_gen): add system l1 support #3221

merged 8 commits into from Oct 13, 2023

Conversation

carlbuchmann
Copy link
Member

@carlbuchmann carlbuchmann commented Oct 6, 2023 •
edited by philippebureau

Change Summary

Add support to define system l1 configuration

Related Issue(s)

Fixes #3202

Component(s) name

`arista.avd.eos_cli_config_gen

Proposed changes

How to test

See molecule scenario

Checklist

User Checklist

  • data model and schema
  • eos cli template
  • eos documentation template
  • create unit tests in molecule

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)

@github-actions github-actions bot added role: eos_cli_config_gen issue related to eos_cli_config_gen role state: Documentation role Updated labels Oct 6, 2023
@carlbuchmann carlbuchmann force-pushed the feat-eos-cli-config-gen-system-l1 branch from 7d67cf7 to bd96ad3 Compare October 11, 2023 15:58
@github-actions github-actions bot added the state: CI Updated CI scenario have been updated in the PR label Oct 12, 2023
@carlbuchmann carlbuchmann force-pushed the feat-eos-cli-config-gen-system-l1 branch from bf823d0 to eae7725 Compare October 12, 2023 01:04
@carlbuchmann carlbuchmann marked this pull request as ready for review October 12, 2023 01:55
@carlbuchmann carlbuchmann requested review from a team as code owners October 12, 2023 01:55
ClausHolbechArista
ClausHolbechArista reviewed Oct 12, 2023
View reviewed changes
ansible_collections/arista/avd/roles/eos_cli_config_gen/templates/documentation/interfaces.j2 Outdated
@@ -14,6 +15,8 @@
or vxlan_interface is arista.avd.defined %}

## Interfaces
{## System l1 #}
Copy link
Contributor

Choose a reason for hiding this comment

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

IMO this should be under a different section. Something with hardware or system settings.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, I debated with myself on this one... I put it in this section because it's about the "Configuration of L1 switch parameters". You can configure parameters for Modules, linecards, and interfaces. But open to suggestions!

Copy link
Contributor

Choose a reason for hiding this comment

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

"System Settings" was my suggestion.

Copy link
Member Author

Choose a reason for hiding this comment

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

addressed in commit: a4804c6

ansible_collections/arista/avd/roles/eos_cli_config_gen/templates/documentation/system-l1.j2 Outdated Show resolved Hide resolved
...ollections/arista/avd/roles/eos_cli_config_gen/schemas/schema_fragments/system_l1.schema.yml Outdated
# Use Ctrl + Space to get suggestions for every field. Autocomplete will pop up after typing 2 letters.
type: dict
keys:
system_l1:
Copy link
Contributor

Choose a reason for hiding this comment

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

We already have a system key, where l1 could just be another option.

Copy link
Member Author

Choose a reason for hiding this comment

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

I separated it since system l1 is in it's on section in the CLI.

Copy link
Member Author

Choose a reason for hiding this comment

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

addressed in commit: 46f87f0

ansible_collections/arista/avd/roles/eos_cli_config_gen/templates/documentation/system-l1.j2 Outdated Show resolved Hide resolved
ansible_collections/arista/avd/roles/eos_cli_config_gen/templates/documentation/system-l1.j2 Outdated Show resolved Hide resolved
ansible_collections/arista/avd/roles/eos_cli_config_gen/templates/documentation/system-l1.j2 Outdated Show resolved Hide resolved
ansible_collections/arista/avd/roles/eos_cli_config_gen/templates/documentation/system-l1.j2 Outdated Show resolved Hide resolved
ansible_collections/arista/avd/roles/eos_cli_config_gen/templates/documentation/system-l1.j2 Outdated Show resolved Hide resolved
@carlbuchmann carlbuchmann force-pushed the feat-eos-cli-config-gen-system-l1 branch from 797a580 to cf758df Compare October 12, 2023 12:23
philippebureau
philippebureau approved these changes Oct 12, 2023
View reviewed changes
Copy link
Contributor

@philippebureau philippebureau 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 2bcfb23 into aristanetworks:devel Oct 13, 2023
43 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@philippebureau philippebureau philippebureau approved these changes

@ClausHolbechArista ClausHolbechArista ClausHolbechArista approved these changes

@gmuloc gmuloc Awaiting requested review from gmuloc

Assignees

@carlbuchmann carlbuchmann

Labels
EPIC - AVD AutoVPN/WAN 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
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Feat(eos_cli_config_gen): EOS 4.30 system l1 defaults
3 participants
@carlbuchmann @philippebureau @ClausHolbechArista