Use SIGUSR1 to notify l3 agent of changing prefix file

It is common for all OpenStack services to use SIGHUP signal to
reload configuration and restart. This functionality, including
defining signal handler, is defined by oslo.service.

Meanwhile, this is currently not so for l3 agent. PrefixDelegation
class that is instantiated in L3NATAgent overrides handler for SIGHUP,
thus removing handler that was set in oslo.service.

The proposed solution is to use another signal, such as SIGUSR1,
instead of SIGHUP to notify l3 agent that a prefix file was
somehow changed.

Added a functional test for restarting L3 agent using SIGHUP.

Change-Id: I48eb4697a5fad97bcf08cfe1f80921a46d94029d
Closes-Bug: #1511401

diff --git a/neutron/agent/linux/pd.py b/neutron/agent/linux/pd.py
index e9be82a37c68b08fe1e155d4b011ad259fee92f7..e4c151f768118e38fe09dc5ed798b425c9a4851e 100644 (file)
--- a/neutron/agent/linux/pd.py
+++ b/neutron/agent/linux/pd.py
@@ -279,12 +279,15 @@ class PrefixDelegation(object):
             self.notifier(self.context, prefix_update)
 
     def after_start(self):
-        LOG.debug('SIGHUP signal handler set')
-        signal.signal(signal.SIGHUP, self._handle_sighup)
+        LOG.debug('SIGUSR1 signal handler set')
+        signal.signal(signal.SIGUSR1, self._handle_sigusr1)
 
-    def _handle_sighup(self, signum, frame):
-        # The external DHCPv6 client uses SIGHUP to notify agent
-        # of prefix changes.
+    def _handle_sigusr1(self, signum, frame):
+        """Update PD on receiving SIGUSR1.
+
+        The external DHCPv6 client uses SIGUSR1 to notify agent
+        of prefix changes.
+        """
         self.pd_update_cb()
 
     def _get_sync_data(self):

diff --git a/neutron/cmd/pd_notify.py b/neutron/cmd/pd_notify.py
index 02f5fdcfe637e98d538935ee378536b21857bf41..9dff494a48e67df810f2566ab9a8ac7c73867655 100644 (file)
--- a/neutron/cmd/pd_notify.py
+++ b/neutron/cmd/pd_notify.py
@@ -35,4 +35,4 @@ def main():
         utils.replace_file(prefix_fname, "%s/64" % prefix)
     elif operation == "delete":
         utils.replace_file(prefix_fname, "::/64")
-    os.kill(int(agent_pid), signal.SIGHUP)
+    os.kill(int(agent_pid), signal.SIGUSR1)

diff --git a/neutron/tests/functional/agent/test_l3_agent.py b/neutron/tests/functional/agent/test_l3_agent.py
index d8eca96307d94b2bcb47264dffc083a00fd200cd..76d21b53577e64b5451b9f41d10ba751e8cbb560 100644 (file)
--- a/neutron/tests/functional/agent/test_l3_agent.py
+++ b/neutron/tests/functional/agent/test_l3_agent.py
@@ -45,12 +45,14 @@ from neutron.callbacks import registry
 from neutron.callbacks import resources
 from neutron.common import config as common_config
 from neutron.common import constants as l3_constants
+from neutron.common import topics
 from neutron.common import utils as common_utils
 from neutron.tests.common import l3_test_common
 from neutron.tests.common import machine_fixtures
 from neutron.tests.common import net_helpers
 from neutron.tests.functional.agent.linux import helpers
 from neutron.tests.functional import base
+from neutron.tests.functional import test_service
 
 
 LOG = logging.getLogger(__name__)
@@ -70,7 +72,9 @@ class L3AgentTestFramework(base.BaseSudoTestCase):
         self.mock_plugin_api = mock.patch(
             'neutron.agent.l3.agent.L3PluginApi').start().return_value
         mock.patch('neutron.agent.rpc.PluginReportStateAPI').start()
-        self.agent = self._configure_agent('agent1')
+        self.conf = self._configure_agent('agent1')
+        self.agent = neutron_l3_agent.L3NATAgentWithStateReport('agent1',
+                                                                self.conf)
 
     def _get_config_opts(self):
         config = cfg.ConfigOpts()
