From 2ad84cb34ff6a6b72c8a65fd91f2a731a149ab3f Mon Sep 17 00:00:00 2001 From: liyingjun Date: Wed, 7 Mar 2018 09:13:48 +0800 Subject: [PATCH] Support description for instance update/rebuild In Nova Compute API microversion 2.19, you can specify a description attribute when creating, rebuilding, or updating a server instance. This description can be retrieved by getting server details, or list details for servers, this patch adds support for this attribute for instance in horizon. This patch adds description for instance update/rebuild Change-Id: I1c561607551fe6ed521772688b643cb27400e24e Closes-bug: #1753661 --- openstack_dashboard/api/nova.py | 9 +- .../dashboards/project/instances/forms.py | 11 +- .../dashboards/project/instances/tests.py | 154 ++++++++++++++++-- .../dashboards/project/instances/views.py | 19 ++- .../instances/workflows/update_instance.py | 18 +- 5 files changed, 192 insertions(+), 19 deletions(-) diff --git a/openstack_dashboard/api/nova.py b/openstack_dashboard/api/nova.py index f5b8ed63c6..8c699ff938 100644 --- a/openstack_dashboard/api/nova.py +++ b/openstack_dashboard/api/nova.py @@ -626,9 +626,12 @@ def server_reboot(request, instance_id, soft_reboot=False): @profiler.trace def server_rebuild(request, instance_id, image_id, password=None, - disk_config=None): - return novaclient(request).servers.rebuild(instance_id, image_id, - password, disk_config) + disk_config=None, description=None): + kwargs = {} + if description: + kwargs['description'] = description + return get_novaclient_with_instance_desc(request).servers.rebuild( + instance_id, image_id, password, disk_config, **kwargs) @profiler.trace diff --git a/openstack_dashboard/dashboards/project/instances/forms.py b/openstack_dashboard/dashboards/project/instances/forms.py index dd232b0f4a..d73e14bcc4 100644 --- a/openstack_dashboard/dashboards/project/instances/forms.py +++ b/openstack_dashboard/dashboards/project/instances/forms.py @@ -56,9 +56,17 @@ class RebuildInstanceForm(forms.SelfHandlingForm): widget=forms.PasswordInput(render_value=False)) disk_config = forms.ThemableChoiceField(label=_("Disk Partition"), required=False) + description = forms.CharField( + label=_("Description"), + widget=forms.Textarea(attrs={'rows': 4}), + max_length=255, + required=False + ) def __init__(self, request, *args, **kwargs): super(RebuildInstanceForm, self).__init__(request, *args, **kwargs) + if not api.nova.is_feature_available(request, "instance_description"): + del self.fields['description'] instance_id = kwargs.get('initial', {}).get('instance_id') self.fields['instance_id'].initial = instance_id @@ -105,9 +113,10 @@ class RebuildInstanceForm(forms.SelfHandlingForm): image = data.get('image') password = data.get('password') or None disk_config = data.get('disk_config', None) + description = data.get('description', None) try: api.nova.server_rebuild(request, instance, image, password, - disk_config) + disk_config, description=description) messages.info(request, _('Rebuilding instance %s.') % instance) except Exception: redirect = reverse('horizon:project:instances:index') diff --git a/openstack_dashboard/dashboards/project/instances/tests.py b/openstack_dashboard/dashboards/project/instances/tests.py index 4e24753900..38b1afd48f 100644 --- a/openstack_dashboard/dashboards/project/instances/tests.py +++ b/openstack_dashboard/dashboards/project/instances/tests.py @@ -1778,7 +1778,7 @@ class InstanceTests(InstanceTestBase): helpers.IsHttpRequest(), server.id) instance_update_get_stubs = { - api.nova: ('server_get',), + api.nova: ('server_get', 'is_feature_available'), api.neutron: ('security_group_list', 'server_security_groups',)} @@ -1789,6 +1789,7 @@ class InstanceTests(InstanceTestBase): self.mock_server_get.return_value = server self.mock_security_group_list.return_value = [] self.mock_server_security_groups.return_value = [] + self.mock_is_feature_available.return_value = False url = reverse('horizon:project:instances:update', args=[server.id]) res = self.client.get(url) @@ -1799,6 +1800,9 @@ class InstanceTests(InstanceTestBase): helpers.IsHttpRequest(), server.id) self.mock_security_group_list(helpers.IsHttpRequest(), tenant_id=None) self.mock_server_security_groups(helpers.IsHttpRequest(), server.id) + self.mock_is_feature_available.assert_called_once_with( + helpers.IsHttpRequest(), "instance_description" + ) @helpers.create_mocks(instance_update_get_stubs) def test_instance_update_get_server_get_exception(self): @@ -1825,7 +1829,7 @@ class InstanceTests(InstanceTestBase): return self.client.post(url, formData) instance_update_post_stubs = { - api.nova: ('server_get', 'server_update'), + api.nova: ('server_get', 'server_update', 'is_feature_available'), api.neutron: ('security_group_list', 'server_security_groups', 'server_update_security_groups')} @@ -1839,6 +1843,7 @@ class InstanceTests(InstanceTestBase): wanted_groups = [secgroups[1].id, secgroups[2].id] self.mock_server_get.return_value = server + self.mock_is_feature_available.return_value = False self.mock_security_group_list.return_value = secgroups self.mock_server_security_groups.return_value = server_groups self.mock_server_update.return_value = server @@ -1855,15 +1860,54 @@ class InstanceTests(InstanceTestBase): self.mock_server_security_groups.assert_called_once_with( helpers.IsHttpRequest(), server.id) self.mock_server_update.assert_called_once_with( - helpers.IsHttpRequest(), server.id, server.name) + helpers.IsHttpRequest(), server.id, server.name, description=None) self.mock_server_update_security_groups.assert_called_once_with( helpers.IsHttpRequest(), server.id, wanted_groups) + self.mock_is_feature_available.assert_called_once_with( + helpers.IsHttpRequest(), "instance_description" + ) + + @helpers.create_mocks(instance_update_post_stubs) + def test_instance_update_post_with_desc(self): + server = self.servers.first() + secgroups = self.security_groups.list()[:3] + + server_groups = [secgroups[0], secgroups[1]] + test_description = 'test description' + + self.mock_server_get.return_value = server + self.mock_is_feature_available.return_value = True + self.mock_security_group_list.return_value = secgroups + self.mock_server_security_groups.return_value = server_groups + self.mock_server_update.return_value = server + + formData = {'name': server.name, + 'description': test_description} + url = reverse('horizon:project:instances:update', + args=[server.id]) + res = self.client.post(url, formData) + self.assertNoFormErrors(res) + self.assertRedirectsNoFollow(res, INDEX_URL) + + self.mock_server_get.assert_called_once_with( + helpers.IsHttpRequest(), server.id) + self.mock_security_group_list.assert_called_once_with( + helpers.IsHttpRequest(), tenant_id=None) + self.mock_server_security_groups.assert_called_once_with( + helpers.IsHttpRequest(), server.id) + self.mock_server_update.assert_called_once_with( + helpers.IsHttpRequest(), server.id, server.name, + description=test_description) + self.mock_is_feature_available.assert_called_once_with( + helpers.IsHttpRequest(), "instance_description" + ) @helpers.create_mocks(instance_update_post_stubs) def test_instance_update_post_api_exception(self): server = self.servers.first() self.mock_server_get.return_value = server + self.mock_is_feature_available.return_value = False self.mock_security_group_list.return_value = [] self.mock_server_security_groups.return_value = [] self.mock_server_update.side_effect = self.exceptions.nova @@ -1879,15 +1923,19 @@ class InstanceTests(InstanceTestBase): self.mock_server_security_groups.assert_called_once_with( helpers.IsHttpRequest(), server.id) self.mock_server_update.assert_called_once_with( - helpers.IsHttpRequest(), server.id, server.name) + helpers.IsHttpRequest(), server.id, server.name, description=None) self.mock_server_update_security_groups.assert_called_once_with( helpers.IsHttpRequest(), server.id, []) + self.mock_is_feature_available.assert_called_once_with( + helpers.IsHttpRequest(), "instance_description" + ) @helpers.create_mocks(instance_update_post_stubs) def test_instance_update_post_secgroup_api_exception(self): server = self.servers.first() self.mock_server_get.return_value = server + self.mock_is_feature_available.return_value = False self.mock_security_group_list.return_value = [] self.mock_server_security_groups.return_value = [] self.mock_server_update.return_value = server @@ -1904,9 +1952,12 @@ class InstanceTests(InstanceTestBase): self.mock_server_security_groups.assert_called_once_with( helpers.IsHttpRequest(), server.id) self.mock_server_update.assert_called_once_with( - helpers.IsHttpRequest(), server.id, server.name) + helpers.IsHttpRequest(), server.id, server.name, description=None) self.mock_server_update_security_groups.assert_called_once_with( helpers.IsHttpRequest(), server.id, []) + self.mock_is_feature_available.assert_called_once_with( + helpers.IsHttpRequest(), "instance_description" + ) class InstanceLaunchInstanceTests(InstanceTestBase, @@ -4316,12 +4367,15 @@ class InstanceTests2(InstanceTestBase, InstanceTableTestMixin): helpers.IsHttpRequest(), server.id, flavor.id, 'AUTO') @helpers.create_mocks({api.glance: ('image_list_detailed',), - api.nova: ('extension_supported', + api.nova: ('server_get', + 'extension_supported', 'is_feature_available',)}) def test_rebuild_instance_get(self, expect_password_fields=True): server = self.servers.first() self._mock_glance_image_list_detailed(self.images.list()) self.mock_extension_supported.return_value = True + self.mock_is_feature_available.return_value = False + self.mock_server_get.return_value = server url = reverse('horizon:project:instances:rebuild', args=[server.id]) res = self.client.get(url) @@ -4334,9 +4388,14 @@ class InstanceTests2(InstanceTestBase, InstanceTableTestMixin): else: self.assertNotContains(res, password_field_label) + self.mock_server_get.assert_called_once_with( + helpers.IsHttpRequest(), server.id) self._check_glance_image_list_detailed(count=3) self.mock_extension_supported.assert_called_once_with( 'DiskConfig', helpers.IsHttpRequest()) + self.mock_is_feature_available.assert_called_once_with( + helpers.IsHttpRequest(), "instance_description" + ) @django.test.utils.override_settings( OPENSTACK_HYPERVISOR_FEATURES={'can_set_password': False}) @@ -4358,7 +4417,8 @@ class InstanceTests2(InstanceTestBase, InstanceTableTestMixin): return self.client.post(url, form_data) instance_rebuild_post_stubs = { - api.nova: ('server_rebuild', + api.nova: ('server_get', + 'server_rebuild', 'extension_supported', 'is_feature_available',), api.glance: ('image_list_detailed',)} @@ -4369,9 +4429,11 @@ class InstanceTests2(InstanceTestBase, InstanceTableTestMixin): image = self.images.first() password = u'testpass' + self.mock_server_get.return_value = server self._mock_glance_image_list_detailed(self.images.list()) self.mock_extension_supported.return_value = True self.mock_server_rebuild.return_value = [] + self.mock_is_feature_available.return_value = False res = self._instance_rebuild_post(server.id, image.id, password=password, @@ -4380,20 +4442,28 @@ class InstanceTests2(InstanceTestBase, InstanceTableTestMixin): self.assertNoFormErrors(res) self.assertRedirectsNoFollow(res, INDEX_URL) + self.mock_server_get.assert_called_once_with( + helpers.IsHttpRequest(), server.id) self._check_glance_image_list_detailed(count=3) self.mock_extension_supported.assert_called_once_with( 'DiskConfig', helpers.IsHttpRequest()) self.mock_server_rebuild.assert_called_once_with( - helpers.IsHttpRequest(), server.id, image.id, password, 'AUTO') + helpers.IsHttpRequest(), server.id, image.id, password, 'AUTO', + description=None) + self.mock_is_feature_available.assert_called_once_with( + helpers.IsHttpRequest(), "instance_description" + ) @helpers.create_mocks(instance_rebuild_post_stubs) def test_rebuild_instance_post_with_password_equals_none(self): server = self.servers.first() image = self.images.first() + self.mock_server_get.return_value = server self._mock_glance_image_list_detailed(self.images.list()) self.mock_extension_supported.return_value = True self.mock_server_rebuild.side_effect = self.exceptions.nova + self.mock_is_feature_available.return_value = False res = self._instance_rebuild_post(server.id, image.id, password=None, @@ -4401,11 +4471,17 @@ class InstanceTests2(InstanceTestBase, InstanceTableTestMixin): disk_config='AUTO') self.assertRedirectsNoFollow(res, INDEX_URL) + self.mock_server_get.assert_called_once_with( + helpers.IsHttpRequest(), server.id) self._check_glance_image_list_detailed(count=3) self.mock_extension_supported.assert_called_once_with( 'DiskConfig', helpers.IsHttpRequest()) self.mock_server_rebuild.assert_called_once_with( - helpers.IsHttpRequest(), server.id, image.id, None, 'AUTO') + helpers.IsHttpRequest(), server.id, image.id, None, 'AUTO', + description=None) + self.mock_is_feature_available.assert_called_once_with( + helpers.IsHttpRequest(), "instance_description" + ) @helpers.create_mocks(instance_rebuild_post_stubs) def test_rebuild_instance_post_password_do_not_match(self): @@ -4414,8 +4490,10 @@ class InstanceTests2(InstanceTestBase, InstanceTableTestMixin): pass1 = u'somepass' pass2 = u'notsomepass' + self.mock_server_get.return_value = server self._mock_glance_image_list_detailed(self.images.list()) self.mock_extension_supported.return_value = True + self.mock_is_feature_available.return_value = False res = self._instance_rebuild_post(server.id, image.id, password=pass1, @@ -4431,19 +4509,26 @@ class InstanceTests2(InstanceTestBase, InstanceTableTestMixin): else: image_list_count = 3 ext_count = 1 + self.mock_server_get.assert_called_once_with( + helpers.IsHttpRequest(), server.id) self._check_glance_image_list_detailed(count=image_list_count) self.assert_mock_multiple_calls_with_same_arguments( self.mock_extension_supported, ext_count, mock.call('DiskConfig', helpers.IsHttpRequest())) + self.assert_mock_multiple_calls_with_same_arguments( + self.mock_is_feature_available, 2, + mock.call(helpers.IsHttpRequest(), 'instance_description')) @helpers.create_mocks(instance_rebuild_post_stubs) def test_rebuild_instance_post_with_empty_string(self): server = self.servers.first() image = self.images.first() + self.mock_server_get.return_value = server self._mock_glance_image_list_detailed(self.images.list()) self.mock_extension_supported.return_value = True self.mock_server_rebuild.return_value = [] + self.mock_is_feature_available.return_value = False res = self._instance_rebuild_post(server.id, image.id, password=u'', @@ -4452,11 +4537,50 @@ class InstanceTests2(InstanceTestBase, InstanceTableTestMixin): self.assertNoFormErrors(res) self.assertRedirectsNoFollow(res, INDEX_URL) + self.mock_server_get.assert_called_once_with( + helpers.IsHttpRequest(), server.id) self._check_glance_image_list_detailed(count=3) self.mock_extension_supported.assert_called_once_with( 'DiskConfig', helpers.IsHttpRequest()) self.mock_server_rebuild.assert_called_once_with( - helpers.IsHttpRequest(), server.id, image.id, None, 'AUTO') + helpers.IsHttpRequest(), server.id, image.id, None, 'AUTO', + description=None) + self.mock_is_feature_available.assert_called_once_with( + helpers.IsHttpRequest(), "instance_description" + ) + + @helpers.create_mocks(instance_rebuild_post_stubs) + def test_rebuild_instance_post_with_desc(self): + server = self.servers.first() + image = self.images.first() + test_description = 'test description' + + self.mock_server_get.return_value = server + self._mock_glance_image_list_detailed(self.images.list()) + self.mock_extension_supported.return_value = True + self.mock_server_rebuild.return_value = [] + self.mock_is_feature_available.return_value = True + + form_data = {'instance_id': server.id, + 'image': image.id, + 'description': test_description} + url = reverse('horizon:project:instances:rebuild', + args=[server.id]) + res = self.client.post(url, form_data) + self.assertNoFormErrors(res) + self.assertRedirectsNoFollow(res, INDEX_URL) + + self.mock_server_get.assert_called_once_with( + helpers.IsHttpRequest(), server.id) + self._check_glance_image_list_detailed(count=3) + self.mock_extension_supported.assert_called_once_with( + 'DiskConfig', helpers.IsHttpRequest()) + self.mock_server_rebuild.assert_called_once_with( + helpers.IsHttpRequest(), server.id, image.id, None, '', + description=test_description) + self.mock_is_feature_available.assert_called_once_with( + helpers.IsHttpRequest(), "instance_description" + ) @helpers.create_mocks(instance_rebuild_post_stubs) def test_rebuild_instance_post_api_exception(self): @@ -4464,9 +4588,11 @@ class InstanceTests2(InstanceTestBase, InstanceTableTestMixin): image = self.images.first() password = u'testpass' + self.mock_server_get.return_value = server self._mock_glance_image_list_detailed(self.images.list()) self.mock_extension_supported.return_value = True self.mock_server_rebuild.side_effect = self.exceptions.nova + self.mock_is_feature_available.return_value = False res = self._instance_rebuild_post(server.id, image.id, password=password, @@ -4474,11 +4600,17 @@ class InstanceTests2(InstanceTestBase, InstanceTableTestMixin): disk_config='AUTO') self.assertRedirectsNoFollow(res, INDEX_URL) + self.mock_server_get.assert_called_once_with( + helpers.IsHttpRequest(), server.id) self._check_glance_image_list_detailed(count=3) self.mock_extension_supported.assert_called_once_with( 'DiskConfig', helpers.IsHttpRequest()) self.mock_server_rebuild.assert_called_once_with( - helpers.IsHttpRequest(), server.id, image.id, password, 'AUTO') + helpers.IsHttpRequest(), server.id, image.id, password, 'AUTO', + description=None) + self.mock_is_feature_available.assert_called_once_with( + helpers.IsHttpRequest(), "instance_description" + ) @django.test.utils.override_settings(API_RESULT_PAGE_SIZE=2) @helpers.create_mocks({ diff --git a/openstack_dashboard/dashboards/project/instances/views.py b/openstack_dashboard/dashboards/project/instances/views.py index fc86a41fac..cc0ff2046d 100644 --- a/openstack_dashboard/dashboards/project/instances/views.py +++ b/openstack_dashboard/dashboards/project/instances/views.py @@ -334,8 +334,10 @@ class UpdateView(workflows.WorkflowView): def get_initial(self): initial = super(UpdateView, self).get_initial() + instance = self.get_object() initial.update({'instance_id': self.kwargs['instance_id'], - 'name': getattr(self.get_object(), 'name', '')}) + 'name': getattr(instance, 'name', ''), + 'description': getattr(instance, 'description', '')}) return initial @@ -352,8 +354,21 @@ class RebuildView(forms.ModalFormView): context['can_set_server_password'] = api.nova.can_set_server_password() return context + @memoized.memoized_method + def get_object(self, *args, **kwargs): + instance_id = self.kwargs['instance_id'] + try: + return api.nova.server_get(self.request, instance_id) + except Exception: + redirect = reverse("horizon:project:instances:index") + msg = _('Unable to retrieve instance details.') + exceptions.handle(self.request, msg, redirect=redirect) + def get_initial(self): - return {'instance_id': self.kwargs['instance_id']} + instance = self.get_object() + initial = {'instance_id': self.kwargs['instance_id'], + 'description': getattr(instance, 'description', '')} + return initial class DecryptPasswordView(forms.ModalFormView): diff --git a/openstack_dashboard/dashboards/project/instances/workflows/update_instance.py b/openstack_dashboard/dashboards/project/instances/workflows/update_instance.py index 9c294637be..8d218e475b 100644 --- a/openstack_dashboard/dashboards/project/instances/workflows/update_instance.py +++ b/openstack_dashboard/dashboards/project/instances/workflows/update_instance.py @@ -128,12 +128,26 @@ class UpdateInstanceSecurityGroups(BaseSecurityGroups): class UpdateInstanceInfoAction(workflows.Action): name = forms.CharField(label=_("Name"), max_length=255) + description = forms.CharField( + label=_("Description"), + widget=forms.Textarea(attrs={'rows': 4}), + max_length=255, + required=False + ) + + def __init__(self, request, *args, **kwargs): + super(UpdateInstanceInfoAction, self).__init__(request, + *args, + **kwargs) + if not api.nova.is_feature_available(request, "instance_description"): + del self.fields["description"] def handle(self, request, data): try: api.nova.server_update(request, data['instance_id'], - data['name']) + data['name'], + description=data.get('description')) except Exception: exceptions.handle(request, ignore=True) return False @@ -148,7 +162,7 @@ class UpdateInstanceInfoAction(workflows.Action): class UpdateInstanceInfo(workflows.Step): action_class = UpdateInstanceInfoAction depends_on = ("instance_id",) - contributes = ("name",) + contributes = ("name", "description") class UpdateInstance(workflows.Workflow):