Dev: ui_corosync: Improve corosync.show and corosync.edit subcommands (#1264)

- corosync.show: validate corosync.conf before showing it
- corosync.edit: edit a temporary copy of corosync.conf, validate it
before saving it,
    then give the user hints on how to sync to other nodes
- Add corosync_ui.feature to collect test cases for crm corosync ui

Author: liangxin1300

.github/workflows/crmsh-ci.yml

@@ -165,6 +165,22 @@ jobs:
         token: ${{ secrets.CODECOV_TOKEN }}
         flags: integration
 
+  functional_test_corosync_ui:
+    runs-on: ubuntu-24.04
+    timeout-minutes: 40
+    steps:
+    - uses: actions/checkout@v4
+    - name: functional test for crm corosync subcommand
+      run:  |
+        echo '{ "exec-opts": ["native.cgroupdriver=systemd"] }' | sudo tee /etc/docker/daemon.json
+        sudo systemctl restart docker.service
+        index=`$GET_INDEX_OF corosync_ui`
+        $DOCKER_SCRIPT $index
+    - uses: codecov/codecov-action@v4
+      with:
+        token: ${{ secrets.CODECOV_TOKEN }}
+        flags: integration
+
   functional_test_bootstrap_options_non_root:
     runs-on: ubuntu-24.04
     timeout-minutes: 40

codecov.yml

@@ -8,9 +8,9 @@ coverage:
         threshold: 0.35%
 codecov: 
   notify:
-    after_n_builds: 26
+    after_n_builds: 27
 comment:
-  after_n_builds: 26
+  after_n_builds: 27
   layout: "condensed_header, flags, files, condensed_footer"
 
 ignore:

crmsh/ui_corosync.py

@@ -121,18 +121,21 @@ def do_edit(self, context):
         '''
         cfg = corosync.conf()
         try:
-            utils.edit_file_ext(cfg, template='')
+            rc = utils.edit_file_ext(cfg, corosync.is_valid_corosync_conf)
+            if rc and len(utils.list_cluster_nodes()) > 1:
+                logger.warning(f"\"{cfg}\" has changed, should be synced with other nodes")
+                logger.info("Use \"crm corosync diff\" to show the difference")
+                logger.info("Use \"crm corosync push\" to sync")
         except IOError as e:
             context.fatal_error(str(e))
 
     def do_show(self, context):
         '''
         Display the corosync configuration.
         '''
-        cfg = corosync.conf()
-        if not os.path.isfile(cfg):
-            context.fatal_error("No corosync configuration found on this node.")
-        utils.page_string(open(cfg).read())
+        if not corosync.is_valid_corosync_conf():
+            return False
+        utils.page_file(corosync.conf())
 
     def do_log(self, context):
         '''

crmsh/utils.py

@@ -1230,31 +1230,27 @@ def edit_file(fname):
     return ext_cmd_nosudo("%s %s" % (config.core.editor, fname))
 
 
-def edit_file_ext(fname, template=''):
+def edit_file_ext(fname: str, validator: typing.Callable[[typing.IO], bool] = None) -> bool:
     '''
     Edit a file via a temporary file.
     Raises IOError on any error.
+
+    returns True if the file was changed
     '''
-    if not os.path.isfile(fname):
-        s = template
-    else:
-        s = open(fname).read()
-    filehash = hash(s)
-    tmpfile = str2tmp(s)
-    try:
-        try:
-            if edit_file(tmpfile) != 0:
-                return
-            s = open(tmpfile, 'r').read()
-            if hash(s) == filehash:  # file unchanged
-                return
-            f2 = open(fname, 'w')
-            f2.write(s)
-            f2.close()
-        finally:
-            os.unlink(tmpfile)
-    except OSError as e:
-        raise IOError(e)
+    with create_tempfile() as tmpfile:
+        shutil.copyfile(fname, tmpfile)
+        if edit_file(tmpfile) != 0:
+            raise IOError(f"Cannot edit file \"{fname}\"")
+        changed_data = read_from_file(tmpfile)
+        source_data = read_from_file(fname)
+        if changed_data != source_data:
+            if validator and not validator(tmpfile):
+                return False
+            # The original file needs to be replaced atomically
+            str2file(changed_data, fname)
+            return True
+        else:
+            return False
 
 
 def need_pager(s, w, h):

data-manifest

@@ -73,6 +73,7 @@ test/features/bootstrap_sbd_normal.feature
 test/features/cluster_api.feature
 test/features/configure_bugs.feature
 test/features/constraints_bugs.feature
+test/features/corosync_ui.feature
 test/features/coveragerc
 test/features/crm_report_bugs.feature
 test/features/crm_report_normal.feature

test/features/corosync_ui.feature

@@ -0,0 +1,10 @@
+@bootstrap
+Feature: crm corosync ui test cases
+
+  Need nodes: hanode1 hanode2
+
+  Scenario: Empty cluster
+    When    Try "crm corosync show" on "hanode1"
+    Then    Except "No such file or directory: '/etc/corosync/corosync.conf'" in stderr
+    When    Try "crm corosync set totem.cluster_name xin" on "hanode1"
+    Then    Except "No such file or directory: '/etc/corosync/corosync.conf'" in stderr