From 983f7211a872a5be9698d7c57fda41020765a64a Mon Sep 17 00:00:00 2001 From: Dan Smith Date: Fri, 17 Mar 2017 07:11:38 -0700 Subject: [PATCH] Fix aggregate_update name and availability_zone clash The name and availability_zone arguments to aggregate update were replaced by optional parameters in change I778ab7ec54a376c60f19dcc89fe62fcab6e59e42. However, the '--name' and 'name' arguments in the parser would conflict, resulting in only the deprecated argument working. Thus, attempting to update the name on an aggregate using --name would end up doing a PUT with no new name provided. Note that there were unit tests for this, but they were not catching this problem. So, this removes those tests and adds functional tests to poke it. Change-Id: Ifef6fdc1a737dd219712a4525d4e34afd3fbd80c Closes-Bug: #1673789 (cherry picked from commit 20f00553d0d8ed791105d5f7fc8798b9ac6a6a53) --- .../tests/functional/v2/test_aggregates.py | 64 +++++++++++++++++++ novaclient/tests/unit/v2/test_shell.py | 24 ------- novaclient/v2/shell.py | 14 ++-- 3 files changed, 72 insertions(+), 30 deletions(-) create mode 100644 novaclient/tests/functional/v2/test_aggregates.py diff --git a/novaclient/tests/functional/v2/test_aggregates.py b/novaclient/tests/functional/v2/test_aggregates.py new file mode 100644 index 000000000..6eb30242e --- /dev/null +++ b/novaclient/tests/functional/v2/test_aggregates.py @@ -0,0 +1,64 @@ +# -*- coding: utf-8 -*- +# 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. + +from oslo_utils import uuidutils + +from novaclient.tests.functional import base + + +class TestAggregatesNovaClient(base.ClientTestBase): + COMPUTE_API_VERSION = '2.1' + + def setUp(self): + super(TestAggregatesNovaClient, self).setUp() + self.agg1 = 'agg-%s' % uuidutils.generate_uuid() + self.agg2 = 'agg-%s' % uuidutils.generate_uuid() + self.addCleanup(self._clean_aggregates) + + def _clean_aggregates(self): + for a in (self.agg1, self.agg2): + try: + self.nova('aggregate-delete', params=a) + except Exception: + pass + + def test_aggregate_update_name_legacy(self): + self.nova('aggregate-create', params=self.agg1) + self.nova('aggregate-update', params='%s %s' % (self.agg1, self.agg2)) + output = self.nova('aggregate-show', params=self.agg2) + self.assertIn(self.agg2, output) + self.nova('aggregate-delete', params=self.agg2) + + def test_aggregate_update_name(self): + self.nova('aggregate-create', params=self.agg1) + self.nova('aggregate-update', + params='--name=%s %s' % (self.agg2, self.agg1)) + output = self.nova('aggregate-show', params=self.agg2) + self.assertIn(self.agg2, output) + self.nova('aggregate-delete', params=self.agg2) + + def test_aggregate_update_az_legacy(self): + self.nova('aggregate-create', params=self.agg2) + self.nova('aggregate-update', + params='%s %s myaz' % (self.agg2, self.agg2)) + output = self.nova('aggregate-show', params=self.agg2) + self.assertIn('myaz', output) + self.nova('aggregate-delete', params=self.agg2) + + def test_aggregate_update_az(self): + self.nova('aggregate-create', params=self.agg2) + self.nova('aggregate-update', + params='--availability-zone=myaz %s' % self.agg2) + output = self.nova('aggregate-show', params=self.agg2) + self.assertIn('myaz', output) + self.nova('aggregate-delete', params=self.agg2) diff --git a/novaclient/tests/unit/v2/test_shell.py b/novaclient/tests/unit/v2/test_shell.py index 45b135f5e..373ec0c49 100644 --- a/novaclient/tests/unit/v2/test_shell.py +++ b/novaclient/tests/unit/v2/test_shell.py @@ -2016,30 +2016,6 @@ class ShellTest(utils.TestCase): self.assert_called('PUT', '/os-aggregates/1', body, pos=-2) self.assert_called('GET', '/os-aggregates/1', pos=-1) - def test_aggregate_update_by_id_legacy(self): - self.run_command('aggregate-update 1 new_name') - body = {"aggregate": {"name": "new_name"}} - self.assert_called('PUT', '/os-aggregates/1', body, pos=-2) - self.assert_called('GET', '/os-aggregates/1', pos=-1) - - def test_aggregate_update_by_name_legacy(self): - self.run_command('aggregate-update test new_name') - body = {"aggregate": {"name": "new_name"}} - self.assert_called('PUT', '/os-aggregates/1', body, pos=-2) - self.assert_called('GET', '/os-aggregates/1', pos=-1) - - def test_aggregate_update_with_availability_zone_by_id_legacy(self): - self.run_command('aggregate-update 1 foo new_zone') - body = {"aggregate": {"name": "foo", "availability_zone": "new_zone"}} - self.assert_called('PUT', '/os-aggregates/1', body, pos=-2) - self.assert_called('GET', '/os-aggregates/1', pos=-1) - - def test_aggregate_update_with_availability_zone_by_name_legacy(self): - self.run_command('aggregate-update test foo new_zone') - body = {"aggregate": {"name": "foo", "availability_zone": "new_zone"}} - self.assert_called('PUT', '/os-aggregates/1', body, pos=-2) - self.assert_called('GET', '/os-aggregates/1', pos=-1) - def test_aggregate_set_metadata_add_by_id(self): out, err = self.run_command('aggregate-set-metadata 3 foo=bar') body = {"set_metadata": {"metadata": {"foo": "bar"}}} diff --git a/novaclient/v2/shell.py b/novaclient/v2/shell.py index 810159544..288675013 100644 --- a/novaclient/v2/shell.py +++ b/novaclient/v2/shell.py @@ -3771,7 +3771,8 @@ def do_aggregate_delete(cs, args): metavar='', help=_('Name or ID of aggregate to update.')) @utils.arg( - 'name', + 'old_name', + metavar='', nargs='?', action=shell.DeprecatedAction, use=_('use "%s"; this option will be removed in ' @@ -3782,7 +3783,7 @@ def do_aggregate_delete(cs, args): dest='name', help=_('Name of aggregate.')) @utils.arg( - 'availability_zone', + 'old_availability_zone', metavar='', nargs='?', default=None, @@ -3799,10 +3800,11 @@ def do_aggregate_update(cs, args): """Update the aggregate's name and optionally availability zone.""" aggregate = _find_aggregate(cs, args.aggregate) updates = {} - if args.name: - updates["name"] = args.name - if args.availability_zone: - updates["availability_zone"] = args.availability_zone + if args.name or args.old_name: + updates["name"] = args.name or args.old_name + if args.availability_zone or args.old_availability_zone: + updates["availability_zone"] = (args.availability_zone or + args.old_availability_zone) aggregate = cs.aggregates.update(aggregate.id, updates) print(_("Aggregate %s has been successfully updated.") % aggregate.id)