Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

openstack: use OS_AUTH_TYPE=token_endpoint when possible #949

Merged
merged 1 commit into from Sep 23, 2016
Merged

openstack: use OS_AUTH_TYPE=token_endpoint when possible #949

merged 1 commit into from Sep 23, 2016

Conversation

ghost
Copy link

@ghost ghost commented Sep 2, 2016

It requires setting the OS_URL depending on the command and avoids
calling the auth endpoint entirely.

Fixes: http://tracker.ceph.com/issues/16893

Signed-off-by: Loic Dachary loic@dachary.org

@ghost
Copy link
Author

ghost commented Sep 2, 2016

@dmick this should avoid 429 errors on OVH

@@ -111,7 +116,7 @@ def create(self, num, os_type, os_version, arch, resources_hint):
flavor = self.flavor(resources_hint['machine'],
config['openstack'].get('flavor-select-regexp'))
cmd = ("flock --close --timeout 28800 /tmp/teuthology-server-create.lock" +
" openstack server create" +
" openstack --quiet server create" +
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems unconnected?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what does unconnected mean in this context ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The change has nothing to do with the rest of the changes or the symptom

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah, right

@dmick
Copy link
Member

dmick commented Sep 8, 2016

Test failures?

@ghost
Copy link
Author

ghost commented Sep 8, 2016

@dmick test failures ? which test failures ;-)

@dmick
Copy link
Member

dmick commented Sep 8, 2016

Thank you, rereviewing

@dmick
Copy link
Member

dmick commented Sep 14, 2016

I've tried several times to find the actual reference for how Openstack auth works that would allow me to understand these changes, and failed. Do you have a pointer?

also, can we separate the unconnected changes into a separate commit for clarity?

@ghost
Copy link
Author

ghost commented Sep 14, 2016

I've tried several times to find the actual reference for how Openstack auth works that would allow me to understand these changes, and failed. Do you have a pointer?

I don't unfortunately. I browsed the code of https://pypi.python.org/pypi/python-openstackclient and https://github.com/openstack/keystone until I figured out how to do what we need which is reusing tokens.

I think auth token throttling is unique to OVH, reason why documented strategies / implementations to cope with that do not exist. It also explains why some code paths have bugs when using an existing token instead of obtaining a new one. It's not a blocker since we can always fallback to the user/password authentication when it is the case.

I will separate the unconnected changes into a separate commit.

It requires setting the OS_URL depending on the command and avoids
calling the auth endpoint entirely.

Fixes: http://tracker.ceph.com/issues/16893

Signed-off-by: Loic Dachary <loic@dachary.org>
@dmick
Copy link
Member

dmick commented Sep 22, 2016

I will separate the unconnected changes into a separate commit.

Do you still plan to do this? Our ovh runs are still failing every night

@ghost
Copy link
Author

ghost commented Sep 22, 2016

I did it right away, did I miss anything ?

@dmick
Copy link
Member

dmick commented Sep 22, 2016

I still only see one commit, so didn't look. Did you just remove that code altogether?

@ghost
Copy link
Author

ghost commented Sep 22, 2016

Yes, the --quiet option that you mentionned as unconnected was unnecessary.

Copy link
Member

@dmick dmick left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK. Well it can't get much worse, so let's give it a shot.

@dmick dmick merged commit 04a68d8 into ceph:master Sep 23, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants