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>

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).

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)

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