error on edit-config dont stop merging the config in database

[GoogleCodeExporter]

What steps will reproduce the problem?
1. Start the server and client
2. Send a edit-config operation with three configurations (that will call three 
callbacks) where the second callback return EXIT_FAILURE.

What is the expected output? What do you see instead?
I expected the third callback not be called (due the failure return of the 
second callback) and the configuration set on database consisting only of the 
first configuration.
I see the third callback is not called (ok), but the database set ALL the 
configurations (got sending a get-config after the edit-config described above)!
This behaviour is very bad because:
1. If the device error condition was gone and I try apply the second 
configuration again, I will can't! (because will be no differences between  
this configuration and configuration on datastore, therefore the callback will 
not be called)
2. The third configuration will never be applied because the callback was not 
called in the first time and in the following times there will not diffs 
(analogous to the case 1). And the user will not know that configuration was 
not applied!

What version of the product are you using? On what operating system?
I am using the branch master commit 3f33930cd3a8 (Thu Sep 5 13:50:40 2013 
+0200) on a Gentoo Linux.

Original issue reported on code.google.com by paulo.zu...@asga.com.br on 5 Sep 2013 at 9:41

[GoogleCodeExporter]

Original comment by rkre...@cesnet.cz on 6 Sep 2013 at 7:31

[GoogleCodeExporter]

Original comment by rkre...@cesnet.cz on 10 Sep 2013 at 7:01

[GoogleCodeExporter]

This issue was updated by revision 974ededda748.


When TransAPI callback fails, ncds_apply_rpc() tries to rollback last
changes inside the libnetconf datastore part. If rollback-on-error is
specified, ncds_apply_rpc2all() then rollbacks all previously changed
datastore parts. So currently, there is no chance for stop-on-error
inside a datastore part - libnetconf performs edit-config as an atomic
operation (for each datastore part).

If rollback fails, client receives error-reply with note, that
configuration is in an inconsistent state.

TODO: stop-on-error with TransAPI callback functions granularity

Original comment by rkre...@cesnet.cz on 10 Sep 2013 at 12:30

[GoogleCodeExporter]

Original comment by rkre...@cesnet.cz on 10 Sep 2013 at 12:32

• Changed state: Started

[GoogleCodeExporter]

Issue 11 has been merged into this issue.

Original comment by mv6...@gmail.com on 11 Sep 2013 at 8:30

[GoogleCodeExporter]

This issue was updated by revision 1de2551d489f.


Changes default behaviour of edit-config's reaction to error. Previously
we were trying rollback, but RFC 6241 (sec 7.2) defines stop-on-error as
the default behaviour.

This still covers only granularity of the whole datastore part
(standalone modules). In future changes, granularity will move to the
transAPI's callbacks.

Original comment by rkre...@cesnet.cz on 11 Sep 2013 at 10:34

[GoogleCodeExporter]

As mentioned, so far the edit-config's error option was connected with changes 
made to a single libnetconf datastore (transAPI module). So stop-on-error, 
continue-on-error or rollback-on-error were (ok, thre were bugs, but they were 
supposed to) applied with this granularity - changes made to a single datastore 
part were atomi (all or nothing).

Now we plan to move granularity from transAPI module to a single transAPI 
callback. The problem is, that changes to the XML configuration data are done 
before calling TransAPI callback and we cannot mix this. So, the proposed 
algorithm is following:

1) perform all edit-config changes on XML configuration data (within a single 
transAPI module)
2) call transAPI callbacks according to the performed changes in XML 
configuration data
3) when a callback fails, then:
3a) if op is XMLDIFF_ADD, node is removed from the XML configuration data
3b) if op is XMLDIFF_REM, node is added back to the XML configuration data
3c) if op is XMLDIFF_MOD or XMLDIFF_CHAIN, node is replaced by the previous 
content of the XML configuration data
4) when a callback fails, then:
4a) if error-option is set to stop-on-error, immediately after failed callback, 
edit-config stops and returns error-reply. Following, not-executed via 
transAPI, changes to XML configuration data are rolled back.
4b) if error-option is set to continue-on-error, edit-config continues to the 
following transAPI callback, but error info is recorded and error-reply with 
all error info structures is returned to the client
4c) if error-option is set to rollback-on-error, edit-config performs 
previously succeeded callbacks with parameters to rollback last change (see 
(3)). Changes to the XML configuration data that were not executed by transAPI 
callbacks are rolled back.

Do you see any problem in this approach?

Original comment by rkre...@cesnet.cz on 11 Sep 2013 at 11:22

[GoogleCodeExporter]

Why we cannot call the TransAPI callbacks together with the changes to the XML 
configuration?

About the algorithm, it seems OK for me. I cannot see any problem about that.

Original comment by paulo.zu...@asga.com.br on 11 Sep 2013 at 1:29

[GoogleCodeExporter]

There are multiple implementations of datastores (currently FILE, EMPTY and 
CUSTOM, for the future we would like to have some kind of DATABASE 
implementation) that include implementation of edit-config's changes to the XML 
configuration. When we would call TransAPI callbacks together with 
modifications of XML tree, it would be duplicated in all datastore 
implementations (CUSTOM would be the most problematic). Furthermore, 
integrating TransAPI calls into the edit-config would be a real hell. In 
addition, current approach allows us to do syntax and semantics validation 
before calling TransAPI functions.

I agree that calling the TransAPI callbacks together with XML tree modification 
would be more straightforward. But I believe that it would bring more cons than 
pros.

Original comment by radek.krejci@gmail.com on 11 Sep 2013 at 1:47

[GoogleCodeExporter]

You have a good point. Thank you for your explanation!

Original comment by paulo.zu...@asga.com.br on 11 Sep 2013 at 2:26

[GoogleCodeExporter]

This issue was updated by revision 8caf69e92ba6.


First part of changing granularity when libnetconf is able to apply
edit-config's error-option (stop|continue|rollback).

This commit simplifies source codes of TransAPI and implements
TransAPI's stop-on-error (4a) for XMLDIFF_ADD (3a) and XMLDIFF_REM (3b)
operations.

TODO: rest of algorithm proposed in issue 9.

Original comment by rkre...@cesnet.cz on 11 Sep 2013 at 4:30

[GoogleCodeExporter]

This issue was updated by revision dd0a3a0136e3.


Second and probably last part.

This commit implements remaining edit-config's error-options with granularity
of a single transAPI's callback. Algorithm described in Issue 9 should
be know fully implemented.

TODO: more testing.

Original comment by rkre...@cesnet.cz on 12 Sep 2013 at 2:19

[GoogleCodeExporter]

I forgot to connect commit with the issue tracker, so the current 
transapi_change branch (commit 98168ab1d784 Sep 12 16:15:57 2013 +0200) 
implements the proposed algorithm (with some minor optimization changes for 
XMLDIFF_CHAIN).

I did some basic testing, but testing on some more complex models is still 
needed. So - bugs are still possible :(

Also note, that if an error occurs during XML tree modification or validation 
(before transAPI calls), it still revert all changes (default, silent, non-RFC 
rollback-on-error - it's better than allow damage of data).

And one more point - validation (in case of test-then-set option) is applied on 
the result of ALL changes required by RPC, but if stop or continue -on-error 
appears, the result actually differs from validated content.

Original comment by rkre...@cesnet.cz on 12 Sep 2013 at 2:48

• Changed state: Fixed