From 0a2b61126a49c81417982e57ce3c660bf0516f96 Mon Sep 17 00:00:00 2001 From: Zhenguo Niu Date: Wed, 17 Feb 2016 18:15:11 +0800 Subject: [PATCH] Add restrictions for changing portgroup-node association * Prevent updates to portgroup.node_uuid if the node is not in a provision_state ENROLL,INSPECTING,MANAGEABLE, that would allow updates to its ports. * Disallow updates to the portgroup.node_uuid field if there are any ports associated with the portgroup. Change-Id: Ia7ef313ab7a31c0ee678a7f4c088b92feefff988 Closes-Bug: #1546371 --- ironic/conductor/manager.py | 43 ++++++++++- ironic/tests/unit/conductor/test_manager.py | 86 +++++++++++++++++++++ 2 files changed, 128 insertions(+), 1 deletion(-) diff --git a/ironic/conductor/manager.py b/ironic/conductor/manager.py index 43cf660e4b..4ebad1f1aa 100644 --- a/ironic/conductor/manager.py +++ b/ironic/conductor/manager.py @@ -1735,7 +1735,9 @@ class ConductorManager(base_manager.BaseConductorManager): @messaging.expected_exceptions(exception.NodeLocked, exception.FailedToUpdateMacOnPort, - exception.PortgroupMACAlreadyExists) + exception.PortgroupMACAlreadyExists, + exception.PortgroupNotEmpty, + exception.InvalidState) def update_portgroup(self, context, portgroup_obj): """Update a portgroup. @@ -1746,6 +1748,11 @@ class ConductorManager(base_manager.BaseConductorManager): failed. :raises: PortgroupMACAlreadyExists if the update is setting a MAC which is registered on another portgroup already. + :raises: InvalidState if portgroup-node association is updated while + node not in a MANAGEABLE or ENROLL or INSPECTING state or not + in MAINTENANCE mode. + :raises: PortgroupNotEmpty if there are ports associated with this + portgroup. """ portgroup_uuid = portgroup_obj.uuid LOG.debug("RPC update_portgroup called for portgroup %s.", @@ -1755,6 +1762,40 @@ class ConductorManager(base_manager.BaseConductorManager): portgroup_obj.node_id, purpose=lock_purpose) as task: node = task.node + + if 'node_id' in portgroup_obj.obj_what_changed(): + # NOTE(zhenguo): If portgroup update is modifying the + # portgroup-node association then node should be in + # MANAGEABLE/INSPECTING/ENROLL provisioning state or in + # maintenance mode, otherwise InvalidState is raised. + allowed_update_states = [states.ENROLL, + states.INSPECTING, + states.MANAGEABLE] + if (node.provision_state not in allowed_update_states + and not node.maintenance): + action = _("Portgroup %(portgroup)s can not be associated " + "to node %(node)s unless the node is in a " + "%(allowed)s state or in maintenance mode.") + + raise exception.InvalidState( + action % {'portgroup': portgroup_uuid, + 'node': node.uuid, + 'allowed': ', '.join(allowed_update_states)}) + + # NOTE(zhenguo): If portgroup update is modifying the + # portgroup-node association then there should not be + # any Port associated to the PortGroup, otherwise + # PortgroupNotEmpty exception is raised. + associated_ports = self.dbapi.get_ports_by_portgroup_id( + portgroup_uuid) + if associated_ports: + action = _("Portgroup %(portgroup)s can not be associated " + "with node %(node)s because there are ports " + "associated with this portgroup.") + raise exception.PortgroupNotEmpty( + action % {'portgroup': portgroup_uuid, + 'node': node.uuid}) + if 'address' in portgroup_obj.obj_what_changed(): vif = portgroup_obj.extra.get('vif_portgroup_id') if vif: diff --git a/ironic/tests/unit/conductor/test_manager.py b/ironic/tests/unit/conductor/test_manager.py index 72e2ea4618..9259ad5fa7 100644 --- a/ironic/tests/unit/conductor/test_manager.py +++ b/ironic/tests/unit/conductor/test_manager.py @@ -2886,6 +2886,92 @@ class UpdatePortgroupTestCase(mgr_utils.ServiceSetUpMixin, portgroup.refresh() self.assertEqual(old_extra, portgroup.extra) + def test_update_portgroup_to_node_in_deleting_state(self): + node = obj_utils.create_test_node(self.context, driver='fake') + portgroup = obj_utils.create_test_portgroup(self.context, + node_id=node.id, + extra={'foo': 'bar'}) + update_node = obj_utils.create_test_node( + self.context, driver='fake', + provision_state=states.DELETING, + uuid=uuidutils.generate_uuid()) + + old_node_id = portgroup.node_id + portgroup.node_id = update_node.id + exc = self.assertRaises(messaging.rpc.ExpectedException, + self.service.update_portgroup, + self.context, portgroup) + self.assertEqual(exception.InvalidState, exc.exc_info[0]) + portgroup.refresh() + self.assertEqual(old_node_id, portgroup.node_id) + + @mock.patch.object(dbapi.IMPL, 'get_ports_by_portgroup_id') + def test_update_portgroup_to_node_in_manageable_state(self, + mock_get_ports): + node = obj_utils.create_test_node(self.context, driver='fake') + portgroup = obj_utils.create_test_portgroup(self.context, + node_id=node.id, + extra={'foo': 'bar'}) + update_node = obj_utils.create_test_node( + self.context, driver='fake', + provision_state=states.MANAGEABLE, + uuid=uuidutils.generate_uuid()) + mock_get_ports.return_value = [] + + self._start_service() + + portgroup.node_id = update_node.id + self.service.update_portgroup(self.context, portgroup) + portgroup.refresh() + self.assertEqual(update_node.id, portgroup.node_id) + mock_get_ports.assert_called_once_with(portgroup.uuid) + + @mock.patch.object(dbapi.IMPL, 'get_ports_by_portgroup_id') + def test_update_portgroup_to_node_in_active_state_and_maintenance( + self, mock_get_ports): + node = obj_utils.create_test_node(self.context, driver='fake') + portgroup = obj_utils.create_test_portgroup(self.context, + node_id=node.id, + extra={'foo': 'bar'}) + update_node = obj_utils.create_test_node( + self.context, driver='fake', + provision_state=states.ACTIVE, + maintenance=True, + uuid=uuidutils.generate_uuid()) + mock_get_ports.return_value = [] + + self._start_service() + + portgroup.node_id = update_node.id + self.service.update_portgroup(self.context, portgroup) + portgroup.refresh() + self.assertEqual(update_node.id, portgroup.node_id) + mock_get_ports.assert_called_once_with(portgroup.uuid) + + @mock.patch.object(dbapi.IMPL, 'get_ports_by_portgroup_id') + def test_update_portgroup_association_with_ports(self, mock_get_ports): + node = obj_utils.create_test_node(self.context, driver='fake') + portgroup = obj_utils.create_test_portgroup(self.context, + node_id=node.id, + extra={'foo': 'bar'}) + update_node = obj_utils.create_test_node( + self.context, driver='fake', + maintenance=True, + uuid=uuidutils.generate_uuid()) + mock_get_ports.return_value = ['test_port'] + + self._start_service() + + old_node_id = portgroup.node_id + portgroup.node_id = update_node.id + exc = self.assertRaises(messaging.rpc.ExpectedException, + self.service.update_portgroup, + self.context, portgroup) + self.assertEqual(exception.PortgroupNotEmpty, exc.exc_info[0]) + portgroup.refresh() + self.assertEqual(old_node_id, portgroup.node_id) + mock_get_ports.assert_called_once_with(portgroup.uuid) + @mock.patch('ironic.dhcp.neutron.NeutronDHCPApi.update_port_address') def test_update_portgroup_address(self, mac_update_mock): node = obj_utils.create_test_node(self.context, driver='fake')