openstack/zun
Code Issues Proposed changes

Implements image_pull_policy

Browse Source 
This patch implements image_pull_policy, which determines if
the image should be pulled prior to starting the container.

It can have following values:
ifnotpresent: only pull the image if it does not already exist on the node.
always: Always pull the image from repositery.
never: never pull the image

If image_pull_policy is not specified then:
1. If the tag is latest, defaults image_pull_policy is always.
2. Otherwise default is ifnotpresent

Partial-Implements: blueprint image-pull-policy
Change-Id: If2715d7724fd5336fb25d380f125e1485ca302d0
This commit is contained in:
Pradeep Kumar Singh 2016-11-17 11:16:48 +00:00
parent 94d565ff0a
commit 2c6ad5a119
17 changed files with 296 additions and 55 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

17
zun/api/controllers/types.py
View File

@ -311,3 +311,20 @@ class ImageSize(object):
in both cases""")
raise exception.InvalidValue(message=message, value=value,
type=cls.type_name)
class EnumType(object):
type_name = 'EnumType'
@classmethod
def validate(cls, value, name=None, values=None):
if value is None:
return None
if value.lower() not in set(values):
message = _(
"%(name)s should be one of: %(values)s") % {
'name': name,
'values': ', '.join(map(six.text_type, values))}
raise exception.InvalidValue(message)
else:
return value.lower()

9
zun/api/controllers/v1/containers.py
View File

@ -120,6 +120,13 @@ class Container(base.APIBase):
'labels': {
'validate': types.Dict(types.String, types.String).validate,
},
'image_pull_policy': {
'validate': types.EnumType.validate,
'validate_args': {
'name': 'image_pull_policy',
'values': ['never', 'always', 'ifnotpresent']
}
}
}
def __init__(self, **kwargs):
@ -131,7 +138,7 @@ class Container(base.APIBase):
container.unset_fields_except([
'uuid', 'name', 'image', 'command', 'status', 'cpu', 'memory',
'environment', 'task_state', 'workdir', 'ports', 'hostname',
'labels'])
'labels', 'image_pull_policy', 'status_reason'])
container.links = [link.Link.make_link(
'self', url,

18
zun/common/utils.py
View File

@ -141,3 +141,21 @@ def check_container_id(function):
return function(*args, **kwargs)
return decorated_function
def get_image_pull_policy(image_pull_policy, image_tag):
if not image_pull_policy:
if image_tag == 'latest':
image_pull_policy = 'always'
else:
image_pull_policy = 'ifnotpresent'
return image_pull_policy
def should_pull_image(image_pull_policy, present):
if image_pull_policy == 'never':
return False
if image_pull_policy == 'always' or \
(image_pull_policy == 'ifnotpresent' and not present):
return True
return False

5
zun/compute/manager.py
View File

@ -69,8 +69,11 @@ class Manager(object):
container.task_state = fields.TaskState.IMAGE_PULLING
container.save()
repo, tag = utils.parse_image_name(container.image)
image_pull_policy = utils.get_image_pull_policy(
container.image_pull_policy, tag)
try:
image = image_driver.pull_image(context, repo, tag)
image = image_driver.pull_image(context, repo,
tag, image_pull_policy)
except exception.ImageNotFound as e:
LOG.error(six.text_type(e))
self._fail_container(container, six.text_type(e))

34
zun/db/sqlalchemy/alembic/versions/43e1088c3389_add_image_pull_policy_column.py Normal file
View File

@ -0,0 +1,34 @@
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.
"""add image_pull_policy column
Revision ID: 43e1088c3389
Revises: c5565cbaa3de
Create Date: 2016-11-17 09:26:22.756296
"""
# revision identifiers, used by Alembic.
revision = '43e1088c3389'
down_revision = 'c5565cbaa3de'
branch_labels = None
depends_on = None
from alembic import op
import sqlalchemy as sa
def upgrade():
op.add_column('container',
sa.Column('image_pull_policy', sa.Text(),
nullable=True))

1
zun/db/sqlalchemy/models.py
View File

@ -142,6 +142,7 @@ class Container(Base):
ports = Column(JSONEncodedList)
hostname = Column(String(255))
labels = Column(JSONEncodedDict)
image_pull_policy = Column(Text, nullable=True)
class Image(Base):

25
zun/image/docker/driver.py
View File

@ -21,6 +21,7 @@ from oslo_utils import excutils
from zun.common import exception
from zun.common.i18n import _
from zun.common import utils
from zun.container.docker import utils as docker_utils
from zun.image import driver
@ -32,6 +33,18 @@ class DockerDriver(driver.ContainerImageDriver):
def __init__(self):
super(DockerDriver, self).__init__()
def _search_image_on_host(self, repo, tag):
with docker_utils.docker_client() as docker:
image = repo + ":" + tag
LOG.debug('Inspecting image locally %s' % image)
try:
image_dict = docker.inspect_image(image)
if image_dict:
return {'image': repo, 'path': None}
except errors.NotFound:
LOG.debug('Image %s not found locally' % image)
return None
def _pull_image(self, repo, tag):
with docker_utils.docker_client() as docker:
for line in docker.pull(repo, tag=tag, stream=True):
@ -42,7 +55,17 @@ class DockerDriver(driver.ContainerImageDriver):
else:
raise exception.DockerError(error['message'])
def pull_image(self, context, repo, tag):
def pull_image(self, context, repo, tag, image_pull_policy):
image = self._search_image_on_host(repo, tag)
if not utils.should_pull_image(image_pull_policy, bool(image)):
if image:
LOG.debug('Image %s present locally' % repo)
return image
else:
message = _('Image %s not present with pull policy of Never'
) % repo
raise exception.ImageNotFound(message)
try:
LOG.debug('Pulling image from docker %s,'
' context %s' % (repo, context))

30
zun/image/driver.py
View File

@ -12,7 +12,6 @@
# License for the specific language governing permissions and limitations
# under the License.
import os
import sys
from oslo_log import log as logging
@ -23,7 +22,6 @@ from zun.common.i18n import _
from zun.common.i18n import _LE
from zun.common.i18n import _LI
import zun.conf
from zun.image.glance import utils
CONF = zun.conf.CONF
LOG = logging.getLogger(__name__)
@ -57,35 +55,13 @@ def load_image_driver(image_driver=None):
sys.exit(1)
def search_image_on_host(context, repo):
LOG.debug('Searching for image %s locally' % repo)
CONF.import_opt('images_directory', 'zun.image.glance.driver',
group='glance')
images_directory = CONF.glance.images_directory
try:
# TODO(mkrai): Change this to search image entry in zun db
# after the image endpoint is merged.
image_meta = utils.find_image(context, repo)
except exception.ImageNotFound:
return None
if image_meta:
out_path = os.path.join(images_directory, image_meta.id + '.tar')
if os.path.isfile(out_path):
return {'image': repo, 'path': out_path}
else:
return None
def pull_image(context, repo, tag):
image = search_image_on_host(context, repo)
if image:
LOG.debug('Found image %s locally.' % repo)
return image
def pull_image(context, repo, tag, image_pull_policy):
image_driver_list = CONF.image_driver_list
for driver in image_driver_list:
try:
image_driver = load_image_driver(driver)
image = image_driver.pull_image(context, repo, tag)
image = image_driver.pull_image(context, repo,
tag, image_pull_policy)
if image:
break
except exception.ImageNotFound:

30
zun/image/glance/driver.py
View File

@ -20,6 +20,7 @@ from oslo_utils import fileutils
from zun.common import exception
from zun.common.i18n import _
from zun.common import utils as common_utils
import zun.conf
from zun.image import driver
from zun.image.glance import utils
@ -34,9 +35,36 @@ class GlanceDriver(driver.ContainerImageDriver):
def __init__(self):
super(GlanceDriver, self).__init__()
def pull_image(self, context, repo, tag):
def _search_image_on_host(self, context, repo):
LOG.debug('Searching for image %s locally' % repo)
images_directory = CONF.glance.images_directory
try:
# TODO(mkrai): Change this to search image entry in zun db
# after the image endpoint is merged.
image_meta = utils.find_image(context, repo)
except exception.ImageNotFound:
return None
if image_meta:
out_path = os.path.join(images_directory,
image_meta.id + '.tar')
if os.path.isfile(out_path):
return {'image': repo, 'path': out_path}
else:
return None
def pull_image(self, context, repo, tag, image_pull_policy):
# TODO(shubhams): glance driver does not handle tags
# once metadata is stored in db then handle tags
image = self._search_image_on_host(context, repo)
if not common_utils.should_pull_image(image_pull_policy, bool(image)):
if image:
LOG.debug('Image %s present locally' % repo)
return image
else:
message = _('Image %s not present with pull policy of Never'
) % repo
raise exception.ImageNotFound(message)
LOG.debug('Pulling image from glance %s' % repo)
try:
glance = utils.create_glanceclient(context)

1
zun/objects/container.py
View File

@ -46,6 +46,7 @@ class Container(base.ZunPersistentObject, base.ZunObject,
'ports': z_fields.ListOfIntegersField(nullable=True),
'hostname': fields.StringField(nullable=True),
'labels': fields.DictOfStringsField(nullable=True),
'image_pull_policy': fields.StringField(nullable=True)
}
@staticmethod

23
zun/tests/unit/api/controllers/test_types.py
View File

@ -243,3 +243,26 @@ class TestTypes(test_base.BaseTestCase):
expected_value = 4096
value = types.ImageSize.validate(test_value)
self.assertEqual(value, expected_value)
def test_enum_type(self):
test_value = 'always'
self.assertEqual(test_value, types.EnumType.validate(
test_value, name='image_pull_policy',
values=['always', 'never', 'ifnotpresent']))
test_value = 'ALWAYS'
self.assertEqual('always', types.EnumType.validate(
test_value, name='image_pull_policy',
values=['always', 'never', 'ifnotpresent']))
test_value = None
self.assertEqual(None, types.EnumType.validate(
test_value, name='image_pull_policy',
values=['always', 'never', 'ifnotpresent']))
test_value = 'xyz'
self.assertRaises(exception.InvalidValue,
types.EnumType.validate,
test_value,
name='image_pull_policy',
values=['always', 'never', 'ifnotpresent'])

5
zun/tests/unit/api/controllers/v1/test_containers.py
View File

@ -226,8 +226,8 @@ class TestContainerController(api_base.FunctionalTest):
@patch('zun.compute.api.API.container_show')
@patch('zun.objects.Container.list')
def test_get_all_has_status_reason(self, mock_container_list,
mock_container_show):
def test_get_all_has_status_reason_and_image_pull_policy(
self, mock_container_list, mock_container_show):
test_container = utils.get_test_container()
containers = [objects.Container(self.context, **test_container)]
mock_container_list.return_value = containers
@ -240,6 +240,7 @@ class TestContainerController(api_base.FunctionalTest):
self.assertEqual(test_container['uuid'],
actual_containers[0].get('uuid'))
self.assertIn('status_reason', actual_containers[0].keys())
self.assertIn('image_pull_policy', actual_containers[0].keys())
@patch('zun.compute.api.API.container_show')
@patch('zun.objects.Container.list')

19
zun/tests/unit/common/test_utils.py
View File

@ -54,3 +54,22 @@ class TestUtils(base.BaseTestCase):
utils.parse_image_name('test:test'))
self.assertEqual(('test-test', 'test'),
utils.parse_image_name('test-test:test'))
def test_get_image_pull_policy(self):
self.assertEqual('always',
utils.get_image_pull_policy('always',
'latest'))
self.assertEqual('always',
utils.get_image_pull_policy(None,
'latest'))
self.assertEqual('ifnotpresent',
utils.get_image_pull_policy(None,
'2.0'))
def test_should_pull_image(self):
self.assertFalse(utils.should_pull_image('never', True))
self.assertFalse(utils.should_pull_image('never', False))
self.assertTrue(utils.should_pull_image('always', True))
self.assertTrue(utils.should_pull_image('always', False))
self.assertTrue(utils.should_pull_image('ifnotpresent', False))
self.assertFalse(utils.should_pull_image('ifnotpresent', True))

2
zun/tests/unit/compute/test_compute_manager.py
View File

@ -69,7 +69,7 @@ class TestManager(base.TestCase):
mock_save.assert_called_with()
mock_pull.assert_called_once_with(self.context,
container.image,
'latest')
'latest', 'always')
mock_create.assert_called_once_with(container, 'fake_path')
@mock.patch.object(Container, 'save')

1
zun/tests/unit/db/utils.py
View File

@ -41,6 +41,7 @@ def get_test_container(**kw):
'ports': kw.get('ports', [80, 443]),
'hostname': kw.get('hostname', 'testhost'),
'labels': kw.get('labels', {'key1': 'val1', 'key2': 'val2'}),
'image_pull_policy': kw.get('image_pull_policy', 'always'),
}

70
zun/tests/unit/image/docker/test_driver.py
View File

@ -34,8 +34,34 @@ class TestDriver(base.BaseTestCase):
self.dfc_context_manager.__enter__.return_value = self.mock_docker
self.addCleanup(dfc_patcher.stop)
def test_pull_image_success(self):
ret = self.driver.pull_image(None, 'test_image', 'latest')
@mock.patch.object(driver.DockerDriver,
'_search_image_on_host')
@mock.patch('zun.common.utils.should_pull_image')
def test_pull_image_should_pull_no_image_not_present_locally(
self, mock_should_pull_image, mock_search):
mock_should_pull_image.return_value = False
mock_search.return_value = None
self.assertRaises(exception.ImageNotFound, self.driver.pull_image,
None, 'nonexisting', 'tag', 'never')
@mock.patch.object(driver.DockerDriver,
'_search_image_on_host')
@mock.patch('zun.common.utils.should_pull_image')
def test_pull_image_should_pull_no_image_present_locally(
self, mock_should_pull_image, mock_search):
mock_should_pull_image.return_value = False
mock_search.return_value = {'image': 'nginx', 'path': 'xyz'}
self.assertEqual({'image': 'nginx', 'path': 'xyz'},
self.driver.pull_image(None, 'nonexisting',
'tag', 'never'))
@mock.patch.object(driver.DockerDriver,
'_search_image_on_host')
@mock.patch('zun.common.utils.should_pull_image')
def test_pull_image_success(self, mock_should_pull_image, mock_search):
mock_should_pull_image.return_value = True
mock_search.return_value = {'image': 'nginx', 'path': 'xyz'}
ret = self.driver.pull_image(None, 'test_image', 'latest', 'always')
self.assertEqual({'image': 'test_image', 'path': None}, ret)
self.mock_docker.pull.assert_called_once_with(
'test_image',
@ -43,14 +69,20 @@ class TestDriver(base.BaseTestCase):
stream=True)
@mock.patch('zun.common.utils.parse_image_name')
def test_pull_image_raises_API_error(self, mock_parse_image):
@mock.patch.object(driver.DockerDriver,
'_search_image_on_host')
@mock.patch('zun.common.utils.should_pull_image')
def test_pull_image_raises_API_error(self, mock_should_pull_image,
mock_search, mock_parse_image):
mock_should_pull_image.return_value = True
mock_search.return_value = {'image': 'nginx', 'path': 'xyz'}
mock_parse_image.return_value = ('repo', 'tag')
with mock.patch.object(errors.APIError, '__str__',
return_value='404 Not Found') as mock_init:
self.mock_docker.pull = mock.Mock(
side_effect=errors.APIError('Error', '', ''))
self.assertRaises(exception.ZunException, self.driver.pull_image,
None, 'repo', 'tag')
None, 'repo', 'tag', 'always')
self.mock_docker.pull.assert_called_once_with(
'repo',
tag='tag',
@ -58,7 +90,13 @@ class TestDriver(base.BaseTestCase):
self.assertEqual(1, mock_init.call_count)
@mock.patch('zun.common.utils.parse_image_name')
def test_pull_image_not_found(self, mock_parse_image):
@mock.patch.object(driver.DockerDriver,
'_search_image_on_host')
@mock.patch('zun.common.utils.should_pull_image')
def test_pull_image_not_found(self, mock_should_pull_image,
mock_search, mock_parse_image):
mock_should_pull_image.return_value = True
mock_search.return_value = {'image': 'nginx', 'path': 'xyz'}
mock_parse_image.return_value = ('repo', 'tag')
pull_return_value = '{"errorDetail":{"message":'\
'"Error: image library/repo not found"},'\
@ -67,7 +105,7 @@ class TestDriver(base.BaseTestCase):
with mock.patch.object(self.mock_docker, 'pull',
return_value=[pull_return_value]) as mock_init:
self.assertRaises(exception.ImageNotFound, self.driver.pull_image,
None, 'repo', 'tag')
None, 'repo', 'tag', 'always')
self.mock_docker.pull.assert_called_once_with(
'repo',
tag='tag',
@ -75,7 +113,13 @@ class TestDriver(base.BaseTestCase):
self.assertEqual(1, mock_init.call_count)
@mock.patch('zun.common.utils.parse_image_name')
def test_pull_image_raises_docker_error(self, mock_parse_image):
@mock.patch.object(driver.DockerDriver,
'_search_image_on_host')
@mock.patch('zun.common.utils.should_pull_image')
def test_pull_image_raises_docker_error(self, mock_should_pull_image,
mock_search, mock_parse_image):
mock_should_pull_image.return_value = True
mock_search.return_value = {'image': 'nginx', 'path': 'xyz'}
mock_parse_image.return_value = ('repo', 'tag')
pull_return_value = '{"errorDetail":{"message":'\
'"Error: image library/repo not"},'\
@ -84,7 +128,7 @@ class TestDriver(base.BaseTestCase):
with mock.patch.object(self.mock_docker, 'pull',
return_value=[pull_return_value]) as mock_init:
self.assertRaises(exception.DockerError, self.driver.pull_image,
None, 'repo', 'tag')
None, 'repo', 'tag', 'always')
self.mock_docker.pull.assert_called_once_with(
'repo',
tag='tag',
@ -92,14 +136,20 @@ class TestDriver(base.BaseTestCase):
self.assertEqual(1, mock_init.call_count)
@mock.patch('zun.common.utils.parse_image_name')
def test_pull_image_exception(self, mock_parse_image):
@mock.patch.object(driver.DockerDriver,
'_search_image_on_host')
@mock.patch('zun.common.utils.should_pull_image')
def test_pull_image_exception(self, mock_should_pull_image,
mock_search, mock_parse_image):
mock_should_pull_image.return_value = True
mock_search.return_value = {'image': 'nginx', 'path': 'xyz'}
mock_parse_image.return_value = ('repo', 'tag')
with mock.patch.object(errors.APIError, '__str__',
return_value='hit error') as mock_init:
self.mock_docker.pull = mock.Mock(
side_effect=errors.APIError('Error', '', ''))
self.assertRaises(exception.ZunException, self.driver.pull_image,
None, 'repo', 'tag')
None, 'repo', 'tag', 'always')
self.mock_docker.pull.assert_called_once_with(
'repo',
tag='tag',

61
zun/tests/unit/image/glance/test_driver.py
View File

@ -36,28 +36,67 @@ class TestDriver(base.BaseTestCase):
super(TestDriver, self).tearDown()
shutil.rmtree(self.test_dir)
@mock.patch.object(driver.GlanceDriver,
'_search_image_on_host')
@mock.patch('zun.common.utils.should_pull_image')
def test_pull_image_should_pull_no_image_not_present_locally(
self, mock_should_pull_image, mock_search):
mock_should_pull_image.return_value = False
mock_search.return_value = None
self.assertRaises(exception.ImageNotFound, self.driver.pull_image,
None, 'nonexisting', 'tag', 'never')
@mock.patch.object(driver.GlanceDriver,
'_search_image_on_host')
@mock.patch('zun.common.utils.should_pull_image')
def test_pull_image_should_pull_no_image_present_locally(
self, mock_should_pull_image, mock_search):
mock_should_pull_image.return_value = False
mock_search.return_value = {'image': 'nginx', 'path': 'xyz'}
self.assertEqual({'image': 'nginx', 'path': 'xyz'},
self.driver.pull_image(None, 'nonexisting',
'tag', 'never'))
@mock.patch('zun.image.glance.utils.create_glanceclient')
def test_pull_image_failure(self, mock_glance):
@mock.patch.object(driver.GlanceDriver,
'_search_image_on_host')
@mock.patch('zun.common.utils.should_pull_image')
def test_pull_image_failure(self, mock_should_pull_image,
mock_search, mock_glance):
mock_should_pull_image.return_value = True
mock_search.return_value = {'image': 'nginx', 'path': 'xyz'}
mock_glance.side_effect = Exception
self.assertRaises(exception.ZunException, self.driver.pull_image,
None, 'nonexisting', 'tag')
@mock.patch('zun.image.glance.utils.create_glanceclient')
def test_pull_image_not_found(self, mock_glance):
with mock.patch('zun.image.glance.utils.find_image') as mock_find:
mock_find.side_effect = exception.ImageNotFound
self.assertRaises(exception.ImageNotFound, self.driver.pull_image,
None, 'nonexisting', 'tag')
None, 'nonexisting', 'tag', 'always')
@mock.patch.object(driver.GlanceDriver,
'_search_image_on_host')
@mock.patch('zun.common.utils.should_pull_image')
@mock.patch('zun.image.glance.utils.create_glanceclient')
@mock.patch('zun.image.glance.utils.find_image')
def test_pull_image_found(self, mock_find_image, mock_glance):
def test_pull_image(self, mock_find_image, mock_glance,
mock_should_pull_image, mock_search):
mock_should_pull_image.return_value = True
mock_search.return_value = {'image': 'nginx', 'path': 'xyz'}
mock_glance.images.data = mock.MagicMock(return_value='content')
image_meta = mock.MagicMock()
image_meta.id = '1234'
mock_find_image.return_value = image_meta
CONF.set_override('images_directory', self.test_dir, group='glance')
out_path = os.path.join(self.test_dir, '1234' + '.tar')
ret = self.driver.pull_image(None, 'image', 'latest')
ret = self.driver.pull_image(None, 'image', 'latest', 'always')
self.assertEqual({'image': 'image', 'path': out_path}, ret)
self.assertTrue(os.path.isfile(ret['path']))
@mock.patch('zun.image.glance.utils.create_glanceclient')
@mock.patch.object(driver.GlanceDriver,
'_search_image_on_host')
@mock.patch('zun.common.utils.should_pull_image')
def test_pull_image_not_found(self, mock_should_pull_image,
mock_search, mock_glance):
mock_should_pull_image.return_value = True
mock_search.return_value = {'image': 'nginx', 'path': 'xyz'}
with mock.patch('zun.image.glance.utils.find_image') as mock_find:
mock_find.side_effect = exception.ImageNotFound
self.assertRaises(exception.ImageNotFound, self.driver.pull_image,
None, 'nonexisting', 'tag', 'always')