Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP] docker connection plugin: make sure docker_extra_args is used during version check #326

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions changelogs/fragments/326-docker-connection-fix.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
bugfixes:
- "docker connection plugin - also use ``docker_extra_args`` during Docker version check. This was accidentally removed by a bugfix in the previous release (https://github.com/ansible-collections/community.docker/issues/325, https://github.com/ansible-collections/community.docker/pull/326)."
14 changes: 8 additions & 6 deletions plugins/connection/docker.py
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,7 @@ def __init__(self, play_context, new_stdin, *args, **kwargs):

self._docker_args = []
self._container_user_cache = {}
self.docker_version = None

# Windows uses Powershell modules
if getattr(self._shell, "_IS_WINDOWS", False):
Expand All @@ -129,12 +130,6 @@ def __init__(self, play_context, new_stdin, *args, **kwargs):
except ValueError:
raise AnsibleError("docker command not found in PATH")

self.docker_version = self._get_docker_version()
if self.docker_version == u'dev':
display.warning(u'Docker version number is "dev". Will assume latest version.')
if self.docker_version != u'dev' and LooseVersion(self.docker_version) < LooseVersion(u'1.3'):
raise AnsibleError('docker connection type requires docker 1.3 or higher')

@staticmethod
def _sanitize_version(version):
version = re.sub(u'[^0-9a-zA-Z.]', u'', version)
Expand Down Expand Up @@ -224,6 +219,13 @@ def _set_conn_data(self):

''' initialize for the connection, cannot do only in init since all data is not ready at that point '''

if self.docker_version is None:
self.docker_version = self._get_docker_version()
if self.docker_version == u'dev':
display.warning(u'Docker version number is "dev". Will assume latest version.')
if self.docker_version != u'dev' and LooseVersion(self.docker_version) < LooseVersion(u'1.3'):
raise AnsibleError('docker connection type requires docker 1.3 or higher')

# TODO: this is mostly for backwards compatibility, play_context is used as fallback for older versions
# docker arguments
del self._docker_args[:]
Expand Down
26 changes: 13 additions & 13 deletions tests/unit/plugins/connection/test_docker.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,13 +41,7 @@ def setUp(self):
self.mock_get_bin_path = mock.patch(
'ansible_collections.community.docker.plugins.connection.docker.get_bin_path', return_value='docker')
self.mock_get_bin_path.start()
with mock.patch(
'ansible_collections.community.docker.plugins.connection.docker.Connection._old_docker_version',
return_value=('false', 'garbage', '', 1)):
with mock.patch(
'ansible_collections.community.docker.plugins.connection.docker.Connection._new_docker_version',
return_value=(['docker', 'version'], '20.10.0', '', 0)):
dc = connection_loader.get('community.docker.docker', self.play_context, self.in_stream)
dc = connection_loader.get('community.docker.docker', self.play_context, self.in_stream)

def tearDown(self):
pass
Expand All @@ -57,22 +51,28 @@ def tearDown(self):
@mock.patch('ansible_collections.community.docker.plugins.connection.docker.Connection._new_docker_version',
return_value=('docker version', '1.2.3', '', 0))
def test_docker_connection_module_too_old(self, mock_new_docker_verison, mock_old_docker_version):
self.assertRaisesRegexp(AnsibleError, '^docker connection type requires docker 1.3 or higher$',
DockerConnection, self.play_context, self.in_stream, docker_command='/fake/docker')
connection = connection_loader.get(
'community.docker.docker', self.play_context, self.in_stream, docker_command='/fake/docker')
self.assertRaisesRegexp(
AnsibleError, '^docker connection type requires docker 1.3 or higher$', connection._connect)

@mock.patch('ansible_collections.community.docker.plugins.connection.docker.Connection._old_docker_version',
return_value=('false', 'garbage', '', 1))
@mock.patch('ansible_collections.community.docker.plugins.connection.docker.Connection._new_docker_version',
return_value=('docker version', '1.3.4', '', 0))
def test_docker_connection_module(self, mock_new_docker_verison, mock_old_docker_version):
self.assertIsInstance(DockerConnection(self.play_context, self.in_stream, docker_command='/fake/docker'),
DockerConnection)
connection = connection_loader.get(
'community.docker.docker', self.play_context, self.in_stream, docker_command='/fake/docker')
self.assertIsInstance(connection, DockerConnection)
connection._connect()

# old version and new version fail
@mock.patch('ansible_collections.community.docker.plugins.connection.docker.Connection._old_docker_version',
return_value=('false', 'garbage', '', 1))
@mock.patch('ansible_collections.community.docker.plugins.connection.docker.Connection._new_docker_version',
return_value=('false', 'garbage', '', 1))
def test_docker_connection_module_wrong_cmd(self, mock_new_docker_version, mock_old_docker_version):
self.assertRaisesRegexp(AnsibleError, '^Docker version check (.*?) failed: ',
DockerConnection, self.play_context, self.in_stream, docker_command='/fake/docker')
connection = connection_loader.get(
'community.docker.docker', self.play_context, self.in_stream, docker_command='/fake/docker')
self.assertRaisesRegexp(
AnsibleError, '^Docker version check (.*?) failed: ', connection._connect)