summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorJhon Honce <jhonce@redhat.com>2018-06-20 19:14:27 -0700
committerAtomic Bot <atomic-devel@projectatomic.io>2018-06-22 17:25:44 +0000
commit2f0f9944b610773d2d547c59cc7d936665b2bbdc (patch)
treee30e0625d099bf0297ce70c88bf48e2955a13494
parent3092d2084761755776994a8febe2279d07e3d03b (diff)
downloadpodman-2f0f9944b610773d2d547c59cc7d936665b2bbdc.tar.gz
podman-2f0f9944b610773d2d547c59cc7d936665b2bbdc.tar.bz2
podman-2f0f9944b610773d2d547c59cc7d936665b2bbdc.zip
Add unittests and fix bugs
* Improved error messages * Improved checking of user input Signed-off-by: Jhon Honce <jhonce@redhat.com> Closes: #978 Approved by: mheon
-rw-r--r--contrib/python/podman/client.py54
-rw-r--r--contrib/python/podman/libs/_containers_attach.py46
-rw-r--r--contrib/python/podman/libs/tunnel.py11
-rw-r--r--contrib/python/test/test_client.py37
-rw-r--r--contrib/python/test/test_system.py4
-rw-r--r--contrib/python/test/test_tunnel.py79
6 files changed, 184 insertions, 47 deletions
diff --git a/contrib/python/podman/client.py b/contrib/python/podman/client.py
index c62c1430a..ad166eb06 100644
--- a/contrib/python/podman/client.py
+++ b/contrib/python/podman/client.py
@@ -1,4 +1,5 @@
"""A client for communicating with a Podman varlink service."""
+import errno
import os
from urllib.parse import urlparse
@@ -32,33 +33,47 @@ class BaseClient(object):
if interface is None:
raise ValueError('interface is required and cannot be None')
+ unsupported = set(kwargs.keys()).difference(
+ ('uri', 'interface', 'remote_uri', 'identity_file'))
+ if unsupported:
+ raise ValueError('Unknown keyword arguments: {}'.format(
+ ', '.join(unsupported)))
+
local_path = urlparse(uri).path
if local_path == '':
- raise ValueError('path is required for uri, format'
- ' "unix://path_to_socket"')
+ raise ValueError('path is required for uri,'
+ ' expected format "unix://path_to_socket"')
if kwargs.get('remote_uri') or kwargs.get('identity_file'):
# Remote access requires the full tuple of information
if kwargs.get('remote_uri') is None:
- raise ValueError('remote is required, format'
- ' "ssh://user@hostname/path_to_socket".')
+ raise ValueError(
+ 'remote is required,'
+ ' expected format "ssh://user@hostname/path_to_socket".')
remote = urlparse(kwargs['remote_uri'])
if remote.username is None:
- raise ValueError('username is required for remote_uri, format'
- ' "ssh://user@hostname/path_to_socket".')
+ raise ValueError(
+ 'username is required for remote_uri,'
+ ' expected format "ssh://user@hostname/path_to_socket".')
if remote.path == '':
- raise ValueError('path is required for remote_uri, format'
- ' "ssh://user@hostname/path_to_socket".')
+ raise ValueError(
+ 'path is required for remote_uri,'
+ ' expected format "ssh://user@hostname/path_to_socket".')
if remote.hostname is None:
- raise ValueError('hostname is required for remote_uri, format'
- ' "ssh://user@hostname/path_to_socket".')
+ raise ValueError(
+ 'hostname is required for remote_uri,'
+ ' expected format "ssh://user@hostname/path_to_socket".')
if kwargs.get('identity_file') is None:
raise ValueError('identity_file is required.')
if not os.path.isfile(kwargs['identity_file']):
- raise ValueError('identity_file "{}" not found.'.format(
- kwargs['identity_file']))
+ raise FileNotFoundError(
+ errno.ENOENT,
+ os.strerror(errno.ENOENT),
+ kwargs['identity_file'],
+ )
+
return RemoteClient(
Context(uri, interface, local_path, remote.path,
remote.username, remote.hostname,
@@ -111,7 +126,7 @@ class RemoteClient(BaseClient):
self._iface = self._client.open(self._context.interface)
return self._iface
except Exception:
- self._close_tunnel(self._context.uri)
+ tunnel.close(self._context.uri)
raise
def __exit__(self, e_type, e, e_traceback):
@@ -154,15 +169,18 @@ class Client(object):
"""
self._client = BaseClient.factory(uri, interface, **kwargs)
+ address = "{}-{}".format(uri, interface)
# Quick validation of connection data provided
try:
if not System(self._client).ping():
- raise ValueError('Failed varlink connection "{}/{}"'.format(
- uri, interface))
+ raise ConnectionRefusedError(
+ errno.ECONNREFUSED,
+ 'Failed varlink connection "{}"'.format(address), address)
except FileNotFoundError:
- raise ValueError('Failed varlink connection "{}/{}".'
- ' Is podman service running?'.format(
- uri, interface))
+ raise ConnectionError(
+ errno.ECONNREFUSED,
+ ('Failed varlink connection "{}".'
+ ' Is podman service running?').format(address), address)
def __enter__(self):
"""Return `self` upon entering the runtime context."""
diff --git a/contrib/python/podman/libs/_containers_attach.py b/contrib/python/podman/libs/_containers_attach.py
index 54e6a009e..bd73542b9 100644
--- a/contrib/python/podman/libs/_containers_attach.py
+++ b/contrib/python/podman/libs/_containers_attach.py
@@ -21,6 +21,9 @@ class Mixin:
stderr is ignored.
"""
+ if not self.containerrunning:
+ raise Exception('you can only attach to running containers')
+
if stdin is None:
stdin = sys.stdin.fileno()
@@ -48,7 +51,7 @@ class Mixin:
packed = fcntl.ioctl(stdout, termios.TIOCGWINSZ,
struct.pack('HHHH', 0, 0, 0, 0))
rows, cols, _, _ = struct.unpack('HHHH', packed)
-
+ # TODO: Need some kind of timeout in case pipe is blocked
with open(ctl_socket, 'w') as skt:
# send conmon window resize message
skt.write('1 {} {}\n'.format(rows, cols))
@@ -73,38 +76,37 @@ class Mixin:
# catch any resizing events and send the resize info
# to the control fifo "socket"
signal.signal(signal.SIGWINCH, resize_handler)
+
except termios.error:
original_attr = None
try:
- # Prepare socket for communicating with conmon/container
+ # TODO: socket.SOCK_SEQPACKET may not be supported in Windows
with socket.socket(socket.AF_UNIX, socket.SOCK_SEQPACKET) as skt:
+ # Prepare socket for communicating with conmon/container
skt.connect(io_socket)
skt.sendall(b'\n')
sources = [skt, stdin]
while sources:
readable, _, _ = select.select(sources, [], [])
- for r in readable:
- if r is skt:
- data = r.recv(CONMON_BUFSZ)
- if not data:
- sources.remove(skt)
-
- # Remove source marker when writing
- os.write(stdout, data[1:])
- elif r is stdin:
- data = os.read(stdin, CONMON_BUFSZ)
- if not data:
- sources.remove(stdin)
-
- skt.sendall(data)
-
- if eot in data:
- sources.clear()
- break
- else:
- raise ValueError('Unknown source in select')
+ if skt in readable:
+ data = skt.recv(CONMON_BUFSZ)
+ if not data:
+ sources.remove(skt)
+
+ # Remove source marker when writing
+ os.write(stdout, data[1:])
+
+ if stdin in readable:
+ data = os.read(stdin, CONMON_BUFSZ)
+ if not data:
+ sources.remove(stdin)
+
+ skt.sendall(data)
+
+ if eot in data:
+ sources.clear()
finally:
if original_attr:
termios.tcsetattr(stdout, termios.TCSADRAIN, original_attr)
diff --git a/contrib/python/podman/libs/tunnel.py b/contrib/python/podman/libs/tunnel.py
index 2cb178644..534326ff0 100644
--- a/contrib/python/podman/libs/tunnel.py
+++ b/contrib/python/podman/libs/tunnel.py
@@ -26,6 +26,7 @@ class Portal(collections.MutableMapping):
self.sweap = sweap
self.ttl = sweap * 2
self.lock = threading.RLock()
+ self._schedule_reaper()
def __getitem__(self, key):
"""Given uri return tunnel and update TTL."""
@@ -73,11 +74,12 @@ class Portal(collections.MutableMapping):
def reap(self):
"""Remove tunnels who's TTL has expired."""
+ now = time.time()
with self.lock:
- now = time.time()
- for entry, timeout in self.data:
- if timeout < now:
- self.__delitem__(entry)
+ reaped_data = self.data.copy()
+ for entry in reaped_data.items():
+ if entry[1][1] < now:
+ del self.data[entry[0]]
else:
# StopIteration as soon as possible
break
@@ -121,7 +123,6 @@ class Tunnel(object):
def close(self, id):
"""Close SSH tunnel."""
- print('Tunnel collapsed!')
if self._tunnel is None:
return
diff --git a/contrib/python/test/test_client.py b/contrib/python/test/test_client.py
new file mode 100644
index 000000000..e642c8add
--- /dev/null
+++ b/contrib/python/test/test_client.py
@@ -0,0 +1,37 @@
+from __future__ import absolute_import
+
+import unittest
+from unittest.mock import patch
+
+import podman
+from podman.client import BaseClient, Client, LocalClient, RemoteClient
+
+
+class TestClient(unittest.TestCase):
+ def setUp(self):
+ pass
+
+ @patch('podman.libs.system.System.ping', return_value=True)
+ def test_local(self, mock_ping):
+ p = Client(
+ uri='unix:/run/podman',
+ interface='io.projectatomic.podman',
+ )
+
+ self.assertIsInstance(p._client, LocalClient)
+ self.assertIsInstance(p._client, BaseClient)
+
+ mock_ping.assert_called_once()
+
+ @patch('os.path.isfile', return_value=True)
+ @patch('podman.libs.system.System.ping', return_value=True)
+ def test_remote(self, mock_ping, mock_isfile):
+ p = Client(
+ uri='unix:/run/podman',
+ interface='io.projectatomic.podman',
+ remote_uri='ssh://user@hostname/run/podmain/podman',
+ identity_file='~/.ssh/id_rsa')
+
+ self.assertIsInstance(p._client, BaseClient)
+ mock_ping.assert_called_once()
+ mock_isfile.assert_called_once()
diff --git a/contrib/python/test/test_system.py b/contrib/python/test/test_system.py
index 93fb9aded..3f6ca57a2 100644
--- a/contrib/python/test/test_system.py
+++ b/contrib/python/test/test_system.py
@@ -32,8 +32,8 @@ class TestSystem(unittest.TestCase):
uri=local_uri,
remote_uri=remote_uri,
identity_file=os.path.expanduser('~/.ssh/id_rsa'),
- ) as pclient:
- pclient.system.ping()
+ ) as remote_client:
+ remote_client.system.ping()
def test_versions(self):
with podman.Client(self.host) as pclient:
diff --git a/contrib/python/test/test_tunnel.py b/contrib/python/test/test_tunnel.py
new file mode 100644
index 000000000..30a80289d
--- /dev/null
+++ b/contrib/python/test/test_tunnel.py
@@ -0,0 +1,79 @@
+from __future__ import absolute_import
+
+import time
+import unittest
+from unittest.mock import MagicMock, patch
+
+import podman
+from podman.libs.tunnel import Context, Portal, Tunnel
+
+
+class TestTunnel(unittest.TestCase):
+ def setUp(self):
+ self.tunnel_01 = MagicMock(spec=Tunnel)
+ self.tunnel_02 = MagicMock(spec=Tunnel)
+
+ def test_portal_ops(self):
+ portal = Portal(sweap=500)
+ portal['unix:/01'] = self.tunnel_01
+ portal['unix:/02'] = self.tunnel_02
+
+ self.assertEqual(portal.get('unix:/01'), self.tunnel_01)
+ self.assertEqual(portal.get('unix:/02'), self.tunnel_02)
+
+ del portal['unix:/02']
+ with self.assertRaises(KeyError):
+ portal['unix:/02']
+ self.assertEqual(len(portal), 1)
+
+ def test_portal_reaping(self):
+ portal = Portal(sweap=0.5)
+ portal['unix:/01'] = self.tunnel_01
+ portal['unix:/02'] = self.tunnel_02
+
+ self.assertEqual(len(portal), 2)
+ for entry in portal:
+ self.assertIn(entry, (self.tunnel_01, self.tunnel_02))
+
+ time.sleep(1)
+ portal.reap()
+ self.assertEqual(len(portal), 0)
+
+ def test_portal_no_reaping(self):
+ portal = Portal(sweap=500)
+ portal['unix:/01'] = self.tunnel_01
+ portal['unix:/02'] = self.tunnel_02
+
+ portal.reap()
+ self.assertEqual(len(portal), 2)
+ for entry in portal:
+ self.assertIn(entry, (self.tunnel_01, self.tunnel_02))
+
+ @patch('subprocess.Popen')
+ @patch('os.path.exists', return_value=True)
+ @patch('weakref.finalize')
+ def test_tunnel(self, mock_finalize, mock_exists, mock_Popen):
+ context = Context(
+ 'unix:/01',
+ 'io.projectatomic.podman',
+ '/tmp/user/socket',
+ '/run/podman/socket',
+ 'user',
+ 'hostname',
+ '~/.ssh/id_rsa',
+ )
+ tunnel = Tunnel(context).bore('unix:/01')
+
+ cmd = [
+ 'ssh',
+ '-nNT',
+ '-L',
+ '{}:{}'.format(context.local_socket, context.remote_socket),
+ '-i',
+ context.identity_file,
+ 'ssh://{}@{}'.format(context.username, context.hostname),
+ ]
+
+ mock_finalize.assert_called_once_with(tunnel, tunnel.close, 'unix:/01')
+ mock_exists.assert_called_once_with(context.local_socket)
+ mock_Popen.assert_called_once_with(cmd, close_fds=True)