diff --git a/config.yaml b/config.yaml index 8156bf1c..39688be6 100644 --- a/config.yaml +++ b/config.yaml @@ -211,3 +211,12 @@ options: . This needs to be set to 1 when deploying a cloud with the nova-lxd hypervisor. + no-bootstrap: + type: boolean + default: False + description: | + Causes the charm to not do any of the initial bootstrapping of the + Ceph monitor cluster. This is only intended to be used when migrating + from the ceph all-in-one charm to a ceph-mon / ceph-osd deployment. + Refer to the Charm Deployment guide at https://docs.openstack.org/charm-deployment-guide/latest/ + for more information. diff --git a/hooks/bootstrap-source-relation-changed b/hooks/bootstrap-source-relation-changed new file mode 120000 index 00000000..52d96630 --- /dev/null +++ b/hooks/bootstrap-source-relation-changed @@ -0,0 +1 @@ +ceph_hooks.py \ No newline at end of file diff --git a/hooks/bootstrap-source-relation-departed b/hooks/bootstrap-source-relation-departed new file mode 120000 index 00000000..52d96630 --- /dev/null +++ b/hooks/bootstrap-source-relation-departed @@ -0,0 +1 @@ +ceph_hooks.py \ No newline at end of file diff --git a/hooks/ceph_hooks.py b/hooks/ceph_hooks.py index 42a9a21b..9de2cd75 100755 --- a/hooks/ceph_hooks.py +++ b/hooks/ceph_hooks.py @@ -178,6 +178,9 @@ JOURNAL_ZAPPED = '/var/lib/ceph/journal_zapped' @hooks.hook('config-changed') @harden() def config_changed(): + # Get the cfg object so we can see if the no-bootstrap value has changed + # and triggered this hook invocation + cfg = config() if config('prefer-ipv6'): assert_charm_supports_ipv6() @@ -192,24 +195,32 @@ def config_changed(): update_nrpe_config() if is_leader(): - if not leader_get('fsid') or not leader_get('monitor-secret'): - if config('fsid'): - fsid = config('fsid') - else: - fsid = "{}".format(uuid.uuid1()) - if config('monitor-secret'): - mon_secret = config('monitor-secret') - else: - mon_secret = "{}".format(ceph.generate_monitor_secret()) - status_set('maintenance', 'Creating FSID and Monitor Secret') - opts = { - 'fsid': fsid, - 'monitor-secret': mon_secret, - } - log("Settings for the cluster are: {}".format(opts)) - leader_set(opts) - else: - if leader_get('fsid') is None or leader_get('monitor-secret') is None: + if not config('no-bootstrap'): + if not leader_get('fsid') or not leader_get('monitor-secret'): + if config('fsid'): + fsid = config('fsid') + else: + fsid = "{}".format(uuid.uuid1()) + if config('monitor-secret'): + mon_secret = config('monitor-secret') + else: + mon_secret = "{}".format(ceph.generate_monitor_secret()) + status_set('maintenance', 'Creating FSID and Monitor Secret') + opts = { + 'fsid': fsid, + 'monitor-secret': mon_secret, + } + log("Settings for the cluster are: {}".format(opts)) + leader_set(opts) + elif cfg.changed('no-bootstrap') and \ + is_relation_made('bootstrap-source'): + # User changed the no-bootstrap config option, we're the leader, + # and the bootstrap-source relation has been made. The charm should + # be in a blocked state indicating that the no-bootstrap option + # must be set. This block is invoked when the user is trying to + # get out of that scenario by enabling no-bootstrap. + bootstrap_source_relation_changed() + elif leader_get('fsid') is None or leader_get('monitor-secret') is None: log('still waiting for leader to setup keys') status_set('waiting', 'Waiting for leader to setup keys') sys.exit(0) @@ -232,7 +243,11 @@ def get_mon_hosts(): addr = get_public_addr() hosts.append('{}:6789'.format(format_ipv6_addr(addr) or addr)) - for relid in relation_ids('mon'): + rel_ids = relation_ids('mon') + if config('no-bootstrap'): + rel_ids += relation_ids('bootstrap-source') + + for relid in rel_ids: for unit in related_units(relid): addr = relation_get('ceph-public-address', unit, relid) if addr is not None: @@ -265,8 +280,70 @@ def mon_relation_joined(): relation_settings={'ceph-public-address': public_addr}) +@hooks.hook('bootstrap-source-relation-changed') +def bootstrap_source_relation_changed(): + """Handles relation data changes on the bootstrap-source relation. + + The bootstrap-source relation to share remote bootstrap information with + the ceph-mon charm. This relation is used to exchange the remote + ceph-public-addresses which are used for the mon's, the fsid, and the + monitor-secret. + """ + if not config('no-bootstrap'): + status_set('blocked', 'Cannot join the bootstrap-source relation when ' + 'no-bootstrap is False') + return + + if not is_leader(): + log('Deferring leader-setting updates to the leader unit') + return + + curr_fsid = leader_get('fsid') + curr_secret = leader_get('monitor-secret') + for relid in relation_ids('bootstrap-source'): + for unit in related_units(relid=relid): + mon_secret = relation_get('monitor-secret', unit, relid) + fsid = relation_get('fsid', unit, relid) + + if not (mon_secret and fsid): + log('Relation data is not ready as the fsid or the ' + 'monitor-secret are missing from the relation: ' + 'mon_secret = %s and fsid = %s ' % (mon_secret, fsid)) + continue + + if not (curr_fsid or curr_secret): + curr_fsid = fsid + curr_secret = mon_secret + else: + # The fsids and secrets need to match or the local monitors + # will fail to join the mon cluster. If they don't, + # bail because something needs to be investigated. + assert curr_fsid == fsid, \ + "bootstrap fsid '%s' != current fsid '%s'" % ( + fsid, curr_fsid) + assert curr_secret == mon_secret, \ + "bootstrap secret '%s' != current secret '%s'" % ( + mon_secret, curr_secret) + + opts = { + 'fsid': fsid, + 'monitor-secret': mon_secret, + } + + log('Updating leader settings for fsid and monitor-secret ' + 'from remote relation data: %s' % opts) + leader_set(opts) + + # The leader unit needs to bootstrap itself as it won't receive the + # leader-settings-changed hook elsewhere. + if curr_fsid: + mon_relation() + + @hooks.hook('mon-relation-departed', - 'mon-relation-changed') + 'mon-relation-changed', + 'leader-settings-changed', + 'bootstrap-source-relation-departed') def mon_relation(): if leader_get('monitor-secret') is None: log('still waiting for leader to setup keys') @@ -320,7 +397,8 @@ def mon_relation(): def notify_osds(): for relid in relation_ids('osd'): - osd_relation(relid) + for unit in related_units(relid): + osd_relation(relid=relid, unit=unit) def notify_radosgws(): @@ -341,7 +419,7 @@ def notify_client(): @hooks.hook('osd-relation-joined') @hooks.hook('osd-relation-changed') -def osd_relation(relid=None): +def osd_relation(relid=None, unit=None): if ceph.is_quorum(): log('mon cluster in quorum - providing fsid & keys') public_addr = get_public_addr() @@ -354,7 +432,7 @@ def osd_relation(relid=None): caps=ceph.osd_upgrade_caps), } - unit = remote_unit() + unit = unit or remote_unit() settings = relation_get(rid=relid, unit=unit) """Process broker request(s).""" if 'broker_req' in settings: @@ -590,6 +668,14 @@ VERSION_PACKAGE = 'ceph-common' def assess_status(): '''Assess status of current unit''' application_version_set(get_upstream_version(VERSION_PACKAGE)) + + # Check that the no-bootstrap config option is set in conjunction with + # having the bootstrap-source relation established + if not config('no-bootstrap') and is_relation_made('bootstrap-source'): + status_set('blocked', 'Cannot join the bootstrap-source relation when ' + 'no-bootstrap is False') + return + moncount = int(config('monitor-count')) units = get_peer_units() # not enough peers and mon_count > 1 diff --git a/hooks/leader-settings-changed b/hooks/leader-settings-changed new file mode 120000 index 00000000..52d96630 --- /dev/null +++ b/hooks/leader-settings-changed @@ -0,0 +1 @@ +ceph_hooks.py \ No newline at end of file diff --git a/metadata.yaml b/metadata.yaml index 82f9d191..bf379593 100644 --- a/metadata.yaml +++ b/metadata.yaml @@ -36,3 +36,6 @@ provides: nrpe-external-master: interface: nrpe-external-master scope: container +requires: + bootstrap-source: + interface: ceph-bootstrap diff --git a/unit_tests/test_ceph_hooks.py b/unit_tests/test_ceph_hooks.py index 44288f94..ef7a0786 100644 --- a/unit_tests/test_ceph_hooks.py +++ b/unit_tests/test_ceph_hooks.py @@ -11,8 +11,28 @@ sys.modules['apt'] = mock_apt mock_apt.apt_pkg = MagicMock() import charmhelpers.contrib.storage.linux.ceph as ceph -import ceph_hooks +import test_utils +with patch('charmhelpers.contrib.hardening.harden.harden') as mock_dec: + mock_dec.side_effect = (lambda *dargs, **dkwargs: lambda f: + lambda *args, **kwargs: f(*args, **kwargs)) + import ceph_hooks + + +TO_PATCH = [ + 'config', + 'is_leader', + 'is_relation_made', + 'leader_get', + 'leader_set', + 'log', + 'mon_relation', + 'relation_ids', + 'related_units', + 'relation_get', + 'relations_of_type', + 'status_set', +] CHARM_CONFIG = {'config-flags': '', 'auth-supported': False, @@ -193,7 +213,7 @@ class RelatedUnitsTestCase(unittest.TestCase): @patch.object(ceph_hooks, 'relation_ids') @patch.object(ceph_hooks, 'related_units') - def test_related_ods_single_relation(self, + def test_related_osd_single_relation(self, related_units, relation_ids): relation_ids.return_value = ['osd:0'] @@ -205,7 +225,7 @@ class RelatedUnitsTestCase(unittest.TestCase): @patch.object(ceph_hooks, 'relation_ids') @patch.object(ceph_hooks, 'related_units') - def test_related_ods_multi_relation(self, + def test_related_osd_multi_relation(self, related_units, relation_ids): relation_ids.return_value = ['osd:0', 'osd:23'] @@ -218,3 +238,119 @@ class RelatedUnitsTestCase(unittest.TestCase): call('osd:0'), call('osd:23') ]) + + +class BootstrapSourceTestCase(test_utils.CharmTestCase): + + def setUp(self): + super(BootstrapSourceTestCase, self).setUp(ceph_hooks, TO_PATCH) + self.config.side_effect = self.test_config.get + self.leader_get.side_effect = self.test_leader_settings.get + self.leader_set.side_effect = self.test_leader_settings.set + self.relation_get.side_effect = self.test_relation.get + self.test_config.set('no-bootstrap', True) + self.is_leader.return_value = True + self.relation_ids.return_value = ['bootstrap-source:0'] + self.related_units.return_value = ['ceph/0', 'ceph/1', 'ceph/2'] + + def test_bootstrap_source_no_bootstrap(self): + """Ensure the config option of no-bootstrap is set to continue""" + self.test_config.set('no-bootstrap', False) + ceph_hooks.bootstrap_source_relation_changed() + self.status_set.assert_called_once_with('blocked', + 'Cannot join the ' + 'bootstrap-source relation ' + 'when no-bootstrap is False') + + def test_bootstrap_source_not_leader(self): + """Ensure the processing is deferred to the leader""" + self.is_leader.return_value = False + ceph_hooks.bootstrap_source_relation_changed() + self.assertEqual(self.leader_set.call_count, 0) + + def test_bootstrap_source_relation_data_not_ready(self): + """Ensures no bootstrapping done if relation data not present""" + ceph_hooks.bootstrap_source_relation_changed() + expected_calls = [] + relid = 'bootstrap-source:0' + for unit in ('ceph/0', 'ceph/1', 'ceph/2'): + expected_calls.append(call('monitor-secret', unit, relid)) + expected_calls.append(call('fsid', unit, relid)) + self.relation_get.has_calls(expected_calls) + self.assertEqual(self.leader_set.call_count, 0) + self.assertEqual(self.mon_relation.call_count, 0) + + def test_bootstrap_source_good_path(self): + """Tests the good path where all is setup and relations established""" + self.test_relation.set({'monitor-secret': 'abcd', + 'fsid': '1234'}) + ceph_hooks.bootstrap_source_relation_changed() + self.leader_set.assert_called_with({'fsid': '1234', + 'monitor-secret': 'abcd'}) + self.mon_relation.assert_called_once_with() + + def test_bootstrap_source_different_fsid_secret(self): + """Tests where the bootstrap relation has a different fsid""" + self.test_relation.set({'monitor-secret': 'abcd', + 'fsid': '1234'}) + self.test_leader_settings.set({'monitor-secret': 'mysecret', + 'fsid': '7890'}) + self.assertRaises(AssertionError, + ceph_hooks.bootstrap_source_relation_changed) + + @patch.object(ceph_hooks, 'emit_cephconf') + @patch.object(ceph_hooks, 'create_sysctl') + @patch.object(ceph_hooks, 'check_for_upgrade') + @patch.object(ceph_hooks, 'get_mon_hosts') + @patch.object(ceph_hooks, 'bootstrap_source_relation_changed') + def test_config_changed_no_bootstrap_changed(self, + bootstrap_source_rel_changed, + get_mon_hosts, + check_for_upgrade, + create_sysctl, + emit_ceph_conf): + """Tests that changing no-bootstrap invokes the bs relation changed""" + self.relations_of_type.return_value = [] + self.is_relation_made.return_value = True + self.test_config.set_changed('no-bootstrap', True) + ceph_hooks.config_changed() + bootstrap_source_rel_changed.assert_called_once() + + @patch.object(ceph_hooks, 'get_public_addr') + def test_get_mon_hosts(self, get_public_addr): + """Tests that bootstrap-source relations are used""" + unit_addrs = { + 'mon:0': { + 'ceph-mon/0': '172.16.0.2', + 'ceph-mon/1': '172.16.0.3', + }, + 'bootstrap-source:1': { + 'ceph/0': '172.16.10.2', + 'ceph/1': '172.16.10.3', + 'cehp/2': '172.16.10.4', + } + } + + def rel_ids_side_effect(relname): + for key in unit_addrs.keys(): + if key.split(':')[0] == relname: + return [key] + return None + + def rel_get_side_effect(attr, unit, relid): + return unit_addrs[relid][unit] + + def rel_units_side_effect(relid): + if relid in unit_addrs: + return unit_addrs[relid].keys() + return [] + + self.relation_ids.side_effect = rel_ids_side_effect + self.related_units.side_effect = rel_units_side_effect + get_public_addr.return_value = '172.16.0.4' + self.relation_get.side_effect = rel_get_side_effect + hosts = ceph_hooks.get_mon_hosts() + self.assertEqual(hosts, [ + '172.16.0.2:6789', '172.16.0.3:6789', '172.16.0.4:6789', + '172.16.10.2:6789', '172.16.10.3:6789', '172.16.10.4:6789', + ]) diff --git a/unit_tests/test_status.py b/unit_tests/test_status.py index 4dfd75fa..33f924f5 100644 --- a/unit_tests/test_status.py +++ b/unit_tests/test_status.py @@ -32,6 +32,7 @@ TO_PATCH = [ 'status_set', 'config', 'ceph', + 'is_relation_made', 'relation_ids', 'relation_get', 'related_units', @@ -64,6 +65,7 @@ class ServiceStatusTestCase(test_utils.CharmTestCase): self.test_config.set('monitor-count', 3) self.local_unit.return_value = 'ceph-mon1' self.get_upstream_version.return_value = '10.2.2' + self.is_relation_made.return_value = False @mock.patch.object(hooks, 'get_peer_units') def test_assess_status_no_peers(self, _peer_units): @@ -123,3 +125,9 @@ class ServiceStatusTestCase(test_utils.CharmTestCase): 'ceph-mon2': True, 'ceph-mon3': True}, hooks.get_peer_units()) + + def test_no_bootstrap_not_set(self): + self.is_relation_made.return_value = True + hooks.assess_status() + self.status_set.assert_called_with('blocked', mock.ANY) + self.application_version_set.assert_called_with('10.2.2') diff --git a/unit_tests/test_utils.py b/unit_tests/test_utils.py index 97d3ee84..1665c161 100644 --- a/unit_tests/test_utils.py +++ b/unit_tests/test_utils.py @@ -66,6 +66,7 @@ class CharmTestCase(unittest.TestCase): self.obj = obj self.test_config = TestConfig() self.test_relation = TestRelation() + self.test_leader_settings = TestLeaderSettings() self.patch_all() def patch(self, method): @@ -83,10 +84,14 @@ class TestConfig(object): def __init__(self): self.config = get_default_config() + self.config_changed = {} + self.config_changed.setdefault(False) def get(self, attr=None): if not attr: - return self.get_all() + # Return a copy of self to allow emulation closer to what + # hookenv.config() returns (not a dict). + return self try: return self.config[attr] except KeyError: @@ -100,6 +105,15 @@ class TestConfig(object): raise KeyError self.config[attr] = value + def __getitem__(self, item): + return self.config[item] + + def changed(self, attr): + return self.config_changed[attr] + + def set_changed(self, attr, changed=True): + self.config_changed[attr] = changed + class TestRelation(object): @@ -117,6 +131,22 @@ class TestRelation(object): return None +class TestLeaderSettings(object): + + def __init__(self, settings={}): + self.settings = settings + + def set(self, settings): + self.settings = settings + + def get(self, attr=None): + if attr is None: + return self.settings + elif attr in self.settings: + return self.settings[attr] + return None + + @contextmanager def patch_open(): '''Patch open() to allow mocking both open() itself and the file that is