From dd0eba2128a892bddef563b09e4a28122c50f458 Mon Sep 17 00:00:00 2001 From: Akihiro Motoki Date: Mon, 30 Oct 2017 18:09:44 +0000 Subject: [PATCH] Support simple FIP disassociation (with FIP release) We previously supported so-called "Simple FIP disassociation" which allows users to disassociate and release a FIP in a single action. We no longer support nova-network based features, but I believe it is worth implemented even in a neutron-only era. This patch introduces a checkbox "Release floating IP" to support this with neutron. At the same time, this patch also fixes a bug that the existing FIP disassociation action disassociates and releases a first FIP of a requested server. Even though it is a rare case where a single server has multiple FIPs, this is a bug. After this patch, FIPs associated with the requested server are listed in the form and a user can select an FIP to be disassociated. This patch drops a setting parameter 'simple_ip_management' without deprecation notice. This is actually no side effect because this setting just toggled the FIP disassociate action in the instance table and it provides nothing more than that. We can do the same thing by the policy file. Change-Id: Ie8053bdd3a3e4c7897c7c906788d40c2a1d3f708 Closes-Bug: #1226003 --- doc/source/configuration/settings.rst | 25 --------- horizon/conf/default.py | 3 -- .../dashboards/project/instances/forms.py | 52 +++++++++++++++++++ .../dashboards/project/instances/tables.py | 45 +++------------- .../templates/instances/_disassociate.html | 28 ++++++++++ .../templates/instances/disassociate.html | 7 +++ .../dashboards/project/instances/tests.py | 48 +++++++++-------- .../dashboards/project/instances/urls.py | 2 + .../dashboards/project/instances/views.py | 16 ++++++ .../local/local_settings.py.example | 4 -- ...e-fip-disassociation-3c751297b467597e.yaml | 12 +++++ 11 files changed, 149 insertions(+), 93 deletions(-) create mode 100644 openstack_dashboard/dashboards/project/instances/templates/instances/_disassociate.html create mode 100644 openstack_dashboard/dashboards/project/instances/templates/instances/disassociate.html create mode 100644 releasenotes/notes/bug-1226003-drop-simple-fip-disassociation-3c751297b467597e.yaml diff --git a/doc/source/configuration/settings.rst b/doc/source/configuration/settings.rst index 0596f0d26d..c09e2e2576 100644 --- a/doc/source/configuration/settings.rst +++ b/doc/source/configuration/settings.rst @@ -428,31 +428,6 @@ there are any. This setting allows you to set rules for passwords if your organization requires them. -simple_ip_management -~~~~~~~~~~~~~~~~~~~~ - -.. versionadded:: 2013.1(Grizzly) - -Default: ``True`` - -Enable or disable simplified floating IP address management. - -"Simple" floating IP address management means that the user does not ever have -to select the specific IP addresses they wish to use, and the process of -allocating an IP and assigning it to an instance is one-click. - -The "advanced" floating IP management allows users to select the floating IP -pool from which the IP should be allocated and to select a specific IP address -when associating one with an instance. - -.. note:: - - Currently "simple" floating IP address management is not compatible with - Neutron. There are two reasons for this. First, Neutron does not support - the default floating IP pool at the moment. Second, a Neutron floating IP - can be associated with each VIF and we need to check whether there is only - one VIF for an instance to enable simple association support. - user_home ~~~~~~~~~ diff --git a/horizon/conf/default.py b/horizon/conf/default.py index e6828dd3ed..f740d534aa 100644 --- a/horizon/conf/default.py +++ b/horizon/conf/default.py @@ -45,9 +45,6 @@ HORIZON_CONFIG = { 'password_autocomplete': 'off', - # Enable or disable simplified floating IP address management. - 'simple_ip_management': True, - 'integration_tests_support': getattr(settings, 'INTEGRATION_TESTS_SUPPORT', False) } diff --git a/openstack_dashboard/dashboards/project/instances/forms.py b/openstack_dashboard/dashboards/project/instances/forms.py index d73e14bcc4..6ba71cac2a 100644 --- a/openstack_dashboard/dashboards/project/instances/forms.py +++ b/openstack_dashboard/dashboards/project/instances/forms.py @@ -15,6 +15,7 @@ from django.template.defaultfilters import filesizeformat from django.urls import reverse +from django.urls import reverse_lazy from django.utils.translation import ugettext_lazy as _ from django.views.decorators.debug import sensitive_variables @@ -418,3 +419,54 @@ class DetachInterface(forms.SelfHandlingForm): exceptions.handle(request, _("Unable to detach interface."), redirect=redirect) return True + + +class Disassociate(forms.SelfHandlingForm): + fip = forms.ThemableChoiceField(label=_('Floating IP')) + is_release = forms.BooleanField(label=_('Release Floating IP'), + required=False) + + def __init__(self, request, *args, **kwargs): + super(Disassociate, self).__init__(request, *args, **kwargs) + instance_id = self.initial['instance_id'] + targets = api.neutron.floating_ip_target_list_by_instance( + request, instance_id) + + target_ids = [t.port_id for t in targets] + + self.fips = [fip for fip + in api.neutron.tenant_floating_ip_list(request) + if fip.port_id in target_ids] + + fip_choices = [(fip.id, fip.ip) for fip in self.fips] + fip_choices.insert(0, ('', _('Select a floating IP to disassociate'))) + self.fields['fip'].choices = fip_choices + self.fields['fip'].initial = self.fips[0].id + + def handle(self, request, data): + redirect = reverse_lazy('horizon:project:instances:index') + fip_id = data['fip'] + fips = [fip for fip in self.fips if fip.id == fip_id] + if not fips: + messages.error(request, + _("The specified floating IP no longer exists."), + redirect=redirect) + fip = fips[0] + try: + if data['is_release']: + api.neutron.tenant_floating_ip_release(request, fip_id) + messages.success( + request, + _("Successfully disassociated and released " + "floating IP %s") % fip.ip) + else: + api.neutron.floating_ip_disassociate(request, fip_id) + messages.success( + request, + _("Successfully disassociated floating IP %s") % fip.ip) + except Exception: + exceptions.handle( + request, + _('Unable to disassociate floating IP %s') % fip.ip, + redirect=redirect) + return True diff --git a/openstack_dashboard/dashboards/project/instances/tables.py b/openstack_dashboard/dashboards/project/instances/tables.py index cc76326ee7..7de62a3ef4 100644 --- a/openstack_dashboard/dashboards/project/instances/tables.py +++ b/openstack_dashboard/dashboards/project/instances/tables.py @@ -17,7 +17,6 @@ import logging from django.conf import settings from django.http import HttpResponse -from django import shortcuts from django import template from django.template.defaultfilters import title from django import urls @@ -29,7 +28,6 @@ from django.utils.translation import string_concat from django.utils.translation import ugettext_lazy as _ from django.utils.translation import ungettext_lazy -from horizon import conf from horizon import exceptions from horizon import messages from horizon import tables @@ -655,14 +653,11 @@ class AssociateIP(policy.PolicyTargetMixin, tables.LinkAction): return "?".join([base_url, params]) -# TODO(amotoki): [drop-nova-network] The current SimpleDisassociateIP -# just disassociates the first found FIP. It looks better to have a form -# which allows to choose which FIP should be disassociated. -# HORIZON_CONFIG['simple_ip_management'] can be dropped then. -class SimpleDisassociateIP(policy.PolicyTargetMixin, tables.Action): +class DisassociateIP(tables.LinkAction): name = "disassociate" verbose_name = _("Disassociate Floating IP") - classes = ("btn-disassociate",) + url = "horizon:project:instances:disassociate" + classes = ("btn-disassociate", 'ajax-modal') policy_rules = (("network", "update_floatingip"),) action_type = "danger" @@ -671,38 +666,12 @@ class SimpleDisassociateIP(policy.PolicyTargetMixin, tables.Action): return False if not api.neutron.floating_ip_supported(request): return False - if not conf.HORIZON_CONFIG["simple_ip_management"]: - return False for addresses in instance.addresses.values(): for address in addresses: if address.get('OS-EXT-IPS:type') == "floating": return not is_deleting(instance) return False - def single(self, table, request, instance_id): - try: - targets = api.neutron.floating_ip_target_list_by_instance( - request, instance_id) - - target_ids = [t.port_id for t in targets] - - fips = [fip for fip in api.neutron.tenant_floating_ip_list(request) - if fip.port_id in target_ids] - # Removing multiple floating IPs at once doesn't work, so this pops - # off the first one. - if fips: - fip = fips.pop() - api.neutron.floating_ip_disassociate(request, fip.id) - messages.success(request, - _("Successfully disassociated " - "floating IP: %s") % fip.ip) - else: - messages.info(request, _("No floating IPs to disassociate.")) - except Exception: - exceptions.handle(request, - _("Unable to disassociate floating IP.")) - return shortcuts.redirect(request.get_full_path()) - class UpdateMetadata(policy.PolicyTargetMixin, tables.LinkAction): name = "update_metadata" @@ -1280,10 +1249,10 @@ class InstancesTable(tables.DataTable): table_actions = launch_actions + (DeleteInstance, InstancesFilterAction) row_actions = (StartInstance, ConfirmResize, RevertResize, - CreateSnapshot, AssociateIP, - SimpleDisassociateIP, AttachInterface, - DetachInterface, EditInstance, AttachVolume, - DetachVolume, UpdateMetadata, DecryptInstancePassword, + CreateSnapshot, AssociateIP, DisassociateIP, + AttachInterface, DetachInterface, EditInstance, + AttachVolume, DetachVolume, + UpdateMetadata, DecryptInstancePassword, EditInstanceSecurityGroups, ConsoleLink, LogLink, TogglePause, ToggleSuspend, ToggleShelve, ResizeLink, LockInstance, UnlockInstance, diff --git a/openstack_dashboard/dashboards/project/instances/templates/instances/_disassociate.html b/openstack_dashboard/dashboards/project/instances/templates/instances/_disassociate.html new file mode 100644 index 0000000000..63007e9432 --- /dev/null +++ b/openstack_dashboard/dashboards/project/instances/templates/instances/_disassociate.html @@ -0,0 +1,28 @@ +{% extends "horizon/common/_modal_form.html" %} +{% load i18n %} + +{% block form_id %}disassociate_fip_form{% endblock %} +{% block form_action %}{% url "horizon:project:instances:disassociate" instance_id %}{% endblock %} + +{% block modal_id %}disassocaite_fip_modal{% endblock %} +{% block modal-header %}{% trans "Disassociate Floating IP" %}{% endblock %} + +{% block modal-body %} +
+
+ {% include "horizon/common/_form_fields.html" %} +
+
+
+

