From 1975c4127121bd026bcbc128c21aeb8585e242a2 Mon Sep 17 00:00:00 2001
From: Liam Young <liam.young@canonical.com>
Date: Tue, 3 Oct 2017 09:54:35 +0000
Subject: [PATCH] Block endpoint reg if cluster partially formed

When an existing cluster of the service is scaled out the new unit
will join with keystone before it is fully clustered. In identity
joined hook the charmhelpers function canonical_url is called which
in turn uses another charmhelpers function, resolve_address.
resolve_address will only return the vip if the vip is set in config
AND the unit is clustered. This means that the units local address
is returned and that is then registered with keystone.

This change gates registering an endpoint if the cluster is
partially formed.

Change-Id: If483147e17dab8de2883058ee0f2718a3b7f8ca6
Partial-Bug: #1544959
---
 .gitignore                      |  1 +
 hooks/cinder_hooks.py           | 16 ++++++++++++----
 unit_tests/test_cinder_hooks.py |  7 +++++++
 3 files changed, 20 insertions(+), 4 deletions(-)

diff --git a/.gitignore b/.gitignore
index 8b9560ff..45f9daaf 100644
--- a/.gitignore
+++ b/.gitignore
@@ -5,3 +5,4 @@ bin
 *.sw[nop]
 *.pyc
 .unit-state.db
+.stestr
diff --git a/hooks/cinder_hooks.py b/hooks/cinder_hooks.py
index 13f7d437..09682f54 100755
--- a/hooks/cinder_hooks.py
+++ b/hooks/cinder_hooks.py
@@ -59,6 +59,7 @@ from charmhelpers.core.hookenv import (
     related_units,
     service_name,
     log,
+    DEBUG,
     ERROR,
     WARNING,
     status_set,
@@ -97,6 +98,7 @@ from charmhelpers.contrib.storage.linux.ceph import (
 )
 
 from charmhelpers.contrib.hahelpers.cluster import (
+    is_clustered,
     is_elected_leader,
     get_hacluster_config,
 )
@@ -307,13 +309,19 @@ def amqp_departed():
 
 @hooks.hook('identity-service-relation-joined')
 def identity_joined(rid=None):
+    if config('vip') and not is_clustered():
+        log('Defering registration until clustered', level=DEBUG)
+        return
+
     settings = {}
 
     if not service_enabled('api'):
-        juju_log('api service not enabled; skipping endpoint registration')
+        juju_log('api service not enabled; skipping endpoint '
+                 'registration')
         return
 
-    if CompareOpenStackReleases(os_release('cinder-common')) < 'pike':
+    cinder_release = os_release('cinder-common')
+    if CompareOpenStackReleases(cinder_release) < 'pike':
         public_url = '{}:{}/v1/$(tenant_id)s'.format(
             canonical_url(CONFIGS, PUBLIC),
             config('api-listening-port')
@@ -338,7 +346,7 @@ def identity_joined(rid=None):
             'cinder_internal_url': internal_url,
             'cinder_admin_url': admin_url,
         })
-    if CompareOpenStackReleases(os_release('cinder-common')) >= 'icehouse':
+    if CompareOpenStackReleases(cinder_release) >= 'icehouse':
         # NOTE(jamespage) register v2 endpoint as well
         public_url = '{}:{}/v2/$(tenant_id)s'.format(
             canonical_url(CONFIGS, PUBLIC),
@@ -359,7 +367,7 @@ def identity_joined(rid=None):
             'cinderv2_internal_url': internal_url,
             'cinderv2_admin_url': admin_url,
         })
-    if CompareOpenStackReleases(os_release('cinder-common')) >= 'pike':
+    if CompareOpenStackReleases(cinder_release) >= 'pike':
         # NOTE(jamespage) register v3 endpoint as well
         public_url = '{}:{}/v3/$(tenant_id)s'.format(
             canonical_url(CONFIGS, PUBLIC),
diff --git a/unit_tests/test_cinder_hooks.py b/unit_tests/test_cinder_hooks.py
index 3a8cb3cd..cbc398d2 100644
--- a/unit_tests/test_cinder_hooks.py
+++ b/unit_tests/test_cinder_hooks.py
@@ -55,6 +55,7 @@ TO_PATCH = [
     'do_openstack_upgrade',
     'ensure_ceph_keyring',
     'git_install',
+    'is_clustered',
     'juju_log',
     'log',
     'lsb_release',
@@ -368,6 +369,12 @@ class TestChangedHooks(CharmTestCase):
         hooks.hooks.execute(['hooks/amqp-relation-changed'])
         self.assertFalse(self.CONFIGS.write.called)
 
+    def test_identity_joined_partial_cluster(self):
+        self.is_clustered.return_value = False
+        self.test_config.set('vip', '10.0.0.10')
+        hooks.identity_joined()
+        self.assertFalse(self.relation_set.called)
+
     @patch.object(hooks, 'configure_https')
     def test_identity_changed(self, conf_https):
         'It writes out api-paste.ini on identity-service changed'