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

ncm-network: nmstate - add additional route rule parameters #1659

Merged
merged 2 commits into from
Nov 12, 2024

Conversation

aka7
Copy link
Contributor

@aka7 aka7 commented Feb 20, 2024

provide additional route rule parameters for nmstate config as defined in https://nmstate.io/devel/yaml_api.html#routes

  • Why the change is necessary.
    allow creation of route rules with all options.
  • What backwards incompatibility it may introduce.
    none.

@aka7 aka7 requested a review from jrha February 20, 2024 18:42
@aka7
Copy link
Contributor Author

aka7 commented Feb 20, 2024

@jrha created this for discussion on if there will a way to create sperate schemas for nmstate only options. something to think about. as discussed before.

this isnt urgent but at some point we may need to create unreachable routes and only way to do this with nmstate is through route-rules.

@aka7 aka7 force-pushed the ncm_network_nmstate_rule_param branch from f47225c to 1450ec3 Compare February 20, 2024 19:13
@jrha
Copy link
Member

jrha commented Feb 22, 2024

I'll try and do some testing to establish a way of making the schema modular, but otherwise having these options as a separate nmstate sub-dict would seem cleanest.

@jrha jrha added this to the 24.3 milestone Feb 22, 2024
@jrha
Copy link
Member

jrha commented Sep 19, 2024

@aka7 I've had a look at how to deal with the schema and it's probably easiest for me to open a PR against your branch.

@stdweird
Copy link
Member

@jrha @aka7 let me know about the PR. but should be something like

in the profile:

  • set QUATTOR_TYPES_NETWORK_BACKEND to nmstate
  • set QUATTOR_TYPES_NETWORK_LEGACY to false

you should be able to do this for at least all el9 hosts.
best to compile and diff the profiles with and without these set. it should be identical

once you did this, you can start to customise the backend schemas:
typically you add the same type to both backends, but eg on initiscripts https://github.com/quattor/configuration-modules-core/blob/master/ncm-network/src/main/pan/components/network/types/network/backend/initscripts.pan you leave it empty

type network_very_special {}

and in the nmstate backend https://github.com/quattor/configuration-modules-core/blob/master/ncm-network/src/main/pan/components/network/types/network/backend/nmstate.pan you define what you want

type network_very_special {
   woohoo: boolean

these special backend templates are included very early in https://github.com/quattor/configuration-modules-core/blob/master/ncm-network/src/main/pan/components/network/types/network.pan (this is used insetad of the core-legacy schema if you set the previous variables).

then eg in the "shared" route schema https://github.com/quattor/configuration-modules-core/blob/master/ncm-network/src/main/pan/components/network/types/network/route.pan you add it like

type network_route = {
    include network_very_special

and now the nmstate profiles will have support for the woohoo attribute

make sure you have a very unique type name for the type you want to include. the should not have the backend in it (even if it is only implemented for a single backend).

(anyway, untested ;)

@jrha
Copy link
Member

jrha commented Sep 20, 2024

Schema migration in #1720.

@jrha jrha force-pushed the ncm_network_nmstate_rule_param branch from b5de236 to 554c477 Compare November 7, 2024 15:34
@jrha
Copy link
Member

jrha commented Nov 7, 2024

Rebased and updated to use new schema.

@jrha jrha force-pushed the ncm_network_nmstate_rule_param branch 2 times, most recently from 12deca5 to fd12f22 Compare November 7, 2024 16:05
@jrha jrha marked this pull request as ready for review November 7, 2024 16:33
@jrha jrha requested a review from stdweird November 8, 2024 17:24
@aka7
Copy link
Contributor Author

aka7 commented Nov 8, 2024

thank you so much @jrha , LGTM

@jrha jrha force-pushed the ncm_network_nmstate_rule_param branch from 1b2eedd to e0aa8c4 Compare November 8, 2024 17:56
@jrha
Copy link
Member

jrha commented Nov 8, 2024

@aka7 I've locked down the types as per my previous comments, if it all makes sense then I'm happy to try and also get this into 24.10.

@aka7
Copy link
Contributor Author

aka7 commented Nov 8, 2024

@aka7 I've locked down the types as per my previous comments, if it all makes sense then I'm happy to try and also get this into 24.10.

@jrha yes please that make sense.

@jrha jrha merged commit fde6175 into quattor:master Nov 12, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

3 participants