openstack/kolla
Code Issues Proposed changes

Transition to DockerClient

Browse Source 
Change to use docker.DockerClient instead of the low level
docker.APIClient.

Signed-off-by: Konstantin Yarovoy <konstantin.yarovoy@tietoevry.com>
Change-Id: I88842c17d8e6ddbe89b984c7b5e0367ed356fa50
This commit is contained in:
Michal Nasiadka 2023-05-17 13:05:35 +00:00
parent 4c31f7a3f2
commit c9af85c584
5 changed files with 90 additions and 81 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
kolla/engine_adapter/engine.py
View File

@ -10,7 +10,6 @@
# See the License for the specific language governing permissions and
# limitations under the License.
from distutils.version import StrictVersion
from enum import Enum
from kolla.image.utils import LOG
@ -45,13 +44,6 @@ def getEngineException(conf):
def getEngineClient(conf):
if conf.engine == Engine.DOCKER.value:
kwargs_env = docker.utils.kwargs_from_env()
return docker.APIClient(version='auto', **kwargs_env)
else:
raise UnsupportedEngineError(conf.engine)
def getEngineVersion(conf):
if conf.engine == Engine.DOCKER.value:
return StrictVersion(docker.__version__)
return docker.DockerClient(version='auto', **kwargs_env)
else:
raise UnsupportedEngineError(conf.engine)

4
kolla/image/kolla_worker.py
View File

