diff options
Diffstat (limited to 'kolla')
| -rw-r--r-- | kolla/common/config.py | 8 | ||||
| -rw-r--r-- | kolla/common/utils.py | 15 | ||||
| -rw-r--r-- | kolla/image/build.py | 10 | ||||
| -rw-r--r-- | kolla/image/tasks.py | 94 | ||||
| -rw-r--r-- | kolla/tests/test_build.py | 302 |
5 files changed, 428 insertions, 1 deletions
diff --git a/kolla/common/config.py b/kolla/common/config.py index c4ebcbf3b..b418552f4 100644 --- a/kolla/common/config.py +++ b/kolla/common/config.py @@ -171,6 +171,14 @@ _CLI_OPTS = [ help='The Docker namespace name'), cfg.StrOpt('network_mode', default='host', help='The network mode for Docker build. Example: host'), + cfg.BoolOpt('buildkit', default=True, + help='Use Docker BuildKit (docker buildx build) when building ' + 'images. Requires the docker-buildx-plugin to be ' + 'installed. Only valid with --engine docker.'), + cfg.StrOpt('buildkit-builder', default=None, + help='Name of the docker buildx builder instance to use. ' + 'If unset the currently active builder is used. ' + 'Only valid with --buildkit.'), cfg.BoolOpt('cache', default=True, help='Use the container engine cache when building'), cfg.StrOpt('patches-path', default=None, diff --git a/kolla/common/utils.py b/kolla/common/utils.py index 4ee80768f..485848ba2 100644 --- a/kolla/common/utils.py +++ b/kolla/common/utils.py @@ -54,6 +54,21 @@ def make_a_logger(conf=None, image_name=None): LOG = make_a_logger() +def check_docker_buildx(): + try: + subprocess.check_output( # nosec + ['docker', 'buildx', 'version'], stderr=subprocess.STDOUT) + except OSError as ex: + if ex.errno == 2: + LOG.error('"docker" command is not found.') + raise + except subprocess.CalledProcessError: + LOG.error('"docker buildx" is not available. ' + 'Install the docker-buildx-plugin or disable BuildKit ' + 'with "--nobuildkit".') + raise + + def get_docker_squash_version(): try: diff --git a/kolla/image/build.py b/kolla/image/build.py index 9b6571d36..a0513b22a 100644 --- a/kolla/image/build.py +++ b/kolla/image/build.py @@ -120,6 +120,10 @@ def run_build(): "Try running 'pip install docker'\n" "Python error: %s", e) sys.exit(1) + if conf.buildkit and conf.squash: + LOG.error('--buildkit and --squash are mutually exclusive: ' + 'docker buildx build does not support squashing.') + sys.exit(1) if conf.squash: squash_version = utils.get_docker_squash_version() LOG.info('Image squash is enabled and "docker-squash" version ' @@ -171,6 +175,12 @@ def run_build(): kolla.list_dependencies() return + if (conf.engine == engine.Engine.DOCKER.value and conf.buildkit): + try: + utils.check_docker_buildx() + except Exception: + sys.exit(1) + push_queue = queue.Queue() build_queue = kolla.build_queue(push_queue) workers = [] diff --git a/kolla/image/tasks.py b/kolla/image/tasks.py index 6bee905f7..757197ef4 100644 --- a/kolla/image/tasks.py +++ b/kolla/image/tasks.py @@ -16,6 +16,7 @@ import hashlib import json import os import shutil +import subprocess # nosec import tarfile try: @@ -156,6 +157,11 @@ class BuildTask(EngineTask): def name(self): return 'BuildTask(%s)' % self.image.name + @property + def _buildkit_active(self): + return (self.conf.engine == engine.Engine.DOCKER.value + and self.conf.buildkit) + def run(self): self.builder(self.image) if self.image.status in (Status.BUILT, Status.SKIPPED): @@ -164,7 +170,7 @@ class BuildTask(EngineTask): @property def followups(self): followups = [] - if self.conf.push and self.success: + if self.conf.push and self.success and not self._buildkit_active: followups.extend([ # If we are supposed to push the image into a # container image repository, @@ -406,6 +412,10 @@ class BuildTask(EngineTask): buildargs = self.update_buildargs() + if self._buildkit_active: + self._build_buildkit(image, pull, buildargs) + return + kwargs = {} if hasattr(image, 'labels') and image.labels: kwargs['labels'] = image.labels @@ -478,6 +488,88 @@ class BuildTask(EngineTask): self.logger.info('Built at %s (took %s)' % (now, now - image.start)) + def _build_buildkit(self, image, pull, buildargs): + platform = self.conf.platform or '' + # Multi-platform (comma-separated): buildx pushes a manifest list + # directly to the registry. --load is not supported for multi-platform + # output, so --push is required. + # + # Single platform: load into the local daemon so child FROM lines + # resolve without a registry round-trip, then push the canonical tag. + # + # No platform: use --push or --load based on conf.push. + multi_platform = ',' in platform + single_platform = bool(platform and not multi_platform) + + if multi_platform and not self.conf.push: + self.logger.error( + 'Multi-platform buildkit build requires --push: ' + 'docker buildx does not support --load for multi-platform ' + 'output.') + image.status = Status.ERROR + return + + cmd = ['docker', 'buildx', 'build', '--progress=plain'] + if self.conf.buildkit_builder: + cmd.extend(['--builder', self.conf.buildkit_builder]) + if pull: + cmd.append('--pull') + if not self.conf.cache: + cmd.append('--no-cache') + if self.conf.network_mode: + cmd.extend(['--network', self.conf.network_mode]) + if platform: + cmd.extend(['--platform', platform]) + + cmd.extend(['-t', image.canonical_name]) + + if self.conf.push and not single_platform: + # Multi-platform or no-platform with push: let buildx push directly + cmd.append('--push') + else: + # Load into the local daemon so child image FROM lines resolve + # locally. For single-platform builds with --push, the canonical + # tag is pushed explicitly after the build. + cmd.append('--load') + + if buildargs: + for k, v in buildargs.items(): + cmd.extend(['--build-arg', '%s=%s' % (k, v)]) + if hasattr(image, 'labels') and image.labels: + for k, v in image.labels.items(): + cmd.extend(['--label', '%s=%s' % (k, v)]) + cmd.append(image.path) + + try: + self._run_cmd(cmd) + if single_platform and self.conf.push: + self._run_cmd(['docker', 'push', image.canonical_name]) + except Exception: + image.status = Status.ERROR + self.logger.exception('Unknown error when building') + else: + image.status = Status.BUILT + now = datetime.datetime.now() + self.logger.info('Built at %s (took %s)' % + (now, now - image.start)) + + def _run_cmd(self, cmd): + env = os.environ.copy() + env['NO_COLOR'] = '1' + with subprocess.Popen( # nosec + cmd, + stdout=subprocess.PIPE, + stderr=subprocess.STDOUT, + universal_newlines=True, + bufsize=1, + env=env, + ) as proc: + for line in proc.stdout: + self.logger.info(line.rstrip()) + proc.wait() + if proc.returncode != 0: + raise subprocess.CalledProcessError(proc.returncode, cmd) + def squash(self): image_tag = self.image.canonical_name image_id = self.engine_client.images.get(image_tag).id diff --git a/kolla/tests/test_build.py b/kolla/tests/test_build.py index 82bcb7d6d..e072860ab 100644 --- a/kolla/tests/test_build.py +++ b/kolla/tests/test_build.py @@ -10,6 +10,7 @@ # See the License for the specific language governing permissions and # limitations under the License. +import datetime import fixtures import jinja2 import os @@ -20,6 +21,7 @@ import tempfile from unittest import mock from kolla.cmd import build as build_cmd +from kolla.common import utils as common_utils from kolla.image import build from kolla.image.kolla_worker import Image from kolla.image import tasks @@ -77,6 +79,9 @@ class TasksTest(base.TestCase): self.build_kwargs["squash"] = False else: self.build_kwargs = {} + # Existing tests exercise the docker-py SDK path; disable BuildKit so + # they are not routed through _build_buildkit. + self.conf.set_override('buildkit', False) @mock.patch.dict(os.environ, clear=True) @mock.patch(engine_client) @@ -479,6 +484,253 @@ class TasksTest(base.TestCase): get_result = builder.followups self.assertEqual(1, len(get_result)) + @mock.patch(engine_client) + def test_followups_buildkit_skips_push(self, mock_client): + """PushTask must not be added when buildkit is active (docker).""" + self.conf.set_override('buildkit', True) + push_queue = mock.Mock() + builder = tasks.BuildTask(self.conf, self.imageChild, push_queue) + builder.success = True + self.conf.push = True + followups = builder.followups + push_tasks = [f for f in followups if + isinstance(f, tasks.PushIntoQueueTask)] + self.assertEqual(0, len(push_tasks)) + + @mock.patch(engine_client) + def test_followups_buildkit_true_podman_includes_push(self, mock_client): + """PushTask must be added for Podman even when buildkit=True.""" + self.conf.set_override('buildkit', True) + self.conf.set_override('engine', 'podman') + push_queue = mock.Mock() + builder = tasks.BuildTask(self.conf, self.imageChild, push_queue) + builder.success = True + self.conf.push = True + followups = builder.followups + push_tasks = [f for f in followups if + isinstance(f, tasks.PushIntoQueueTask)] + self.assertEqual(1, len(push_tasks)) + + @mock.patch.object(tasks.BuildTask, '_run_cmd') + def test_build_buildkit_no_platform_no_push(self, mock_run_cmd): + """No --platform and no push: command uses --load.""" + self.conf.set_override('buildkit', True) + push_queue = mock.Mock() + builder = tasks.BuildTask(self.conf, self.image, push_queue) + self.image.start = datetime.datetime.now() + builder._build_buildkit(self.image, pull=True, buildargs=None) + + mock_run_cmd.assert_called_once() + cmd = mock_run_cmd.call_args[0][0] + self.assertIn('--load', cmd) + self.assertNotIn('--push', cmd) + self.assertNotIn('--platform', cmd) + self.assertEqual(utils.Status.BUILT, self.image.status) + + @mock.patch.object(tasks.BuildTask, '_run_cmd') + def test_build_buildkit_no_platform_with_push(self, mock_run_cmd): + """No platform + push=True: buildx pushes directly.""" + self.conf.set_override('buildkit', True) + self.conf.set_override('push', True) + push_queue = mock.Mock() + builder = tasks.BuildTask(self.conf, self.image, push_queue) + self.image.start = datetime.datetime.now() + builder._build_buildkit(self.image, pull=False, buildargs=None) + + mock_run_cmd.assert_called_once() + cmd = mock_run_cmd.call_args[0][0] + self.assertIn('--push', cmd) + self.assertNotIn('--load', cmd) + self.assertEqual(utils.Status.BUILT, self.image.status) + + @mock.patch.object(tasks.BuildTask, '_run_cmd') + def test_build_buildkit_single_platform_no_push(self, mock_run_cmd): + """Single platform without push: --load, no push_tag, no extra push.""" + self.conf.set_override('buildkit', True) + self.conf.set_override('platform', 'linux/amd64') + push_queue = mock.Mock() + builder = tasks.BuildTask(self.conf, self.image, push_queue) + self.image.start = datetime.datetime.now() + builder._build_buildkit(self.image, pull=False, buildargs=None) + + mock_run_cmd.assert_called_once() + cmd = mock_run_cmd.call_args[0][0] + self.assertIn('--platform', cmd) + self.assertIn('linux/amd64', cmd) + self.assertIn('--load', cmd) + self.assertNotIn('--push', cmd) + self.assertEqual(utils.Status.BUILT, self.image.status) + + @mock.patch.object(tasks.BuildTask, '_run_cmd') + def test_build_buildkit_single_platform_with_push(self, mock_run_cmd): + """Single platform + push: --load then push canonical tag.""" + self.conf.set_override('buildkit', True) + self.conf.set_override('platform', 'linux/amd64') + self.conf.set_override('push', True) + push_queue = mock.Mock() + builder = tasks.BuildTask(self.conf, self.image, push_queue) + self.image.start = datetime.datetime.now() + builder._build_buildkit(self.image, pull=False, buildargs=None) + + self.assertEqual(2, mock_run_cmd.call_count) + build_cmd = mock_run_cmd.call_args_list[0][0][0] + push_cmd = mock_run_cmd.call_args_list[1][0][0] + + self.assertIn('--load', build_cmd) + self.assertNotIn('--push', build_cmd) + self.assertNotIn('image-base:latest-amd64', build_cmd) + self.assertEqual( + ['docker', 'push', self.image.canonical_name], push_cmd) + self.assertEqual(utils.Status.BUILT, self.image.status) + + @mock.patch.object(tasks.BuildTask, '_run_cmd') + def test_build_buildkit_multi_platform_with_push(self, mock_run_cmd): + """Multi-platform + push: single --push invocation, no separate push""" + self.conf.set_override('buildkit', True) + self.conf.set_override('platform', 'linux/amd64,linux/arm64') + self.conf.set_override('push', True) + push_queue = mock.Mock() + builder = tasks.BuildTask(self.conf, self.image, push_queue) + self.image.start = datetime.datetime.now() + builder._build_buildkit(self.image, pull=False, buildargs=None) + + mock_run_cmd.assert_called_once() + cmd = mock_run_cmd.call_args[0][0] + self.assertIn('--push', cmd) + self.assertNotIn('--load', cmd) + self.assertIn('linux/amd64,linux/arm64', cmd) + self.assertEqual(utils.Status.BUILT, self.image.status) + + @mock.patch.object(tasks.BuildTask, '_run_cmd') + def test_build_buildkit_multi_platform_no_push(self, mock_run_cmd): + """Multi-platform without push: fails fast (--load unsupported).""" + self.conf.set_override('buildkit', True) + self.conf.set_override('platform', 'linux/amd64,linux/arm64') + push_queue = mock.Mock() + builder = tasks.BuildTask(self.conf, self.image, push_queue) + self.image.start = datetime.datetime.now() + builder._build_buildkit(self.image, pull=False, buildargs=None) + + mock_run_cmd.assert_not_called() + self.assertEqual(utils.Status.ERROR, self.image.status) + + @mock.patch.object(tasks.BuildTask, '_run_cmd') + def test_build_buildkit_with_named_builder(self, mock_run_cmd): + """--buildkit-builder is forwarded as --builder to the command.""" + self.conf.set_override('buildkit', True) + self.conf.set_override('buildkit_builder', 'mybuilder') + push_queue = mock.Mock() + builder = tasks.BuildTask(self.conf, self.image, push_queue) + self.image.start = datetime.datetime.now() + builder._build_buildkit(self.image, pull=False, buildargs=None) + + cmd = mock_run_cmd.call_args[0][0] + self.assertIn('--builder', cmd) + idx = cmd.index('--builder') + self.assertEqual('mybuilder', cmd[idx + 1]) + + @mock.patch.object(tasks.BuildTask, '_run_cmd') + def test_build_buildkit_no_cache(self, mock_run_cmd): + """cache=False adds --no-cache to the command.""" + self.conf.set_override('buildkit', True) + self.conf.set_override('cache', False) + push_queue = mock.Mock() + builder = tasks.BuildTask(self.conf, self.image, push_queue) + self.image.start = datetime.datetime.now() + builder._build_buildkit(self.image, pull=False, buildargs=None) + + cmd = mock_run_cmd.call_args[0][0] + self.assertIn('--no-cache', cmd) + + @mock.patch.object(tasks.BuildTask, '_run_cmd') + def test_build_buildkit_with_buildargs(self, mock_run_cmd): + """Build args are forwarded as --build-arg KEY=VALUE pairs.""" + self.conf.set_override('buildkit', True) + push_queue = mock.Mock() + builder = tasks.BuildTask(self.conf, self.image, push_queue) + self.image.start = datetime.datetime.now() + buildargs = {'HTTP_PROXY': 'http://proxy:8080', + 'NO_PROXY': '127.0.0.1'} + builder._build_buildkit(self.image, pull=False, buildargs=buildargs) + + cmd = mock_run_cmd.call_args[0][0] + self.assertIn('--build-arg', cmd) + self.assertIn('HTTP_PROXY=http://proxy:8080', cmd) + self.assertIn('NO_PROXY=127.0.0.1', cmd) + + @mock.patch.object(tasks.BuildTask, '_run_cmd') + def test_build_buildkit_with_labels(self, mock_run_cmd): + """Image labels are forwarded as --label key=value pairs.""" + self.conf.set_override('buildkit', True) + push_queue = mock.Mock() + builder = tasks.BuildTask(self.conf, self.image, push_queue) + self.image.start = datetime.datetime.now() + self.image.labels = {'maintainer': 'kolla', 'version': '1.0'} + builder._build_buildkit(self.image, pull=False, buildargs=None) + + cmd = mock_run_cmd.call_args[0][0] + self.assertIn('--label', cmd) + self.assertIn('maintainer=kolla', cmd) + self.assertIn('version=1.0', cmd) + + @mock.patch.object(tasks.BuildTask, '_run_cmd') + def test_build_buildkit_error_sets_status(self, mock_run_cmd): + """An exception from _run_cmd marks the image as ERROR.""" + self.conf.set_override('buildkit', True) + mock_run_cmd.side_effect = Exception('build failed') + push_queue = mock.Mock() + builder = tasks.BuildTask(self.conf, self.image, push_queue) + self.image.start = datetime.datetime.now() + builder._build_buildkit(self.image, pull=False, buildargs=None) + + self.assertEqual(utils.Status.ERROR, self.image.status) + + @mock.patch('subprocess.Popen') + def test_run_cmd_success(self, mock_popen): + """Zero exit code does not raise.""" + mock_proc = mock.MagicMock() + mock_proc.__enter__.return_value = mock_proc + mock_proc.stdout = [] + mock_proc.returncode = 0 + mock_popen.return_value = mock_proc + + push_queue = mock.Mock() + builder = tasks.BuildTask(self.conf, self.image, push_queue) + builder._run_cmd(['docker', 'buildx', 'build', self.image.path]) + + @mock.patch('subprocess.Popen') + def test_run_cmd_failure_raises(self, mock_popen): + """Non-zero exit code raises CalledProcessError.""" + import subprocess as sp # nosec + mock_proc = mock.MagicMock() + mock_proc.__enter__.return_value = mock_proc + mock_proc.stdout = [] + mock_proc.returncode = 1 + mock_popen.return_value = mock_proc + + push_queue = mock.Mock() + builder = tasks.BuildTask(self.conf, self.image, push_queue) + cmd = ['docker', 'buildx', 'build', self.image.path] + self.assertRaises(sp.CalledProcessError, builder._run_cmd, cmd) + + @mock.patch(engine_client) + def test_build_image_buildkit_dispatches(self, mock_client): + """When buildkit=True and engine=docker, calls _build_buildkit.""" + self.conf.set_override('buildkit', True) + push_queue = mock.Mock() + builder = tasks.BuildTask(self.conf, self.image, push_queue) + with mock.patch.object(builder, '_build_buildkit') as mock_bk: + builder.run() + mock_bk.assert_called_once() + + @mock.patch(engine_client) + def test_build_image_no_buildkit_uses_sdk(self, mock_client): + """When buildkit=False, builder uses docker-py SDK (images.build).""" + push_queue = mock.Mock() + builder = tasks.BuildTask(self.conf, self.image, push_queue) + builder.run() + mock_client().images.build.assert_called_once() + class KollaWorkerTest(base.TestCase): @@ -869,8 +1121,49 @@ class KollaWorkerTest(base.TestCase): self.assertEqual('tmp/foo/docker', kolla.working_dir) +class BuildKitCheckTest(base.TestCase): + + @mock.patch('subprocess.check_output') + def test_check_docker_buildx_success(self, mock_check_output): + mock_check_output.return_value = b'github.com/docker/buildx v0.21.0' + common_utils.check_docker_buildx() + mock_check_output.assert_called_once_with( + ['docker', 'buildx', 'version'], stderr=mock.ANY) + + @mock.patch('subprocess.check_output') + def test_check_docker_buildx_not_installed(self, mock_check_output): + import subprocess as sp # nosec + mock_check_output.side_effect = sp.CalledProcessError(1, 'docker') + self.assertRaises(sp.CalledProcessError, + common_utils.check_docker_buildx) + + @mock.patch('subprocess.check_output') + def test_check_docker_buildx_docker_missing(self, mock_check_output): + mock_check_output.side_effect = OSError(2, 'No such file or directory') + self.assertRaises(OSError, common_utils.check_docker_buildx) + + class MainTest(base.TestCase): + def setUp(self): + super(MainTest, self).setUp() + self.conf.set_override('buildkit', False) + # Mock subprocess so that _build_buildkit and check_docker_buildx + # succeed when the full build pipeline is exercised (buildkit=True is + # the default for a fresh conf). + mock_proc = mock.MagicMock() + mock_proc.__enter__.return_value = mock_proc + mock_proc.stdout = [] + mock_proc.returncode = 0 + popen_patcher = mock.patch('subprocess.Popen', return_value=mock_proc) + popen_patcher.start() + self.addCleanup(popen_patcher.stop) + check_output_patcher = mock.patch( + 'subprocess.check_output', + return_value=b'github.com/docker/buildx v0.21.0') + check_output_patcher.start() + self.addCleanup(check_output_patcher.stop) + @mock.patch.object(build, 'run_build') def test_images_built(self, mock_run_build): image_statuses = ({}, {'img': 'built'}, {}, {}, {}, {}) @@ -904,6 +1197,15 @@ class MainTest(base.TestCase): result = build.run_build() self.assertTrue(result) + @mock.patch('sys.argv') + @mock.patch('kolla.common.utils.check_docker_buildx') + @mock.patch(engine_client) + def test_run_build_exits_when_buildx_missing( + self, mock_client, mock_check_buildx, mock_sys): + import subprocess as sp # nosec + mock_check_buildx.side_effect = sp.CalledProcessError(1, 'docker') + self.assertRaises(SystemExit, build.run_build) + @mock.patch.object(build, 'run_build') def test_skipped_images(self, mock_run_build): image_statuses = ({}, {}, {}, {'img': 'skipped'}, {}, {}) |