openstack/horizon
Code Issues Proposed changes

Save instace_id inside Associate Floating IP workflow

Browse Source 
Also, rename instance_id field to proper port_id.

Before this change, when instance_id was passed from the Instances
page in the URL, the information would be lost on form submission,
and if the form contained an error, the it would be redisplayed with
the port drop-down containing all ports from all instances. This also
made submitting this form slow, as the drop-down would be populated
with all ports on form validation.

It also creates a bug, where if there are more instances than Nova's
pagination allows, the ports from newer instances would no longer
appear in the drop-down, and the form would fail to validate, even
though the choice of port on initial form was correct.

Closes-bug: #1920010
Change-Id: I3ab26c19dc9ea1ed23fcff790d0db919039099eb
This commit is contained in:
Radomir Dopieralski 2021-03-16 17:09:21 +01:00
parent 6ac0917950
commit 3d82d57353
3 changed files with 22 additions and 16 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
openstack_dashboard/dashboards/project/floating_ips/tests.py
View File

@ -139,7 +139,7 @@ class FloatingIpViewTests(test.TestCase):
self._get_fip_targets()
self.mock_floating_ip_associate.return_value = None
form_data = {'instance_id': port_target_id,
form_data = {'port_id': port_target_id,
'ip_id': floating_ip.id}
url = reverse('%s:associate' % NAMESPACE)
res = self.client.post(url, form_data)
@ -168,7 +168,7 @@ class FloatingIpViewTests(test.TestCase):
self.mock_floating_ip_associate.return_value = None
next = reverse("horizon:project:instances:index")
form_data = {'instance_id': port_target_id,
form_data = {'port_id': port_target_id,
'next': next,
'ip_id': floating_ip.id}
url = reverse('%s:associate' % NAMESPACE)
@ -197,7 +197,7 @@ class FloatingIpViewTests(test.TestCase):
self._get_fip_targets()
self.mock_floating_ip_associate.side_effect = self.exceptions.nova
form_data = {'instance_id': port_target_id,
form_data = {'port_id': port_target_id,
'ip_id': floating_ip.id}
url = reverse('%s:associate' % NAMESPACE)
res = self.client.post(url, form_data)

26
openstack_dashboard/dashboards/project/floating_ips/workflows.py
View File

@ -36,7 +36,11 @@ class AssociateIPAction(workflows.Action):
coerce=filters.get_int_or_uuid,
empty_value=None
)
instance_id = forms.ThemableChoiceField(
instance_id = forms.CharField(
widget=forms.widgets.HiddenInput(),
required=False,
)
port_id = forms.ThemableChoiceField(
label=_("Port to be associated")
)
@ -53,7 +57,8 @@ class AssociateIPAction(workflows.Action):
# an association target is not an instance but a port, so we need
# to get an association target based on a received instance_id
# and set the initial value of instance_id ChoiceField.
q_instance_id = self.request.GET.get('instance_id')
q_instance_id = self.data.get('instance_id',
self.request.GET.get('instance_id'))
q_port_id = self.request.GET.get('port_id')
if policy.check((("network", "create_floatingip"),),
@ -61,20 +66,21 @@ class AssociateIPAction(workflows.Action):
self.fields['ip_id'].widget.add_item_link = ALLOCATE_URL
if q_instance_id:
self.initial['instance_id'] = q_instance_id
targets = self._get_target_list(q_instance_id)
# Setting the initial value here is required to avoid a situation
# where instance_id passed in the URL is used as the initial value
# unexpectedly. (This always happens if the form is invoked from
# the instance table.)
if targets:
self.initial['instance_id'] = targets[0].id
self.initial['port_id'] = targets[0].id
else:
self.initial['instance_id'] = ''
self.initial['port_id'] = ''
elif q_port_id:
targets = self._get_target_list()
for target in targets:
if target.port_id == q_port_id:
self.initial['instance_id'] = target.id
self.initial['port_id'] = target.id
break
def populate_ip_id_choices(self, request, context):
@ -112,9 +118,9 @@ class AssociateIPAction(workflows.Action):
redirect=redirect)
return targets
# TODO(amotoki): [drop-nova-network] Rename instance_id to port_id
def populate_instance_id_choices(self, request, context):
q_instance_id = self.request.GET.get('instance_id')
def populate_port_id_choices(self, request, context):
q_instance_id = self.data.get('instance_id',
self.initial.get('instance_id'))
# The reason of specifying an empty tuple when q_instance_id is None
# is to make memoized_method _get_target_list work. Two calls of
# _get_target_list from here and __init__ must have a same arguments.
@ -132,7 +138,7 @@ class AssociateIPAction(workflows.Action):
class AssociateIP(workflows.Step):
action_class = AssociateIPAction
contributes = ("ip_id", "instance_id", "ip_address")
contributes = ("ip_id", "instance_id", "ip_address", "port_id")
def contribute(self, data, context):
context = super().contribute(data, context)
@ -162,7 +168,7 @@ class IPAssociationWorkflow(workflows.Workflow):
try:
api.neutron.floating_ip_associate(request,
data['ip_id'],
data['instance_id'])
data['port_id'])
except neutron_exc.Conflict:
msg = _('The requested instance port is already'
' associated with another floating IP.')

6
openstack_dashboard/test/integration_tests/pages/project/network/floatingipspage.py
View File

@ -25,7 +25,7 @@ from openstack_dashboard.test.integration_tests.regions import tables
class FloatingIPTable(tables.TableRegion):
name = 'floating_ips'
FLOATING_IP_ASSOCIATIONS = (
("ip_id", "instance_id"))
("ip_id", "port_id"))
@tables.bind_table_action('allocate')
def allocate_ip(self, allocate_button):
@ -93,8 +93,8 @@ class FloatingipsPage(basepage.BaseNavigationPage):
instance_ip=None):
row = self._get_row_with_floatingip(floatingip)
floatingip_form = self.floatingips_table.associate_ip(row)
floatingip_form.instance_id.text = "{}: {}".format(instance_name,
instance_ip)
floatingip_form.port_id.text = "{}: {}".format(instance_name,
instance_ip)
floatingip_form.submit()
def disassociate_floatingip(self, floatingip):