Merge "Make metadata_server_url config optional"

heat/common/config.py

@@ -38,8 +38,10 @@ service_opts = [
                default=60,
                help=_('Seconds between running periodic tasks.')),
     cfg.StrOpt('heat_metadata_server_url',
-               default="",
+               help=_('URL of the Heat metadata server. '
-               help=_('URL of the Heat metadata server.')),
+                      'NOTE: Setting this is only needed if you require '
+                      'instances to use a different endpoint than in the '
+                      'keystone catalog')),
     cfg.StrOpt('heat_waitcondition_server_url',
                help=_('URL of the Heat waitcondition server.')),
     cfg.StrOpt('heat_watch_server_url',

heat/engine/clients/os/heat_plugin.py

@@ -11,6 +11,8 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
+from oslo_config import cfg
+
 from heatclient import client as hc
 from heatclient import exc
 
@@ -76,3 +78,21 @@ class HeatClientPlugin(client_plugin.ClientPlugin):
         heat_cfn_url = self.url_for(service_type=self.CLOUDFORMATION,
                                     endpoint_type=endpoint_type)
         return heat_cfn_url
+
+    def get_cfn_metadata_server_url(self):
+        # Historically, we've required heat_metadata_server_url set in
+        # heat.conf, which simply points to the heat-api-cfn endpoint in
+        # most cases, so fall back to looking in the catalog when not set
+        config_url = cfg.CONF.heat_metadata_server_url
+        if config_url is None:
+            config_url = self.get_heat_cfn_url()
+        # Backwards compatibility, previous heat_metadata_server_url
+        # values didn't have to include the version path suffix
+        # Also, we always added a trailing "/" in nova/server.py,
+        # which looks not required by os-collect-config, but maintain
+        # to avoid any risk other folks have scripts which expect it.
+        if '/v1' not in config_url:
+            config_url += '/v1'
+        if config_url and config_url[-1] != "/":
+            config_url += '/'
+        return config_url

heat/engine/clients/os/nova.py

@@ -356,12 +356,14 @@ echo -e '%s\tALL=(ALL)\tNOPASSWD: ALL' >> /etc/sudoers
                             'cfn-watch-server', 'x-cfninitdata'))
 
         if is_cfntools:
-            attachments.append((cfg.CONF.heat_metadata_server_url,
+            heat_client_plugin = self.context.clients.client_plugin('heat')
+            cfn_md_url = heat_client_plugin.get_cfn_metadata_server_url()
+            attachments.append((cfn_md_url,
                                 'cfn-metadata-server', 'x-cfninitdata'))
 
             # Create a boto config which the cfntools on the host use to know
             # where the cfn and cw API's are to be accessed
-            cfn_url = urlparse.urlparse(cfg.CONF.heat_metadata_server_url)
+            cfn_url = urlparse.urlparse(cfn_md_url)
             cw_url = urlparse.urlparse(cfg.CONF.heat_watch_server_url)
             is_secure = cfg.CONF.instance_connection_is_secure
             vcerts = cfg.CONF.instance_connection_https_validate_certificates

heat/engine/resources/openstack/nova/server.py

@@ -600,8 +600,10 @@ class Server(stack_user.StackUser, sh.SchedulerHintsMixin,
                 'queue_id': queue_id}})
 
         elif self.transport_poll_server_cfn(props):
