From 1bf5a44a77b4019838bf8bfa9791b7ee85bacaa3 Mon Sep 17 00:00:00 2001 From: Corubba <97832352+corubba@users.noreply.github.com> Date: Mon, 9 Oct 2023 09:11:11 +0200 Subject: [PATCH] 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 429d8c8cfb3d95c4f2a04f9accbd26f53832ac9b. --------- Co-authored-by: Felix Fontein --- changelogs/fragments/7369-fix-lxc-options.yml | 3 + plugins/connection/lxc.py | 8 +- tests/unit/plugins/connection/test_lxc.py | 77 +++++++++++++++++-- 3 files changed, 77 insertions(+), 11 deletions(-) create mode 100644 changelogs/fragments/7369-fix-lxc-options.yml diff --git a/changelogs/fragments/7369-fix-lxc-options.yml b/changelogs/fragments/7369-fix-lxc-options.yml new file mode 100644 index 0000000000..c909f34603 --- /dev/null +++ b/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). diff --git a/plugins/connection/lxc.py b/plugins/connection/lxc.py index 7ad5807244..35cc0b3c81 100644 --- a/plugins/connection/lxc.py +++ b/plugins/connection/lxc.py @@ -60,7 +60,7 @@ 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): @@ -68,12 +68,14 @@ class Connection(ConnectionBase): 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 @@ class Connection(ConnectionBase): 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 diff --git a/tests/unit/plugins/connection/test_lxc.py b/tests/unit/plugins/connection/test_lxc.py index 8733a92e09..fe2253df62 100644 --- a/tests/unit/plugins/connection/test_lxc.py +++ b/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 -class TestLXCConnectionClass(unittest.TestCase): +@pytest.fixture(autouse=True) +def lxc(request): + """Fixture to import/load the lxc plugin module. - def test_lxc_connection_module(self): + 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(): + + @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