From e7072b05e39d19034106cef61dce28415b01428b Mon Sep 17 00:00:00 2001 From: Isaku Yamahata Date: Thu, 15 Dec 2016 17:05:26 -0800 Subject: [PATCH] Kill the metadata proxy process unconditionally When force_metadata=True and enable_isolated_metadata=False, the namespace metadata proxy process might not be terminated when the network is deleted because the subnets and ports will have already been deleted, so we could incorrectly determine it was started. Calling destroy_monitored_metadata_proxy() is a noop when there is no process running. Change-Id: I77ff545ce02f2dca4c38e587b37ea809ad6f072c Closes-Bug: #1648095 (cherry picked from commit a502c96b8efa62627e245ffa69a23f4b6d3f90a5) --- neutron/agent/dhcp/agent.py | 14 +++++---- neutron/tests/unit/agent/dhcp/test_agent.py | 35 +++++++++++++-------- 2 files changed, 30 insertions(+), 19 deletions(-) diff --git a/neutron/agent/dhcp/agent.py b/neutron/agent/dhcp/agent.py index a6d5756cea6..85768ade769 100644 --- a/neutron/agent/dhcp/agent.py +++ b/neutron/agent/dhcp/agent.py @@ -284,12 +284,14 @@ class DhcpAgent(manager.Manager): """Disable DHCP for a network known to the agent.""" network = self.cache.get_network_by_id(network_id) if network: - if self.conf.enable_isolated_metadata: - # NOTE(jschwarz): In the case where a network is deleted, all - # the subnets and ports are deleted before this function is - # called, so checking if 'should_enable_metadata' is True - # for any subnet is false logic here. - self.disable_isolated_metadata_proxy(network) + # NOTE(yamahata): Kill the metadata proxy process + # unconditionally, as in the case where a network + # is deleted, all the subnets and ports are deleted + # before this function is called, so determining if + # the proxy should be terminated is error prone. + # destroy_monitored_metadata_proxy() is a noop when + # there is no process running. + self.disable_isolated_metadata_proxy(network) if self.call_driver('disable', network): self.cache.remove(network) diff --git a/neutron/tests/unit/agent/dhcp/test_agent.py b/neutron/tests/unit/agent/dhcp/test_agent.py index 60ea37acb78..6891753346f 100644 --- a/neutron/tests/unit/agent/dhcp/test_agent.py +++ b/neutron/tests/unit/agent/dhcp/test_agent.py @@ -31,6 +31,7 @@ from neutron.agent.dhcp import agent as dhcp_agent from neutron.agent import dhcp_agent as entry from neutron.agent.linux import dhcp from neutron.agent.linux import interface +from neutron.agent.metadata import driver as metadata_driver from neutron.common import config as common_config from neutron.common import constants as n_const from neutron.common import utils @@ -469,6 +470,20 @@ class TestDhcpAgent(base.BaseTestCase): dhcp.configure_dhcp_for_network(fake_network) self.assertEqual({fake_port1.id}, dhcp.dhcp_ready_ports) + def test_dhcp_metadata_destroy(self): + cfg.CONF.set_override('force_metadata', True) + cfg.CONF.set_override('enable_isolated_metadata', False) + with mock.patch.object(metadata_driver, + 'MetadataDriver') as md_cls: + dhcp = dhcp_agent.DhcpAgent(HOSTNAME) + dhcp.configure_dhcp_for_network(fake_network) + md_cls.spawn_monitored_metadata_proxy.assert_called_once_with( + mock.ANY, mock.ANY, mock.ANY, mock.ANY, + network_id=fake_network.id) + dhcp.disable_dhcp_helper(fake_network.id) + md_cls.destroy_monitored_metadata_proxy.assert_called_once_with( + mock.ANY, fake_network.id, mock.ANY) + def test_report_state_revival_logic(self): dhcp = dhcp_agent.DhcpAgentWithStateReport(HOSTNAME) with mock.patch.object(dhcp.state_rpc, @@ -780,12 +795,9 @@ class TestDhcpAgentEventHandler(base.BaseTestCase): self.cache.assert_has_calls( [mock.call.get_network_by_id(fake_network.id)]) self.call_driver.assert_called_once_with('disable', fake_network) - if isolated_metadata: - self.external_process.assert_has_calls([ - self._process_manager_constructor_call(ns=None), - mock.call().disable()]) - else: - self.assertFalse(self.external_process.call_count) + self.external_process.assert_has_calls([ + self._process_manager_constructor_call(ns=None), + mock.call().disable()]) def test_disable_dhcp_helper_known_network_isolated_metadata(self): self._disable_dhcp_helper_known_network(isolated_metadata=True) @@ -812,13 +824,10 @@ class TestDhcpAgentEventHandler(base.BaseTestCase): self.call_driver.assert_called_once_with('disable', fake_network) self.cache.assert_has_calls( [mock.call.get_network_by_id(fake_network.id)]) - if isolated_metadata: - self.external_process.assert_has_calls([ - self._process_manager_constructor_call(ns=None), - mock.call().disable() - ]) - else: - self.assertFalse(self.external_process.call_count) + self.external_process.assert_has_calls([ + self._process_manager_constructor_call(ns=None), + mock.call().disable() + ]) def test_disable_dhcp_helper_driver_failure_isolated_metadata(self): self._disable_dhcp_helper_driver_failure(isolated_metadata=True)