Fixed: ["commit diff" after applying a service always seems to show a…

… diff](#70)
clicon · Dec 14, 2023 · 496a304 · 496a304
1 parent 4102c6a
commit 496a304
Show file tree

Hide file tree

Showing 5 changed files with 11 additions and 7 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -23,6 +23,7 @@ Expected: February 2024
 
 ### Corrected Bugs
 
+* Fixed: ["commit diff" after applying a service always seems to show a diff](https://github.com/clicon/clixon-controller/issues/70)
 * Fixed: [Backend may exit with assertion if the device return warnings](https://github.com/clicon/clixon-controller/issues/77)
 * Fixed: [apply template with multiple variables do not work in some cases](https://github.com/clicon/clixon-controller/issues/74)
 

diff --git a/src/controller_configure.cli b/src/controller_configure.cli
@@ -18,9 +18,9 @@ delete("Delete a configuration item") {
       all("Delete whole candidate configuration"), delete_all("candidate");
 }
 quit("Quit"), cli_quit();
-commit("Commit the changes, trigger services scripts"), cli_rpc_controller_commit("candidate", "CHANGE", "COMMIT");{
+commit("Run services, commit and push to devices"), cli_rpc_controller_commit("candidate", "CHANGE", "COMMIT");{
     diff("Show the result of running the services but do not commit"), cli_rpc_controller_commit("candidate", "CHANGE", "NONE");
-    push("Run services and push to devices"), cli_rpc_controller_commit("candidate", "CHANGE", "COMMIT"); 
+    push("Run services, commit and push to devices"), cli_rpc_controller_commit("candidate", "CHANGE", "COMMIT"); 
     local("Local commit, do not push to devices"), cli_commit();
 }
 validate("Validate changes"), cli_validate();{

diff --git a/src/controller_rpc.c b/src/controller_rpc.c
@@ -915,9 +915,6 @@ controller_commit_actions(clixon_handle           h,
     /* 1) copy candidate to actions and remove all device config tagged with services */
     if (xmldb_copy(h, "candidate", "actions") < 0)
         goto done;
-    /* Strip service data in device config for services that changed */
-    if (strip_service_data_from_device_config(h, "actions", cvv) < 0)
-        goto done;
     if (services){
         if (actions == AT_FORCE || cvec_len(cvv) > 0){     /* There are service changes */
             if ((notifycb = cbuf_new()) == NULL){
@@ -934,7 +931,9 @@ controller_commit_actions(clixon_handle           h,
                 cprintf(notifycb, "</service>");
             }
             cprintf(notifycb, "</services-commit>");
-
+            /* Strip service data in device config for services that changed */
+            if (strip_service_data_from_device_config(h, "actions", cvv) < 0)
+                goto done;
             clicon_debug(1, "%s stream_notify: services-commit: %" PRIu64, __FUNCTION__, ct->ct_id);
             if (stream_notify(h, "services-commit", "%s", cbuf_get(notifycb)) < 0)
                 goto done;

diff --git a/test/test-c-service.sh b/test/test-c-service.sh
@@ -347,6 +347,10 @@ new "commit push"
 set +e
 expectpart "$(${clixon_cli} -m configure -1f $CFG commit push 2>&1)" 0 OK --not-- Error
 
+# see https://github.com/clicon/clixon-controller/issues/70
+new "commit diff"
+expectpart "$(${clixon_cli} -m configure -1f $CFG commit diff 2>&1)" 0 OK --not-- "<interface"
+
 CREATORSA="<creator><name>testA\[name='foo'\]</name><path>/devices/device\[name=\"openconfig1\"\]/config/interfaces/interface\[name=\"A0x\"\]</path><path>/devices/device\[name=\"openconfig1\"\]/config/interfaces/interface\[name=\"A0y\"\]</path><path>/devices/device\[name=\"openconfig1\"\]/config/interfaces/interface\[name=\"ABx\"\]</path><path>/devices/device\[name=\"openconfig1\"\]/config/interfaces/interface\[name=\"ABy\"\]</path><path>/devices/device\[name=\"openconfig1\"\]/config/interfaces/interface\[name=\"Ax\"\]</path><path>/devices/device\[name=\"openconfig1\"\]/config/interfaces/interface\[name=\"Ay\"\]</path><path>/devices/device\[name=\"openconfig2\"\]/config/interfaces/interface\[name=\"A0x\"\]</path><path>/devices/device\[name=\"openconfig2\"\]/config/interfaces/interface\[name=\"A0y\"\]</path><path>/devices/device\[name=\"openconfig2\"\]/config/interfaces/interface\[name=\"ABx\"\]</path><path>/devices/device\[name=\"openconfig2\"\]/config/interfaces/interface\[name=\"ABy\"\]</path><path>/devices/device\[name=\"openconfig2\"\]/config/interfaces/interface\[name=\"Ax\"\]</path><path>/devices/device\[name=\"openconfig2\"\]/config/interfaces/interface\[name=\"Ay\"\]</path></creator>"
 
 CREATORSB="<creator><name>testB\[name='foo'\]</name><path>/devices/device\[name=\"openconfig1\"\]/config/interfaces/interface\[name=\"A0x\"\]</path><path>/devices/device\[name=\"openconfig1\"\]/config/interfaces/interface\[name=\"A0y\"\]</path><path>/devices/device\[name=\"openconfig1\"\]/config/interfaces/interface\[name=\"ABx\"\]</path><path>/devices/device\[name=\"openconfig1\"\]/config/interfaces/interface\[name=\"ABy\"\]</path><path>/devices/device\[name=\"openconfig1\"\]/config/interfaces/interface\[name=\"ABz\"\]</path><path>/devices/device\[name=\"openconfig1\"\]/config/interfaces/interface\[name=\"Bx\"\]</path><path>/devices/device\[name=\"openconfig2\"\]/config/interfaces/interface\[name=\"A0x\"\]</path><path>/devices/device\[name=\"openconfig2\"\]/config/interfaces/interface\[name=\"A0y\"\]</path><path>/devices/device\[name=\"openconfig2\"\]/config/interfaces/interface\[name=\"ABx\"\]</path><path>/devices/device\[name=\"openconfig2\"\]/config/interfaces/interface\[name=\"ABy\"\]</path><path>/devices/device\[name=\"openconfig2\"\]/config/interfaces/interface\[name=\"ABz\"\]</path><path>/devices/device\[name=\"openconfig2\"\]/config/interfaces/interface\[name=\"Bx\"\]</path></creator></creators>"

diff --git a/test/test-cli-edit-config.sh b/test/test-cli-edit-config.sh
@@ -161,6 +161,6 @@ new "Sleep and verify devices are open 3"
 sleep_open
 
 new "Testrun after restart"
-testrun
+testrun # XXX This fails on regression occasionally
 
 endtest