simplestreams

Merge lp:~thedac/simplestreams/keystone-v3-support into lp:simplestreams

  1. keystone-v3-support
  2. Merge into trunk
Proposed by David Ames
Status: Merged
Merged at revision: 450
Proposed branch: lp:~thedac/simplestreams/keystone-v3-support
Merge into: lp:simplestreams
Diff against target: 209 lines (+106/-16)
3 files modified
simplestreams/mirrors/glance.py (+7/-2)
simplestreams/objectstores/swift.py (+5/-1)
simplestreams/openstack.py (+94/-13)
To merge this branch: bzr merge lp:~thedac/simplestreams/keystone-v3-support
Reviewer Review Type Date Requested Status
Scott Moser (community) Approve
Server Team CI bot continuous-integration Approve
Alex Kavanagh (community) Approve
Review via email: mp+325781@code.launchpad.net

Commit message

Keystone v3 Support

Get the correct environment variables for Keystone v3.
Use keystoneauth1 sessions when available.

Description of the change

Keystone v3 Support

Get the correct environment variables for Keystone v3.
Use keystoneauth1 sessions when available.

To post a comment you must log in.
Revision history for this message
Alex Kavanagh (ajkavanagh) wrote :

LGTM

review: Approve
Revision history for this message
Server Team CI bot (server-team-bot) wrote :

PASSED: Continuous integration, rev:451
https://jenkins.ubuntu.com/server/job/simplestreams-ci/37/
Executed test runs:
    SUCCESS: https://jenkins.ubuntu.com/server/job/simplestreams-ci/nodes=metal-amd64/37
    SUCCESS: https://jenkins.ubuntu.com/server/job/simplestreams-ci/nodes=metal-arm64/37
    SUCCESS: https://jenkins.ubuntu.com/server/job/simplestreams-ci/nodes=metal-ppc64el/37
    SUCCESS: https://jenkins.ubuntu.com/server/job/simplestreams-ci/nodes=metal-s390x/37
    SUCCESS: https://jenkins.ubuntu.com/server/job/simplestreams-ci/nodes=vm-i386/37

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/simplestreams-ci/37/rebuild

review: Approve (continuous-integration)
Revision history for this message
Scott Moser (smoser) wrote :

Hi,

Very slow in responding, sorry.

I think this looks really good. There are 2 points I'd like addressed though:
a.) I think we need to take the v3 route only if 'OS_AUTH_VERSION' is set to 3 (or possibly OS_IDENTITY_API_VERSION). i'm not sure which is right, i see both of these in my 'serverstack' creds file. We should respect this if set, rather than your 'get_ks_api_version'

 b.) I suspect it is possible that the 'import' of 'v3' would fail . looking at my "example sync" from
http://bazaar.launchpad.net/~smoser/simplestreams/example-sync/view/head:/setup , this previously worked on trusty, so i'd like to have it continue to work there. I think we need to add some ImportError handling to address that.

review: Needs Fixing
Revision history for this message
Scott Moser (smoser) wrote :

David,

Can you try this out and see if it works for you?

https://code.launchpad.net/~smoser/simplestreams/keystone-v3-support/+merge/329379

Revision history for this message
Server Team CI bot (server-team-bot) wrote :

FAILED: Continuous integration, rev:453
https://jenkins.ubuntu.com/server/job/simplestreams-ci/39/
Executed test runs:
    FAILURE: https://jenkins.ubuntu.com/server/job/simplestreams-ci/nodes=metal-amd64/39/console
    FAILURE: https://jenkins.ubuntu.com/server/job/simplestreams-ci/nodes=metal-arm64/39/console
    FAILURE: https://jenkins.ubuntu.com/server/job/simplestreams-ci/nodes=metal-ppc64el/39/console
    FAILURE: https://jenkins.ubuntu.com/server/job/simplestreams-ci/nodes=metal-s390x/39/console
    FAILURE: https://jenkins.ubuntu.com/server/job/simplestreams-ci/nodes=vm-i386/39/console

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/simplestreams-ci/39/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
David Ames (thedac) wrote :