@ -73,8 +73,8 @@ class Image(object):
return c
def in_engine_cache(self):
return len(self.engine_client.images(name=self.canonical_name,
quiet=True)) == 1
return len(self.engine_client.images.list(
name=self.canonical_name)) >= 1
def __repr__(self):
return ("Image(%s, %s, %s, parent_name=%s,"

42
kolla/image/tasks.py
View File

@ -108,8 +108,8 @@ class PushTask(EngineTask):
def push_image(self, image):
kwargs = dict(stream=True, decode=True)
for response in self.engine_client.push(
image.canonical_name, **kwargs):
for response in self.engine_client.images.push(image.canonical_name,
**kwargs):
if 'stream' in response:
self.logger.info(response['stream'])
elif 'errorDetail' in response:
@ -367,16 +367,15 @@ class BuildTask(EngineTask):
buildargs = self.update_buildargs()
try:
for stream in \
self.engine_client.build(path=image.path,
tag=image.canonical_name,
nocache=not self.conf.cache,
rm=True,
decode=True,
network_mode=self.conf.network_mode,
pull=pull,
forcerm=self.forcerm,
buildargs=buildargs):
for stream in self.engine_client.images.build(
path=image.path,
tag=image.canonical_name,
nocache=not self.conf.cache,
rm=True,
network_mode=self.conf.network_mode,
pull=pull,
forcerm=self.forcerm,
buildargs=buildargs)[1]:
if 'stream' in stream:
for line in stream['stream'].split('\n'):
if line:
@ -407,12 +406,23 @@ class BuildTask(EngineTask):
def squash(self):
image_tag = self.image.canonical_name
image_id = self.engine_client.inspect_image(image_tag)['Id']
image_id = self.engine_client.images.get(image_tag).id
parent_history = self.engine_client.history(self.image.parent_name)
parent_last_layer = parent_history[0]['Id']
self.logger.info('Parent lastest layer is: %s' % parent_last_layer)
parent = self.engine_client.images.get(self.image.parent_name)
parent_history = parent.history()
parent_last_layer = ""
for layer in parent_history:
if layer["Tags"] is not None and \
self.image.parent_name in layer["Tags"]:
parent_last_layer = layer["Id"]
break
if not parent_last_layer:
self.logger.error('Parent last layer is not found, '
'cannot squash.')
self.image.status = Status.ERROR
return
self.logger.info('Parent last layer is: %s' % parent_last_layer)
utils.squash(image_id, image_tag, from_layer=parent_last_layer,
cleanup=self.conf.squash_cleanup,
tmp_dir=self.conf.squash_tmp_dir)

110
kolla/tests/test_build.py
View File

@ -65,37 +65,37 @@ class TasksTest(base.TestCase):
self.imageChild.path = self.useFixture(fixtures.TempDir()).path
@mock.patch.dict(os.environ, clear=True)
@mock.patch('docker.APIClient')
@mock.patch('docker.DockerClient')
def test_push_image(self, mock_client):
self.dc = mock_client
self.engine_client = mock_client
pusher = tasks.PushTask(self.conf, self.image)
pusher.run()
mock_client().push.assert_called_once_with(
mock_client().images.push.assert_called_once_with(
self.image.canonical_name, decode=True, stream=True)
self.assertTrue(pusher.success)
@mock.patch.dict(os.environ, clear=True)
@mock.patch('docker.APIClient')
@mock.patch('docker.DockerClient')
def test_push_image_failure(self, mock_client):
"""failure on connecting Docker API"""
self.dc = mock_client
mock_client().push.side_effect = Exception
self.engine_client = mock_client
mock_client().images.push.side_effect = Exception
pusher = tasks.PushTask(self.conf, self.image)
pusher.run()
mock_client().push.assert_called_once_with(
mock_client().images.push.assert_called_once_with(
self.image.canonical_name, decode=True, stream=True)
self.assertFalse(pusher.success)
self.assertEqual(utils.Status.PUSH_ERROR, self.image.status)
@mock.patch.dict(os.environ, clear=True)
@mock.patch('docker.APIClient')
@mock.patch('docker.DockerClient')
def test_push_image_failure_retry(self, mock_client):
"""failure on connecting Docker API, success on retry"""
self.dc = mock_client
mock_client().push.side_effect = [Exception, []]
self.engine_client = mock_client
mock_client().images.push.side_effect = [Exception, []]
pusher = tasks.PushTask(self.conf, self.image)
pusher.run()
mock_client().push.assert_called_once_with(
mock_client().images.push.assert_called_once_with(
self.image.canonical_name, decode=True, stream=True)
self.assertFalse(pusher.success)
self.assertEqual(utils.Status.PUSH_ERROR, self.image.status)
@ -103,82 +103,83 @@ class TasksTest(base.TestCase):
# Try again, this time without exception.
pusher.reset()
pusher.run()
self.assertEqual(2, mock_client().push.call_count)
self.assertEqual(2, mock_client().images.push.call_count)
self.assertTrue(pusher.success)
self.assertEqual(utils.Status.BUILT, self.image.status)
@mock.patch.dict(os.environ, clear=True)
@mock.patch('docker.APIClient')
@mock.patch('docker.DockerClient')
def test_push_image_failure_error(self, mock_client):
"""Docker connected, failure to push"""
self.dc = mock_client
mock_client().push.return_value = [{'errorDetail': {'message':
'mock push fail'}}]
self.engine_client = mock_client
mock_client().images.push.return_value = [{'errorDetail': {'message':
'mock push fail'}}]
pusher = tasks.PushTask(self.conf, self.image)
pusher.run()
mock_client().push.assert_called_once_with(
mock_client().images.push.assert_called_once_with(
self.image.canonical_name, decode=True, stream=True)
self.assertFalse(pusher.success)
self.assertEqual(utils.Status.PUSH_ERROR, self.image.status)
@mock.patch.dict(os.environ, clear=True)
@mock.patch('docker.APIClient')
@mock.patch('docker.DockerClient')
def test_push_image_failure_error_retry(self, mock_client):
"""Docker connected, failure to push, success on retry"""
self.dc = mock_client
mock_client().push.return_value = [{'errorDetail': {'message':
'mock push fail'}}]
self.engine_client = mock_client
mock_client().images.push.return_value = [{'errorDetail': {'message':
'mock push fail'}}]
pusher = tasks.PushTask(self.conf, self.image)
pusher.run()
mock_client().push.assert_called_once_with(
mock_client().images.push.assert_called_once_with(
self.image.canonical_name, decode=True, stream=True)
self.assertFalse(pusher.success)
self.assertEqual(utils.Status.PUSH_ERROR, self.image.status)
# Try again, this time without exception.
mock_client().push.return_value = [{'stream': 'mock push passes'}]
mock_client().images.push.return_value = [{'stream':
'mock push passes'}]
pusher.reset()
pusher.run()
self.assertEqual(2, mock_client().push.call_count)
self.assertEqual(2, mock_client().images.push.call_count)
self.assertTrue(pusher.success)
self.assertEqual(utils.Status.BUILT, self.image.status)
@mock.patch.dict(os.environ, clear=True)
@mock.patch('docker.APIClient')
@mock.patch('docker.DockerClient')
def test_build_image(self, mock_client):
self.dc = mock_client
self.engine_client = mock_client
push_queue = mock.Mock()
builder = tasks.BuildTask(self.conf, self.image, push_queue)
builder.run()
mock_client().build.assert_called_once_with(
path=self.image.path, tag=self.image.canonical_name, decode=True,
mock_client().images.build.assert_called_once_with(
path=self.image.path, tag=self.image.canonical_name,
network_mode='host', nocache=False, rm=True, pull=True,
forcerm=True, buildargs=None)
self.assertTrue(builder.success)
@mock.patch.dict(os.environ, clear=True)
@mock.patch('docker.APIClient')
@mock.patch('docker.DockerClient')
def test_build_image_with_network_mode(self, mock_client):
self.dc = mock_client
self.engine_client = mock_client
push_queue = mock.Mock()
self.conf.set_override('network_mode', 'bridge')
builder = tasks.BuildTask(self.conf, self.image, push_queue)
builder.run()
mock_client().build.assert_called_once_with(
path=self.image.path, tag=self.image.canonical_name, decode=True,
mock_client().images.build.assert_called_once_with(
path=self.image.path, tag=self.image.canonical_name,
network_mode='bridge', nocache=False, rm=True, pull=True,
forcerm=True, buildargs=None)
self.assertTrue(builder.success)
@mock.patch.dict(os.environ, clear=True)
@mock.patch('docker.APIClient')
@mock.patch('docker.DockerClient')
def test_build_image_with_build_arg(self, mock_client):
self.dc = mock_client
self.engine_client = mock_client
build_args = {
'HTTP_PROXY': 'http://localhost:8080',
'NO_PROXY': '127.0.0.1'
@ -188,8 +189,8 @@ class TasksTest(base.TestCase):
builder = tasks.BuildTask(self.conf, self.image, push_queue)
builder.run()
mock_client().build.assert_called_once_with(
path=self.image.path, tag=self.image.canonical_name, decode=True,
mock_client().images.build.assert_called_once_with(
path=self.image.path, tag=self.image.canonical_name,
network_mode='host', nocache=False, rm=True, pull=True,
forcerm=True, buildargs=build_args)
@ -197,18 +198,18 @@ class TasksTest(base.TestCase):
@mock.patch.dict(os.environ, {'http_proxy': 'http://FROM_ENV:8080'},
clear=True)
@mock.patch('docker.APIClient')
@mock.patch('docker.DockerClient')
def test_build_arg_from_env(self, mock_client):
push_queue = mock.Mock()
self.dc = mock_client
self.engine_client = mock_client
build_args = {
'http_proxy': 'http://FROM_ENV:8080',
}
builder = tasks.BuildTask(self.conf, self.image, push_queue)
builder.run()
mock_client().build.assert_called_once_with(
path=self.image.path, tag=self.image.canonical_name, decode=True,
mock_client().images.build.assert_called_once_with(
path=self.image.path, tag=self.image.canonical_name,
network_mode='host', nocache=False, rm=True, pull=True,
forcerm=True, buildargs=build_args)
@ -216,9 +217,9 @@ class TasksTest(base.TestCase):
@mock.patch.dict(os.environ, {'http_proxy': 'http://FROM_ENV:8080'},
clear=True)
@mock.patch('docker.APIClient')
@mock.patch('docker.DockerClient')
def test_build_arg_precedence(self, mock_client):
self.dc = mock_client
self.engine_client = mock_client
build_args = {
'http_proxy': 'http://localhost:8080',
}
@ -228,17 +229,17 @@ class TasksTest(base.TestCase):
builder = tasks.BuildTask(self.conf, self.image, push_queue)
builder.run()
mock_client().build.assert_called_once_with(
path=self.image.path, tag=self.image.canonical_name, decode=True,
mock_client().images.build.assert_called_once_with(
path=self.image.path, tag=self.image.canonical_name,
network_mode='host', nocache=False, rm=True, pull=True,
forcerm=True, buildargs=build_args)
self.assertTrue(builder.success)
@mock.patch('docker.APIClient')
@mock.patch('docker.DockerClient')
@mock.patch('requests.get')
def test_requests_get_timeout(self, mock_get, mock_client):
self.dc = mock_client
self.engine_client = mock_client
self.image.source = {
'source': 'http://fake/source',
'type': 'url',
@ -260,7 +261,7 @@ class TasksTest(base.TestCase):
@mock.patch('os.utime')
@mock.patch('shutil.copyfile')
@mock.patch('shutil.rmtree')
@mock.patch('docker.APIClient')
@mock.patch('docker.DockerClient')
@mock.patch('requests.get')
def test_process_source(self, mock_get, mock_client,
mock_rmtree, mock_copyfile, mock_utime):
@ -292,7 +293,7 @@ class TasksTest(base.TestCase):
self.assertIsNotNone(get_result)
@mock.patch.dict(os.environ, clear=True)
@mock.patch('docker.APIClient')
@mock.patch('docker.DockerClient')
def test_local_directory(self, mock_client):
tmpdir = tempfile.mkdtemp()
file_name = 'test.txt'
@ -313,7 +314,6 @@ class TasksTest(base.TestCase):
push_queue = mock.Mock()
builder = tasks.BuildTask(self.conf, self.image, push_queue)
builder.run()
self.assertTrue(builder.success)
except IOError:
print('IOError')
@ -323,8 +323,10 @@ class TasksTest(base.TestCase):
os.umask(saved_umask)
os.rmdir(tmpdir)
self.assertTrue(builder.success)
@mock.patch.dict(os.environ, clear=True)
@mock.patch('docker.APIClient')
@mock.patch('docker.DockerClient')
def test_malicious_tar(self, mock_client):
tmpdir = tempfile.mkdtemp()
file_name = 'test.txt'
@ -364,7 +366,7 @@ class TasksTest(base.TestCase):
os.rmdir(tmpdir)
@mock.patch.dict(os.environ, clear=True)
@mock.patch('docker.APIClient')
@mock.patch('docker.DockerClient')
def test_malicious_tar_gz(self, mock_client):
tmpdir = tempfile.mkdtemp()
file_name = 'test.txt'
@ -426,7 +428,7 @@ class TasksTest(base.TestCase):
self.assertFalse(builder.success)
self.assertIsNone(get_result)
@mock.patch('docker.APIClient')
@mock.patch('docker.DockerClient')
def test_followups_docker_image(self, mock_client):
self.imageChild.source = {
'source': 'http://fake/source',
@ -461,7 +463,7 @@ class KollaWorkerTest(base.TestCase):
self.images = [image, image_child, image_unmatched,
image_error, image_built]
patcher = mock.patch('docker.APIClient')
patcher = mock.patch('docker.DockerClient')
self.addCleanup(patcher.stop)
self.mock_client = patcher.start()
@ -765,7 +767,7 @@ class MainTest(base.TestCase):
self.assertEqual(1, result)
@mock.patch('sys.argv')
@mock.patch('docker.APIClient')
@mock.patch('docker.DockerClient')
def test_run_build(self, mock_client, mock_sys):
result = build.run_build()
self.assertTrue(result)

5
releasenotes/notes/docker-3-0-0-e5f43f9b78ad644d.yaml Normal file
View File

@ -0,0 +1,5 @@
---
features:
- |
Implements transition from docker.APIClient low level client to
docker.DockerClient.