mgr/ansible: Simplify Orchestrator wait implementation

- Changed return value (list to boolean) in the ansible orchestrator wait method
- Improved and simplify the orchestrator_cli wait method using a new property, should_wait in completion objects.

Signed-off-by: Juan Miguel Olmo Martínez <jolmomar@redhat.com>

diff --git a/src/pybind/mgr/ansible/module.py b/src/pybind/mgr/ansible/module.py
index 862e53aebf293c7abaf041f0879dfb05cf2f4bf6..fcca13ef2d20e6cefc5eb1c6af22b1a1baf0a5ae 100644 (file)
--- a/src/pybind/mgr/ansible/module.py
+++ b/src/pybind/mgr/ansible/module.py
@@ -218,7 +218,7 @@ class Module(MgrModule, orchestrator.Orchestrator):
            incomplete.
 
            @param completions: list of Completion instances
-           @Returns          : List with completions operations pending
+           @Returns          : true if everything is done.
         """
 
         # Check progress and update status in each operation
@@ -229,9 +229,10 @@ class Module(MgrModule, orchestrator.Orchestrator):
 
         completions = filter(lambda x: not x.is_complete, completions)
 
-        self.log.info("Operations pending: %s", len(completions))
+        ops_pending = len(completions)
+        self.log.info("Operations pending: %s", ops_pending)
 
-        return completions
+        return ops_pending == 0
 
     def serve(self):
         """ Mandatory for standby modules

diff --git a/src/pybind/mgr/orchestrator.py b/src/pybind/mgr/orchestrator.py
index 4a0190fad7db72b0165b0b0be5dff6ffe2175c4d..a4bfa4b195a2a0a5142ba24e23c330c9a7f4ab0a 100644 (file)
--- a/src/pybind/mgr/orchestrator.py
+++ b/src/pybind/mgr/orchestrator.py
@@ -45,6 +45,13 @@ class ReadCompletion(_Completion):
     def is_read(self):
         return True
 
+    @property
+    def should_wait(self):
+        """Could the external operation be deemed as complete,
+        or should we wait?
+        We must wait for a read operation only if it is not complete.
+        """
+        return not self.is_complete
 
 class WriteCompletion(_Completion):
     """
@@ -83,6 +90,14 @@ class WriteCompletion(_Completion):
     def is_read(self):
         return False
 
+    @property
+    def should_wait(self):
+        """Could the external operation be deemed as complete,
+        or should we wait?
+        We must wait for a write operation only if we know
+        it is not persistent yet.
+        """
+        return not self.is_persistent
 
 class Orchestrator(object):
     """

diff --git a/src/pybind/mgr/orchestrator_cli/module.py b/src/pybind/mgr/orchestrator_cli/module.py
index 756f932636612285e6c64cece9f1a7e8c1c2d3c0..575ef2f727e6d4d9634608c8c86c8e27ec5a37d1 100644 (file)
--- a/src/pybind/mgr/orchestrator_cli/module.py
+++ b/src/pybind/mgr/orchestrator_cli/module.py
@@ -82,34 +82,19 @@ class OrchestratorCli(MgrModule):
 
         Waits for writes to be *persistent* but not *effective*.
         """
-        done = False
-
-        while done is False:
-            done = self._oremote("wait", completions) == []
-
-            if not done:
-                any_nonpersistent = False
-                for c in completions:
-                    if c.is_read:
-                        if not c.is_complete:
-                            any_nonpersistent = True
-                            break
-                    else:
-                        if not c.is_persistent:
-                            any_nonpersistent = True
-                            break
-
-                if any_nonpersistent:
-                    time.sleep(5)
-                else:
-                    done = True
+
+        while not self._oremote("wait", completions):
+
+            if any(c.should_wait for c in completions):
+                time.sleep(5)
+            else:
+                break
 
         if all(hasattr(c, 'error') and getattr(c, 'error')for c in completions):
             raise Exception([getattr(c, 'error') for c in completions])
 
     def _list_devices(self, cmd):
         """
-
         This (all lines starting with ">") is how it is supposed to work. As of
         now, it's not yet implemented:
         > :returns: Either JSON:
@@ -124,7 +109,7 @@ class OrchestratorCli(MgrModule):
         >
         > or human readable:
         >
-        >     HOST  DEV  SIZE  DEVID(vendor\_model\_serial)   IN-USE  TIMESTAMP
+        >     HOST  DEV  SIZE  DEVID(vendor\\_model\\_serial)   IN-USE  TIMESTAMP
         >
         > Note: needs ceph-volume on the host.