+            heat_client_plugin = self.stack.clients.client_plugin('heat')
+            config_url = heat_client_plugin.get_cfn_metadata_server_url()
             occ.update({'cfn': {
-                'metadata_url': '%s/v1/' % cfg.CONF.heat_metadata_server_url,
+                'metadata_url': config_url,
                 'access_key_id': self.access_key,
                 'secret_access_key': self.secret_key,
                 'stack_name': self.stack.name,

heat/tests/clients/test_clients.py

@@ -73,18 +73,32 @@ class ClientsTest(common.HeatTestCase):
         obj._get_client_option.return_value = result
         self.assertEqual(result, obj.get_heat_url())
 
-    def test_clients_get_heat_cfn_url(self):
+    def _client_cfn_url(self):
         con = mock.Mock()
         c = clients.Clients(con)
         con.clients = c
-
         obj = c.client_plugin('heat')
         obj._get_client_option = mock.Mock()
         obj._get_client_option.return_value = None
         obj.url_for = mock.Mock(name="url_for")
-        heat_cfn_url = "http://0.0.0.0:8000/v1"
+        obj.url_for.return_value = "http://0.0.0.0:8000/v1/"
-        obj.url_for.return_value = heat_cfn_url
+        return obj
-        self.assertEqual(heat_cfn_url, obj.get_heat_cfn_url())
+
+    def test_clients_get_heat_cfn_url(self):
+        obj = self._client_cfn_url()
+        self.assertEqual("http://0.0.0.0:8000/v1/", obj.get_heat_cfn_url())
+
+    def test_clients_get_heat_cfn_metadata_url(self):
+        obj = self._client_cfn_url()
+        self.assertEqual("http://0.0.0.0:8000/v1/",
+                         obj.get_cfn_metadata_server_url())
+
+    def test_clients_get_heat_cfn_metadata_url_conf(self):
+        cfg.CONF.set_override('heat_metadata_server_url',
+                              'http://server.test:123')
+        obj = self._client_cfn_url()
+        self.assertEqual("http://server.test:123/v1/",
+                         obj.get_cfn_metadata_server_url())
 
     @mock.patch.object(heatclient, 'Client')
     def test_clients_heat(self, mock_call):

heat/tests/openstack/nova/test_server.py

@@ -28,6 +28,7 @@ from heat.common import exception
 from heat.common.i18n import _
 from heat.common import template_format
 from heat.engine.clients.os import glance
+from heat.engine.clients.os import heat_plugin
 from heat.engine.clients.os import neutron
 from heat.engine.clients.os import nova
 from heat.engine.clients.os import swift
@@ -747,14 +748,16 @@ class ServersTest(common.HeatTestCase):
         else:
             return server
 
-    def test_server_create_software_config(self):
+    @mock.patch.object(heat_plugin.HeatClientPlugin, 'url_for')
+    def test_server_create_software_config(self, fake_url):
+        fake_url.return_value = 'the-cfn-url'
         server = self._server_create_software_config()
 
         self.assertEqual({
             'os-collect-config': {
                 'cfn': {
                     'access_key_id': '4567',
-                    'metadata_url': '/v1/',
+                    'metadata_url': 'the-cfn-url/v1/',
                     'path': 'WebServer.Metadata',
                     'secret_access_key': '8901',
                     'stack_name': 'software_config_s'
@@ -763,15 +766,17 @@ class ServersTest(common.HeatTestCase):
             'deployments': []
         }, server.metadata_get())
 
-    def test_server_create_software_config_metadata(self):
+    @mock.patch.object(heat_plugin.HeatClientPlugin, 'url_for')
+    def test_server_create_software_config_metadata(self, fake_url):
         md = {'os-collect-config': {'polling_interval': 10}}
+        fake_url.return_value = 'the-cfn-url'
         server = self._server_create_software_config(md=md)
 
         self.assertEqual({
             'os-collect-config': {
                 'cfn': {
                     'access_key_id': '4567',
-                    'metadata_url': '/v1/',
+                    'metadata_url': 'the-cfn-url/v1/',
                     'path': 'WebServer.Metadata',
                     'secret_access_key': '8901',
                     'stack_name': 'software_config_s'
@@ -1603,7 +1608,9 @@ class ServersTest(common.HeatTestCase):
         self.assertEqual({'test': 456}, server.metadata_get())
         self.m.VerifyAll()
 
-    def test_server_update_metadata_software_config(self):
+    @mock.patch.object(heat_plugin.HeatClientPlugin, 'url_for')
+    def test_server_update_metadata_software_config(self, fake_url):
+        fake_url.return_value = 'the-cfn-url'
         server, ud_tmpl = self._server_create_software_config(
             stack_name='update_meta_sc', ret_tmpl=True)
 
@@ -1611,7 +1618,7 @@ class ServersTest(common.HeatTestCase):
             'os-collect-config': {
                 'cfn': {
                     'access_key_id': '4567',
-                    'metadata_url': '/v1/',
+                    'metadata_url': 'the-cfn-url/v1/',
                     'path': 'WebServer.Metadata',
                     'secret_access_key': '8901',
                     'stack_name': 'update_meta_sc'
@@ -1634,8 +1641,10 @@ class ServersTest(common.HeatTestCase):
         self.assertEqual(expected_md, server.metadata_get())
         self.m.VerifyAll()
 
-    def test_server_update_metadata_software_config_merge(self):
+    @mock.patch.object(heat_plugin.HeatClientPlugin, 'url_for')
+    def test_server_update_metadata_software_config_merge(self, fake_url):
         md = {'os-collect-config': {'polling_interval': 10}}
+        fake_url.return_value = 'the-cfn-url'
         server, ud_tmpl = self._server_create_software_config(
             stack_name='update_meta_sc', ret_tmpl=True,
             md=md)
@@ -1644,7 +1653,7 @@ class ServersTest(common.HeatTestCase):
             'os-collect-config': {
                 'cfn': {
                     'access_key_id': '4567',
-                    'metadata_url': '/v1/',
+                    'metadata_url': 'the-cfn-url/v1/',
                     'path': 'WebServer.Metadata',
                     'secret_access_key': '8901',
                     'stack_name': 'update_meta_sc'
@@ -1668,8 +1677,10 @@ class ServersTest(common.HeatTestCase):
         self.assertEqual(expected_md, server.metadata_get())
         self.m.VerifyAll()
 
-    def test_server_update_software_config_transport(self):
+    @mock.patch.object(heat_plugin.HeatClientPlugin, 'url_for')
+    def test_server_update_software_config_transport(self, fake_url):
         md = {'os-collect-config': {'polling_interval': 10}}
+        fake_url.return_value = 'the-cfn-url'
         server = self._server_create_software_config(
             stack_name='update_meta_sc', md=md)
 
@@ -1677,7 +1688,7 @@ class ServersTest(common.HeatTestCase):
             'os-collect-config': {
                 'cfn': {
                     'access_key_id': '4567',
-                    'metadata_url': '/v1/',
+                    'metadata_url': 'the-cfn-url/v1/',
                     'path': 'WebServer.Metadata',
                     'secret_access_key': '8901',
                     'stack_name': 'update_meta_sc'