Merge lp:~billy-olsen/charms/trusty/ceph-radosgw/backport-lp1398182 into lp:~openstack-charmers-archive/charms/trusty/ceph-radosgw/trunk

[Billy Olsen]

[uosci-testing-bot (uosci-testing-bot)]

charm_lint_check #5403 ceph-radosgw for billy-olsen mp262000
    LINT OK: passed

Build: http://10.245.162.77:8080/job/charm_lint_check/5403/

[uosci-testing-bot (uosci-testing-bot)]

charm_unit_test #5035 ceph-radosgw for billy-olsen mp262000
    UNIT OK: passed

Build: http://10.245.162.77:8080/job/charm_unit_test/5035/

[uosci-testing-bot (uosci-testing-bot)]

charm_amulet_test #4646 ceph-radosgw for billy-olsen mp262000
    AMULET OK: passed

Build: http://10.245.162.77:8080/job/charm_amulet_test/4646/

[Corey Bryant (corey.bryant)]

review: Approve

=== modified file 'config.yaml'
--- config.yaml	2015-01-14 13:27:02 +0000
+++ config.yaml	2015-06-15 18:43:37 +0000
@@ -87,3 +87,15 @@
     description: |
       Default multicast port number that will be used to communicate between
       HA Cluster nodes.
+  os-public-hostname:
+    type: string
+    default:
+    description: |
+      The hostname or address of the public endpoints created for ceph-radosgw
+      in the keystone identity provider.
+      .
+      This value will be used for public endpoints. For example, an
+      os-public-hostname set to 'files.example.com' with will create
+      the following public endpoint for the ceph-radosgw:
+      .
+      https://files.example.com:80/swift/v1

=== modified file 'hooks/charmhelpers/contrib/openstack/ip.py'
--- hooks/charmhelpers/contrib/openstack/ip.py	2015-02-24 11:02:02 +0000
+++ hooks/charmhelpers/contrib/openstack/ip.py	2015-06-15 18:43:37 +0000
@@ -17,6 +17,7 @@
 from charmhelpers.core.hookenv import (
     config,
     unit_get,
+    service_name,
 )
 from charmhelpers.contrib.network.ip import (
     get_address_in_network,
@@ -26,8 +27,6 @@
 )
 from charmhelpers.contrib.hahelpers.cluster import is_clustered
 
-from functools import partial
-
 PUBLIC = 'public'
 INTERNAL = 'int'
 ADMIN = 'admin'
@@ -35,15 +34,18 @@
 ADDRESS_MAP = {
     PUBLIC: {
         'config': 'os-public-network',
-        'fallback': 'public-address'
+        'fallback': 'public-address',
+        'override': 'os-public-hostname',
     },
     INTERNAL: {
         'config': 'os-internal-network',
-        'fallback': 'private-address'
+        'fallback': 'private-address',
+        'override': 'os-internal-hostname',
     },
     ADMIN: {
         'config': 'os-admin-network',
-        'fallback': 'private-address'
+        'fallback': 'private-address',
+        'override': 'os-admin-hostname',
     }
 }
 
@@ -57,15 +59,50 @@
     :param endpoint_type: str endpoint type to resolve.
     :param returns: str base URL for services on the current service unit.
     """
-    scheme = 'http'
-    if 'https' in configs.complete_contexts():
-        scheme = 'https'
+    scheme = _get_scheme(configs)
+
     address = resolve_address(endpoint_type)
     if is_ipv6(address):
         address = "[{}]".format(address)
+
     return '%s://%s' % (scheme, address)
 
 
+def _get_scheme(configs):
+    """Returns the scheme to use for the url (either http or https)
+    depending upon whether https is in the configs value.
+
+    :param configs: OSTemplateRenderer config templating object to inspect
+                    for a complete https context.
+    :returns: either 'http' or 'https' depending on whether https is
+              configured within the configs context.
+    """
+    scheme = 'http'
+    if configs and 'https' in configs.complete_contexts():
+        scheme = 'https'
+    return scheme
+
+
+def _get_address_override(endpoint_type=PUBLIC):
+    """Returns any address overrides that the user has defined based on the
+    endpoint type.
+
+    Note: this function allows for the service name to be inserted into the
+    address if the user specifies {service_name}.somehost.org.
+
+    :param endpoint_type: the type of endpoint to retrieve the override
+                          value for.
+    :returns: any endpoint address or hostname that the user has overridden
+              or None if an override is not present.
+    """
+    override_key = ADDRESS_MAP[endpoint_type]['override']
+    addr_override = config(override_key)
+    if not addr_override:
+        return None
+    else:
+        return addr_override.format(service_name=service_name())
+
+
 def resolve_address(endpoint_type=PUBLIC):
     """Return unit address depending on net config.
 
