wip: save

(cherry picked from commit 2ccbe48)

srv/modules/runners/proposal.py

@@ -317,22 +317,22 @@ def _find_minions_to_replace(profile_dir):
     :param profile_dir: Profile directory, e.g. "/srv/pillar/ceph/proposals/profile-default"
     """
     ToReplace = namedtuple('ToReplace', ['fullpath', 'filename'])
-    base_dir = '{}/stack/default/ceph/minions'.format(profile_dir)
-    files = [f for f in os.listdir(base_dir) if isfile(join(base_dir, f))]
+    dir = '{}/stack/default/ceph/minions'.format(profile_dir)
+    files = [f for f in os.listdir(dir) if isfile(join(dir, f))]
 
-    return [ToReplace(join(base_dir, f), f) for f in files
-            if f.endswith('-replace')]
+    return [ToReplace(join(dir, f), f) for f in files if f.endswith('-replace')]
 
 
-class ReplaceDisk(object):
+class ReplaceDiskOn(object):
     """
     Handle replacement of disks on OSD minions.
 
     This class encapsulates everything that is needed to parse an old
-    proposal, compare it to the new disks on the given minion and adapt the
-    old proposal if necessary. When the same physical slots for are used, the
-    old proposal is reused. Otherwise the disks are replaced by new, unused
-    ones, using the old proposal and only swapping the disk paths.
+    proposal, compare it to the new/unused disks on the given minion and adapt
+    the old proposal if necessary.
+    When the same physical slots for are used, the only change of the proposal
+    is that the "replace: true" attribute gets stripped out of the proposal.
+    Otherwise the disks identifiers in the proposal are replaced by new ones.
 
     Public method: replace() - trigger replacement of all flagged disks
 
@@ -349,7 +349,7 @@ def __init__(self, minion):
         self.disks = self._query_node_disks()
         self.device_files = self._extract_device_files()
         self.old_proposal_disks = self._extract_old_proposal_disks()
-        self.new_disks = self._new_disks()
+        self.unused_disks = self._unused_disks()
         self.flagged_replace = self._flagged_replace()
 
     def _proposal_basename(self):
@@ -364,7 +364,7 @@ def _minion_name_from_file(self):
         return self.proposal_basename.replace(".yml", "")
 
     def _load_proposal(self):
-        with open(self.minion.filename, 'rb') as f:
+        with open(self.minion.fullpath, 'rb') as f:
             return yaml.safe_load(f)
 
     def _query_node_disks(self):
@@ -373,52 +373,87 @@ def _query_node_disks(self):
         # old disks present
         local_client.cmd(self.name, 'mine.delete', ['cephdisks.list'],
                          tgt_type='compound')
+        # the return of this call is a dictionary with the targets as keys
+        # even if there is only a single target
         return local_client.cmd(self.name, 'cephdisks.list',
-                                tgt_type='compound')
+                                tgt_type='compound')[self.name]
 
     def _extract_device_files(self):
-        return sorted([x['Device File'] for x in self.disks[self.name]])
+        # TODO: This does not return the 'Device File' we actually want in most cases,
+        # base that on https://github.com/SUSE/DeepSea/pull/1222
+        return sorted([x['Device File'] for x in self.disks])
 
     def _extract_old_proposal_disks(self):
         return sorted([x for x in self.proposal['ceph']['storage']['osds']])
 
-    def _new_disks(self):
-        return [x for x in self.device_files if x not in self.old_proposal_disks]
+    def _unused_disks(self):
+        return [x for x in self.device_files
+                if (x not in self.old_proposal_disks) or
+                (x in self.old_proposal_disks and 'replace' in self.proposal['ceph']['storage']['osds'][x]
+                    and self.proposal['ceph']['storage']['osds'][x]['replace'] is True)]
 
     def _flagged_replace(self):
         return [x for x in self.old_proposal_disks
                 if 'replace' in self.proposal['ceph']['storage']['osds'][x]]
 
-    def _enough_new_disks(self):
-        return (self.new_disks >= self.flagged_replace) and (len(self.new_disks) > 0)
+    def _enough_unused_disks(self):
+        return len(self.unused_disks) >= len(self.flagged_replace)
 
     def _setup_changed(self):
         return self.device_files != self.old_proposal_disks
 
     def _swap_disks_in_proposal(self):
-        for disk_path, conf in self.proposal['ceph']['storage']['osds'].items():
-            if 'replace' in conf and conf['replace'] is True:
-                self._change_disk_path(disk_path, conf)
+        for disk in self.flagged_replace:
+            if self.proposal['ceph']['storage']['osds'][disk]['replace'] is True:
+                self._change_disk_path(disk)
 
-    def _change_disk_path(self, old_disk, conf):
-        new_disk = self.new_disks.pop(0)
-        del conf['replace']
+    def _change_disk_path(self, old_disk):
+        unused_disk = self.unused_disks.pop(0)
+        temp = self.proposal['ceph']['storage']['osds'][old_disk]
         del self.proposal['ceph']['storage']['osds'][old_disk]
-        self.proposal['ceph']['storage']['osds'][new_disk] = conf
+        self.proposal['ceph']['storage']['osds'][unused_disk] = temp
+
+    def _strip_replace_flags(self):
+        for disk in self.proposal['ceph']['storage']['osds']:
+            if 'replace' in self.proposal['ceph']['storage']['osds'][disk]:
+                del self.proposal['ceph']['storage']['osds'][disk]['replace']
+
+    def _write_new_proposal(self):
+        with open(self.proposal_basepath, 'w') as f:
+            yaml.dump(self.proposal, f, default_flow_style=False)
+
+    def _delete_old_proposal(self):
+        os.remove(self.minion.fullpath)
 
     def replace(self):
-        if not self._enough_new_disks():
-            log.error("Fewer new disks than disks to replace")
-            return False
+        """
+        Adapt the proposal for replaced disks.
+
+        Following steps take place:
+        (0.) Use a new disk (by-path) for the proposal if the physical location
+            has changed.
+        1. Remove all "replace: " attributes from the proposal
+        2. Write proposal to the "normal" proposal file
+        3. Remove proposal file with "-replace" in its name
+        """
+
+        # import ipdb; ipdb.set_trace()
+        if self._setup_changed():
+            if not self._enough_unused_disks():
+                log.error("Fewer unused disks than disks to replace!")
+                return False
 
-        if not self._setup_changed():
-            os.rename(self.minion.fullpath, self.proposal_basepath)
-            log.info("Proposal did not change")
-        else:
             self._swap_disks_in_proposal()
-            _write_proposal(self.proposal, self.proposal_basepath)
+
+        self._strip_replace_flags()
+
+        try:
+            self._write_new_proposal()
             log.info("New proposal was written to {}".format(
                      self.proposal_basepath))
+            self._delete_old_proposal()
+        except:
+            log.error("Writing new proposal failed.")
 
 
 def populate(**kwargs):
@@ -435,7 +470,7 @@ def populate(**kwargs):
 
     if minions_to_replace:
         for minion in minions_to_replace:
-            ReplaceDisk(minion).replace()
+            ReplaceDiskOn(minion).replace()
         return True
     else:
         proposals = local_client.cmd(args['target'], 'proposal.generate',