Skip to content

T7510: ospfd.frr.j2 ospf nssa translation error - fix template #4536

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

Merged
merged 3 commits into from
Jun 10, 2025

Conversation

ig0rb
Copy link
Contributor

@ig0rb ig0rb commented May 31, 2025

Change summary

Hi, I have updated ospf frr template to fix nssa translation

Types of changes

Jinja2 template patching as suggested in the forum.
https://forum.vyos.io/t/ospf-area-type-nssa-translate-never/16593/8

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes)
  • Migration from an old Vyatta component to vyos-1x, please link to related PR inside obsoleted component
  • Other (please describe):

Related Task(s)

Related PR(s)

How to test / Smoketest result

vyos@vyos:/tmp$ sh conf commands | grep ospf
set protocols ospf area 0.0.0.10 area-type nssa translate 'never'
vyos@vyos:/tmp$ vtysh -e "show run ospfd no-header"
frr version 10.2.2
frr defaults traditional
hostname localhost
log syslog
log facility local7
hostname vyos
service integrated-vtysh-config
!
router ospf
 auto-cost reference-bandwidth 100
 timers throttle spf 200 1000 10000
 area 0.0.0.10 nssa translate-never
exit
!
end
vyos@vyos:/tmp$ conf 
WARNING: You are currently configuring a live-ISO environment, changes will not persist until installed
[edit]
vyos@vyos# set protocols ospf area 0.0.0.10 area-type nssa translate always 
[edit]
vyos@vyos# commit
[edit]
vyos@vyos# exit
Warning: configuration changes have not been saved.
exit
vyos@vyos:/tmp$ vtysh -e "show run ospfd no-header"
frr version 10.2.2
frr defaults traditional
hostname localhost
log syslog
log facility local7
hostname vyos
service integrated-vtysh-config
!
router ospf
 auto-cost reference-bandwidth 100
 timers throttle spf 200 1000 10000
 area 0.0.0.10 nssa translate-always
exit
!
end
vyos@vyos:/tmp$ conf
WARNING: You are currently configuring a live-ISO environment, changes will not persist until installed
[edit]
vyos@vyos# del protocols ospf area 0.0.0.10 area-type nssa translate
[edit]
vyos@vyos# commit
[edit]
vyos@vyos# exit
Warning: configuration changes have not been saved.
exit
vyos@vyos:/tmp$ vtysh -e "show run ospfd no-header"
frr version 10.2.2
frr defaults traditional
hostname localhost
log syslog
log facility local7
hostname vyos
service integrated-vtysh-config
!
router ospf
 auto-cost reference-bandwidth 100
 timers throttle spf 200 1000 10000
 area 0.0.0.10 nssa
exit
!
end
vyos@vyos:/tmp$ 

Checklist:

  • I have read the CONTRIBUTING document
  • I have linked this PR to one or more Phabricator Task(s)
  • I have run the components SMOKETESTS if applicable
  • My commit headlines contain a valid Task id
  • My change requires a change to the documentation
  • I have updated the documentation accordingly

Copy link

github-actions bot commented May 31, 2025

👍
No issues in PR Title / Commit Title

Copy link
Member

@c-po c-po left a comment

Choose a reason for hiding this comment

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

Please also update the smoketests to check if the entry is available in Frr config when specified on the CLI.

https://docs.vyos.io/en/latest/contributing/testing.html#manual-smoketest-run

@c-po c-po added bp/sagitta Create automatic backport for sagitta LTS version bp/circinus Create automatic backport for circinus labels Jun 2, 2025
@ig0rb
Copy link
Contributor Author

ig0rb commented Jun 4, 2025

Please also update the smoketests to check if the entry is available in Frr config when specified on the CLI.

https://docs.vyos.io/en/latest/contributing/testing.html#manual-smoketest-run

Hi, sorry, if my question look stupid, did I need to modify "test_protocols_ospf.py" to check the command ?

@c-po
Copy link
Member

c-po commented Jun 4, 2025

Hi, sorry, if my question look stupid, did I need to modify "test_protocols_ospf.py" to check the command ?

