Remove creation of unused partitions, implement recommendation

The bluestore partition process has been working from original
assumptions.  The refactor of bluestore args created a disconnect
between the two steps.  Partitions are getting created, but never
used nor removed.

This also implements the recommendation to use a single DB partition
since the WAL will naturally use an existing DB.  This reduces the
number of managed partitions from two to one for each OSD.  Note
that osd.report is intentionally not changed to give the admin an
indication that the configuration can be improved.

Signed-off-by: Eric Jackson <[email protected]>

srv/salt/_modules/osd.py

@@ -898,8 +898,10 @@ def _bluestore_partitions(self):
                     log.warning(("WAL size is unsupported for same device of "
                                  "{}".format(self.osd.device)))
                 else:
-                    # Create wal of wal_size on wal device
-                    self.create(self.osd.wal, [('wal', self.osd.wal_size)])
+                    log.info(("WAL will reside on same device {} as db - "
+                              "recommend removing the WAL entry from the "
+                              "configuration for device "
+                              "{}").format(self.osd.db, self.osd.device))
             else:
                 # pylint: disable=line-too-long
                 log.warning("No size specified for wal {}. Using default sizes.".format(self.osd.wal))
@@ -923,11 +925,9 @@ def _bluestore_partitions(self):
                         log.warning(("WAL size is unsupported for same device of "
                                      "{}".format(self.osd.device)))
                     else:
-                        log.warning("Setting db to same device {} as wal".format(self.osd.wal))
+                        log.info("DB will reside on device {}".format(self.osd.device))
                         # Create wal of wal_size on wal device
