Merge lp:~ajkavanagh/openstack-mojo-specs/fix-os-project-id-nova-creds into lp:~ost-maintainers/openstack-mojo-specs/mojo-openstack-specs-1709

Proposed by Alex Kavanagh
Status: Merged
Approved by: Liam Young
Approved revision: 314
Merged at revision: 335
Proposed branch: lp:~ajkavanagh/openstack-mojo-specs/fix-os-project-id-nova-creds
Merge into: lp:~ost-maintainers/openstack-mojo-specs/mojo-openstack-specs-1709
Diff against target: 58 lines (+15/-7)
2 files modified
helper/utils/mojo_os_utils.py (+12/-7)
helper/utils/mojo_utils.py (+3/-0)
To merge this branch: bzr merge lp:~ajkavanagh/openstack-mojo-specs/fix-os-project-id-nova-creds
Reviewer Review Type Date Requested Status
Liam Young (community) Needs Fixing
Ryan Beisner Needs Information
Review via email: mp+330298@code.launchpad.net

Description of the change

Fix OS_PROJECT_ID polution from env to overcloud nova creds
Also some fixes for keystone client and service_catalog.

To post a comment you must log in.
Revision history for this message
Ryan Beisner (1chb1n) wrote :

I've not exercised this, and I've also not had any mojo specs fail without this (hundres have run in OSCI). Please have gnuoy and/or thedac review. Thank you.

review: Needs Information
Revision history for this message
Liam Young (gnuoy) wrote :

I think the issue may come from whether you have a novarc sourced when you run mojo. I'm +1 on removing the os.environ.get('OS_PROJECT_ID') from helper/utils/mojo_os_utils.py

Only issue for me seems to be some code duplication.

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

Agree with Liam. The project id move is fine. The get_keystone_session_client supersedes get_keystone_client so that bit can left as is.

Revision history for this message
Alex Kavanagh (ajkavanagh) wrote :

David, Liam: I don't understand either of your comments. What do you want to keep? What do you want to remove?

Revision history for this message
Liam Young (gnuoy) wrote :

Alex, a function exists that returns a keystone session object based on the api version. That function is called get_keystone_session. You have amended (admittedly pre-existing) code in get_keystone_client that creates a keystone session object based on the api version. I am suggesting that rather than have logic for getting a keystone session object based on the api version in get_keystone_client, get_keystone_client should call get_keystone_session.

Revision history for this message
Alex Kavanagh (ajkavanagh) wrote :

Liam, please see inline comment. Please could you review your 'needs fixing' or suggest a solution that maintains backwards compatibility. Thanks.

Revision history for this message
Liam Young (gnuoy) wrote :

ok

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'helper/utils/mojo_os_utils.py'
--- helper/utils/mojo_os_utils.py 2017-08-30 17:25:03 +0000
+++ helper/utils/mojo_os_utils.py 2017-09-06 15:12:56 +0000
@@ -28,8 +28,8 @@
28# Openstack Client helpers28# Openstack Client helpers
29def get_nova_creds(cloud_creds):29def get_nova_creds(cloud_creds):
30 auth = get_ks_creds(cloud_creds)30 auth = get_ks_creds(cloud_creds)
31 if os.environ.get('OS_PROJECT_ID'):31 if cloud_creds.get('OS_PROJECT_ID'):
32 auth['project_id'] = os.environ.get('OS_PROJECT_ID')32 auth['project_id'] = cloud_creds.get('OS_PROJECT_ID')
33 return auth33 return auth
3434
3535
@@ -111,11 +111,16 @@
111def get_keystone_client(novarc_creds, insecure=True):111def get_keystone_client(novarc_creds, insecure=True):
112 keystone_creds = get_ks_creds(novarc_creds)112 keystone_creds = get_ks_creds(novarc_creds)
113 if novarc_creds.get('API_VERSION', 2) == 2:113 if novarc_creds.get('API_VERSION', 2) == 2:
114 sess = v2.Password(**keystone_creds)114 auth = v2.Password(**keystone_creds)
115 return keystoneclient_v2.Client(session=sess)115 sess = session.Session(auth=auth, verify=True)
116 client = keystoneclient_v2.Client(session=sess)
116 else:117 else:
117 sess = v3.Password(**keystone_creds)118 auth = v3.Password(**keystone_creds)
118 return keystoneclient_v3.Client(session=sess)119 sess = get_keystone_session(novarc_creds, insecure)
120 client = keystoneclient_v3.Client(session=sess)
121 # This populates the client.service_catalog
122 client.auth_ref = auth.get_access(sess)
123 return client
119124
120125
121def get_swift_client(novarc_creds, insecure=True):126def get_swift_client(novarc_creds, insecure=True):
@@ -136,7 +141,7 @@
136 if novarc_creds.get('API_VERSION', 2) == 2:141 if novarc_creds.get('API_VERSION', 2) == 2:
137 kc = get_keystone_client(novarc_creds)142 kc = get_keystone_client(novarc_creds)
138 glance_ep_url = kc.service_catalog.url_for(service_type='image',143 glance_ep_url = kc.service_catalog.url_for(service_type='image',
139 endpoint_type='publicURL')144 interface='publicURL')
140 else:145 else:
141 keystone_creds = get_ks_creds(novarc_creds, scope='PROJECT')146 keystone_creds = get_ks_creds(novarc_creds, scope='PROJECT')
142 kc = keystoneclient_v3.Client(**keystone_creds)147 kc = keystoneclient_v3.Client(**keystone_creds)
143148
=== modified file 'helper/utils/mojo_utils.py'
--- helper/utils/mojo_utils.py 2017-09-05 22:27:05 +0000
+++ helper/utils/mojo_utils.py 2017-09-06 15:12:56 +0000
@@ -354,6 +354,9 @@
354 os.environ.get('OS_PROJECT_NAME'))354 os.environ.get('OS_PROJECT_NAME'))
355 auth_settings['OS_PROJECT_DOMAIN_NAME'] = (355 auth_settings['OS_PROJECT_DOMAIN_NAME'] = (
356 os.environ.get('OS_PROJECT_DOMAIN_NAME'))356 os.environ.get('OS_PROJECT_DOMAIN_NAME'))
357 os_project_id = os.environ.get('OS_PROJECT_ID')
358 if os_project_id is not None:
359 auth_settings['OS_PROJECT_ID'] = os_project_id
357360
358 # Validate settings361 # Validate settings
359 for key, settings in auth_settings.items():362 for key, settings in auth_settings.items():

Subscribers

People subscribed via source and target branches