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

Refactor(eos_designs): Setting ospf.area default values to 0.0.0.0 #4536

Merged

Conversation

Shivani-gslab
Copy link
Contributor

@Shivani-gslab Shivani-gslab commented Oct 1, 2024

Change Summary

OSPF default values should default to "0.0.0.0" since that is how EOS translates "0"

Related Issue(s)

Fixes #3006

Component(s) name

arista.avd.eos_designs

Proposed changes

Setting ospf.area default values to 0.0.0.0 in schema and code.

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 state: CI Updated CI scenario have been updated in the PR state: Documentation role Updated role: eos_designs issue related to eos_designs role labels Oct 1, 2024
Copy link

github-actions bot commented Oct 1, 2024

Review docs on Read the Docs

To test this pull request:

# Create virtual environment for this testing below the current directory
python -m venv test-avd-pr-4536
# Activate the virtual environment
source test-avd-pr-4536/bin/activate
# Install all requirements including PyAVD
pip install "pyavd[ansible] @ git+https://github.com/Shivani-gslab/avd.git@ospf_default_to_0.0.0.0#subdirectory=python-avd" --force
# Point Ansible collections path to the Python virtual environment
export ANSIBLE_COLLECTIONS_PATH=$VIRTUAL_ENV/ansible_collections
# Install Ansible collection
ansible-galaxy collection install git+https://github.com/Shivani-gslab/avd.git#/ansible_collections/arista/avd/,ospf_default_to_0.0.0.0 --force
# Optional: Install AVD examples
cd test-avd-pr-4536
ansible-playbook arista.avd.install_examples

@github-actions github-actions bot added the type: documentation Improvements or additions to documentation label Oct 1, 2024
@Shivani-gslab Shivani-gslab force-pushed the ospf_default_to_0.0.0.0 branch from 7085ad6 to 69d83df Compare October 1, 2024 10:34
@Shivani-gslab Shivani-gslab force-pushed the ospf_default_to_0.0.0.0 branch from ed7ea6e to f690d2c Compare October 1, 2024 11:45
@MaheshGSLAB MaheshGSLAB added this to the v5.0.0 milestone Oct 1, 2024
@Shivani-gslab Shivani-gslab marked this pull request as ready for review October 3, 2024 07:31
@Shivani-gslab Shivani-gslab requested review from a team as code owners October 3, 2024 07:31
@gmuloc
Copy link
Contributor

gmuloc commented Oct 3, 2024

@Shivani-gslab we have a couple of places in our molecule scenarios where the value was hardcoded to 0 for area under tenants:

./eos_designs-mpls-isis-sr-ldp/inventory/group_vars/MPLS_TENANTS_NETWORKS.yml:              area: 0
./eos_designs-mpls-isis-sr-ldp/inventory/group_vars/MPLS_TENANTS_NETWORKS.yml:              area: 0
./eos_designs_unit_tests/inventory/group_vars/DC1_TENANTS_NETWORKS/Tenant_A.yml:              area: 0
./eos_designs_unit_tests/inventory/group_vars/DC1_TENANTS_NETWORKS/Tenant_A.yml:              area: 0
./eos_designs_unit_tests/inventory/group_vars/DC1_TENANTS_NETWORKS/Tenant_A.yml:              area: 0

I think for the sake of them being example it would be best to update them to 0.0.0.0 in this PR.

Co-authored-by: Vibhu-gslab <[email protected]>
Copy link
Contributor

@alexeygorbunov alexeygorbunov left a comment

Choose a reason for hiding this comment

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

./ansible_collections/arista/avd/molecule/eos_designs-mpls-isis-sr-ldp/documentation/devices/SITE1-LER1.md

285: ip ospf area 0
600:| Ethernet6.100 | 0 | 10 | True |

@Shivani-gslab Shivani-gslab marked this pull request as draft October 4, 2024 05:24
@Shivani-gslab
Copy link
Contributor Author

./ansible_collections/arista/avd/molecule/eos_designs-mpls-isis-sr-ldp/documentation/devices/SITE1-LER1.md

285: ip ospf area 0 600:| Ethernet6.100 | 0 | 10 | True |

it is provided as input here and is not generated by default - https://github.com/aristanetworks/avd/blob/devel/ansible_collections/arista/avd/molecule/eos_designs-mpls-isis-sr-ldp/inventory/group_vars/MPLS_TENANTS_NETWORKS.yml#L79

@Shivani-gslab Shivani-gslab marked this pull request as ready for review October 4, 2024 05:41
@carlbuchmann carlbuchmann self-assigned this Oct 4, 2024
@ClausHolbechArista
Copy link
Contributor

@gmuloc @alexeygorbunov To be clear, we still need to test that giving ospf area 0 still works. So it is ok to have molecule vars with area 0. It is just the default we are changing.

@gmuloc gmuloc closed this Oct 7, 2024
@gmuloc gmuloc reopened this Oct 7, 2024
Copy link

sonarqubecloud bot commented Oct 7, 2024

Please retry analysis of this Pull-Request directly on SonarCloud

@ClausHolbechArista
Copy link
Contributor

Guillaume told me we have some test with another integer area, so that we can keep that and then change all the 0 -> 0.0.0.0 in molecule inputs.

…/inventory/group_vars/DC1_TENANTS_NETWORKS/Tenant_A.yml
@Shivani-gslab Shivani-gslab marked this pull request as draft October 7, 2024 12:23
@Shivani-gslab Shivani-gslab marked this pull request as ready for review October 7, 2024 12:45
Copy link

sonarqubecloud bot commented Oct 7, 2024

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 dismissed ClausHolbechArista’s stale review October 7, 2024 13:26

reviews comments have been addressed

@carlbuchmann carlbuchmann merged commit 5abfaec into aristanetworks:devel Oct 7, 2024
40 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
rn: Refactor(eos_designs) role: eos_designs issue related to eos_designs role state: CI Updated CI scenario have been updated in the PR state: Documentation role Updated type: documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Feat(eos_designs): OSFP default values should default to "0.0.0.0" since that is how EOS translates "0"
8 participants