-                        # Create db on wal device
-                        self.create(self.osd.wal, [('wal', self.osd.wal_size),
-                                                   ('db', self._halve(self.osd.wal_size))])
+                        self.create(self.osd.wal, [('wal', self.osd.wal_size)])
             else:
                 if self.osd.wal_size:
                     log.warning(("WAL size is unsupported for same device of "
@@ -1173,11 +1173,17 @@ def _bluestore_args(self):
 
             # WAL cornercase with sizes
             if self.osd.wal and self.osd.wal_size and self.osd.wal != self.osd.device:
-                _partition = self.highest_partition(self.osd.wal, 'wal')
-                if _partition:
-                    args += "--block.wal {}{} ".format(self.osd.wal, _partition)
+                if self.osd.db:
+                    log.warning(("Ignoring WAL setting - "
+                                "No need for two partitions, "
+                                "WAL will use same device as DB "
+                                "{}").format(self.osd.db))
                 else:
-                    args += "--block.wal {} ".format(self.osd.wal)
+                    _partition = self.highest_partition(self.osd.wal, 'wal')
+                    if _partition:
+                        args += "--block.wal {}{} ".format(self.osd.wal, _partition)
+                    else:
+                        args += "--block.wal {} ".format(self.osd.wal)
 
             # DB cornercase with sizes
             if self.osd.db and self.osd.db_size and self.osd.db != self.osd.device:

tests/unit/_modules/test_osd.py

@@ -814,8 +814,7 @@ def test_bluestore_partitions_wal_and_db_all_size_no_eq(self, create_mock):
         And I have a wal_size
         And I have a db_size
         And wal is not equivalent to the device
-        Expect to call create('/dev/sddb', [('db', db_size)])
-        Expect to call create('/dev/sdwal', [('wal', wal_size)])
+        Expect to call create('/dev/sddb', [('db', db_size)]) only
         """
         kwargs = {'format': 'bluestore',
                   'wal': '/dev/sdwal',
@@ -826,8 +825,8 @@ def test_bluestore_partitions_wal_and_db_all_size_no_eq(self, create_mock):
         test_module = helper_specs(module=DEFAULT_MODULE)()
         obj = test_module.OSDPartitions(osd_config)
         obj._bluestore_partitions()
-        create_mock.assert_any_call('/dev/sdwal', [('wal', 'walsize')])
         create_mock.assert_any_call('/dev/sddb', [('db', 'dbsize')])
+        assert create_mock.call_count == 1
 
     @mock.patch('srv.salt._modules.osd.OSDPartitions.create')
     @mock.patch('srv.salt._modules.osd.log')
@@ -860,7 +859,8 @@ def test_bluestore_partitions_wal_and_db_wal_size_no_eq(self, mock_log, create_m
         And I have a wal_size
         And I do not have a db_size
         And wal is not equivalent to the device
-        Expect to call create('/dev/sdx', [('wal', wal_size)])
+
+        Expect WAL to reside on DB device
         """
         kwargs = {'format': 'bluestore',
                   'wal': '/dev/sdwal',
@@ -871,7 +871,7 @@ def test_bluestore_partitions_wal_and_db_wal_size_no_eq(self, mock_log, create_m
         test_module = helper_specs(module=DEFAULT_MODULE)()
         obj = test_module.OSDPartitions(osd_config)
         obj._bluestore_partitions()
-        create_mock.assert_any_call('/dev/sdwal', [('wal', 'walsize')])
+        create_mock.call_count == 0
 
     @mock.patch('srv.salt._modules.osd.log')
     def test_bluestore_partitions_wal_and_db_no_sizes_no_eq(self, mock_log):
@@ -913,16 +913,12 @@ def test_bluestore_partitions_no_waldb_only_wal_and_size(self, mock_log):
         obj._bluestore_partitions()
         mock_log.warning.assert_any_call('WAL size is unsupported for same device of /dev/sdx')
 
-    @mock.patch('srv.salt._modules.osd.OSDPartitions._halve')
     @mock.patch('srv.salt._modules.osd.OSDPartitions.create')
-    @mock.patch('srv.salt._modules.osd.log')
-    def test_bluestore_partitions_no_waldb_only_wal_and_size_no_eq(self, mock_log, create_mock, halve_mock):
+    def test_bluestore_partitions_no_waldb_only_wal_and_size_no_eq(self, create_mock):
         """
         Given I defined only wal
         And I have a wal_size
         And wal is not equivalent to the device
-        Expect to call log()
-        Expect to call _halve()
         Expect to call create()
         """
         kwargs = {'format': 'bluestore',
@@ -934,9 +930,7 @@ def test_bluestore_partitions_no_waldb_only_wal_and_size_no_eq(self, mock_log, c
         test_module = helper_specs(module=DEFAULT_MODULE)()
         obj = test_module.OSDPartitions(osd_config)
         obj._bluestore_partitions()
-        mock_log.warning.assert_called_with('Setting db to same device /dev/sdwal as wal')
-        create_mock.assert_called_with('/dev/sdwal', [('wal', 100000), ('db', halve_mock('100000'))])
-        halve_mock.assert_called_with('100000')
+        create_mock.assert_called_with('/dev/sdwal', [('wal', 100000)])
 
     @mock.patch('srv.salt._modules.osd.log')
     def test_bluestore_partitions_no_waldb_only_wal_and_no_size(self, mock_log):
@@ -1591,8 +1585,9 @@ def test_bluestore_args_1(self, ip_mock, hp_mock, osdc_o):
         And and the device is a NVME
         Expect args to be:
 
-        --block.wal /dev/sdwal1 --block.db /dev/sddb1 /dev/nvme0n1p1
+        --block.db /dev/sddb1 /dev/nvme0n1p1
 
+        since the WAL will use the DB
         """
         kwargs = {'wal': '/dev/sdwal',
                   'wal_size': '2G',
@@ -1604,7 +1599,7 @@ def test_bluestore_args_1(self, ip_mock, hp_mock, osdc_o):
         ip_mock.side_effect = [True]
         hp_mock.side_effect = [1, 1]
         ret = obj._bluestore_args()
-        assert ret == "--block.wal /dev/sdwal1 --block.db /dev/sddb1 /dev/nvme0n1p1"
+        assert ret == "--block.db /dev/sddb1 /dev/nvme0n1p1"
 
     @mock.patch('srv.salt._modules.osd.OSDCommands.highest_partition')
     @mock.patch('srv.salt._modules.osd.OSDCommands.is_partitioned')