Bug 1756687

Hi Dan,

The `libnmstate.apply(state, commit=False, rollback_timeout=60)` will revert back to the network state after 60 seconds.

You can also either `libnmstate.commit()` or `libnmstate.rollback()` to make it disappear or permanent.

Does that fit your need?

Regarding the boot stage, are you talking about initd ? If so, NetworkManager does not provides dbus in initd hence nmstate
cannot work in initd yet.

The networkmanager in initd can change kernel-only(not saved in profile) state via nm-initrd-generator
https://developer.gnome.org/NetworkManager/stable/nm-initrd-generator.html

I can express my request as

   libnmstate.apply(state, commit=False, rollback_timeout=NEXT_BOOT)

no commit takes place implicitly; rollback takes place during (just before?) next boot. I did not understand what you said about initrd.

(In reply to Dan Kenigsberg from comment #2)
> I can express my request as
> 
>    libnmstate.apply(state, commit=False, rollback_timeout=NEXT_BOOT)
> 
> no commit takes place implicitly; rollback takes place during (just before?)
> next boot. I did not understand what you said about initrd.

IMHO this approach would have some problems
- The uncommitted/unreverted checkpoint will block any later attempts to change the network state
- Currently the changes are already persisted but reverting the checkpoint will persist the old configuration. Therefore a crash/unexpected reboot would result in the system booting the originally uncommited changes
- There is nothing currently running at shutdown that could handle this

NM supports to change active profiles only in memory instead of persisting them on disk. I guess this should be used here instead. However, given that Nmstate supports partial changes by combining the specified state with the current (runtime) state, this might make it rather complex to understand what is happening. For example:

1) on-disk state: IP is defined on ethernet interface
2) runtime change: Move IP to OVS port
3) later (partial) change request for on-disk state: change MTU of ethernet interface - this would now also persist the changes to the IP configuration

Hi Dan Kenigsberg,

Supporting `libnmstate.apply(state, memory_only=True)` is possible.

Before I ack this RFE, need to clarify thing out:

 * The memory only profiles will be dropped when NetworkManager restart.

 * The memory only `apply()` will not touch existing on-disk profile.
   Which means after reboot, the on-disk profile will be activated.

 * After any normal `apply()` been invoked, the memory only profile of specific interfaces will be dropped.

 * For memory only master profiles, all changes to its slave profiles will be memory only also.

 * The `libnmstate.show()` will only show runtime(memory) status, not config after reboot.
   do you need new API like `libnmstate.show_saved_config()`? If so, Can it wait RHEL 8.4?

(In reply to Till Maas from comment #3)
> (In reply to Dan Kenigsberg from comment #2)
> > I can express my request as
> > 
> >    libnmstate.apply(state, commit=False, rollback_timeout=NEXT_BOOT)
> > 
> > no commit takes place implicitly; rollback takes place during (just before?)
> > next boot. I did not understand what you said about initrd.
> 
> IMHO this approach would have some problems
> - The uncommitted/unreverted checkpoint will block any later attempts to
> change the network state

Ok, I did not know that you cannot modify the current state if it was unccommitted.

> - Currently the changes are already persisted but reverting the checkpoint
> will persist the old configuration. Therefore a crash/unexpected reboot
> would result in the system booting the originally uncommited changes

The whole point of this request is to apply a possibly-dangerous new state. If it is so bad that it causes the node to be lost, I would like the node to be revived to the last known-good committed changes. If on NEXT_BOOT we have uncommitted changes, it is a bug.

> - There is nothing currently running at shutdown that could handle this

No code should be expected to run on shutdown. The host may crash at any moment. My request is that after next boot, we're back at the formerly-committed state.

> 
> NM supports to change active profiles only in memory instead of persisting
> them on disk. I guess this should be used here instead. However, given that
> Nmstate supports partial changes by combining the specified state with the
> current (runtime) state, this might make it rather complex to understand
> what is happening. For example:
> 
> 1) on-disk state: IP is defined on ethernet interface
> 2) runtime change: Move IP to OVS port
> 3) later (partial) change request for on-disk state: change MTU of ethernet
> interface - this would now also persist the changes to the IP configuration

I think that I am not qualified to speak about the complexity of the implementation.


