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 enhancements #1647

Merged
merged 3 commits into from
Feb 20, 2024

Conversation

aka7
Copy link
Contributor

@aka7 aka7 commented Jan 11, 2024

Add the following enhancements:

nmstatectl apply does not replace running config, it instead merges current state config with desired state. This is not useful when settings have been removed such as routing and policy routing and rules.

state: absent will remove config routes entry that matches the interface as next hop and therefore will only apply the routes configured in host profile profile. so following changes added.

  1. absent entry added when route for that interface. will clean all routes matching that interface when applying:

  2. absent entry add route-rules to match any rules with the table id and clear it when applying rules.

  3. made nmstate mimic bootproto setting of network.pm

  4. refactored configuration settings of each interfaces type.

  5. add more advance testing for interface configuration, bonding and valn.

nmstatectl apply merges current route state config with desired state.
Adding state: absent to route config for the interface will clear routes
before adding the desired state instead of merge. Therefore keeping only
the routes configured through the profiles.
@aka7
Copy link
Contributor Author

aka7 commented Jan 11, 2024

@ned21 thank your for review to the recent pr's I created, I thought its best just do one PR because the changes are related, especially with the testing that I've added, it requires all 3 commits. first 2 you already reviewed. hope that is ok. All changes are now in this one.

@jrha please can this be looked into in your own time.

@jrha jrha added this to the 23.12 milestone Jan 12, 2024
@aka7
Copy link
Contributor Author

aka7 commented Jan 23, 2024

any of the maintainers are able to review this? I like to know if I need make any changes before I do a internal release please. or does it look good to you? @jrha @stdweird ?

@jrha jrha changed the title ncm-netwrok: nmstate enhancements ncm-network: nmstate enhancements Jan 25, 2024
- add a default absent rule to clear route rules entries for the table.
  nmstate by default will merge rules therefore won't clear if there are any
  old rules already present. This will make sure only rules defined by profile is present.
- mimic bootproto settings to default to static, to match network.pm.
- refactored the conditions for setting config based on bootproto.
- support to configure dhcp interface.
- fixed the way bonded interface is recognised
- add advance testing for nmstate interface config
@aka7 aka7 force-pushed the ncm_network_nmstate_enhancements branch from 3b6aff5 to 7a72ad9 Compare February 5, 2024 09:08
@aka7
Copy link
Contributor Author

aka7 commented Feb 5, 2024

@jrha made some changes based on what @wpoely86 discovered which I think makes code a bit simpler.
please can you review this please? would be great if we can get this merged.

StephaneGerardVUB added a commit to StephaneGerardVUB/configuration-modules-core that referenced this pull request Feb 5, 2024
Copy link
Member

@jrha jrha left a comment

Choose a reason for hiding this comment

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

Looks like a lot of good improvements here, I'll have more of a think about how best to integrate the nmstate specific options.

@wdpypere
Copy link
Contributor

@aka7 I've been running some tests with this and it's working a lot better now, thanks for the work! However I ran into an issue where nmstate deletes the default gateway 😄 . I've logged it at #1655 because it has little to do with this PR.

@aka7
Copy link
Contributor Author

aka7 commented Feb 19, 2024

@jrha is there anything outstanding on this PR or can this be merged?
I have a follow up PR but need to rebase once this is merge before I create it.

Regards

@jrha jrha merged commit 125da86 into quattor:master Feb 20, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

ncm-network: nmstate backend policy routing removal does not work without downing the interface.
4 participants