From e6a150c0d2894dadb870a4a356f1f9ab2336faa8 Mon Sep 17 00:00:00 2001
From: Syed Mohammad Adnan Karim <syed.karim@canonical.com>
Date: Mon, 26 Nov 2018 16:30:22 +0000
Subject: [PATCH] Update hook to remove old ceph.conf alternatives

When adding ceph-mon relation to cinder, the charm installs ceph.conf
with the update-alternatives via cinder_utils.resource_map().
However when the relation is removed, the alternative isn't cleaned up.
This can cause issues if installing a cinder-ceph subordinate charm.
The cinder-ceph charm also installs a ceph.conf alternative that will
point to the leftover ceph.conf installed by the ceph-mon charm.

Added remove_alternative() in ceph-relation-broken hook to ensure
that leftover ceph.conf alternatives is removed upon relation removal.

Change-Id: If9a8d460ee8209ef917fa55ec970379e9c741ec6
Related-Bug: 1778084
---
 hooks/cinder_hooks.py           | 4 ++++
 hooks/cinder_utils.py           | 3 ++-
 unit_tests/test_cinder_hooks.py | 6 ++++++
 3 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/hooks/cinder_hooks.py b/hooks/cinder_hooks.py
index e3ca16c2..cf6bccdc 100755
--- a/hooks/cinder_hooks.py
+++ b/hooks/cinder_hooks.py
@@ -39,6 +39,7 @@ from cinder_utils import (
     CLUSTER_RES,
     CINDER_CONF,
     CINDER_API_CONF,
+    CEPH_CONF,
     setup_ipv6,
     check_local_db_actions_complete,
     filesystem_mounted,
@@ -92,6 +93,8 @@ from charmhelpers.contrib.openstack.utils import (
     series_upgrade_complete,
 )
 
+from charmhelpers.contrib.openstack.alternatives import remove_alternative
+
 from charmhelpers.contrib.storage.linux.ceph import (
     send_request_if_needed,
     is_request_complete,
@@ -435,6 +438,7 @@ def ceph_broken():
     service = service_name()
     delete_keyring(service=service)
     CONFIGS.write_all()
+    remove_alternative(os.path.basename(CEPH_CONF), ceph_config_file())
 
 
 @hooks.hook('cluster-relation-joined')
diff --git a/hooks/cinder_utils.py b/hooks/cinder_utils.py
index ebd92405..0d7f20d5 100644
--- a/hooks/cinder_utils.py
+++ b/hooks/cinder_utils.py
@@ -37,6 +37,7 @@ from charmhelpers.core.hookenv import (
     related_units,
     log,
     DEBUG,
+    hook_name,
 )
 
 from charmhelpers.fetch import (
@@ -272,7 +273,7 @@ def resource_map(release=None):
         resource_map[CINDER_CONF]['services'].append('cinder-backup')
         resource_map[ceph_config_file()]['services'].append('cinder-backup')
 
-    if relation_ids('ceph'):
+    if relation_ids('ceph') and hook_name() != 'ceph-relation-broken':
         # need to create this early, new peers will have a relation during
         # registration # before they've run the ceph hooks to create the
         # directory.
diff --git a/unit_tests/test_cinder_hooks.py b/unit_tests/test_cinder_hooks.py
index dd910b2c..f6d8100d 100644
--- a/unit_tests/test_cinder_hooks.py
+++ b/unit_tests/test_cinder_hooks.py
@@ -12,6 +12,7 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+import os
 import json
 
 from six.moves import reload_module
@@ -57,6 +58,7 @@ TO_PATCH = [
     'service_enabled',
     'CONFIGS',
     'CLUSTER_RES',
+    'CEPH_CONF',
     'ceph_config_file',
     'update_nrpe_config',
     'remove_old_packages',
@@ -87,6 +89,7 @@ TO_PATCH = [
     'execd_preinstall',
     'sync_db_with_multi_ipv6_addresses',
     'delete_keyring',
+    'remove_alternative',
     'get_relation_ip',
     'services',
 ]
@@ -579,6 +582,9 @@ class TestJoinedHooks(CharmTestCase):
         hooks.hooks.execute(['hooks/ceph-relation-broken'])
         self.delete_keyring.assert_called_with(service='cinder')
         self.assertTrue(self.CONFIGS.write_all.called)
+        self.remove_alternative.assert_called_with(
+            os.path.basename(self.CEPH_CONF),
+            self.ceph_config_file())
 
     def test_ceph_changed_no_leadership(self):
         '''It does not attempt to create ceph pool if not leader'''