-
Notifications
You must be signed in to change notification settings - Fork 235
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
Add support for configuring interface_profiles and setting them on et… #983
Add support for configuring interface_profiles and setting them on et… #983
Conversation
49bdf13
to
10d551b
Compare
Rebased to current devel and added the Molecule test for the new Didn't expect the rest of the configs to change, but there was actually a |
Hello @quulah
I will go ahead and remove other references in Besides that, all the PR is nice ! |
Actually, I tried to push, but seems you have not enabled push from maintainers. So, you can simply remove all references to |
👍 Did that, should be sorted out now. |
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.
All templates and CI definition look good. But interface-profile is missing from master templates for configuration and documentation.
Please add entry to cover your feature in global rendering. Indeed, output of CI is not rendering your unit test.
Good catch! Fixed now. |
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!
Thanks
As per Molecule, it seems there is a conflict with a key ingested by Will take it on hold, time to review and see how to fix that with @carlbuchmann and @ClausHolbechArista |
@titom73 Not sure why, but for some reason we have always set the |
For AVD3.0 we could consider renaming the AVD "profiles" to "templates" to avoid the naming conflict. |
Conflicts have been resolved. A maintainer will review the pull request shortly. |
PR #1008 has merged which removed the unintentional profiles. Please rerun molecule and commit updated artifacts. |
…hernet_interfaces
b1cf512
to
d17d1ab
Compare
Rebased and pushed as per @ClausHolbechArista request. Something funky going on in the pipelines though:
|
Yeah we tried to fix it with #1023 |
nevermind. I found another link. Time for weekend! :) |
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
Types of changes
Related Issue(s)
Related to #979
Requires merge of PR #1008 first.
Component(s) name
arista.avd.eos_cli_config_gen
Proposed changes
interface profile
parameters as well as setting them on Ethernet interfacesHow to test
Checklist:
pre-commit
,make linting
andmake sanity-lint
).