(In reply to Gris Ge from comment #4)
> Hi Dan Kenigsberg,
> 
> Supporting `libnmstate.apply(state, memory_only=True)` is possible.
> 
> Before I ack this RFE, need to clarify thing out:
> 
>  * The memory only profiles will be dropped when NetworkManager restart.

I don't quite like this semantics. I was asking for rollback_timeout=NEXT_BOOT, not rollback_timeout=NEXT_DAEMON_RESTART
But I suppose it would be good enough - assuming NM does not crash too often.

> 
>  * The memory only `apply()` will not touch existing on-disk profile.
>    Which means after reboot, the on-disk profile will be activated.

This sounds like a valid implementation

> 
>  * After any normal `apply()` been invoked, the memory only profile of
> specific interfaces will be dropped.

I don't understand this point.

> 
>  * For memory only master profiles, all changes to its slave profiles will
> be memory only also.

This one is too technical to me, too.

> 
>  * The `libnmstate.show()` will only show runtime(memory) status, not config
> after reboot.
>    do you need new API like `libnmstate.show_saved_config()`?

yes, I think that it would make a lot of sense to have (possibly multiple) registers of configurations. E.g libnmstate.show(configname=MEMORY_ONLY)

>  If so, Can it
> wait RHEL 8.4?

I suppose so. I'll leave this to current maintainers of knmstate to discuss.

(In reply to Dan Kenigsberg from comment #5)
> (In reply to Till Maas from comment #3)
> > (In reply to Dan Kenigsberg from comment #2)
> > > I can express my request as
> > > 
> > >    libnmstate.apply(state, commit=False, rollback_timeout=NEXT_BOOT)
> > > 
> > > no commit takes place implicitly; rollback takes place during (just before?)
> > > next boot. I did not understand what you said about initrd.
> > 
> > IMHO this approach would have some problems
> > - The uncommitted/unreverted checkpoint will block any later attempts to
> > change the network state
> 
> Ok, I did not know that you cannot modify the current state if it was
> unccommitted.

This is the locking feature as described in:
https://bugzilla.redhat.com/show_bug.cgi?id=1694961


> 
> > - Currently the changes are already persisted but reverting the checkpoint
> > will persist the old configuration. Therefore a crash/unexpected reboot
> > would result in the system booting the originally uncommited changes
> 
> The whole point of this request is to apply a possibly-dangerous new state.
> If it is so bad that it causes the node to be lost, I would like the node to
> be revived to the last known-good committed changes. If on NEXT_BOOT we have
> uncommitted changes, it is a bug.

This bug seems to be about a different scenario. Could you please file a new bug against Nmstate for this so we can discuss this there. Thank you.

(In reply to Till Maas from comment #6)

> > 
> > The whole point of this request is to apply a possibly-dangerous new state.
> > If it is so bad that it causes the node to be lost, I would like the node to
> > be revived to the last known-good committed changes. If on NEXT_BOOT we have
> > uncommitted changes, it is a bug.
> 
> This bug seems to be about a different scenario. Could you please file a new
> bug against Nmstate for this so we can discuss this there. Thank you.

I filed this bug on 2019-09-29, and it is all about booting from a cleanly-committed version, with no uncommitted changes. I don't understand which other bug you want me to file.

(In reply to Dan Kenigsberg from comment #7)
> (In reply to Till Maas from comment #6)
> 
> > > 
> > > The whole point of this request is to apply a possibly-dangerous new state.
> > > If it is so bad that it causes the node to be lost, I would like the node to
> > > be revived to the last known-good committed changes. If on NEXT_BOOT we have
> > > uncommitted changes, it is a bug.
> > 
> > This bug seems to be about a different scenario. Could you please file a new
> > bug against Nmstate for this so we can discuss this there. Thank you.
> 
> I filed this bug on 2019-09-29, and it is all about booting from a
> cleanly-committed version, with no uncommitted changes. I don't understand
> which other bug you want me to file.

I was looking at the initial description, it stated:

| I would like to configure my host so that its primary IP address is assigned to an OvS port. However, during boot time, I may not have OvS available. Therefore, I would like to keep the persisted boot-time network configuration unchanged, and modify only the currently-effective network configuration.

This sounds like a feature to me and does not describe any expectations regarding the commit/rollback feature.

But I also understood that you expect that if a system crashed after setting "nmstatectl set --no-commit some_state.yml", the changes from some_state.yml should not be visible.

This might be a bug but I start to remember that the behavior regarding persistent configuration was not strictly defined since there was the assumption that something else like will use nmstate to apply the desired runtime state. If the current behavior is not a bug from your POV, I am fine with this.

Till, I feel that we are speaking in parallel planes. I don't know much about current implementation. I don't want to talk about it before the requirements are clear. My request has not changed since comment 0.

> I would like to configure my host so that its primary IP address is assigned to an OvS port. However, during boot time, I may not have OvS available. Therefore, I would like to keep the persisted boot-time network configuration unchanged, and modify only the currently-effective network configuration.

But I would like to stress that the persisted boot-time is kept no matter how the node ended up rebooting. I don't care if it crashed or shut down orderly; it should boot with the known-good persisted configuration, unharmed by changes to currently-effective configuration.

Would you confirm that this request is clear?

(In reply to Dan Kenigsberg from comment #9)
> Till, I feel that we are speaking in parallel planes. I don't know much
> about current implementation. I don't want to talk about it before the
> requirements are clear. My request has not changed since comment 0.

Good, just to be clear, we can ignore the request in comment:2 that proposed an implementation detail that raises problems?


Regarding this:

> > - Currently the changes are already persisted but reverting the checkpoint
> > will persist the old configuration. Therefore a crash/unexpected reboot
> > would result in the system booting the originally uncommited changes
> 
> The whole point of this request is to apply a possibly-dangerous new state.
> If it is so bad that it causes the node to be lost, I would like the node to
> be revived to the last known-good committed changes. If on NEXT_BOOT we have
> uncommitted changes, it is a bug.

The behavior that I described above was designed with your involvement (AFAIR), it is shown in demo 4: https://nmstate.github.io/demos.html
So, I understand this, now, but might have misunderstood it before:

1) The described behavior would be a bug if it was a solution to the request you made her.
2) The described behavior is not a problem in the current situation/the way it was requested for vdsm.


> > I would like to configure my host so that its primary IP address is assigned to an OvS port. However, during boot time, I may not have OvS available. Therefore, I would like to keep the persisted boot-time network configuration unchanged, and modify only the currently-effective network configuration.
> 
> But I would like to stress that the persisted boot-time is kept no matter
> how the node ended up rebooting. I don't care if it crashed or shut down
> orderly; it should boot with the known-good persisted configuration,
> unharmed by changes to currently-effective configuration.
> 
> Would you confirm that this request is clear?

Yes, the idea here is clear. 
Let's define the configuration that is active till shutdown as runtime configuration and the configuration that will be loaded on boot as persisted configuration. The runtime state is the state of the system/the Linux kernel.

Still it needs clarification how the runtime and persistent configuration should (and can) affect each other. Open questions that Gris and I have are AFAIU:

Do you require that changes to the persisted configuration
a) do not change the runtime configuration
b) change the runtime configuration, too

Or do you not care about this relationship?

Do you need to be able to query
a) the runtime configuration
b) the persistent configuration
c) the runtime state

Currently, nmstate is required to show the runtime state and not the configured state and the non-well defined requirement is that changes to the configuration change the runtime state and might also change the persistent configuration.


In case you required this in the previous question:
Currently, nmstate verifies that the requested state matches the runtime state. If you request changes to the persistent state (but now for the runtime state), what kind of verification do you expect for the persistent state?
Currently, nmstate supports partial states when setting a state and complements it with the current runtime state. What do you expect to happen when a partial state for the persistent configuration is provided?
a) Should only these changes be added to the persistent configuration
b) should these changes + the runtime configuration be added to the persistent configuration
c) Should these changes + the runtime state be added to the persistent configuration