@@ -77,7 +114,10 @@
 
     :param endpoint_type: Network endpoing type
     """
-    resolved_address = None
+    resolved_address = _get_address_override(endpoint_type)
+    if resolved_address:
+        return resolved_address
+
     vips = config('vip')
     if vips:
         vips = vips.split()
@@ -109,38 +149,3 @@
                          "clustered=%s)" % (net_type, clustered))
 
     return resolved_address
-
-
-def endpoint_url(configs, url_template, port, endpoint_type=PUBLIC,
-                 override=None):
-    """Returns the correct endpoint URL to advertise to Keystone.
-
-    This method provides the correct endpoint URL which should be advertised to
-    the keystone charm for endpoint creation. This method allows for the url to
-    be overridden to force a keystone endpoint to have specific URL for any of
-    the defined scopes (admin, internal, public).
-
-    :param configs: OSTemplateRenderer config templating object to inspect
-                    for a complete https context.
-    :param url_template: str format string for creating the url template. Only
-                         two values will be passed - the scheme+hostname
-                        returned by the canonical_url and the port.
-    :param endpoint_type: str endpoint type to resolve.
-    :param override: str the name of the config option which overrides the
-                     endpoint URL defined by the charm itself. None will
-                     disable any overrides (default).
-    """
-    if override:
-        # Return any user-defined overrides for the keystone endpoint URL.
-        user_value = config(override)
-        if user_value:
-            return user_value.strip()
-
-    return url_template % (canonical_url(configs, endpoint_type), port)
-
-
-public_endpoint = partial(endpoint_url, endpoint_type=PUBLIC)
-
-internal_endpoint = partial(endpoint_url, endpoint_type=INTERNAL)
-
-admin_endpoint = partial(endpoint_url, endpoint_type=ADMIN)

=== modified file 'hooks/hooks.py'
--- hooks/hooks.py	2015-04-28 21:18:24 +0000
+++ hooks/hooks.py	2015-06-15 18:43:37 +0000
@@ -50,10 +50,9 @@
 from charmhelpers.contrib.network.ip import (
     get_iface_for_address,
     get_netmask_for_address,
-    is_ipv6,
 )
 from charmhelpers.contrib.openstack.ip import (
-    resolve_address,
+    canonical_url,
     PUBLIC, INTERNAL, ADMIN,
 )
 
@@ -273,16 +272,6 @@
     open_port(port=80)
 
 
-# XXX Define local canonical_url until charm has been updated to use the
-#     standard context architecture.
-def canonical_url(configs, endpoint_type=PUBLIC):
-    scheme = 'http'
-    address = resolve_address(endpoint_type)
-    if is_ipv6(address):
-        address = "[{}]".format(address)
-    return '%s://%s' % (scheme, address)
-
-
 @hooks.hook('identity-service-relation-joined')
 def identity_joined(relid=None):
     if cmp_pkgrevno('radosgw', '0.55') < 0:
@@ -290,11 +279,11 @@
         sys.exit(1)
 
     port = 80
-    admin_url = '%s:%i/swift' % (canonical_url(ADMIN), port)
+    admin_url = '%s:%i/swift' % (canonical_url(None, ADMIN), port)
     internal_url = '%s:%s/swift/v1' % \
-        (canonical_url(INTERNAL), port)
+        (canonical_url(None, INTERNAL), port)
     public_url = '%s:%s/swift/v1' % \
-        (canonical_url(PUBLIC), port)
+        (canonical_url(None, PUBLIC), port)
     relation_set(service='swift',
                  region=config('region'),
                  public_url=public_url, internal_url=internal_url,

=== modified file 'unit_tests/test_hooks.py'
--- unit_tests/test_hooks.py	2015-04-28 21:18:24 +0000
+++ unit_tests/test_hooks.py	2015-06-15 18:43:37 +0000
@@ -8,6 +8,7 @@
     CharmTestCase,
     patch_open
 )
+from charmhelpers.contrib.openstack.ip import PUBLIC
 
 dnsmock = MagicMock()
 modules = {
@@ -45,7 +46,6 @@
     'relation_set',
     'relation_get',
     'render_template',
-    'resolve_address',
     'shutil',
     'subprocess',
     'sys',
@@ -323,14 +323,22 @@
         cmd = ['service', 'radosgw', 'restart']
         self.subprocess.call.assert_called_with(cmd)
 
-    def test_identity_joined_early_version(self):
+    @patch('charmhelpers.contrib.openstack.ip.service_name',
+           lambda *args: 'ceph-radosgw')
+    @patch('charmhelpers.contrib.openstack.ip.config')
+    def test_identity_joined_early_version(self, _config):
         self.cmp_pkgrevno.return_value = -1
         ceph_hooks.identity_joined()
         self.sys.exit.assert_called_with(1)
 
-    def test_identity_joined(self):
+    @patch('charmhelpers.contrib.openstack.ip.service_name',
+           lambda *args: 'ceph-radosgw')
+    @patch('charmhelpers.contrib.openstack.ip.resolve_address')
+    @patch('charmhelpers.contrib.openstack.ip.config')
+    def test_identity_joined(self, _config, _resolve_address):
         self.cmp_pkgrevno.return_value = 1
-        self.resolve_address.return_value = 'myserv'
+        _resolve_address.return_value = 'myserv'
+        _config.side_effect = self.test_config.get
         self.test_config.set('region', 'region1')
         self.test_config.set('operator-roles', 'admin')
         self.unit_get.return_value = 'myserv'
@@ -344,6 +352,27 @@
             relation_id='rid',
             admin_url='http://myserv:80/swift')
 
+    @patch('charmhelpers.contrib.openstack.ip.service_name',
+           lambda *args: 'ceph-radosgw')
+    @patch('charmhelpers.contrib.openstack.ip.is_clustered')
+    @patch('charmhelpers.contrib.openstack.ip.unit_get')
+    @patch('charmhelpers.contrib.openstack.ip.config')
+    def test_identity_joined_public_name(self, _config, _unit_get,
+                                         _is_clustered):
+        _config.side_effect = self.test_config.get
+        self.test_config.set('os-public-hostname', 'files.example.com')
+        _unit_get.return_value = 'myserv'
+        _is_clustered.return_value = False
+        ceph_hooks.identity_joined(relid='rid')
+        self.relation_set.assert_called_with(
+            service='swift',
+            region='RegionOne',
+            public_url='http://files.example.com:80/swift/v1',
+            internal_url='http://myserv:80/swift/v1',
+            requested_roles='Member,Admin',
+            relation_id='rid',
+            admin_url='http://myserv:80/swift')
+
     def test_identity_changed(self):
         _emit_cephconf = self.patch('emit_cephconf')
         _restart = self.patch('restart')
@@ -351,10 +380,15 @@
         _emit_cephconf.assert_called()
         _restart.assert_called()
 
-    def test_canonical_url_ipv6(self):
+    @patch('charmhelpers.contrib.openstack.ip.is_clustered')
+    @patch('charmhelpers.contrib.openstack.ip.unit_get')
+    @patch('charmhelpers.contrib.openstack.ip.config')
+    def test_canonical_url_ipv6(self, _config, _unit_get, _is_clustered):
         ipv6_addr = '2001:db8:85a3:8d3:1319:8a2e:370:7348'
-        self.resolve_address.return_value = ipv6_addr
-        self.assertEquals(ceph_hooks.canonical_url({}),
+        _config.side_effect = self.test_config.get
+        _unit_get.return_value = ipv6_addr
+        _is_clustered.return_value = False
+        self.assertEquals(ceph_hooks.canonical_url({}, PUBLIC),
                           'http://[%s]' % ipv6_addr)
 
     @patch.object(ceph_hooks, 'CONFIGS')