This should be ready now. I cannot test against serverstack until we are done with release.

The final remaining issue was swiftclient authentication. By sending it the session that seems to have fixed it in my test environments.

Revision history for this message
Scott Moser (smoser) wrote :

You can test with any credentials. Do not need any special privileges.

On September 1, 2017 7:31:00 PM EDT, David Ames <email address hidden> wrote:
>This should be ready now. I cannot test against serverstack until we
>are done with release.
>
>The final remaining issue was swiftclient authentication. By sending it
>the session that seems to have fixed it in my test environments.
>--
>https://code.launchpad.net/~thedac/simplestreams/keystone-v3-support/+merge/325781
>You are reviewing the proposed merge of
>lp:~thedac/simplestreams/keystone-v3-support into lp:simplestreams.

Revision history for this message
Joshua Powers (powersj) wrote :

Four CI failures:

https://jenkins.ubuntu.com/server/job/simplestreams-ci/39/nodes=metal-ppc64el/console

simplestreams/openstack.py:138:8: E111 indentation is not a multiple of four
simplestreams/openstack.py:144:8: E111 indentation is not a multiple of four

======================================================================
FAIL: test_create_glance_properties (tests.unittests.test_glancemirror.TestGlanceMirror)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/var/lib/jenkins/slaves/axew/workspace/simplestreams-ci/nodes/metal-ppc64el/ss-39/tests/unittests/test_glancemirror.py", line 256, in test_create_glance_properties
    properties)
