From 8f4331a6bdc12bb7632f2d170cc94176e0897fbf Mon Sep 17 00:00:00 2001 From: Alejandro Emanuel Paredes Date: Mon, 10 Mar 2014 18:14:07 -0300 Subject: [PATCH] Improve Host Aggregates handle method. Added update conditions to the handle method (remove and add hosts). Previously, to perform an update, first all existing hosts were removed and then all new hosts were added. Now, the update method only remove or add the changed hosts. Added missing unit tests for the update. Change-Id: I52acdd1226be504cf2d0cf029353dbf80e4aa01a Closes-bug: #1287192 --- .../dashboards/admin/aggregates/tests.py | 170 +++++++++++++++++- .../dashboards/admin/aggregates/workflows.py | 40 ++--- .../test/test_data/nova_data.py | 8 + 3 files changed, 191 insertions(+), 27 deletions(-) diff --git a/openstack_dashboard/dashboards/admin/aggregates/tests.py b/openstack_dashboard/dashboards/admin/aggregates/tests.py index 27810966dac..0bbf344e11f 100644 --- a/openstack_dashboard/dashboards/admin/aggregates/tests.py +++ b/openstack_dashboard/dashboards/admin/aggregates/tests.py @@ -103,7 +103,6 @@ def test_create_aggregate(self): 'aggregate_create', 'add_host_to_aggregate'), }) def test_create_aggregate_with_hosts(self): - aggregate = self.aggregates.first() hosts = self.hosts.list() @@ -222,6 +221,7 @@ class ManageHostsTests(test.BaseAdminViewTests): @test.create_stubs({api.nova: ('aggregate_get', 'host_list')}) def test_manage_hosts(self): aggregate = self.aggregates.first() + api.nova.aggregate_get(IsA(http.HttpRequest), str(aggregate.id)) \ .AndReturn(aggregate) api.nova.host_list(IsA(http.HttpRequest)) \ @@ -235,15 +235,21 @@ def test_manage_hosts(self): constants.AGGREGATES_MANAGE_HOSTS_TEMPLATE) @test.create_stubs({api.nova: ('aggregate_get', 'add_host_to_aggregate', + 'remove_host_from_aggregate', 'host_list')}) - def test_manage_hosts_update_empty_aggregate(self): + def test_manage_hosts_update_add_remove_not_empty_aggregate(self): aggregate = self.aggregates.first() - aggregate.hosts = [] - host = self.hosts.get(service="compute") - + aggregate.hosts = ['host1', 'host2'] + host = self.hosts.list()[0] form_data = {'manageaggregatehostsaction_role_member': [host.host_name]} + api.nova.remove_host_from_aggregate(IsA(http.HttpRequest), + str(aggregate.id), + 'host2') + api.nova.remove_host_from_aggregate(IsA(http.HttpRequest), + str(aggregate.id), + 'host1') api.nova.aggregate_get(IsA(http.HttpRequest), str(aggregate.id)) \ .AndReturn(aggregate) api.nova.host_list(IsA(http.HttpRequest)) \ @@ -257,7 +263,161 @@ def test_manage_hosts_update_empty_aggregate(self): res = self.client.post(reverse(constants.AGGREGATES_MANAGE_HOSTS_URL, args=[aggregate.id]), form_data) + self.assertNoFormErrors(res) + self.assertRedirectsNoFollow(res, + reverse(constants.AGGREGATES_INDEX_URL)) + + @test.create_stubs({api.nova: ('aggregate_get', 'add_host_to_aggregate', + 'remove_host_from_aggregate', + 'host_list')}) + def test_manage_hosts_update_add_not_empty_aggregate_should_fail(self): + aggregate = self.aggregates.first() + aggregate.hosts = ['devstack001'] + host1 = self.hosts.list()[0] + host3 = self.hosts.list()[2] + form_data = {'manageaggregatehostsaction_role_member': + [host1.host_name, host3.host_name]} + + api.nova.aggregate_get(IsA(http.HttpRequest), str(aggregate.id)) \ + .AndReturn(aggregate) + api.nova.host_list(IsA(http.HttpRequest)) \ + .AndReturn(self.hosts.list()) + api.nova.aggregate_get(IsA(http.HttpRequest), str(aggregate.id)) \ + .AndReturn(aggregate) + api.nova.add_host_to_aggregate(IsA(http.HttpRequest), + str(aggregate.id), host3.host_name)\ + .AndRaise(self.exceptions.nova) + self.mox.ReplayAll() + + res = self.client.post(reverse(constants.AGGREGATES_MANAGE_HOSTS_URL, + args=[aggregate.id]), + form_data) + self.assertNoFormErrors(res) + self.assertMessageCount(error=2) + self.assertRedirectsNoFollow(res, + reverse(constants.AGGREGATES_INDEX_URL)) + + @test.create_stubs({api.nova: ('aggregate_get', 'add_host_to_aggregate', + 'remove_host_from_aggregate', + 'host_list')}) + def test_manage_hosts_update_clean_not_empty_aggregate_should_fail(self): + aggregate = self.aggregates.first() + aggregate.hosts = ['host1', 'host2', 'host3'] + form_data = {'manageaggregatehostsaction_role_member': + []} + + api.nova.remove_host_from_aggregate(IsA(http.HttpRequest), + str(aggregate.id), + 'host3') + api.nova.remove_host_from_aggregate(IsA(http.HttpRequest), + str(aggregate.id), + 'host2')\ + .AndRaise(self.exceptions.nova) + api.nova.aggregate_get(IsA(http.HttpRequest), str(aggregate.id)) \ + .AndReturn(aggregate) + api.nova.host_list(IsA(http.HttpRequest)) \ + .AndReturn(self.hosts.list()) + api.nova.aggregate_get(IsA(http.HttpRequest), str(aggregate.id)) \ + .AndReturn(aggregate) + self.mox.ReplayAll() + + res = self.client.post(reverse(constants.AGGREGATES_MANAGE_HOSTS_URL, + args=[aggregate.id]), + form_data) + self.assertNoFormErrors(res) + self.assertMessageCount(error=2) + self.assertRedirectsNoFollow(res, + reverse(constants.AGGREGATES_INDEX_URL)) + + @test.create_stubs({api.nova: ('aggregate_get', 'add_host_to_aggregate', + 'remove_host_from_aggregate', + 'host_list')}) + def _test_manage_hosts_update(self, + host, + aggregate, + form_data, + addAggregate=False, + cleanAggregates=False): + if cleanAggregates: + api.nova.remove_host_from_aggregate(IsA(http.HttpRequest), + str(aggregate.id), + 'host3') + api.nova.remove_host_from_aggregate(IsA(http.HttpRequest), + str(aggregate.id), + 'host2') + api.nova.remove_host_from_aggregate(IsA(http.HttpRequest), + str(aggregate.id), + 'host1') + api.nova.aggregate_get(IsA(http.HttpRequest), str(aggregate.id)) \ + .AndReturn(aggregate) + api.nova.host_list(IsA(http.HttpRequest)) \ + .AndReturn(self.hosts.list()) + api.nova.aggregate_get(IsA(http.HttpRequest), str(aggregate.id)) \ + .AndReturn(aggregate) + if addAggregate: + api.nova.add_host_to_aggregate(IsA(http.HttpRequest), + str(aggregate.id), + host.host_name) + self.mox.ReplayAll() + res = self.client.post(reverse(constants.AGGREGATES_MANAGE_HOSTS_URL, + args=[aggregate.id]), + form_data) self.assertNoFormErrors(res) self.assertRedirectsNoFollow(res, reverse(constants.AGGREGATES_INDEX_URL)) + + def test_manage_hosts_update_nothing_not_empty_aggregate(self): + aggregate = self.aggregates.first() + host = self.hosts.list()[0] + aggregate.hosts = [host.host_name] + form_data = {'manageaggregatehostsaction_role_member': + [host.host_name]} + self._test_manage_hosts_update(host, + aggregate, + form_data, + addAggregate=False) + + def test_manage_hosts_update_nothing_empty_aggregate(self): + aggregate = self.aggregates.first() + aggregate.hosts = [] + form_data = {'manageaggregatehostsaction_role_member': + []} + self._test_manage_hosts_update(None, + aggregate, + form_data, + addAggregate=False) + + def test_manage_hosts_update_add_empty_aggregate(self): + aggregate = self.aggregates.first() + aggregate.hosts = [] + host = self.hosts.list()[0] + form_data = {'manageaggregatehostsaction_role_member': + [host.host_name]} + self._test_manage_hosts_update(host, + aggregate, + form_data, + addAggregate=True) + + def test_manage_hosts_update_add_not_empty_aggregate(self): + aggregate = self.aggregates.first() + aggregate.hosts = ['devstack001'] + host1 = self.hosts.list()[0] + host3 = self.hosts.list()[2] + form_data = {'manageaggregatehostsaction_role_member': + [host1.host_name, host3.host_name]} + self._test_manage_hosts_update(host3, + aggregate, + form_data, + addAggregate=True) + + def test_manage_hosts_update_clean_not_empty_aggregate(self): + aggregate = self.aggregates.first() + aggregate.hosts = ['host1', 'host2', 'host3'] + form_data = {'manageaggregatehostsaction_role_member': + []} + self._test_manage_hosts_update(None, + aggregate, + form_data, + addAggregate=False, + cleanAggregates=True) diff --git a/openstack_dashboard/dashboards/admin/aggregates/workflows.py b/openstack_dashboard/dashboards/admin/aggregates/workflows.py index c35a87ba7fa..867ccfdfabf 100644 --- a/openstack_dashboard/dashboards/admin/aggregates/workflows.py +++ b/openstack_dashboard/dashboards/admin/aggregates/workflows.py @@ -111,7 +111,7 @@ def __init__(self, request, *args, **kwargs): aggregate_id = self.initial['id'] aggregate = api.nova.aggregate_get(request, aggregate_id) - aggregate_hosts = aggregate.hosts + current_aggregate_hosts = aggregate.hosts hosts = [] try: @@ -128,7 +128,7 @@ def __init__(self, request, *args, **kwargs): self.fields[field_name].choices = \ [(host_name, host_name) for host_name in host_names] - self.fields[field_name].initial = aggregate_hosts + self.fields[field_name].initial = current_aggregate_hosts class Meta: name = _("Hosts within aggregate") @@ -181,9 +181,6 @@ class CreateAggregateWorkflow(workflows.Workflow): success_url = constants.AGGREGATES_INDEX_URL default_steps = (SetAggregateInfoStep, AddHostsToAggregateStep) - def format_status_message(self, message): - return message % self.context['name'] - def handle(self, request, context): try: self.object = \ @@ -195,8 +192,8 @@ def handle(self, request, context): exceptions.handle(request, _('Unable to create host aggregate.')) return False - hosts = context['hosts_aggregate'] - for host in hosts: + context_hosts_aggregate = context['hosts_aggregate'] + for host in context_hosts_aggregate: try: api.nova.add_host_to_aggregate(request, self.object.id, host) except Exception: @@ -216,23 +213,22 @@ class ManageAggregateHostsWorkflow(workflows.Workflow): success_url = constants.AGGREGATES_INDEX_URL default_steps = (ManageAggregateHostsStep, ) - def format_status_message(self, message): - return message - def handle(self, request, context): - hosts_aggregate = context['hosts_aggregate'] aggregate_id = context['id'] aggregate = api.nova.aggregate_get(request, aggregate_id) - aggregate_hosts = aggregate.hosts - for host in aggregate_hosts: - api.nova.remove_host_from_aggregate(request, aggregate_id, host) - - for host in hosts_aggregate: - try: + current_aggregate_hosts = set(aggregate.hosts) + context_hosts_aggregate = set(context['hosts_aggregate']) + removed_hosts = current_aggregate_hosts - context_hosts_aggregate + added_hosts = context_hosts_aggregate - current_aggregate_hosts + try: + for host in removed_hosts: + api.nova.remove_host_from_aggregate(request, + aggregate_id, + host) + for host in added_hosts: api.nova.add_host_to_aggregate(request, aggregate_id, host) - except Exception: - exceptions.handle( - request, _('Error updating the aggregate.')) - return False - + except Exception: + exceptions.handle( + request, _('Error when adding or removing hosts.')) + return False return True diff --git a/openstack_dashboard/test/test_data/nova_data.py b/openstack_dashboard/test/test_data/nova_data.py index eb52d53608e..56cd6987be5 100644 --- a/openstack_dashboard/test/test_data/nova_data.py +++ b/openstack_dashboard/test/test_data/nova_data.py @@ -723,5 +723,13 @@ def generate_fip(conf): } ) + host3 = hosts.Host(hosts.HostManager(None), + { + "host_name": "devstack003", + "service": "compute", + "zone": "testing" + } + ) TEST.hosts.add(host1) TEST.hosts.add(host2) + TEST.hosts.add(host3)