From bc628ac6efa99781e547075e99a2d72b4afc867b Mon Sep 17 00:00:00 2001 From: Dmitry Tantsur Date: Fri, 25 Sep 2020 17:34:01 +0200 Subject: [PATCH] Route conductor notification RPC to the same conductor RPC continue_node_{deploy,clean} are called from a conductor handling the node, so they don't have to go through the hash ring. This avoids situation when take over happens in the middle of a deploy/clean step processing, breaking it. Eventually, we should stop using RPC for that at all, but that will be a much more invasive change. Story: #2008200 Task: #40984 Change-Id: I76293f8ec30d5957b99bdbce5b70e87e8378d135 --- ironic/conductor/rpcapi.py | 4 ++++ ironic/conductor/utils.py | 2 +- ironic/tests/unit/conductor/test_utils.py | 9 ++++----- releasenotes/notes/notify-topic-451493784ce45e73.yaml | 6 ++++++ 4 files changed, 15 insertions(+), 6 deletions(-) create mode 100644 releasenotes/notes/notify-topic-451493784ce45e73.yaml diff --git a/ironic/conductor/rpcapi.py b/ironic/conductor/rpcapi.py index 16b66e6373..65b234d163 100644 --- a/ironic/conductor/rpcapi.py +++ b/ironic/conductor/rpcapi.py @@ -202,6 +202,10 @@ class ConductorAPI(object): host = random.choice(list(ring.nodes)) return self.topic + "." + host + def get_current_topic(self): + """Get RPC topic name for the current conductor.""" + return self.topic + "." + CONF.host + def can_send_create_port(self): """Return whether the RPCAPI supports the create_port method.""" return self.client.can_send_version("1.41") diff --git a/ironic/conductor/utils.py b/ironic/conductor/utils.py index 26c590f4d5..2e97be54d3 100644 --- a/ironic/conductor/utils.py +++ b/ironic/conductor/utils.py @@ -828,7 +828,7 @@ def notify_conductor_resume_operation(task, operation): from ironic.conductor import rpcapi uuid = task.node.uuid rpc = rpcapi.ConductorAPI() - topic = rpc.get_topic_for(task.node) + topic = rpc.get_current_topic() # Need to release the lock to let the conductor take it task.release_resources() getattr(rpc, method)(task.context, uuid, topic=topic) diff --git a/ironic/tests/unit/conductor/test_utils.py b/ironic/tests/unit/conductor/test_utils.py index 9f917142fe..2b1ea74cc1 100644 --- a/ironic/tests/unit/conductor/test_utils.py +++ b/ironic/tests/unit/conductor/test_utils.py @@ -1728,15 +1728,14 @@ class MiscTestCase(db_base.DbTestCase): @mock.patch.object(rpcapi.ConductorAPI, 'continue_node_deploy', autospec=True) - @mock.patch.object(rpcapi.ConductorAPI, 'get_topic_for', autospec=True) - def test_notify_conductor_resume_operation(self, mock_topic, - mock_rpc_call): - mock_topic.return_value = 'topic' + def test_notify_conductor_resume_operation(self, mock_rpc_call): + self.config(host='fake-host') with task_manager.acquire( self.context, self.node.uuid, shared=False) as task: conductor_utils.notify_conductor_resume_operation(task, 'deploy') mock_rpc_call.assert_called_once_with( - mock.ANY, task.context, self.node.uuid, topic='topic') + mock.ANY, task.context, self.node.uuid, + topic='ironic.conductor_manager.fake-host') @mock.patch.object(conductor_utils, 'notify_conductor_resume_operation', autospec=True) diff --git a/releasenotes/notes/notify-topic-451493784ce45e73.yaml b/releasenotes/notes/notify-topic-451493784ce45e73.yaml new file mode 100644 index 0000000000..18f8668b30 --- /dev/null +++ b/releasenotes/notes/notify-topic-451493784ce45e73.yaml @@ -0,0 +1,6 @@ +--- +fixes: + - | + Prevents a take over from happening in the middle of a deploy step + processing. This could happen if the RPC call ``continue_node_deploy`` + is routed to a different conductor.