@@ -108,9 +112,8 @@ class L3AgentTestFramework(base.BaseSudoTestCase):
                           get_temp_file_path('external/pids'))
         conf.set_override('host', host)
         conf.set_override('agent_mode', agent_mode)
-        agent = neutron_l3_agent.L3NATAgentWithStateReport(host, conf)
 
-        return agent
+        return conf
 
     def _get_agent_ovs_integration_bridge(self, agent):
         return get_ovs_bridge(agent.conf.ovs_integration_bridge)
@@ -809,7 +812,9 @@ class L3HATestFramework(L3AgentTestFramework):
 
     def setUp(self):
         super(L3HATestFramework, self).setUp()
-        self.failover_agent = self._configure_agent('agent2')
+        self.conf = self._configure_agent('agent2')
+        self.failover_agent = neutron_l3_agent.L3NATAgentWithStateReport(
+            'agent2', self.conf)
 
         br_int_1 = self._get_agent_ovs_integration_bridge(self.agent)
         br_int_2 = self._get_agent_ovs_integration_bridge(self.failover_agent)
@@ -919,6 +924,23 @@ class L3HATestFramework(L3AgentTestFramework):
         verify_ip_in_keepalived_config(router, internal_iface)
 
 
+class TestL3AgentRestart(test_service.TestServiceRestart,
+                         L3AgentTestFramework):
+
+    def _start_l3_agent(self, workers=1):
+        with mock.patch("neutron.service.Service.start") as start_method:
+            start_method.side_effect = self._fake_start
+            self._start_service(
+                host='agent1', binary='neutron-l3-agent',
+                topic=topics.L3_AGENT,
+                manager='neutron.agent.l3.agent.L3NATAgentWithStateReport',
+                workers=workers, conf=self.conf)
+
+    def test_restart_l3_agent_on_sighup(self):
+        self._test_restart_service_on_sighup(service=self._start_l3_agent,
+                                             workers=1)
+
+
 class MetadataFakeProxyHandler(object):
 
     def __init__(self, status):
@@ -1553,7 +1575,9 @@ class TestDvrRouter(L3AgentTestFramework):
     def _setup_dvr_ha_agents(self):
         self.agent.conf.agent_mode = 'dvr_snat'
 
-        self.failover_agent = self._configure_agent('agent2')
+        self.conf = self._configure_agent('agent2')
+        self.failover_agent = neutron_l3_agent.L3NATAgentWithStateReport(
+            'agent2', self.conf)
         self.failover_agent.conf.agent_mode = 'dvr_snat'
 
     def _setup_dvr_ha_bridges(self):

diff --git a/neutron/tests/functional/test_service.py b/neutron/tests/functional/test_service.py
index d3bed3ee048e9b83435acb48fb4f2351a31fc4ef..9206aefa207742cdfcf4992882fc61023e57e8a7 100644 (file)
--- a/neutron/tests/functional/test_service.py
+++ b/neutron/tests/functional/test_service.py
@@ -14,18 +14,29 @@
 
 from oslo_concurrency import processutils
 from oslo_config import cfg
+from oslo_service import service
 
-from neutron import service
+from neutron import service as neutron_service
 from neutron.tests import base
+from neutron.tests.functional import test_server
 
 
 class TestService(base.BaseTestCase):
 
     def test_api_workers_default(self):
         self.assertEqual(processutils.get_worker_count(),
-                         service._get_api_workers())
+                         neutron_service._get_api_workers())
 
     def test_api_workers_from_config(self):
         cfg.CONF.set_override('api_workers', 1234)
         self.assertEqual(1234,
-                         service._get_api_workers())
+                         neutron_service._get_api_workers())
+
+
+class TestServiceRestart(test_server.TestNeutronServer):
+
+    def _start_service(self, host, binary, topic, manager, workers,
+                       *args, **kwargs):
+        server = neutron_service.Service(host, binary, topic, manager,
+                                         *args, **kwargs)
+        service.launch(cfg.CONF, server, workers).wait()