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 - down the interface connection before applying #1644

Closed
wants to merge 1 commit into from

Conversation

aka7
Copy link
Contributor

@aka7 aka7 commented Dec 29, 2023

take down the connection using nmcli before applying changes.

nmstatectl apply does not replace running config instead it seems to append. This is not useful when settings have been removed such as policy routing. Therefore down the interface connection during configuration changes. This will be now similar to original main ncm network module where it runs ifdown and ifup during config changes.
#1643

  • Why the change is necessary.
    Get correct configs applied after a change to profile

  • What backwards incompatibility it may introduce.
    None

nmstatectl apply does not replace running config instead it seems to append.
This is not useful when settings have been removed such as policy routing.
Therefore down the interface connection during configuration changes.
This will be now similar to original main ncm network module where it runs ifdown and ifup during config changes.
@aka7 aka7 force-pushed the ncm_network_nmstate_ifdown branch from 834c8da to 0b9945d Compare January 2, 2024 10:21
@aka7 aka7 assigned jrha Jan 2, 2024
@aka7 aka7 marked this pull request as draft January 3, 2024 11:24
@aka7
Copy link
Contributor Author

aka7 commented Jan 4, 2024

nmstate allows a way clear routes during apply. replaced with.
#1645
so I think we can close this unless anyone thinks we should also down the interface before applying, just like in ifcfg approach? would love to get some input in this? @ned21

@aka7
Copy link
Contributor Author

aka7 commented Jan 11, 2024

closing PR in favor of one, #1647

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.

3 participants