From: Juan Miguel Olmo Martínez <jolmomar@redhat.com>
Date: Tue, 4 Dec 2018 20:16:47 +0000 (+0100)
Subject: mgr/ansible: Simplify Orchestrator wait implementation
X-Git-Tag: v14.1.0~660^2
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=refs%2Fpull%2F25401%2Fhead;p=ceph.git

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 862e53aebf29..fcca13ef2d20 100644
--- 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 4a0190fad7db..a4bfa4b195a2 100644
--- 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 756f93263661..575ef2f727e6 100644
--- 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.