From 71817c79abb921e4ea2da307ab0d2365da3f8c86 Mon Sep 17 00:00:00 2001 From: Hongbin Lu Date: Tue, 7 Mar 2017 15:58:48 -0600 Subject: [PATCH] Refactor load_image out of inspect_image This allows load_image to be reused by others. In addition, reduce the max-complexity from 10 to 20. Max complexity of 10 is hard to achieve and much lower than other openstack projects. Change-Id: I6185e3b8fc5e5b132656ff416e08917edd938fc8 --- tools/flake8wrap.sh | 4 +- zun/compute/manager.py | 6 ++- zun/container/docker/driver.py | 13 +++-- .../container/docker/test_docker_driver.py | 51 +------------------ zun/tests/unit/container/fake_driver.py | 8 ++- 5 files changed, 21 insertions(+), 61 deletions(-) diff --git a/tools/flake8wrap.sh b/tools/flake8wrap.sh index 919ea6729..da6072b01 100755 --- a/tools/flake8wrap.sh +++ b/tools/flake8wrap.sh @@ -13,8 +13,8 @@ if test "x$1" = "x-HEAD" ; then shift files=$(git diff --name-only HEAD~1 | tr '\n' ' ') echo "Running flake8 on ${files}" - diff -u --from-file /dev/null ${files} | flake8 --max-complexity 10 --diff "$@" + diff -u --from-file /dev/null ${files} | flake8 --max-complexity 20 --diff "$@" else echo "Running flake8 on all files" - exec flake8 --max-complexity 10 "$@" + exec flake8 --max-complexity 20 "$@" fi diff --git a/zun/compute/manager.py b/zun/compute/manager.py index 7220b20a2..723e022ec 100644 --- a/zun/compute/manager.py +++ b/zun/compute/manager.py @@ -76,6 +76,7 @@ class Manager(object): image = image_driver.pull_image(context, repo, tag, sandbox_image_pull_policy, sandbox_image_driver) + self.driver.load_image(sandbox_image, image['path']) sandbox_id = self.driver.create_sandbox(context, container, image=sandbox_image) except Exception as e: @@ -96,6 +97,7 @@ class Manager(object): image = image_driver.pull_image(context, repo, tag, image_pull_policy, image_driver_name) + self.driver.load_image(container.image, image['path']) except exception.ImageNotFound as e: with excutils.save_and_reraise_exception(reraise=reraise): LOG.error(six.text_type(e)) @@ -432,8 +434,8 @@ class Manager(object): try: pulled_image = image_driver.pull_image(context, image.repo, image.tag) - image_dict = self.driver.inspect_image(repo_tag, - pulled_image['path']) + self.driver.load_image(repo_tag, pulled_image['path']) + image_dict = self.driver.inspect_image(repo_tag) image.image_id = image_dict['Id'] image.size = image_dict['Size'] image.save() diff --git a/zun/container/docker/driver.py b/zun/container/docker/driver.py index 966cf9f8e..4c88548cd 100644 --- a/zun/container/docker/driver.py +++ b/zun/container/docker/driver.py @@ -43,12 +43,15 @@ class DockerDriver(driver.ContainerDriver): def __init__(self): super(DockerDriver, self).__init__() - def inspect_image(self, image, image_path=None): + def load_image(self, image, image_path=None): with docker_utils.docker_client() as docker: if image_path: - LOG.debug('Loading local image in docker %s' % image) - with open(image_path, 'r') as fd: + with open(image_path, 'rb') as fd: + LOG.debug('Loading local image %s into docker', image_path) docker.load_image(fd.read()) + + def inspect_image(self, image): + with docker_utils.docker_client() as docker: LOG.debug('Inspecting image %s' % image) image_dict = docker.inspect_image(image) return image_dict @@ -61,10 +64,6 @@ class DockerDriver(driver.ContainerDriver): def create(self, context, container, sandbox_id, image): with docker_utils.docker_client() as docker: name = container.name - if image['path']: - LOG.debug('Loading local image %s in docker' % container.image) - with open(image['path'], 'r') as fd: - docker.load_image(fd.read()) image = container.image LOG.debug('Creating container with image %s name %s' % (image, name)) diff --git a/zun/tests/unit/container/docker/test_docker_driver.py b/zun/tests/unit/container/docker/test_docker_driver.py index da8a968cb..9dcb85be3 100644 --- a/zun/tests/unit/container/docker/test_docker_driver.py +++ b/zun/tests/unit/container/docker/test_docker_driver.py @@ -59,16 +59,13 @@ class TestDockerDriver(base.DriverTestCase): self.driver.inspect_image(mock_image) self.mock_docker.inspect_image.assert_called_once_with(mock_image) - def test_inspect_image_path_is_not_none(self): + def test_load_image(self): self.mock_docker.load_image = mock.Mock() - self.mock_docker.inspect_image = mock.Mock() mock_open_file = mock.mock_open(read_data='test_data') with mock.patch('zun.container.docker.driver.open', mock_open_file): mock_image = mock.MagicMock() - self.driver.inspect_image(mock_image, 'test') + self.driver.load_image(mock_image, 'test') self.mock_docker.load_image.assert_called_once_with('test_data') - self.mock_docker.inspect_image.assert_called_once_with( - mock_image) def test_images(self): self.mock_docker.images = mock.Mock() @@ -112,50 +109,6 @@ class TestDockerDriver(base.DriverTestCase): self.assertEqual(result_container.status, fields.ContainerStatus.STOPPED) - @mock.patch('zun.objects.container.Container.save') - def test_create_image_path_is_not_none(self, mock_save): - self.mock_docker.load_image = mock.Mock(return_value='load_test') - self.mock_docker.create_host_config = mock.Mock( - return_value={'Id1': 'val1', 'key2': 'val2'}) - self.mock_docker.create_container = mock.Mock( - return_value={'Id': 'val1', 'key1': 'val2'}) - mock_open_file = mock.mock_open(read_data='test_data') - with mock.patch('zun.container.docker.driver.open', mock_open_file): - mock_container = self.mock_default_container - result_container = self.driver.create(self.context, - mock_container, - 'test_sandbox', - {'path': 'test_path'}) - self.mock_docker.load_image.assert_called_once_with('test_data') - - host_config = {} - host_config['network_mode'] = 'container:test_sandbox' - host_config['ipc_mode'] = 'container:test_sandbox' - host_config['volumes_from'] = 'test_sandbox' - host_config['mem_limit'] = '512m' - host_config['cpu_quota'] = 100000 - host_config['cpu_period'] = 100000 - host_config['restart_policy'] = {'Name': 'no', - 'MaximumRetryCount': 0} - self.mock_docker.create_host_config.assert_called_once_with( - **host_config) - - kwargs = { - 'command': 'fake_command', - 'environment': {'key1': 'val1', 'key2': 'val2'}, - 'working_dir': '/home/ubuntu', - 'labels': {'key1': 'val1', 'key2': 'val2'}, - 'host_config': {'Id1': 'val1', 'key2': 'val2'}, - 'name': 'zun-ea8e2a25-2901-438d-8157-de7ffd68d051', - 'stdin_open': True, - 'tty': True, - } - self.mock_docker.create_container.assert_called_once_with( - mock_container.image, **kwargs) - self.assertEqual(result_container.container_id, 'val1') - self.assertEqual(result_container.status, - fields.ContainerStatus.STOPPED) - def test_delete_success(self): self.mock_docker.remove_container = mock.Mock() mock_container = mock.MagicMock() diff --git a/zun/tests/unit/container/fake_driver.py b/zun/tests/unit/container/fake_driver.py index 44221b7e8..1aefdec9a 100644 --- a/zun/tests/unit/container/fake_driver.py +++ b/zun/tests/unit/container/fake_driver.py @@ -22,7 +22,13 @@ class FakeDriver(driver.ContainerDriver): def __init__(self): super(FakeDriver, self).__init__() - def pull_image(self, image): + def load_image(self, image, image_path=None): + pass + + def inspect_image(self, image): + pass + + def images(self, repo, **kwargs): pass def create(self, container):