From 85bb3280e19e8ef28c552f38e179d07d124d79f0 Mon Sep 17 00:00:00 2001 From: Nathan Cutler Date: Fri, 12 May 2017 13:48:05 +0200 Subject: [PATCH] openstack: prefer vps-ssd-* flavors in OVH OVH has effectively two price tiers for VM flavors, which could be given the unofficial descriptive names "El Cheapo" and "Regular". Under the flavor scheme we are currently using the "El Cheapo" flavors are those that match the regular expression '^vps-ssd-' (just three flavors); all others are "Regular". Prefer the "El Cheapo" flavors over "Regular" flavors when the selection criteria yields matches in both OVH price tiers. For OpenStack providers other than OVH, the behavior should not change. In addition to the above, this patch removes duplication of the regular expression used for selecting OpenStack flavors. While it's convenient not to have to change the regular expression in three or more places, this comes with a disadvantage: the patch removes "flavor-select-regexp" from .teuthology.yaml, so it is no longer possible to adjust the flavor-selection heuristics by editing the configuration file. Such adjustment _must_ be done by modifying __flavor_wrapper(). This is not really a loss, though, because - as the OVH case demonstrates - a single regular expression match may not be enough to select the least expensive flavor matching the selection criteria. Fixes: https://github.com/SUSE/teuthology/issues/75 Signed-off-by: Nathan Cutler Conflicts: teuthology/openstack/__init__.py --- teuthology/openstack/__init__.py | 48 ++++++++++++++++----- teuthology/openstack/setup-openstack.sh | 19 ++------ teuthology/openstack/test/test_openstack.py | 14 +++--- teuthology/provision/openstack.py | 4 +- teuthology/task/buildpackages.py | 8 +--- 5 files changed, 50 insertions(+), 43 deletions(-) diff --git a/teuthology/openstack/__init__.py b/teuthology/openstack/__init__.py index 35196d9a97..18ecc90e2d 100644 --- a/teuthology/openstack/__init__.py +++ b/teuthology/openstack/__init__.py @@ -385,9 +385,7 @@ class OpenStack(object): return self.image_name(name) def get_sorted_flavors(self, arch, select): - """ - Return the smallest flavor that satisfies the desired size. - """ + log.debug("flavor selection regex: " + select) flavors_string = self.run("flavor list -f json") flavors = json.loads(flavors_string) found = [] @@ -404,7 +402,7 @@ class OpenStack(object): log.debug("sorted flavors = " + str(sorted_flavors)) return sorted_flavors - def flavor(self, hint, arch, select): + def __flavor(self, hint, arch, select): """ Return the smallest flavor that satisfies the desired size. """ @@ -418,7 +416,7 @@ class OpenStack(object): " does not contain a flavor in which" + " the desired " + str(hint) + " can fit") - def flavor_range(self, min, good, arch, select): + def __flavor_range(self, min, good, arch, select): """ Return the smallest flavor that satisfies the good hint. If no such flavor, get the largest flavor smaller than good @@ -443,6 +441,37 @@ class OpenStack(object): " does not contain a flavor which" + " is larger than " + str(min)) + def __flavor_wrapper(self, min, good, hint, arch): + """ + Wrapper for __flavor_range() and __flavor(), to hide the messiness of + the real world. + + This is the one, single place for coding OpenStack-provider-specific + heuristics for selecting flavors. + """ + select = None + if self.get_provider() == 'ovh': + log.debug("Looking for a match among the El Cheapo flavors...") + select = '^vps-ssd-' + try: + if (hint is not None): + return self.__flavor(hint, arch, select) + return self.__flavor_range(min, good, arch, select) + except NoFlavorException: + pass + log.debug("No El Cheapo flavors match the selection criteria. " + "Looking for a match among the more expensive flavors...") + select = '^(hg|sg|c2)-.*ssd' + if (hint is not None): + return self.__flavor(hint, arch, select) + return self.__flavor_range(min, good, arch, select) + + def flavor(self, hint, arch): + return self.__flavor_wrapper(None, None, hint, arch) + + def flavor_range(self, min, good, arch): + return self.__flavor_wrapper(min, good, None, arch) + def interpret_hints(self, defaults, hints): """ Return a hint hash which is the interpretation of a list of hints @@ -884,7 +913,7 @@ ssh access : ssh {identity}{username}@{ip} # logs in /usr/share/nginx/ log.exception("flavor list") raise Exception("verify openrc.sh has been sourced") - def flavor(self, arch): + def teuthology_openstack_flavor(self, arch): """ Return an OpenStack flavor fit to run the teuthology cluster. The RAM size depends on the maximum number of workers that @@ -908,10 +937,7 @@ ssh access : ssh {identity}{username}@{ip} # logs in /usr/share/nginx/ if self.args.controller_disk > 0: hint['disk'] = self.args.controller_disk - select = None - if self.get_provider() == 'ovh': - select = '^(vps|hg|sg|c2)-.*ssd' - return super(TeuthologyOpenStack, self).flavor(hint, arch, select) + return self.flavor(hint, arch) def net(self): """ @@ -1085,7 +1111,7 @@ openstack security group rule create --proto udp --dst-port 16000:65535 teutholo self.run( "server create " + " --image '" + self.image('ubuntu', '16.04', arch) + "' " + - " --flavor '" + self.flavor(arch) + "' " + + " --flavor '" + self.teuthology_openstack_flavor(arch) + "' " + " " + self.net() + " --key-name " + self.args.key_name + " --user-data " + user_data + diff --git a/teuthology/openstack/setup-openstack.sh b/teuthology/openstack/setup-openstack.sh index d1b7596f4c..0f6972d822 100755 --- a/teuthology/openstack/setup-openstack.sh +++ b/teuthology/openstack/setup-openstack.sh @@ -33,13 +33,8 @@ function create_config() { local nameserver="$3" local labdomain="$4" local ip="$5" - local flavor_select="$6" - local archive_upload="$7" - local canonical_tags="$8" - - if test "$flavor_select" ; then - flavor_select="flavor-select-regexp: $flavor_select" - fi + local archive_upload="$6" + local canonical_tags="$7" if test "$network" ; then network="network: $network" @@ -92,7 +87,6 @@ openstack: volumes: count: 0 size: 1 # GB - $flavor_select subnet: $subnet $network EOF @@ -530,7 +524,6 @@ function main() { local nameserver local labdomain=teuthology local nworkers=2 - local flavor_select local keypair=teuthology local archive_upload local ceph_workbench_git_url @@ -676,12 +669,6 @@ function main() { ;; esac - case $provider in - ovh) - flavor_select='^(vps|hg|sg|c2)-.*ssd' - ;; - esac - local ip for dev in eth0 ens3 ; do ip=$(ip a show dev $dev 2>/dev/null | sed -n "s:.*inet \(.*\)/.*:\1:p") @@ -690,7 +677,7 @@ function main() { : ${nameserver:=$ip} if $do_create_config ; then - create_config "$network" "$subnets" "$nameserver" "$labdomain" "$ip" "$flavor_select" "$archive_upload" "$canonical_tags" || return 1 + create_config "$network" "$subnets" "$nameserver" "$labdomain" "$ip" "$archive_upload" "$canonical_tags" || return 1 setup_ansible "$subnets" $labdomain || return 1 setup_ssh_config || return 1 setup_authorized_keys || return 1 diff --git a/teuthology/openstack/test/test_openstack.py b/teuthology/openstack/test/test_openstack.py index c1d0214347..a96c979d63 100644 --- a/teuthology/openstack/test/test_openstack.py +++ b/teuthology/openstack/test/test_openstack.py @@ -1310,7 +1310,7 @@ class TestOpenStack(TestOpenStackBase): ): with pytest.raises(NoFlavorException): hint = { 'ram': 1000, 'disk': 40, 'cpus': 2 } - OpenStack().flavor(hint, 'arch', None) + OpenStack().flavor(hint, 'arch') flavor = 'good-flavor' def get_sorted_flavors(self, arch, select): @@ -1327,7 +1327,7 @@ class TestOpenStack(TestOpenStackBase): get_sorted_flavors=get_sorted_flavors, ): hint = { 'ram': 1000, 'disk': 40, 'cpus': 2 } - assert flavor == OpenStack().flavor(hint, 'arch', None) + assert flavor == OpenStack().flavor(hint, 'arch') def test_flavor_range(self): flavors = [ @@ -1352,7 +1352,7 @@ class TestOpenStack(TestOpenStackBase): get_sorted_flavors=get_sorted_flavors, ): with pytest.raises(NoFlavorException): - OpenStack().flavor_range(min, good, 'arch', None) + OpenStack().flavor_range(min, good, 'arch') # # there is one flavor in the required range @@ -1369,7 +1369,7 @@ class TestOpenStack(TestOpenStackBase): get_sorted_flavors=get_sorted_flavors, ): - assert 'min' == OpenStack().flavor_range(min, good, 'arch', None) + assert 'min' == OpenStack().flavor_range(min, good, 'arch') # # out of the two flavors in the required range, get the bigger one @@ -1386,7 +1386,7 @@ class TestOpenStack(TestOpenStackBase): get_sorted_flavors=get_sorted_flavors, ): - assert 'good' == OpenStack().flavor_range(min, good, 'arch', None) + assert 'good' == OpenStack().flavor_range(min, good, 'arch') # # there is one flavor bigger or equal to good, get this one @@ -1403,7 +1403,7 @@ class TestOpenStack(TestOpenStackBase): get_sorted_flavors=get_sorted_flavors, ): - assert 'best' == OpenStack().flavor_range(min, good, 'arch', None) + assert 'best' == OpenStack().flavor_range(min, good, 'arch') # # there are two flavors bigger or equal to good, get the smallest one @@ -1420,7 +1420,7 @@ class TestOpenStack(TestOpenStackBase): get_sorted_flavors=get_sorted_flavors, ): - assert 'best' == OpenStack().flavor_range(min, good, 'arch', None) + assert 'best' == OpenStack().flavor_range(min, good, 'arch') def test_interpret_hints(self): diff --git a/teuthology/provision/openstack.py b/teuthology/provision/openstack.py index 724a7a56f1..937bdf85cd 100644 --- a/teuthology/provision/openstack.py +++ b/teuthology/provision/openstack.py @@ -121,9 +121,7 @@ class ProvisionOpenStack(OpenStack): net = "--nic net-id=" + str(self.net_id(config['openstack']['network'])) else: net = '' - flavor = self.flavor(resources_hint['machine'], - arch, - config['openstack'].get('flavor-select-regexp')) + flavor = self.flavor(resources_hint['machine'], arch) cmd = ("flock --close --timeout 28800 /tmp/teuthology-server-create.lock" + " openstack server create" + " " + config['openstack'].get('server-create', '') + diff --git a/teuthology/task/buildpackages.py b/teuthology/task/buildpackages.py index 40df7f20ca..b41971e1d4 100644 --- a/teuthology/task/buildpackages.py +++ b/teuthology/task/buildpackages.py @@ -181,22 +181,18 @@ def task(ctx, config): sha1) openstack = OpenStack() openstack.set_provider() - if openstack.provider == 'ovh': - select = '^(vps|hg|sg|c2)-.*ssd' - else: - select = '' network = openstack.net() if network != "": network = " OPENSTACK_NETWORK='" + network + "' " openstack.image(os_type, os_version, arch) # create if it does not exist build_flavor = openstack.flavor_range( - config['min_machine'], config['good_machine'], arch, select) + config['min_machine'], config['good_machine'], arch) default_arch = openstack.get_default_arch() http_flavor = openstack.flavor({ 'disk': 30, # GB 'ram': 1024, # MB 'cpus': 1, - }, default_arch, select) + }, default_arch) lock = "/tmp/buildpackages-" + sha1 + "-" + os_type + "-" + os_version cmd = (". " + os.environ['HOME'] + "/.ssh_agent ; " + " flock --close " + lock + -- 2.39.5