From 3ae19aa28f49c380f89d593217085ac7774e28f6 Mon Sep 17 00:00:00 2001
From: Corubba <97832352+corubba@users.noreply.github.com>
Date: Wed, 18 Oct 2023 20:54:24 +0200
Subject: [PATCH] Handle remote_addr change in lxc connection plugin (#7373)

* Stopped container error test

* Handle remote_addr change

Detect if the remote_addr option changed, and properly "reconnect" aka
update the internal state of the plugin instance.

* Add changelog fragment

* Apply suggestions from code review

Co-authored-by: Felix Fontein <felix@fontein.de>

---------

Co-authored-by: Felix Fontein <felix@fontein.de>
---
 .../fragments/7373-lxc-remote-addr-change.yml |  3 +
 plugins/connection/lxc.py                     |  5 +-
 tests/unit/plugins/connection/test_lxc.py     | 61 ++++++++++++++++++-
 3 files changed, 65 insertions(+), 4 deletions(-)
 create mode 100644 changelogs/fragments/7373-lxc-remote-addr-change.yml

diff --git a/changelogs/fragments/7373-lxc-remote-addr-change.yml b/changelogs/fragments/7373-lxc-remote-addr-change.yml
new file mode 100644
index 0000000000..9e5ba057a5
--- /dev/null
+++ b/changelogs/fragments/7373-lxc-remote-addr-change.yml
@@ -0,0 +1,3 @@
+---
+minor_changes:
+  - lxc connection plugin - properly handle a change of the ``remote_addr`` option (https://github.com/ansible-collections/community.general/pull/7373).
diff --git a/plugins/connection/lxc.py b/plugins/connection/lxc.py
index 35cc0b3c81..7bb5824fac 100644
--- a/plugins/connection/lxc.py
+++ b/plugins/connection/lxc.py
@@ -71,10 +71,11 @@ class Connection(ConnectionBase):
             msg = "lxc python bindings are not installed"
             raise errors.AnsibleError(msg)
 
-        if self.container:
+        container_name = self.get_option('remote_addr')
+        if self.container and self.container_name == container_name:
             return
 
-        self.container_name = self.get_option('remote_addr')
+        self.container_name = container_name
 
         self._display.vvv("THIS IS A LOCAL LXC DIR", host=self.container_name)
         self.container = _lxc.Container(self.container_name)
diff --git a/tests/unit/plugins/connection/test_lxc.py b/tests/unit/plugins/connection/test_lxc.py
index fe2253df62..bebd42772a 100644
--- a/tests/unit/plugins/connection/test_lxc.py
+++ b/tests/unit/plugins/connection/test_lxc.py
@@ -27,11 +27,17 @@ def lxc(request):
     """
     liblxc_present = getattr(request, 'param', True)
 
-    class ContainerMock(mock.MagicMock):
+    class ContainerMock():
+        # dict of container name to its state
+        _container_states = {}
+
         def __init__(self, name):
             super(ContainerMock, self).__init__()
             self.name = name
-            self.state = 'STARTED'
+
+        @property
+        def state(self):
+            return ContainerMock._container_states.get(self.name, 'STARTED')
 
     liblxc_module_mock = mock.MagicMock()
     liblxc_module_mock.Container = ContainerMock
@@ -84,3 +90,54 @@ class TestLXCConnectionClass():
 
         conn._connect()
         assert conn.container_name == container_name
+
+    def test_error_when_stopped(self, lxc):
+        """Test that on connect an error is thrown if the container is stopped."""
+        play_context = PlayContext()
+        in_stream = StringIO()
+        conn = connection_loader.get('lxc', play_context, in_stream)
+        conn.set_option('remote_addr', 'my-container')
+
+        lxc._lxc.Container._container_states['my-container'] = 'STOPPED'
+
+        with pytest.raises(AnsibleError, match='my-container is not running'):
+            conn._connect()
+
+    def test_container_name_change(self):
+        """Test connect method reconnects when remote_addr changes"""
+        play_context = PlayContext()
+        in_stream = StringIO()
+        conn = connection_loader.get('lxc', play_context, in_stream)
+
+        # setting the option does nothing
+        container1_name = 'my-container'
+        conn.set_option('remote_addr', container1_name)
+        assert conn.container_name is None
+        assert conn.container is None
+
+        # first call initializes the connection
+        conn._connect()
+        assert conn.container_name is container1_name
+        assert conn.container is not None
+        assert conn.container.name == container1_name
+        container1 = conn.container
+
+        # second call is basically a no-op
+        conn._connect()
+        assert conn.container_name is container1_name
+        assert conn.container is container1
+        assert conn.container.name == container1_name
+
+        # setting the option does again nothing
+        container2_name = 'my-other-container'
+        conn.set_option('remote_addr', container2_name)
+        assert conn.container_name == container1_name
+        assert conn.container is container1
+        assert conn.container.name == container1_name
+
+        # first call with a different remote_addr changes the connection
+        conn._connect()
+        assert conn.container_name == container2_name
+        assert conn.container is not None
+        assert conn.container is not container1
+        assert conn.container.name == container2_name