From 22961c5ba5c6573b615a9bf7cfbb78c365aa07aa Mon Sep 17 00:00:00 2001 From: "James E. Blair" Date: Mon, 24 Mar 2014 15:13:04 -0700 Subject: [PATCH] Add tests for the allocator And fix a bug in it that caused too-small allocations in some circumstances. The main demonstration of this failure was: nodepool.tests.test_allocator.TwoLabels.test_allocator(two_nodes) Which allocated 1,2 instead of 2,2. But the following tests also failed: nodepool.tests.test_allocator.TwoProvidersTwoLabels.test_allocator(four_nodes_over_quota) nodepool.tests.test_allocator.TwoProvidersTwoLabels.test_allocator(four_nodes) nodepool.tests.test_allocator.TwoProvidersTwoLabels.test_allocator(three_nodes) nodepool.tests.test_allocator.TwoProvidersTwoLabels.test_allocator(four_nodes_at_quota) nodepool.tests.test_allocator.TwoProvidersTwoLabels.test_allocator(one_node) Change-Id: Idba0e52b2775132f52386785b3d5f0974c5e0f8e --- .testr.conf | 4 + nodepool/allocation.py | 8 +- nodepool/tests/__init__.py | 62 +++++++++++ nodepool/tests/test_allocator.py | 177 +++++++++++++++++++++++++++++++ test-requirements.txt | 2 + 5 files changed, 249 insertions(+), 4 deletions(-) create mode 100644 .testr.conf create mode 100644 nodepool/tests/__init__.py create mode 100644 nodepool/tests/test_allocator.py diff --git a/.testr.conf b/.testr.conf new file mode 100644 index 000000000..15cea5ee1 --- /dev/null +++ b/.testr.conf @@ -0,0 +1,4 @@ +[DEFAULT] +test_command=OS_TEST_TIMEOUT=60 ${PYTHON:-python} -m subunit.run discover -t ./ nodepool/tests/ $LISTOPT $IDOPTION +test_id_option=--load-list $IDFILE +test_list_option=--list diff --git a/nodepool/allocation.py b/nodepool/allocation.py index 15505c7da..0ce53290c 100644 --- a/nodepool/allocation.py +++ b/nodepool/allocation.py @@ -74,8 +74,8 @@ class AllocationProvider(object): reqs.sort(lambda a, b: cmp(a.getPriority(), b.getPriority())) for req in reqs: total_requested = 0.0 - # Within a specific priority, grant requests - # proportionally. + # Within a specific priority, limit the number of + # available nodes to a value proportionate to the request. reqs_at_this_level = [r for r in reqs if r.getPriority() == req.getPriority()] for r in reqs_at_this_level: @@ -84,8 +84,8 @@ class AllocationProvider(object): ratio = float(req.amount) / total_requested else: ratio = 0.0 - grant = int(round(req.amount * ratio)) - grant = min(grant, self.available) + grant = int(round(req.amount)) + grant = min(grant, int(round(self.available * ratio))) # This adjusts our availability as well as the values of # other requests, so values will be correct the next time # through the loop. diff --git a/nodepool/tests/__init__.py b/nodepool/tests/__init__.py new file mode 100644 index 000000000..f6abab6af --- /dev/null +++ b/nodepool/tests/__init__.py @@ -0,0 +1,62 @@ +# Copyright (C) 2013 Hewlett-Packard Development Company, L.P. +# Copyright (C) 2014 OpenStack Foundation +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +"""Common utilities used in testing""" + +import os + +import fixtures +import testresources +import testtools + +TRUE_VALUES = ('true', '1', 'yes') + + +class BaseTestCase(testtools.TestCase, testresources.ResourcedTestCase): + + def setUp(self): + super(BaseTestCase, self).setUp() + test_timeout = os.environ.get('OS_TEST_TIMEOUT', 30) + try: + test_timeout = int(test_timeout) + except ValueError: + # If timeout value is invalid, fail hard. + print("OS_TEST_TIMEOUT set to invalid value" + " defaulting to no timeout") + test_timeout = 0 + if test_timeout > 0: + self.useFixture(fixtures.Timeout(test_timeout, gentle=True)) + + if os.environ.get('OS_STDOUT_CAPTURE') in TRUE_VALUES: + stdout = self.useFixture(fixtures.StringStream('stdout')).stream + self.useFixture(fixtures.MonkeyPatch('sys.stdout', stdout)) + if os.environ.get('OS_STDERR_CAPTURE') in TRUE_VALUES: + stderr = self.useFixture(fixtures.StringStream('stderr')).stream + self.useFixture(fixtures.MonkeyPatch('sys.stderr', stderr)) + + self.useFixture(fixtures.FakeLogger()) + self.useFixture(fixtures.NestedTempfile()) + + +class AllocatorTestCase(BaseTestCase): + def setUp(self): + super(AllocatorTestCase, self).setUp() + self.agt = [] + + def test_allocator(self): + for i, amount in enumerate(self.results): + print self.agt[i] + for i, amount in enumerate(self.results): + self.assertEqual(self.agt[i].amount, amount) diff --git a/nodepool/tests/test_allocator.py b/nodepool/tests/test_allocator.py new file mode 100644 index 000000000..8bc511ca0 --- /dev/null +++ b/nodepool/tests/test_allocator.py @@ -0,0 +1,177 @@ +# Copyright (C) 2014 OpenStack Foundation +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or +# implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +import testscenarios + +from nodepool import tests +from nodepool import allocation + + +class OneLabel(tests.AllocatorTestCase): + """The simplest case: one each of providers, labels, and + targets. + + Result AGT is: + * label1 from provider1 + """ + + scenarios = [ + ('one_node', + dict(provider1=10, label1=1, results=[1])), + ('two_nodes', + dict(provider1=10, label1=2, results=[2])), + ] + + def setUp(self): + super(OneLabel, self).setUp() + ap1 = allocation.AllocationProvider('provider1', self.provider1) + at1 = allocation.AllocationTarget('target1') + ar1 = allocation.AllocationRequest('label1', self.label1) + ar1.addTarget(at1, 0) + self.agt.append(ar1.addProvider(ap1, at1, 0)[1]) + ap1.makeGrants() + + +class TwoLabels(tests.AllocatorTestCase): + """Two labels from one provider. + + Result AGTs are: + * label1 from provider1 + * label1 from provider2 + """ + + scenarios = [ + ('one_node', + dict(provider1=10, label1=1, label2=1, results=[1, 1])), + ('two_nodes', + dict(provider1=10, label1=2, label2=2, results=[2, 2])), + ] + + def setUp(self): + super(TwoLabels, self).setUp() + ap1 = allocation.AllocationProvider('provider1', self.provider1) + at1 = allocation.AllocationTarget('target1') + ar1 = allocation.AllocationRequest('label1', self.label1) + ar2 = allocation.AllocationRequest('label2', self.label2) + ar1.addTarget(at1, 0) + ar2.addTarget(at1, 0) + self.agt.append(ar1.addProvider(ap1, at1, 0)[1]) + self.agt.append(ar2.addProvider(ap1, at1, 0)[1]) + ap1.makeGrants() + + +class TwoProvidersTwoLabels(tests.AllocatorTestCase): + """Two labels, each of which is supplied by both providers. + + Result AGTs are: + * label1 from provider1 + * label2 from provider1 + * label1 from provider2 + * label2 from provider2 + """ + + scenarios = [ + ('one_node', + dict(provider1=10, provider2=10, label1=1, label2=1, + results=[1, 1, 0, 0])), + ('two_nodes', + dict(provider1=10, provider2=10, label1=2, label2=2, + results=[1, 1, 1, 1])), + ('three_nodes', + dict(provider1=10, provider2=10, label1=3, label2=3, + results=[2, 2, 1, 1])), + ('four_nodes', + dict(provider1=10, provider2=10, label1=4, label2=4, + results=[2, 2, 2, 2])), + ('four_nodes_at_quota', + dict(provider1=4, provider2=4, label1=4, label2=4, + results=[2, 2, 2, 2])), + ('four_nodes_over_quota', + dict(provider1=2, provider2=2, label1=4, label2=4, + results=[1, 1, 1, 1])), + ] + + def setUp(self): + super(TwoProvidersTwoLabels, self).setUp() + ap1 = allocation.AllocationProvider('provider1', self.provider1) + ap2 = allocation.AllocationProvider('provider2', self.provider1) + at1 = allocation.AllocationTarget('target1') + ar1 = allocation.AllocationRequest('label1', self.label1) + ar2 = allocation.AllocationRequest('label2', self.label2) + ar1.addTarget(at1, 0) + ar2.addTarget(at1, 0) + self.agt.append(ar1.addProvider(ap1, at1, 0)[1]) + self.agt.append(ar2.addProvider(ap1, at1, 0)[1]) + self.agt.append(ar1.addProvider(ap2, at1, 0)[1]) + self.agt.append(ar2.addProvider(ap2, at1, 0)[1]) + ap1.makeGrants() + ap2.makeGrants() + + +class TwoProvidersTwoLabelsOneShared(tests.AllocatorTestCase): + """One label is served by both providers, the other can only come + from one. This tests that the allocator uses the diverse provider + to supply the label that can come from either while reserving + nodes from the more restricted provider for the label that can + only be supplied by it. + + label1 is supplied by provider1 and provider2. + label2 is supplied only by provider2. + + Result AGTs are: + * label1 from provider1 + * label2 from provider1 + * label2 from provider2 + """ + + scenarios = [ + ('one_node', + dict(provider1=10, provider2=10, label1=1, label2=1, + results=[1, 1, 0])), + ('two_nodes', + dict(provider1=10, provider2=10, label1=2, label2=2, + results=[2, 1, 1])), + ('three_nodes', + dict(provider1=10, provider2=10, label1=3, label2=3, + results=[3, 2, 1])), + ('four_nodes', + dict(provider1=10, provider2=10, label1=4, label2=4, + results=[4, 2, 2])), + ('four_nodes_at_quota', + dict(provider1=4, provider2=4, label1=4, label2=4, + results=[4, 0, 4])), + ('four_nodes_over_quota', + dict(provider1=2, provider2=2, label1=4, label2=4, + results=[2, 0, 2])), + ] + + def setUp(self): + super(TwoProvidersTwoLabelsOneShared, self).setUp() + ap1 = allocation.AllocationProvider('provider1', self.provider1) + ap2 = allocation.AllocationProvider('provider2', self.provider1) + at1 = allocation.AllocationTarget('target1') + ar1 = allocation.AllocationRequest('label1', self.label1) + ar2 = allocation.AllocationRequest('label2', self.label2) + ar1.addTarget(at1, 0) + ar2.addTarget(at1, 0) + self.agt.append(ar1.addProvider(ap1, at1, 0)[1]) + self.agt.append(ar2.addProvider(ap1, at1, 0)[1]) + self.agt.append(ar2.addProvider(ap2, at1, 0)[1]) + ap1.makeGrants() + ap2.makeGrants() + + +def load_tests(loader, in_tests, pattern): + return testscenarios.load_tests_apply_scenarios(loader, in_tests, pattern) diff --git a/test-requirements.txt b/test-requirements.txt index d5dcfcb89..bb7a4e27b 100644 --- a/test-requirements.txt +++ b/test-requirements.txt @@ -7,4 +7,6 @@ discover fixtures>=0.3.12 python-subunit testrepository>=0.0.13 +testresources +testscenarios testtools>=0.9.27