{% trans "Description:" %}

+

{% blocktrans trimmed %} + Select the floating IP to be disassociated from the instance. + {% endblocktrans %}

+
+
{% trans "Release Floating IP" %}
+
{% blocktrans trimmed %} + If checked, the selected floating IP will be released at the same time. + {% endblocktrans %}
+
+
+{% endblock %} diff --git a/openstack_dashboard/dashboards/project/instances/templates/instances/disassociate.html b/openstack_dashboard/dashboards/project/instances/templates/instances/disassociate.html new file mode 100644 index 0000000000..7338335472 --- /dev/null +++ b/openstack_dashboard/dashboards/project/instances/templates/instances/disassociate.html @@ -0,0 +1,7 @@ +{% extends "base.html" %} +{% load i18n %} +{% block title %}{% trans "Disassociate Floating IP" %}{% endblock %} + +{% block main %} + {% include "project/instances/_disassociate.html" %} +{% endblock %} diff --git a/openstack_dashboard/dashboards/project/instances/tests.py b/openstack_dashboard/dashboards/project/instances/tests.py index 38b1afd48f..0bcf04bbe1 100644 --- a/openstack_dashboard/dashboards/project/instances/tests.py +++ b/openstack_dashboard/dashboards/project/instances/tests.py @@ -4156,13 +4156,10 @@ class InstanceTests2(InstanceTestBase, InstanceTableTestMixin): @helpers.create_mocks({ api.neutron: ('floating_ip_target_list_by_instance', 'tenant_floating_ip_list', - 'floating_ip_disassociate',), - api.network: ('servers_update_addresses',), - api.glance: ('image_list_detailed',), - api.nova: ('server_list', - 'flavor_list'), + 'floating_ip_disassociate', + 'tenant_floating_ip_release'), }) - def test_disassociate_floating_ip(self): + def _test_disassociate_floating_ip(self, is_release): servers = self.servers.list() server = servers[0] port = [p for p in self.ports.list() if p.device_id == server.id][0] @@ -4171,35 +4168,40 @@ class InstanceTests2(InstanceTestBase, InstanceTableTestMixin): fip = self.floating_ips.first() fip.port_id = port.id - self.mock_server_list.return_value = [servers, False] - self.mock_servers_update_addresses.return_value = None - self.mock_flavor_list.return_value = self.flavors.list() - self.mock_image_list_detailed.return_value = (self.images.list(), - False, False) self.mock_floating_ip_target_list_by_instance.return_value = \ [fip_target] self.mock_tenant_floating_ip_list.return_value = [fip] self.mock_floating_ip_disassociate.return_value = None + self.mock_tenant_floating_ip_release.return_value = None - formData = {'action': 'instances__disassociate__%s' % server.id} - res = self.client.post(INDEX_URL, formData) + url = reverse('horizon:project:instances:disassociate', + args=[server.id]) + form_data = {'fip': fip.id, + 'is_release': is_release} + res = self.client.post(url, form_data) self.assertRedirectsNoFollow(res, INDEX_URL) - search_opts = {'marker': None, 'paginate': True} - self.mock_server_list.assert_called_once_with( - helpers.IsHttpRequest(), search_opts=search_opts) - self.mock_servers_update_addresses.assert_called_once_with( - helpers.IsHttpRequest(), servers) - self.mock_flavor_list.assert_called_once_with(helpers.IsHttpRequest()) - self.mock_image_list_detailed.assert_called_once_with( - helpers.IsHttpRequest()) self.mock_floating_ip_target_list_by_instance.assert_called_once_with( helpers.IsHttpRequest(), server.id) self.mock_tenant_floating_ip_list.assert_called_once_with( helpers.IsHttpRequest()) - self.mock_floating_ip_disassociate.assert_called_once_with( - helpers.IsHttpRequest(), fip.id) + if is_release: + self.mock_floating_ip_disassociate.assert_not_called() + self.mock_tenant_floating_ip_release.assert_called_once_with( + helpers.IsHttpRequest(), fip.id) + else: + self.mock_floating_ip_disassociate.assert_called_once_with( + helpers.IsHttpRequest(), fip.id) + self.mock_tenant_floating_ip_release.assert_not_called() + + @helpers.create_mocks({api.neutron: ('floating_ip_disassociate',)}) + def test_disassociate_floating_ip(self): + self._test_disassociate_floating_ip(is_release=False) + + @helpers.create_mocks({api.neutron: ('tenant_floating_ip_release',)}) + def test_disassociate_floating_ip_with_release(self): + self._test_disassociate_floating_ip(is_release=True) @helpers.create_mocks({api.nova: ('server_get', 'flavor_list', diff --git a/openstack_dashboard/dashboards/project/instances/urls.py b/openstack_dashboard/dashboards/project/instances/urls.py index efbe9e85ac..4e5a594839 100644 --- a/openstack_dashboard/dashboards/project/instances/urls.py +++ b/openstack_dashboard/dashboards/project/instances/urls.py @@ -41,6 +41,8 @@ urlpatterns = [ url(INSTANCES % 'resize', views.ResizeView.as_view(), name='resize'), url(INSTANCES_KEYPAIR % 'decryptpassword', views.DecryptPasswordView.as_view(), name='decryptpassword'), + url(INSTANCES % 'disassociate', + views.DisassociateView.as_view(), name='disassociate'), url(INSTANCES % 'attach_interface', views.AttachInterfaceView.as_view(), name='attach_interface'), url(INSTANCES % 'detach_interface', diff --git a/openstack_dashboard/dashboards/project/instances/views.py b/openstack_dashboard/dashboards/project/instances/views.py index cc0ff2046d..4c37343f69 100644 --- a/openstack_dashboard/dashboards/project/instances/views.py +++ b/openstack_dashboard/dashboards/project/instances/views.py @@ -388,6 +388,22 @@ class DecryptPasswordView(forms.ModalFormView): 'keypair_name': self.kwargs['keypair_name']} +class DisassociateView(forms.ModalFormView): + form_class = project_forms.Disassociate + template_name = 'project/instances/disassociate.html' + success_url = reverse_lazy('horizon:project:instances:index') + page_title = _("Disassociate floating IP") + submit_label = _("Disassocaite") + + def get_context_data(self, **kwargs): + context = super(DisassociateView, self).get_context_data(**kwargs) + context['instance_id'] = self.kwargs['instance_id'] + return context + + def get_initial(self): + return {'instance_id': self.kwargs['instance_id']} + + class DetailView(tabs.TabView): tab_group_class = project_tabs.InstanceDetailTabs template_name = 'horizon/common/_detail.html' diff --git a/openstack_dashboard/local/local_settings.py.example b/openstack_dashboard/local/local_settings.py.example index 4c36ec2053..5013f8110d 100644 --- a/openstack_dashboard/local/local_settings.py.example +++ b/openstack_dashboard/local/local_settings.py.example @@ -128,10 +128,6 @@ WEBROOT = '/' # "help_text": _("Your password does not meet the requirements."), #} -# Disable simplified floating IP address management for deployments with -# multiple floating IP pools or complex network requirements. -#HORIZON_CONFIG["simple_ip_management"] = False - # Turn off browser autocompletion for forms including the login form and # the database creation workflow if so desired. #HORIZON_CONFIG["password_autocomplete"] = "off" diff --git a/releasenotes/notes/bug-1226003-drop-simple-fip-disassociation-3c751297b467597e.yaml b/releasenotes/notes/bug-1226003-drop-simple-fip-disassociation-3c751297b467597e.yaml new file mode 100644 index 0000000000..a63316d697 --- /dev/null +++ b/releasenotes/notes/bug-1226003-drop-simple-fip-disassociation-3c751297b467597e.yaml @@ -0,0 +1,12 @@ +--- +features: + - | + Floating IP can be released when it is disassociated from a server. + "Release Floating IP" checkbox is now available in "Disassociate + Floating IP" form. +upgrade: + - | + ``simple_ip_management`` setting in ``HORIZON_CONFIG`` was dropped. + This actually has no meaning after nova-network support was dropped in Pike. + If you use this setting to hide ``Disaccoaite Floating IP`` button in the + instance table, use the policy file instead.