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): Implement managing RCF. #3798

Merged
merged 16 commits into from
May 3, 2024
Merged

Feat(eos_cli_config_gen): Implement managing RCF. #3798

merged 16 commits into from
May 3, 2024

Conversation

laxmikantchintakindi
Copy link
Contributor

@laxmikantchintakindi laxmikantchintakindi commented Apr 1, 2024 •
edited

Change Summary

Route Control Functions getting traction to replace Route-Maps. They should be managable using eos_cli_config_gen.

Related Issue(s)

Fixes #2891

Component(s) name

arista.avd.eos_cli_config_gen

Proposed changes

Added keys for code units.

How to test

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)

@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 Apr 1, 2024
@laxmikantchintakindi laxmikantchintakindi changed the title Feature(eos_cli_config_gen): Implement managing RCF. Feat(eos_cli_config_gen): Implement managing RCF. Apr 1, 2024
@Shivani-gslab
Copy link
Contributor

Shivani-gslab commented Apr 1, 2024 •
edited

I think we should update the data-model to add separate keys for operations like commit compile etc.
@MaheshGSLAB @Vibhu-gslab

@laxmikantchintakindi
Copy link
Contributor Author

We cannot commit the function without compilation. So we cannot just give true boolean value to commit and false to compile. May be if it stores the function flash after the compile then we can give true to compile and false on commit. Does this work?

@laxmikantchintakindi laxmikantchintakindi marked this pull request as ready for review April 1, 2024 16:58
@ankudinov ankudinov self-requested a review April 2, 2024 07:27
@ankudinov
Copy link
Contributor

@laxmikantchintakindi few questions based on quick PR review:

  1. Was this tested against lab devices with eos_config_deploy_eapi? RCF deployment breaks due to lack of RCF syntax support in eos_config. Are we aiming at CVP only? If yes, this must be agreed and documented.
  2. I don't see any way to apply RCF policy in this PR.
  3. I'd suggest adding more RCF policies to the test. Ideally more realistic. There are few RCF examples in the tech library - may be take them.
  4. Add some detailed description of the planned design and testing expected to the conversation tab of this PR if possible.

@laxmikantchintakindi laxmikantchintakindi marked this pull request as draft April 3, 2024 12:31
@laxmikantchintakindi
Copy link
Contributor Author

@ankudinov I tested this PR with eos_config_deploy_cvp role and RCFs are generated. We have another issue(#2397) opened for of eos_cli_config_gen to apply RCFs to bgp peers.

@laxmikantchintakindi laxmikantchintakindi marked this pull request as ready for review April 5, 2024 06:19
@laxmikantchintakindi laxmikantchintakindi marked this pull request as draft April 8, 2024 07:44
bjmeuer
bjmeuer reviewed Apr 8, 2024
View reviewed changes
Copy link
Contributor

@bjmeuer bjmeuer left a comment

Choose a reason for hiding this comment

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

If you remove these and say it is basically only for CVP than this needs to be documented that RCF push is only working via CVP.

@github-actions github-actions bot added the role: eos_designs issue related to eos_designs role label Apr 16, 2024
@laxmikantchintakindi laxmikantchintakindi marked this pull request as ready for review April 16, 2024 12:28
@github-actions github-actions bot added the state: conflict PR with conflict label Apr 17, 2024
@github-actions GitHub Actions
Copy link

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@github-actions github-actions bot removed the state: conflict PR with conflict label Apr 18, 2024
@github-actions GitHub Actions
Copy link

Conflicts have been resolved. A maintainer will review the pull request shortly.

@laxmikantchintakindi laxmikantchintakindi marked this pull request as draft April 25, 2024 09:42
@laxmikantchintakindi laxmikantchintakindi marked this pull request as ready for review April 25, 2024 10:22
ClausHolbechArista
ClausHolbechArista requested changes Apr 29, 2024
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.

Minor code optimization. Please add documentation template for this as well.

ansible_collections/arista/avd/roles/eos_cli_config_gen/templates/eos/router-general.j2 Outdated Show resolved Hide resolved
@laxmikantchintakindi laxmikantchintakindi marked this pull request as draft April 30, 2024 05:21
@laxmikantchintakindi laxmikantchintakindi marked this pull request as ready for review April 30, 2024 07:27
@ClausHolbechArista ClausHolbechArista requested a review from a team May 3, 2024 06:53
@ClausHolbechArista ClausHolbechArista added the one approval This PR has one approval and is only missing one more. label May 3, 2024
carlbuchmann
carlbuchmann approved these changes May 3, 2024
View reviewed changes
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.

LGTM

@carlbuchmann carlbuchmann merged commit 16d31f9 into aristanetworks:devel May 3, 2024
43 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bjmeuer bjmeuer bjmeuer left review comments

@MaheshGSLAB MaheshGSLAB MaheshGSLAB left review comments

@Shivani-gslab Shivani-gslab Shivani-gslab left review comments

@carlbuchmann carlbuchmann carlbuchmann approved these changes

@ClausHolbechArista ClausHolbechArista ClausHolbechArista approved these changes

@ankudinov ankudinov Awaiting requested review from ankudinov

Assignees

@laxmikantchintakindi laxmikantchintakindi

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 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.

Feature(eos_cli_config_gen): Implement managing RCF
7 participants
@laxmikantchintakindi @Shivani-gslab @ankudinov @carlbuchmann @bjmeuer @ClausHolbechArista @MaheshGSLAB