Please note that nmstate only verifies the runtime configuration against the runtime state at the time the runtime configuration is applied. Afterwards something might change the runtime state without changing the runtime configuration. Therefore they might not be in sync when only the persistent configuration is changed.

It would be great if we could get concise user stories about the expected behavior but we can also try to build them together.

Just realized, that there might actually be no requirement for Nmstate to change the on-disk/boot-time configuration but only the runtime configuration. What are you requirements regarding changes to the boot-time configuration? Would it be enough if Nmstate supports only to change the runtime configuration and to make the current runtime configuration persistent? Maybe this is where we are out of sync.

(In reply to Till Maas from comment #11)
> Just realized, that there might actually be no requirement for Nmstate to
> change the on-disk/boot-time configuration but only the runtime
> configuration. What are you requirements regarding changes to the boot-time
> configuration? Would it be enough if Nmstate supports only to change the
> runtime configuration and to make the current runtime configuration
> persistent? Maybe this is where we are out of sync.

Right. I am fine with runtime config being a staging area for the boottime one. I'd like to modify the runtime config, and at one point commit it to the boottime config (or have it completely reverted on reboot).

It could be cooler if we had multiple named configs, which can be edited independently, with "boottime" and "runtime" being only two special ones. This, however, is not a strict requirement of this RFE.

What we can do for this RFE:

 * The default behaviour of `libnmstate.apply()` will not change, still be persistent on disk.
 * New option `memory_only=True` added to `libnmstate.apply()` allowing network state been persistent tile system reboot.
 * NetworkManager daemon restart will not discard memory only state.
 * If anyone want to convert memory-only state to on-disk persistent state, they need to apply the same state with `memory_only=False` again.
 * With `memory_only=True`, can add interface, change interface.
 * With `memory_only=True`, can delete the virtual interface but on-disk profile of that interface will not be impacted.
 * With `memory_only=True`, can bring interface down(state:down) with on-disk profile un-touched.
 * With `memory_only=True`, can change route, route rule, and DNS.
 * With `memory_only=True`, cannot delete a profile(state:absent) for certain interface.

Hi Dan,

Does above meet your initial need on this managing memory only network state?

If so, I will approve this RFE and create sub-RFE to split the works.

I am afraid we don't have capacity to support the different type of network state(running state, running config, saved config)
in RHEL 8.3. I have created a dedicate RFE to track it https://bugzilla.redhat.com/show_bug.cgi?id=1817925 in RHEL 8.4.

Gris, can you explain

 * With `memory_only=True`, cannot delete a profile(state:absent) for certain interface.

? Which are the "certain interface" this is impossible to do? state:absent seems like a critical requirement to me.

A typical use case for this whole feature is the following. I have a vlan device configured on multiple hosts. I'd like to remove this device from all hosts, but I would like to to first verify that the hosts keep their mutual connectivity in the new configuration. I want to make sure that I don't end up with some hosts using the vlan, and some others use native. So I remove all vlan devices with memory_only=True, do my network-wide validation, and then repeat with memory_only=False.

Would I still be able to do that?

(In reply to Dan Kenigsberg from comment #14)
> Gris, can you explain
> 
>  * With `memory_only=True`, cannot delete a profile(state:absent) for
> certain interface.

The `memory_only` action cannot delete on-disk profils.

You may use `state:down` to deactivate a on-disk profile. 
For software interface, it will got removed from kernel.
Next boot, NM will create it using on-disk profile.

> 
> ? Which are the "certain interface" this is impossible to do? state:absent
> seems like a critical requirement to me.

The `certain interface` is the specificed interface. The memory only action
cannot delete a on-disk profile, but can deactivate it and create new overriding it
in memory.

> 
> A typical use case for this whole feature is the following. I have a vlan
> device configured on multiple hosts. I'd like to remove this device from all
> hosts, but I would like to to first verify that the hosts keep their mutual
> connectivity in the new configuration. I want to make sure that I don't end
> up with some hosts using the vlan, and some others use native. So I remove
> all vlan devices with memory_only=True, do my network-wide validation, and
> then repeat with memory_only=False.
> 
> Would I still be able to do that?

You can, you will mark all VLAN interface as `state: down` along with other memory-only
changes.

Just `state: absent` is illegal in memory only action.

I'm not very happy about this constraint. Testing one yaml with memory_only=true and then having to modify it before committing it is a recipe for errors. I'd like to test and apply the very same string. Let nmstate mock "absent" into "offline"  when it's memory_only.

(In reply to Dan Kenigsberg from comment #16)
> I'm not very happy about this constraint. Testing one yaml with
> memory_only=true and then having to modify it before committing it is a
> recipe for errors. I'd like to test and apply the very same string. Let
> nmstate mock "absent" into "offline"  when it's memory_only.

Thanks for the suggestion. I will take that approach.
Approving RFE.

I would love to see a high level design document on this topic in order to adjust the requirements and solutions to fit all raised points.
The non-persistent configuration has roots in telco network devices (routers/switches) but also on projects like oVirt.
It's main purpose is to make sure changes do not cause a permanent loss of remote connectivity to the device, allowing a recovery to occur by rebooting.
It also prevents an unexpected crash of the system to leave it in an intermediate state when booting back.

Here are a few points I would like to see discussed in a design document:
- Looking at this requirement from a provider agnostic angle.
  As an example, a solution that may also work with a netlink or iproute solution.
  Or simply with additions to the NM provider (e.g. ethtool).
- Use the NM non persisted config for all configurations, then for the persisted (default) patch, persist at the end.
  This will allow solving a few points in one shot:
  * Utilize the same path and limit the conditions along the setup.
  * If the system crashes, it will not boot with an intermediate state.
- Persist a startup-config and use of a systemd one-time job to perform a restoration on boot (if needed).
  - Not sure if applicable for containerized projects like CNV.

It is fine to implement only what is needed under this RFE, however, the implementation should take into account the overall needs in order to avoid future revolutions.

(In reply to Edward Haas from comment #18)
> I would love to see a high level design document on this topic in order to
> adjust the requirements and solutions to fit all raised points.

If there are other,unconsidered requirements, please provide them as user storiees.

> Here are a few points I would like to see discussed in a design document:
> - Looking at this requirement from a provider agnostic angle.
>   As an example, a solution that may also work with a netlink or iproute
> solution.
>   Or simply with additions to the NM provider (e.g. ethtool).

IMHO the providers need to implement persistence or can declare that they do not support this. Also all nmstate users can build persistence on top of Nmstate by calling it after boot to setup the state as required.


> - Use the NM non persisted config for all configurations, then for the
> persisted (default) patch, persist at the end.

IMHO this is a implementation detail that does not need to be discussed here.

>   This will allow solving a few points in one shot:
>   * Utilize the same path and limit the conditions along the setup.
>   * If the system crashes, it will not boot with an intermediate state.

If these are requirements, please specify them as user stories. Then we can see if/how they can be fulfilled.

> - Persist a startup-config and use of a systemd one-time job to perform a
> restoration on boot (if needed).

This is also an implementation detail.

>   - Not sure if applicable for containerized projects like CNV.

Who can tell us if this is a requirement for CNV?


> It is fine to implement only what is needed under this RFE, however, the
> implementation should take into account the overall needs in order to avoid
> future revolutions.

I am more in favor of doing the simplest thing that might actually work to avoid working too much in the wrong direction. For this it would be best to work based on the actual requirements from a user instead of guessing what might be needed.

(In reply to Till Maas from comment #19)
> > I would love to see a high level design document on this topic in order to
> > adjust the requirements and solutions to fit all raised points.
> 
> If there are other,unconsidered requirements, please provide them as user
> storiees.

As I mentioned, I am looking to discuss it in a design document and not here.
> > Here are a few points I would like to see discussed in a design document

There is one base user story which Danken opened this BZ with. However, the
design and implementation will imply other requirements and behaviors.
In any case, I see no point in discussing these in a BZ, my list was a starting point
to give some initial content to look at while designing this.

Every question we will have during the design phase will probably raise a user-story specific point.
That is the nature of things. As with other features/RFEs, I expect things to be build on the go
as we see the implications revealed (hopefully in the design document).

 
> I am more in favor of doing the simplest thing that might actually work to
> avoid working too much in the wrong direction. For this it would be best to
> work based on the actual requirements from a user instead of guessing what
> might be needed.

Unless the requirements will come in the form of zeros and ones (i.e. the solution), the team will always guess and assume things.
We have a feedback loop to mitigate this in the form of reviews and demos.
Having a design document and asking for review on it is a good start, there we have the opertunity to get feedback.
I'm fine with the "simplest" quest but only as long as it is not a dead end (i.e. changing it is not possible and a full rework will be needed).
The high level picture must be understood and then you can proceed with small and simple steps.

Created attachment 1702149 [details]
error log

A sketchy test with versions:
nmstate-0.3.3-2.el8.noarch
NetworkManager-1.26.0-2.el8.x86_64
DISTRO=RHEL-8.3.0-20200721.n.0
Linux dell-r330-15.gsslab.brq.redhat.com 4.18.0-226.el8.x86_64 #1 SMP Wed Jul 15 07:40:39 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux
openvswitch-selinux-extra-policy-1.0-19.el8fdp.noarch
openvswitch2.12-2.12.0-12.el8fdp.x86_64

Use --memory-only to set a state with all supported interfaces and reboot, after that all interfaces are removed except for the ovs things: no NM profiles, but partially stored in ovsdb. A worse thing is, now you might not easy to edit these ovs interfaces, before you remove them all. Sometimes nmstate can do removing successfully but sometimes not, mostly I have to use ovs-vsctl command.

See the attachment for the details. Briefly it was doing:
1. Use --memory-only to set a state with all supported interfaces
2. Reboot the system
3. After rebooting, check state: ip link, nmstatectl show 'ovs*', nmcli con, ovs-vsctl show
4. Apply the state in step 1 again: all failed (during my test, the errors were not always same)
5. Remove ovs interfaces with nmstate successfully, but actually still partially stored in ovsdb

Created attachment 1702150 [details]
noovsbond.yaml

Use this yaml file, in case #bz1858758 impacts, I removed the ovs bond config

For ovs issue:
1. Temporarily add warning message to warn user
2. For fixing, to open a RFE to NetworkManager

Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory (nmstate bug fix and enhancement update), and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

https://access.redhat.com/errata/RHBA-2020:4696

The needinfo request[s] on this closed bug have been removed as they have been unresolved for 500 days