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

Update ovh flavors #109

Closed
wants to merge 1 commit into from
Closed

Update ovh flavors #109

wants to merge 1 commit into from

Conversation

kshtsk
Copy link

@kshtsk kshtsk commented Oct 25, 2017

Add s1 flavor to make teuthology working in new regions in OVH
Add workaround for floating ip exception for some regions in OVH

@kshtsk
Copy link
Author

kshtsk commented Oct 25, 2017

Related issues:
#77
#108

@kshtsk kshtsk requested a review from smithfarm October 25, 2017 09:41
if e.returncode == 1:
return None
else:
raise e
Copy link
Collaborator

Choose a reason for hiding this comment

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

When it was just one line [1] being duplicated it wasn't a big deal. But now that you add this try..except block the amount of code being duplicated increases considerably. Can you split this out into a helper method to eliminate the code duplication?

[1] ips = json.loads(OpenStack().run("ip floating list -f json"))

Copy link
Author

Choose a reason for hiding this comment

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

agree

Copy link
Collaborator

@smithfarm smithfarm left a comment

Choose a reason for hiding this comment

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

Other than the code duplication nit, this looks good to me. Needs some basic testing before merge.

@kshtsk kshtsk force-pushed the wip-update-ovh-flavors branch 3 times, most recently from 1e8e42d to 3cc921b Compare October 27, 2017 20:16
Add s1 flavor to make teuthology working in new regions in OVH
Add workaround for floating ip exception for some regions in OVH
@kshtsk kshtsk force-pushed the wip-update-ovh-flavors branch from 3cc921b to dd3a62d Compare October 31, 2017 14:16
@@ -454,7 +465,7 @@ def __flavor_wrapper(self, min, good, hint, arch):
select = None
if self.get_provider() == 'ovh':
log.debug("Looking for a match among the El Cheapo flavors...")
select = '^vps-ssd-'
select = '^(vps-ssd|s1)-'
Copy link
Collaborator

Choose a reason for hiding this comment

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

@kshtsk Did you want to reverse the order here?

Copy link
Author

Choose a reason for hiding this comment

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

@smithfarm yes... probably this can be implemented with ECP feature.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@kshtsk Good idea. Then this PR could be closed.

@kshtsk
Copy link
Author

kshtsk commented Apr 26, 2018

#124 is covering this request

@kshtsk kshtsk closed this Apr 26, 2018
@kshtsk kshtsk deleted the wip-update-ovh-flavors branch May 8, 2018 21:41
@kshtsk kshtsk mentioned this pull request Oct 5, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants