From 11db34a48ab9e5b98155a4ff9ec2bf2a11b3209f Mon Sep 17 00:00:00 2001
From: Xiaobin Qu <jxiaobin@ebaysf.com>
Date: Thu, 10 Mar 2016 01:05:11 -0800
Subject: [PATCH] Pass region_name to SwiftAPI

Currently when SwiftAPI(common/swift.py) is initialized, region_name
is not specified; with multi-region setup, ironic may pick wrong swift
endpoint.

This fix loads region_name from keystone_authtoken/region_name, and
pass the value to SwiftAPI

Closes-Bug: #1554933

Change-Id: I7295fc96c3b9ed50214576e80723d1d9db1a7e38
---
 ironic/common/swift.py                        |  9 +++++-
 ironic/tests/unit/common/test_swift.py        | 29 ++++++++++++++-----
 ...egion-to-swiftclient-c8c8bf1020f62ebc.yaml |  5 ++++
 3 files changed, 34 insertions(+), 9 deletions(-)
 create mode 100644 releasenotes/notes/pass-region-to-swiftclient-c8c8bf1020f62ebc.yaml

diff --git a/ironic/common/swift.py b/ironic/common/swift.py
index d351b89a80..b5c66c6625 100644
--- a/ironic/common/swift.py
+++ b/ironic/common/swift.py
@@ -50,6 +50,8 @@ CONF.import_opt('insecure', 'keystonemiddleware.auth_token',
                 group='keystone_authtoken')
 CONF.import_opt('cafile', 'keystonemiddleware.auth_token',
                 group='keystone_authtoken')
+CONF.import_opt('region_name', 'keystonemiddleware.auth_token',
+                group='keystone_authtoken')
 
 
 class SwiftAPI(object):
@@ -60,7 +62,8 @@ class SwiftAPI(object):
                  tenant_name=None,
                  key=None,
                  auth_url=None,
-                 auth_version=None):
+                 auth_version=None,
+                 region_name=None):
         """Constructor for creating a SwiftAPI object.
 
         :param user: the name of the user for Swift account
@@ -68,6 +71,7 @@ class SwiftAPI(object):
         :param key: the 'password' or key to authenticate with
         :param auth_url: the url for authentication
         :param auth_version: the version of api to use for authentication
+        :param region_name: the region used for getting endpoints of swift
         """
         user = user or CONF.keystone_authtoken.admin_user
         tenant_name = tenant_name or CONF.keystone_authtoken.admin_tenant_name
@@ -83,6 +87,9 @@ class SwiftAPI(object):
                   'key': key,
                   'authurl': auth_url,
                   'auth_version': auth_version}
+        region_name = region_name or CONF.keystone_authtoken.region_name
+        if region_name:
+            params['os_options'] = {'region_name': region_name}
 
         self.connection = swift_client.Connection(**params)
 
diff --git a/ironic/tests/unit/common/test_swift.py b/ironic/tests/unit/common/test_swift.py
index 92fece6138..f6961458c1 100644
--- a/ironic/tests/unit/common/test_swift.py
+++ b/ironic/tests/unit/common/test_swift.py
@@ -47,17 +47,30 @@ class SwiftTestCase(base.TestCase):
         self.config(swift_max_retries=2, group='swift')
         self.config(insecure=0, group='keystone_authtoken')
         self.config(cafile='/path/to/ca/file', group='keystone_authtoken')
+        self.expected_params = {'retries': 2,
+                                'insecure': 0,
+                                'user': 'admin',
+                                'tenant_name': 'tenant',
+                                'key': 'password',
+                                'authurl': 'http://authurl/v2.0',
+                                'cacert': '/path/to/ca/file',
+                                'auth_version': '2'}
 
     def test___init__(self, connection_mock):
         swift.SwiftAPI()
-        params = {'retries': 2,
-                  'insecure': 0,
-                  'user': 'admin',
-                  'tenant_name': 'tenant',
-                  'key': 'password',
-                  'authurl': 'http://authurl/v2.0',
-                  'cacert': '/path/to/ca/file',
-                  'auth_version': '2'}
+        connection_mock.assert_called_once_with(**self.expected_params)
+
+    def test__init__with_region_from_config(self, connection_mock):
+        self.config(region_name='region1', group='keystone_authtoken')
+        swift.SwiftAPI()
+        params = self.expected_params.copy()
+        params['os_options'] = {'region_name': 'region1'}
+        connection_mock.assert_called_once_with(**params)
+
+    def test__init__with_region_from_constructor(self, connection_mock):
+        swift.SwiftAPI(region_name='region1')
+        params = self.expected_params.copy()
+        params['os_options'] = {'region_name': 'region1'}
         connection_mock.assert_called_once_with(**params)
 
     @mock.patch.object(__builtin__, 'open', autospec=True)
diff --git a/releasenotes/notes/pass-region-to-swiftclient-c8c8bf1020f62ebc.yaml b/releasenotes/notes/pass-region-to-swiftclient-c8c8bf1020f62ebc.yaml
new file mode 100644
index 0000000000..0887f2ddde
--- /dev/null
+++ b/releasenotes/notes/pass-region-to-swiftclient-c8c8bf1020f62ebc.yaml
@@ -0,0 +1,5 @@
+---
+fixes:
+  - Fixes a bug where the keystone_authtoken/region_name wasn't passed
+    to Swift when instantiating its client, in a multi-region environment
+    this is needed so the client can choose the correct swift endpoint.