From 804deed0199772cd2baf990ad26451e007faa3e6 Mon Sep 17 00:00:00 2001 From: James Page Date: Mon, 18 Feb 2019 09:28:14 +0000 Subject: [PATCH] Update pool creation for >= Jewel The ceph broker request missed some pools for later Ceph versions, and created pools which where no longer required. Update pool list and tweak weights inline with current best practice. Change-Id: I4ed7e08d557c33a05aa8f8c6305914ef9734bad6 Closes-Bug: 1685536 --- hooks/ceph_rgw.py | 54 ++++++++---------- unit_tests/test_ceph.py | 122 ++++++++++++++-------------------------- 2 files changed, 64 insertions(+), 112 deletions(-) diff --git a/hooks/ceph_rgw.py b/hooks/ceph_rgw.py index 5efde943..3af951fd 100644 --- a/hooks/ceph_rgw.py +++ b/hooks/ceph_rgw.py @@ -16,8 +16,6 @@ import os import subprocess -from utils import get_pkg_version - from charmhelpers.core.hookenv import ( config, ) @@ -93,7 +91,7 @@ def get_create_rgw_pools_rq(prefix=None): # Per the Ceph PG Calculator, all of the lightweight pools get 0.10% # of the data by default and only the .rgw.buckets.* get higher values weights = { - '.rgw.buckets.index': 1.00, + '.rgw.buckets.index': 3.00, '.rgw.buckets.extra': 1.00 } w = weights.get(pool, 0.10) @@ -112,18 +110,13 @@ def get_create_rgw_pools_rq(prefix=None): rq = CephBrokerRq() replicas = config('ceph-osd-replication-count') - # Jewel and above automatically always prefix pool names with zone when - # creating them (see LP: 1573549). - if prefix is None: - vc = apt_pkg.version_compare(get_pkg_version('radosgw'), '10.0.0') - if vc >= 0: - prefix = 'default' - else: - prefix = '' + prefix = prefix or 'default' - # Buckets likely to contain the most data and therefore requiring the most - # PGs - heavy = ['.rgw.buckets'] + # Buckets likely to contain the most data and therefore + # requiring the most PGs + heavy = [ + '.rgw.buckets.data' + ] bucket_weight = config('rgw-buckets-pool-weight') for pool in heavy: pool = "{prefix}{pool}".format(prefix=prefix, pool=pool) @@ -132,27 +125,26 @@ def get_create_rgw_pools_rq(prefix=None): # NOTE: we want these pools to have a smaller pg_num/pgp_num than the # others since they are not expected to contain as much data - light = ['.rgw', - '.rgw.root', - '.rgw.control', - '.rgw.gc', - '.rgw.buckets.index', - '.rgw.buckets.extra', - '.log', - '.intent-log', - '.usage', - '.users', - '.users.email', - '.users.swift', - '.users.uid'] + light = [ + '.rgw.control', + '.rgw.data.root', + '.rgw.gc', + '.rgw.log', + '.rgw.intent-log', + '.rgw.meta', + '.rgw.usage', + '.rgw.users.keys', + '.rgw.users.email', + '.rgw.users.swift', + '.rgw.users.uid', + '.rgw.buckets.extra', + '.rgw.buckets.index', + ] pg_num = config('rgw-lightweight-pool-pg-num') for pool in light: _add_light_pool(rq, pool, pg_num, prefix) - if prefix: - light_unprefixed = ['.rgw.root'] - for pool in light_unprefixed: - _add_light_pool(rq, pool, pg_num) + _add_light_pool(rq, '.rgw.root', pg_num) if config('restrict-ceph-pools'): rq.add_op_request_access_to_group(name="objects", diff --git a/unit_tests/test_ceph.py b/unit_tests/test_ceph.py index e13d7da1..a6b6e231 100644 --- a/unit_tests/test_ceph.py +++ b/unit_tests/test_ceph.py @@ -66,78 +66,38 @@ class CephRadosGWCephTests(CharmTestCase): self.test_config.set('rgw-buckets-pool-weight', 19) ceph.get_create_rgw_pools_rq(prefix='us-east') mock_broker.assert_has_calls([ - call(replica_count=3, weight=19, name='us-east.rgw.buckets', - group='objects'), - call(pg_num=10, replica_count=3, name='us-east.rgw', - group='objects'), - call(pg_num=10, replica_count=3, name='us-east.rgw.root', + call(replica_count=3, weight=19, name='us-east.rgw.buckets.data', group='objects'), call(pg_num=10, replica_count=3, name='us-east.rgw.control', group='objects'), + call(pg_num=10, replica_count=3, name='us-east.rgw.data.root', + group='objects'), call(pg_num=10, replica_count=3, name='us-east.rgw.gc', group='objects'), - call(pg_num=10, replica_count=3, name='us-east.rgw.buckets.index', + call(pg_num=10, replica_count=3, name='us-east.rgw.log', + group='objects'), + call(pg_num=10, replica_count=3, name='us-east.rgw.intent-log', + group='objects'), + call(pg_num=10, replica_count=3, name='us-east.rgw.meta', + group='objects'), + call(pg_num=10, replica_count=3, name='us-east.rgw.usage', + group='objects'), + call(pg_num=10, replica_count=3, name='us-east.rgw.users.keys', + group='objects'), + call(pg_num=10, replica_count=3, name='us-east.rgw.users.email', + group='objects'), + call(pg_num=10, replica_count=3, name='us-east.rgw.users.swift', + group='objects'), + call(pg_num=10, replica_count=3, name='us-east.rgw.users.uid', group='objects'), call(pg_num=10, replica_count=3, name='us-east.rgw.buckets.extra', group='objects'), - call(pg_num=10, replica_count=3, name='us-east.log', - group='objects'), - call(pg_num=10, replica_count=3, name='us-east.intent-log', - group='objects'), - call(pg_num=10, replica_count=3, name='us-east.usage', - group='objects'), - call(pg_num=10, replica_count=3, name='us-east.users', - group='objects'), - call(pg_num=10, replica_count=3, name='us-east.users.email', - group='objects'), - call(pg_num=10, replica_count=3, name='us-east.users.swift', - group='objects'), - call(pg_num=10, replica_count=3, name='us-east.users.uid', + call(pg_num=10, replica_count=3, name='us-east.rgw.buckets.index', group='objects'), call(pg_num=10, replica_count=3, name='.rgw.root', - group='objects')] + group='objects')], ) - @patch.object(mock_apt.apt_pkg, 'version_compare', lambda *args: -1) - @patch('charmhelpers.contrib.storage.linux.ceph.CephBrokerRq' - '.add_op_create_pool') - def test_create_rgw_pools_rq_no_prefix_pre_jewel(self, mock_broker): - self.test_config.set('rgw-lightweight-pool-pg-num', -1) - self.test_config.set('ceph-osd-replication-count', 3) - self.test_config.set('rgw-buckets-pool-weight', 19) - ceph.get_create_rgw_pools_rq(prefix=None) - mock_broker.assert_has_calls([ - call(weight=19, replica_count=3, name='.rgw.buckets', - group='objects'), - call(weight=0.10, replica_count=3, name='.rgw', - group='objects'), - call(weight=0.10, replica_count=3, name='.rgw.root', - group='objects'), - call(weight=0.10, replica_count=3, name='.rgw.control', - group='objects'), - call(weight=0.10, replica_count=3, name='.rgw.gc', - group='objects'), - call(weight=1.00, replica_count=3, name='.rgw.buckets.index', - group='objects'), - call(weight=1.00, replica_count=3, name='.rgw.buckets.extra', - group='objects'), - call(weight=0.10, replica_count=3, name='.log', - group='objects'), - call(weight=0.10, replica_count=3, name='.intent-log', - group='objects'), - call(weight=0.10, replica_count=3, name='.usage', - group='objects'), - call(weight=0.10, replica_count=3, name='.users', - group='objects'), - call(weight=0.10, replica_count=3, name='.users.email', - group='objects'), - call(weight=0.10, replica_count=3, name='.users.swift', - group='objects'), - call(weight=0.10, replica_count=3, name='.users.uid', - group='objects')] - ) - - @patch.object(mock_apt.apt_pkg, 'version_compare', lambda *args: 0) @patch('charmhelpers.contrib.storage.linux.ceph.CephBrokerRq' '.add_op_request_access_to_group') @patch('charmhelpers.contrib.storage.linux.ceph.CephBrokerRq' @@ -150,38 +110,38 @@ class CephRadosGWCephTests(CharmTestCase): self.test_config.set('restrict-ceph-pools', True) ceph.get_create_rgw_pools_rq(prefix=None) mock_broker.assert_has_calls([ - call(weight=19, replica_count=3, name='default.rgw.buckets', - group='objects'), - call(weight=0.10, replica_count=3, name='default.rgw', - group='objects'), - call(weight=0.10, replica_count=3, name='default.rgw.root', + call(replica_count=3, weight=19, name='default.rgw.buckets.data', group='objects'), call(weight=0.10, replica_count=3, name='default.rgw.control', group='objects'), + call(weight=0.10, replica_count=3, name='default.rgw.data.root', + group='objects'), call(weight=0.10, replica_count=3, name='default.rgw.gc', group='objects'), - call(weight=1.00, replica_count=3, - name='default.rgw.buckets.index', + call(weight=0.10, replica_count=3, name='default.rgw.log', + group='objects'), + call(weight=0.10, replica_count=3, name='default.rgw.intent-log', + group='objects'), + call(weight=0.10, replica_count=3, name='default.rgw.meta', + group='objects'), + call(weight=0.10, replica_count=3, name='default.rgw.usage', + group='objects'), + call(weight=0.10, replica_count=3, name='default.rgw.users.keys', + group='objects'), + call(weight=0.10, replica_count=3, name='default.rgw.users.email', + group='objects'), + call(weight=0.10, replica_count=3, name='default.rgw.users.swift', + group='objects'), + call(weight=0.10, replica_count=3, name='default.rgw.users.uid', group='objects'), call(weight=1.00, replica_count=3, name='default.rgw.buckets.extra', group='objects'), - call(weight=0.10, replica_count=3, name='default.log', - group='objects'), - call(weight=0.10, replica_count=3, name='default.intent-log', - group='objects'), - call(weight=0.10, replica_count=3, name='default.usage', - group='objects'), - call(weight=0.10, replica_count=3, name='default.users', - group='objects'), - call(weight=0.10, replica_count=3, name='default.users.email', - group='objects'), - call(weight=0.10, replica_count=3, name='default.users.swift', - group='objects'), - call(weight=0.10, replica_count=3, name='default.users.uid', + call(weight=3.00, replica_count=3, + name='default.rgw.buckets.index', group='objects'), call(weight=0.10, replica_count=3, name='.rgw.root', - group='objects')] + group='objects')], ) mock_request_access.assert_called_with(key_name='radosgw.gateway', name='objects',