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): Flow tracking table size #2838

Merged
merged 13 commits into from
Jul 25, 2023

Conversation

chetryan
Copy link
Contributor

Adds functionality to specify the flow tracking table size.

Change Summary

At the moment, specifying the flow tracking table size for a flow tracker is cumbersome as it requires using eos_cli and specifying the context commands to get to the flow tracker context.

This PR makes it easy to specify the optional table size.

Related Issue(s)

No Tickets currently raised for this.

Component(s) name

arista.avd.eos_cli_conifg_gen

Proposed changes

  • Update flow tracking schema
  • Update flow tracking eos template
  • Update the flow tracking documentation template

How to test

  • run molecule test -s eos_cli_config_gen

Checklist

User Checklist

  • [] Does the updated Documentation make sense?
  • [] Is the updated schema easy to undrstand?

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)

Adds functionality to specify the flow tracking table size.
@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 May 15, 2023
@chetryan chetryan marked this pull request as ready for review May 17, 2023 11:02
@chetryan chetryan requested a review from a team as a code owner May 17, 2023 11:02
@github-actions github-actions bot added the state: conflict PR with conflict label May 19, 2023
@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 May 22, 2023
@github-actions
Copy link

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

@carlbuchmann carlbuchmann added this to the v4.1.0 milestone May 30, 2023
@github-actions github-actions bot added the state: conflict PR with conflict label Jun 13, 2023
@github-actions
Copy link

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

@chetryan chetryan requested a review from a team as a code owner June 16, 2023 12:57
@github-actions github-actions bot removed the state: conflict PR with conflict label Jun 16, 2023
@github-actions
Copy link

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

Adds functionality to specify the flow tracking table size.
type: int
convert_types:
- str
min: 1
Copy link
Contributor

Choose a reason for hiding this comment

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

if we know the maximum value, please set it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maximum depends on the platform. I have not found documentation of a definite upper limit, but on the platforms I tested, the minimum was always 1, hence why I defined a minimum but no max.

Copy link
Contributor

Choose a reason for hiding this comment

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

Can we add a max at 614400 (cf internal discussion with link to eos-trunk model) and add in the description that the max is platform dependent ?

chetryan and others added 5 commits July 3, 2023 16:34
…s/schema_fragments/flow_trackings.schema.yml

Co-authored-by: Claus Holbech <holbech@arista.com>
…tes/documentation/flow-trackings.j2

Co-authored-by: Claus Holbech <holbech@arista.com>
@ClausHolbechArista ClausHolbechArista requested a review from a team July 4, 2023 09:08
@@ -12,6 +12,8 @@ flow_trackings:
mpls: true
# no exporter
- name: T2
# table size as string
table_size: "999999"
Copy link
Contributor

Choose a reason for hiding this comment

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

I have read the close comments about the upper limit being platform dependent.
However can we use in the molecule test some values that would be accepted on vEOS / cEOS in case we start trying the configs on such devices:

vEOS 4.27.0F output:

Spine1(config-ftr-sampled-tr-T2)#flow table size ?
  <1-614400>  Maximum number of entries in flow table

type: int
convert_types:
- str
min: 1
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we add a max at 614400 (cf internal discussion with link to eos-trunk model) and add in the description that the max is platform dependent ?

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 cd099ff into aristanetworks:devel Jul 25, 2023
35 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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.

None yet

4 participants