Skip to content

Commit

Permalink
Improve Host Aggregates handle method.
Browse files Browse the repository at this point in the history
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
  • Loading branch information
Alejandro Emanuel Paredes committed Apr 3, 2014
1 parent af87ddc commit 8f4331a
Show file tree
Hide file tree
Showing 3 changed files with 191 additions and 27 deletions.
170 changes: 165 additions & 5 deletions openstack_dashboard/dashboards/admin/aggregates/tests.py
Expand Up @@ -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()

Expand Down Expand Up @@ -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)) \
Expand All @@ -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)) \
Expand All @@ -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)
40 changes: 18 additions & 22 deletions openstack_dashboard/dashboards/admin/aggregates/workflows.py
Expand Up @@ -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:
Expand All @@ -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")
Expand Down Expand Up @@ -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 = \
Expand All @@ -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:
Expand All @@ -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
8 changes: 8 additions & 0 deletions openstack_dashboard/test/test_data/nova_data.py
Expand Up @@ -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)

0 comments on commit 8f4331a

Please sign in to comment.