AssertionError: {'con[19 chars]1', 'os_version': '16.04', 'version_name': 'X'[190 chars]ntu'} != {'con[19 chars]1', 'version_name': 'X', 'simplestreams_metada[144 chars]ntu'}
  {'content_id': 'content-1',
   'item_name': 'disk1.img',
- 'os_distro': 'ubuntu',
- 'os_version': '16.04',
   'product_name': 'foobuntu',
   'simplestreams_metadata': '{"extra": "value", "os": "ubuntu", "version": '
                             '"16.04"}',
   'source_content_id': 'source-1',
   'version_name': 'X'}

======================================================================
FAIL: test_insert_item_full (tests.unittests.test_glancemirror.TestGlanceMirror)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/var/lib/jenkins/slaves/axew/workspace/simplestreams-ci/nodes/metal-ppc64el/ss-39/tests/unittests/test_glancemirror.py", line 595, in test_insert_item_full
    self.assertEqual(expected_create_kwargs, passed_create_kwargs)
AssertionError: {'con[62 chars]c', 'os_version': '14.04', 'version_name': '20[878 chars]3f1'} != {'con[62 chars]c', 'architecture': 'x86_64', 'version_name': [832 chars]3f1'}
Diff is 1866 characters long. Set self.maxDiff to None to see it.
-------------------- >> begin captured stdout << ---------------------
created image-1: auto-sync/ubuntu-trusty-14.04-amd64-server-20160602-disk1.img

--------------------- >> end captured stdout << ----------------------
-------------------- >> begin captured logging << --------------------
sstreams: DEBUG: getting local copy of http://image-store/fooubuntu-X-disk1.img
--------------------- >> end captured logging << ---------------------

Revision history for this message
David Ames (thedac) wrote :

Scott,

Note, I had to revert the following to get unit tests to pass.

=== modified file 'simplestreams/mirrors/glance.py'
--- simplestreams/mirrors/glance.py 2017-08-22 17:41:18 +0000
+++ simplestreams/mirrors/glance.py 2017-09-11 16:44:40 +0000
@@ -272,8 +272,8 @@
                 name_old, name_new = carry_over_property
             else:
                 name_old = name_new = carry_over_property
- if name_new in image_metadata:
- properties[name_new] = image_metadata[name_old]
+ properties[name_new] = image_metadata[name_old]

         if 'arch' in image_metadata:
             properties['architecture'] = canonicalize_arch(

Revision history for this message
Server Team CI bot (server-team-bot) wrote :

PASSED: Continuous integration, rev:455
https://jenkins.ubuntu.com/server/job/simplestreams-ci/40/
Executed test runs:
    SUCCESS: https://jenkins.ubuntu.com/server/job/simplestreams-ci/nodes=metal-amd64/40
    SUCCESS: https://jenkins.ubuntu.com/server/job/simplestreams-ci/nodes=metal-arm64/40
    SUCCESS: https://jenkins.ubuntu.com/server/job/simplestreams-ci/nodes=metal-ppc64el/40
    SUCCESS: https://jenkins.ubuntu.com/server/job/simplestreams-ci/nodes=metal-s390x/40
    SUCCESS: https://jenkins.ubuntu.com/server/job/simplestreams-ci/nodes=vm-i386/40

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/simplestreams-ci/40/rebuild

review: Approve (continuous-integration)
Revision history for this message
David Ames (thedac) wrote :

That change is required. Looking at how to fix the unit tests.

Revision history for this message
Server Team CI bot (server-team-bot) wrote :

PASSED: Continuous integration, rev:456
https://jenkins.ubuntu.com/server/job/simplestreams-ci/41/
Executed test runs:
    SUCCESS: https://jenkins.ubuntu.com/server/job/simplestreams-ci/nodes=metal-amd64/41
    SUCCESS: https://jenkins.ubuntu.com/server/job/simplestreams-ci/nodes=metal-arm64/41
    SUCCESS: https://jenkins.ubuntu.com/server/job/simplestreams-ci/nodes=metal-ppc64el/41
    SUCCESS: https://jenkins.ubuntu.com/server/job/simplestreams-ci/nodes=metal-s390x/41
    SUCCESS: https://jenkins.ubuntu.com/server/job/simplestreams-ci/nodes=vm-i386/41

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/simplestreams-ci/41/rebuild

review: Approve (continuous-integration)
Revision history for this message
David Ames (thedac) wrote :

This has been tested against v3 and v2 clouds. Ready for merge.

Revision history for this message
Scott Moser (smoser) wrote :

Thank you David!

Revision history for this message
Scott Moser (smoser) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'simplestreams/mirrors/glance.py'
2--- simplestreams/mirrors/glance.py 2016-06-14 20:21:05 +0000
3+++ simplestreams/mirrors/glance.py 2017-09-11 20:33:57 +0000
4@@ -36,7 +36,11 @@
5 kwargs['endpoint'] = _strip_version(kwargs['endpoint'])
6 pt = ('endpoint', 'token', 'insecure', 'cacert')
7 kskw = {k: kwargs.get(k) for k in pt if k in kwargs}
8- return glanceclient.Client(version, **kskw)
9+ if kwargs.get('session'):
10+ sess = kwargs.get('session')
11+ return glanceclient.Client(version, session=sess)
12+ else:
13+ return glanceclient.Client(version, **kskw)
14
15
16 def empty_iid_products(content_id):
17@@ -65,6 +69,7 @@
18 LXC_FTYPES = [
19 'root.tar.gz',
20 'root.tar.xz',
21+ 'squashfs',
22 ]
23
24 QEMU_FTYPES = [
25@@ -267,7 +272,7 @@
26 name_old, name_new = carry_over_property
27 else:
28 name_old = name_new = carry_over_property
29- properties[name_new] = image_metadata[name_old]
30+ properties[name_new] = image_metadata.get(name_old)
31
32 if 'arch' in image_metadata:
33 properties['architecture'] = canonicalize_arch(
34
35=== modified file 'simplestreams/objectstores/swift.py'
36--- simplestreams/objectstores/swift.py 2014-07-15 21:10:44 +0000
37+++ simplestreams/objectstores/swift.py 2017-09-11 20:33:57 +0000
38@@ -33,7 +33,11 @@
39
40 connargs = {v: kwargs.get(k) for k, v in nmap.items() if k in kwargs}
41 connargs.update({k: kwargs.get(k) for k in pt if k in kwargs})
42- return Connection(**connargs)
43+ if kwargs.get('session'):
44+ sess = kwargs.get('session')
45+ return Connection(session=sess)
46+ else:
47+ return Connection(**connargs)
48
49
50 class SwiftContentSource(cs.IteratorContentSource):
51
52=== modified file 'simplestreams/openstack.py'
53--- simplestreams/openstack.py 2014-12-12 16:36:00 +0000
54+++ simplestreams/openstack.py 2017-09-11 20:33:57 +0000
55@@ -15,16 +15,47 @@
56 # You should have received a copy of the GNU Affero General Public License
57 # along with Simplestreams. If not, see <http://www.gnu.org/licenses/>.
58
59-from keystoneclient.v2_0 import client as ksclient
60+import collections
61 import os
62
63+from keystoneclient.v2_0 import client as ksclient_v2
64+from keystoneclient.v3 import client as ksclient_v3
65+try:
66+ from keystoneauth1 import session
67+ from keystoneauth1.identity import (v2, v3)
68+ _LEGACY_CLIENTS = False
69+except ImportError:
70+ # 14.04 level packages do not have this.
71+ session, v2, v3 = (None, None, None)
72+ _LEGACY_CLIENTS = True
73+
74+
75 OS_ENV_VARS = (
76 'OS_AUTH_TOKEN', 'OS_AUTH_URL', 'OS_CACERT', 'OS_IMAGE_API_VERSION',
77 'OS_IMAGE_URL', 'OS_PASSWORD', 'OS_REGION_NAME', 'OS_STORAGE_URL',
78- 'OS_TENANT_ID', 'OS_TENANT_NAME', 'OS_USERNAME', 'OS_INSECURE'
79+ 'OS_TENANT_ID', 'OS_TENANT_NAME', 'OS_USERNAME', 'OS_INSECURE',
80+ 'OS_USER_DOMAIN_NAME', 'OS_PROJECT_DOMAIN_NAME',
81+ 'OS_USER_DOMAIN_ID', 'OS_PROJECT_DOMAIN_ID', 'OS_PROJECT_NAME',
82+ 'OS_PROJECT_ID'
83 )
84
85
86+PT_V2 = ('username', 'password', 'tenant_id', 'tenant_name', 'auth_url',
87+ 'cacert', 'insecure', )
88+PT_V3 = ('username', 'password', 'project_id', 'project_name', 'auth_url',
89+ 'cacert', 'insecure', 'user_domain_name', 'project_domain_name',
90+ 'user_domain_id', 'project_domain_id', )
91+
92+
93+Settings = collections.namedtuple('Settings', 'mod ident arg_set')
94+KS_VERSION_RESOLVER = {2: Settings(mod=ksclient_v2,
95+ ident=v2,
96+ arg_set=PT_V2),
97+ 3: Settings(mod=ksclient_v3,
98+ ident=v3,
99+ arg_set=PT_V3), }
100+
101+
102 def load_keystone_creds(**kwargs):
103 # either via arguments or OS_* values in environment, the kwargs
104 # that are required are:
105@@ -57,9 +88,18 @@
106 if not (ret.get('auth_token') or ret.get('password')):
107 missing.append("(auth_token or password)")
108
109- if not (ret.get('tenant_id') or ret.get('tenant_name')):
110+ api_version = get_ks_api_version(ret.get('auth_url', '')) or 2
111+ if (api_version == 2 and
112+ not (ret.get('tenant_id') or ret.get('tenant_name'))):
113 raise ValueError("(tenant_id or tenant_name)")
114
115+ if (api_version == 3 and
116+ not (ret.get('user_domain_name') and
117+ ret.get('project_domain_name') and
118+ ret.get('project_name'))):
119+ raise ValueError("(user_domain_name, project_domain_name "
120+ "or project_name)")
121+
122 if missing:
123 raise ValueError("Need values for: %s" % missing)
124
125@@ -88,11 +128,49 @@
126 return list(regions)
127
128
129+def get_ks_api_version(auth_url=None, env=None):
130+ """Get the keystone api version based on the end of the auth url.
131+
132+ @param auth_url: String
133+ @returns: 2 or 3 (int)
134+ """
135+ if env is None:
136+ env = os.environ
137+
138+ if env.get('OS_IDENTITY_API_VERSION'):
139+ return int(env['OS_IDENTITY_API_VERSION'])
140+
141+ if auth_url is None:
142+ auth_url = ""
143+
144+ if auth_url.endswith('/v3'):
145+ return 3
146+ elif auth_url.endswith('/v2.0'):
147+ return 2
148+ # Return None if we can't determine the keystone version
149+ return None
150+
151+
152+def _legacy_ksclient(**kwargs):
153+ """14.04 does not have session available."""
154+ kskw = {k: kwargs.get(k) for k in PT_V2 if k in kwargs}
155+ return ksclient_v2.Client(**kskw)
156+
157+
158 def get_ksclient(**kwargs):
159- pt = ('username', 'password', 'tenant_id', 'tenant_name', 'auth_url',
160- 'cacert', 'insecure')
161- kskw = {k: kwargs.get(k) for k in pt if k in kwargs}
162- return ksclient.Client(**kskw)
163+ # api version will be force to 3 or 2
164+ if _LEGACY_CLIENTS:
165+ return _legacy_ksclient(**kwargs)
166+
167+ api_version = get_ks_api_version(kwargs.get('auth_url', '')) or 2
168+ arg_set = KS_VERSION_RESOLVER[api_version].arg_set
169+ # Filter/select the args for the api version from the kwargs dictionary
170+ kskw = {k: v for k, v in kwargs.items() if k in arg_set}
171+ auth = KS_VERSION_RESOLVER[api_version].ident.Password(**kskw)
172+ sess = session.Session(auth=auth)
173+ client = KS_VERSION_RESOLVER[api_version].mod.Client(session=sess)
174+ client.auth_ref = auth.get_access(sess)
175+ return client
176
177
178 def get_service_conn_info(service='image', client=None, **kwargs):
179@@ -101,21 +179,24 @@
180 client = get_ksclient(**kwargs)
181
182 endpoint = _get_endpoint(client, service, **kwargs)
183- return {'token': client.auth_token, 'insecure': kwargs.get('insecure'),
184+ info = {'token': client.auth_token, 'insecure': kwargs.get('insecure'),
185 'cacert': kwargs.get('cacert'), 'endpoint': endpoint,
186 'tenant_id': client.tenant_id}
187+ if not _LEGACY_CLIENTS:
188+ info['session'] = client.session
189+
190+ return info
191
192
193 def _get_endpoint(client, service, **kwargs):
194 """Get an endpoint using the provided keystone client."""
195 endpoint_kwargs = {
196 'service_type': service,
197- 'endpoint_type': kwargs.get('endpoint_type') or 'publicURL',
198+ 'interface': kwargs.get('endpoint_type') or 'publicURL',
199+ 'region_name': kwargs.get('region_name'),
200 }
201-
202- if kwargs.get('region_name'):
203- endpoint_kwargs['attr'] = 'region'
204- endpoint_kwargs['filter_value'] = kwargs.get('region_name')
205+ if _LEGACY_CLIENTS:
206+ del endpoint_kwargs['interface']
207
208 endpoint = client.service_catalog.url_for(**endpoint_kwargs)
209 return endpoint

Subscribers

People subscribed via source and target branches

to all changes: