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_designs): l3ls: svi_profiles should support referencing a base svi_profile #1531

Merged
merged 9 commits into from
Mar 14, 2022
Merged

Feat(eos_designs): l3ls: svi_profiles should support referencing a base svi_profile #1531

merged 9 commits into from
Mar 14, 2022

Conversation

c-po
Copy link
Contributor

@c-po c-po commented Feb 21, 2022

Change Summary

Similar to #875 but instead of a port profile support for SVI profiles should be extended to support 2 stage nested profiles.

Related Issue(s)

Fixes #1370

Component(s) name

arista.avd.eos_designs

Proposed changes

svi_profiles:
  < profile_name >:
    parent_profile: < parent svi profile name >
    ...

How to test

molecule

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)

@c-po c-po requested a review from a team as a code owner February 21, 2022 16:05
@github-actions github-actions bot added role: eos_designs issue related to eos_designs role state: CI Updated CI scenario have been updated in the PR state: Documentation role Updated labels Feb 21, 2022
@tgodaA
Copy link
Contributor

tgodaA commented Feb 21, 2022 •
edited
Loading

The PR title should start with Feat or Fix.

@c-po c-po changed the title eos_designs: l3ls: svi_profiles should support referencing a base svi_profile Feat(eos_designs): l3ls: svi_profiles should support referencing a base svi_profile Feb 21, 2022
@tgodaA
Copy link
Contributor

tgodaA commented Mar 3, 2022

run molecule converge -s evpn_underlay_ebgp_overlay_ebgp and then molecule converge -s eos_designs-twodc-5stage-clos. This should fix the artifacts.

@c-po
Copy link
Contributor Author

c-po commented Mar 7, 2022

Anything missing from my side?

@c-po
Copy link
Contributor Author

c-po commented Mar 7, 2022

Thank you for the feedback @ClausHolbechArista - will add the changes

@xaviramon xaviramon mentioned this pull request Mar 7, 2022
Closed
1 task
xaviramon
xaviramon previously approved these changes Mar 7, 2022
View reviewed changes
Copy link
Contributor

@xaviramon xaviramon left a comment

Choose a reason for hiding this comment

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

LGTM. Tested and it and works as you mentioned and expected.
After discussion with Claus I filled bug #1567 to address a behaviour that I consider unexpected, where not all parameters defined in the profile are inherited to the SVI.

@c-po c-po force-pushed the issue-1370-svi-profile branch 2 times, most recently from 5cae0f9 to 66bc360 Compare March 7, 2022 16:08
@c-po
Copy link
Contributor Author

c-po commented Mar 7, 2022

Added all the requested changed from @ClausHolbechArista

@xaviramon xaviramon self-requested a review March 7, 2022 22:16
@ClausHolbechArista ClausHolbechArista dismissed xaviramon’s stale review March 14, 2022 13:11

Too many changes. Please review again

ClausHolbechArista
ClausHolbechArista approved these changes Mar 14, 2022
View reviewed changes
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

@ClausHolbechArista ClausHolbechArista requested a review from a team March 14, 2022 13:52
emilarista
emilarista approved these changes Mar 14, 2022
View reviewed changes
Copy link
Contributor

@emilarista emilarista 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 d897537 into aristanetworks:devel Mar 14, 2022
@c-po c-po deleted the issue-1370-svi-profile branch March 24, 2022 07:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@emilarista emilarista emilarista approved these changes

@ClausHolbechArista ClausHolbechArista ClausHolbechArista approved these changes

@xaviramon xaviramon Awaiting requested review from xaviramon

Assignees

@c-po c-po

Labels
role: eos_designs issue related to eos_designs 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.

eos_designs: l3ls: svi_profiles should support referencing a base svi_profile
5 participants
@c-po @tgodaA @xaviramon @ClausHolbechArista @emilarista