Yes, there you can add the code path to perform the smoketest. Set the CLI node and validate that it's present int the resulting FRR configuration.

ig0rb and others added 3 commits June 8, 2025 08:44
To keep existing CLI behavior use a Warning() to prompt the user for an invalid
configuration. It is not possible to have more the one area-type defined per
area logically - the CLI does support it. In addition the backbone area cannot
be of type STUB or NSSA.

CLI configuration should be cleaned up using a migrator in the future.
@c-po c-po force-pushed the fix/T7510-ospf-nssa-translation-error branch from f696473 to 323ef23 Compare June 8, 2025 07:29
Copy link
Member

@c-po c-po left a comment

Choose a reason for hiding this comment

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

Implemented smoketests:

$ /usr/libexec/vyos/tests/smoke/cli/test_protocols_ospf.py
test_ospf_18_area_translate_no_summary (__main__.TestProtocolsOSPF.test_ospf_0118_area_translate_no_summary) ... ok
test_ospf_01_defaults (__main__.TestProtocolsOSPF.test_ospf_01_defaults) ... ok
test_ospf_02_simple (__main__.TestProtocolsOSPF.test_ospf_02_simple) ... ok
test_ospf_03_access_list (__main__.TestProtocolsOSPF.test_ospf_03_access_list) ... ok
test_ospf_04_default_originate (__main__.TestProtocolsOSPF.test_ospf_04_default_originate) ... ok
test_ospf_05_options (__main__.TestProtocolsOSPF.test_ospf_05_options) ... ok
test_ospf_06_neighbor (__main__.TestProtocolsOSPF.test_ospf_06_neighbor) ... ok
test_ospf_07_redistribute (__main__.TestProtocolsOSPF.test_ospf_07_redistribute) ... ok
test_ospf_08_virtual_link (__main__.TestProtocolsOSPF.test_ospf_08_virtual_link) ... ok
test_ospf_09_interface_configuration (__main__.TestProtocolsOSPF.test_ospf_09_interface_configuration) ... ok
test_ospf_11_interface_area (__main__.TestProtocolsOSPF.test_ospf_11_interface_area) ... ok
test_ospf_12_vrfs (__main__.TestProtocolsOSPF.test_ospf_12_vrfs) ... ok
test_ospf_13_export_list (__main__.TestProtocolsOSPF.test_ospf_13_export_list) ... ok
test_ospf_14_segment_routing_configuration (__main__.TestProtocolsOSPF.test_ospf_14_segment_routing_configuration) ... ok
test_ospf_15_ldp_sync (__main__.TestProtocolsOSPF.test_ospf_15_ldp_sync) ... ok
test_ospf_16_graceful_restart (__main__.TestProtocolsOSPF.test_ospf_16_graceful_restart) ... ok
test_ospf_17_duplicate_area_network (__main__.TestProtocolsOSPF.test_ospf_17_duplicate_area_network) ... ok

----------------------------------------------------------------------
Ran 17 tests in 271.259s

OK

@c-po c-po requested a review from sever-sever June 8, 2025 07:30
Copy link

github-actions bot commented Jun 8, 2025

CI integration 👍 passed!

Details

CI logs

  • CLI Smoketests (no interfaces) 👍 passed
  • CLI Smoketests (interfaces only) 👍 passed
  • Config tests 👍 passed
  • RAID1 tests 👍 passed
  • TPM tests 👍 passed

Copy link
Member

@dmbaturin dmbaturin left a comment

Choose a reason for hiding this comment

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

A straightforward addition, covered by smoke tests. No objections.

@dmbaturin dmbaturin merged commit a3b5108 into vyos:current Jun 10, 2025
13 of 14 checks passed
@github-actions github-actions bot added the mirror-initiated This PR initiated for mirror sync workflow label Jun 10, 2025
@vyosbot vyosbot added mirror-completed and removed mirror-initiated This PR initiated for mirror sync workflow labels Jun 10, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bp/circinus Create automatic backport for circinus bp/sagitta Create automatic backport for sagitta LTS version current mirror-completed
Development

Successfully merging this pull request may close these issues.

4 participants