clicon / clixon-controller

Clixon network controller
Apache License 2.0
14 stars 5 forks source link

commit (push) does not work if no devices are connected/mounted #5

Closed olofhagsand closed 1 year ago

olofhagsand commented 1 year ago

If you do a "commit" in configure mode (same as "commit push") and there are no devices opened, the commit will not take place: the changed config will remain in candidate-db and will not appear in running-db. The reason is that the commit push command starts a transaction where the last step is the controller commit when the device interaction is completed. In this case there is no device interaction, so no controller commit takes place. Work-around: use "commit local" instead.

olofhagsand commented 1 year ago

It is somewhat complex issue that depends on the fact that there are two different types of transactions that currently cannot mix:

  1. connection establishment from CLOSED to OPEN (or OPEN->OPEN via reconnect). This is triggered by: 1a) rpc_config_pull, rpc_connection_change, or 1b) change of device/enable to "true" automatically triggered by regular commit plugins
  2. device PUSH commit/validate, which is triggered by rpc_controller_commit

In the case of this issue, the commit push operation starts a transaciton of type (2), but the devices enable flag is changed to "true", which should trigger a transaction of type (1b), but it does not.

This problem also exists if there is a mix of open devices, and one or more that are requested to be opened.

Fixing it by triggering (1b) is not straightforward, since only one transaction is allowed at a time, so that a (1b) transaction cannot go in parallell with a (2). It would also be wrong, since the (1b) transaction should complete BEFORE the (2) is started since it affects the PUSH operation.

Solutions (if both PUSH and one or more devices are set in ENABLE state): 1) Make a combined transaction, so that the devices are connected first, before the PUSH is started. This is probably the most correct solution. But it is technically complicated 2) Do not allow transactions of type (1b) together with (2). Give an error if you change the value of device/enable at the same time as you make a COMMIT PUSH. Probably easiest solution, but it is a special case (looking at changes to device/enable flag) and more such corenercases may evolve. 3) Change the design so that triggering by commit (1b) is not done, only after a rpc_connection_change will the connection be setup. This may make the UI more elaborate, since you need to FIRST enable the device and commit, and THEN send a separate RPC when you enable devices, but it makes the mixture of triggering of transactions more deterministic.

Reflecting on this, it seems that (3) is the most logical. It will make the solution cleaner, so that there is only one way to start a transaction (via RPC). But it changes the way devices are connected

olofhagsand commented 1 year ago

Fixed, but with changed behvaiour: after you add and commit a new device, you need to explitly connect:

cli> connection <devices> open
krihal commented 1 year ago

Tested working.