Fix lxc plugin options (#7369)

* Fixture for liblxc

Add a fixture to allow testing the lxc connection plugin both with and
without liblxc being present.

Also change the test from unittest to pytest.

* Update liblxc error message

The error is not specific to python2, so remove the version. Also add
a test for it.

* Migrate to options

Because the lxc plugin was only using PlayContext properties, using host
vars like `ansible_lxc_host` didn't work. This is fixed by instead using
the `get_option` method inherited from `AnsiblePlugin`.
The options are not yet available in the `__init__` function, so the
determination of the container name is moved to the `_connect` method,
which is the first time it is actually needed.
The default for the `remote_addr` option is removed, because the string
`inventory_hostname` is not very useful. At all. This seams to have been
spread with copy&paste and a bit of cargo culting. The variable priority
already takes care of setting the value.

* Add changelog fragment

* Fix for Py2.7

`TypeError: super() takes at least 1 argument (0 given)`

* Add plugin type to changelog fragment.

* Restore untemplated default

This partially reverts commit 429d8c8.

---------

Co-authored-by: Felix Fontein <[email protected]>

changelogs/fragments/7369-fix-lxc-options.yml

@@ -0,0 +1,3 @@
+---
+bugfixes:
+  - lxc connection plugin - properly evaluate options (https://github.com/ansible-collections/community.general/pull/7369).

plugins/connection/lxc.py

@@ -60,20 +60,22 @@ class Connection(ConnectionBase):
     def __init__(self, play_context, new_stdin, *args, **kwargs):
         super(Connection, self).__init__(play_context, new_stdin, *args, **kwargs)
 
-        self.container_name = self._play_context.remote_addr
+        self.container_name = None
         self.container = None
 
     def _connect(self):
         """ connect to the lxc; nothing to do here """
         super(Connection, self)._connect()
 
         if not HAS_LIBLXC:
-            msg = "lxc bindings for python2 are not installed"
+            msg = "lxc python bindings are not installed"
             raise errors.AnsibleError(msg)
 
         if self.container:
             return
 
+        self.container_name = self.get_option('remote_addr')
+
         self._display.vvv("THIS IS A LOCAL LXC DIR", host=self.container_name)
         self.container = _lxc.Container(self.container_name)
         if self.container.state == "STOPPED":
@@ -118,7 +120,7 @@ def exec_command(self, cmd, in_data=None, sudoable=False):
         super(Connection, self).exec_command(cmd, in_data=in_data, sudoable=sudoable)
 
         # python2-lxc needs bytes. python3-lxc needs text.
-        executable = to_native(self._play_context.executable, errors='surrogate_or_strict')
+        executable = to_native(self.get_option('executable'), errors='surrogate_or_strict')
         local_cmd = [executable, '-c', to_native(cmd, errors='surrogate_or_strict')]
 
         read_stdout, write_stdout = None, None

tests/unit/plugins/connection/test_lxc.py

@@ -6,20 +6,81 @@
 from __future__ import (absolute_import, division, print_function)
 __metaclass__ = type
 
+import pytest
+import sys
+
 from io import StringIO
 
-from ansible_collections.community.general.tests.unit.compat import unittest
-from ansible_collections.community.general.plugins.connection import lxc
+from ansible.errors import AnsibleError
 from ansible.playbook.play_context import PlayContext
+from ansible.plugins.loader import connection_loader
+from ansible_collections.community.general.tests.unit.compat import mock
+
+
+@pytest.fixture(autouse=True)
+def lxc(request):
+    """Fixture to import/load the lxc plugin module.
+
+    The fixture parameter is used to determine the presence of liblxc.
+    When true (default), a mocked liblxc module is injected. If False,
+    no liblxc will be present.
+    """
+    liblxc_present = getattr(request, 'param', True)
+
+    class ContainerMock(mock.MagicMock):
+        def __init__(self, name):
+            super(ContainerMock, self).__init__()
+            self.name = name
+            self.state = 'STARTED'
+
+    liblxc_module_mock = mock.MagicMock()
+    liblxc_module_mock.Container = ContainerMock
+
+    with mock.patch.dict('sys.modules'):
+        if liblxc_present:
+            sys.modules['lxc'] = liblxc_module_mock
+        elif 'lxc' in sys.modules:
+            del sys.modules['lxc']
+
+        from ansible_collections.community.general.plugins.connection import lxc as lxc_plugin_module
+
+        assert lxc_plugin_module.HAS_LIBLXC == liblxc_present
+        assert bool(getattr(lxc_plugin_module, '_lxc', None)) == liblxc_present
+
+        yield lxc_plugin_module
 
 
-class TestLXCConnectionClass(unittest.TestCase):
+class TestLXCConnectionClass():
 
-    def test_lxc_connection_module(self):
+    @pytest.mark.parametrize('lxc', [True, False], indirect=True)
+    def test_lxc_connection_module(self, lxc):
+        """Test that a connection can be created with the plugin."""
         play_context = PlayContext()
-        play_context.prompt = (
-            '[sudo via ansible, key=ouzmdnewuhucvuaabtjmweasarviygqq] password: '
-        )
         in_stream = StringIO()
 
-        self.assertIsInstance(lxc.Connection(play_context, in_stream), lxc.Connection)
+        conn = connection_loader.get('lxc', play_context, in_stream)
+        assert conn
+        assert isinstance(conn, lxc.Connection)
+
+    @pytest.mark.parametrize('lxc', [False], indirect=True)
+    def test_lxc_connection_liblxc_error(self, lxc):
+        """Test that on connect an error is thrown if liblxc is not present."""
+        play_context = PlayContext()
+        in_stream = StringIO()
+        conn = connection_loader.get('lxc', play_context, in_stream)
+
+        with pytest.raises(AnsibleError, match='lxc python bindings are not installed'):
+            conn._connect()
+
+    def test_remote_addr_option(self):
+        """Test that the remote_addr option is used"""
+        play_context = PlayContext()
+        in_stream = StringIO()
+        conn = connection_loader.get('lxc', play_context, in_stream)
+
+        container_name = 'my-container'
+        conn.set_option('remote_addr', container_name)
+        assert conn.get_option('remote_addr') == container_name
+
+        conn._connect()
+        assert conn.container_name == container_name