Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Build opm cache locally to enable multi-arch builds #465

Merged
merged 1 commit into from
Dec 19, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
78 changes: 78 additions & 0 deletions iib/workers/tasks/opm_operations.py
Original file line number Diff line number Diff line change
Expand Up @@ -406,6 +406,10 @@ def opm_generate_dockerfile(
log.info('Rewriting Dockerfile %s with newly generated by opm.', dockerfile_path)
os.rename(dockerfile_path_opm_default, dockerfile_path)

local_cache_path = os.path.join(base_dir, 'cache')
generate_cache_locally(base_dir, fbc_dir, local_cache_path)
insert_cache_into_dockerfile(dockerfile_path, local_cache_path)

db_path = get_worker_config()['hidden_index_db_path']
rel_path_index_db = os.path.relpath(index_db, base_dir)
with open(dockerfile_path, 'a') as f:
Expand All @@ -415,6 +419,80 @@ def opm_generate_dockerfile(
return dockerfile_path


def verify_cache_insertion_edit_dockerfile(file_list: list, local_cache_path: str) -> None:
"""
Verify Dockerfile edit to insert local cache was successful.

:param list file_list: Generated dockerfile as a list.
:param str local_cache_path: path to the locally generated cache.
:raises: IIBError when the Dockerfile edit is unsuccessful.
"""
copied_cache_found = False
match_str = f'COPY {local_cache_path} /tmp/cache'
for line in file_list:
if match_str in line:
copied_cache_found = True
break
if not copied_cache_found:
raise IIBError('Dockerfile edit to insert locally built cache failed.')


def insert_cache_into_dockerfile(dockerfile_path: str, local_cache_path: str) -> None:
"""
Insert built cache into the Dockerfile.

:param str dockerfile_path: path to the generated Dockerfile.
:param str local_cache_path: path to the locally generated cache.
"""
with open(dockerfile_path, 'r') as f:
file_data = f.read()

file_data = file_data.replace(
'RUN ["/bin/opm", "serve", "/configs", "--cache-dir=/tmp/cache", "--cache-only"]',
f'COPY {local_cache_path} /tmp/cache',
)
JAVGan marked this conversation as resolved.
Show resolved Hide resolved

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unrelated question for this replacement. Is there a way to detect if the replacement didn't replace anything? In the future, if the dockerfile generation changes again, it would be nice if the failure message is something like: "failed to replace cache building execution in dockerfile" rather than the eventual/cryptic: "exec /bin/opm: exec format error" error that would happen during the image build.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a check to confirm the COPY line is added to the file and throw a more comprehendible error message if the replacement fails.

with open(dockerfile_path, 'w') as f:
f.write(file_data)

with open(dockerfile_path, 'r') as f:
verify_cache_insertion_edit_dockerfile(f.readlines(), local_cache_path)


def generate_cache_locally(base_dir: str, fbc_dir: str, local_cache_path: str) -> None:
"""
Generate the cache for the index image locally before building it.

:param str base_dir: base directory where cache should be created.
:param str fbc_dir: directory containing file-based catalog (JSON or YAML files).
:param str local_cache_path: path to the locally generated cache.
:return: Returns path to generated cache
:rtype: str
:raises: IIBError when cache was not generated

"""
from iib.workers.tasks.utils import run_cmd

cmd = [
'opm',
lipoja marked this conversation as resolved.
Show resolved Hide resolved
'serve',
os.path.abspath(fbc_dir),
f'--cache-dir={local_cache_path}',
'--cache-only',
lipoja marked this conversation as resolved.
Show resolved Hide resolved
'--termination-log',
'/dev/null',
]

log.info('Generating cache for the file-based catalog')
run_cmd(cmd, {'cwd': base_dir}, exc_msg='Failed to generate cache for file-based catalog')

# Check if the opm command generated cache successfully
if not os.path.isdir(local_cache_path):
error_msg = f"Cannot find generated cache at {local_cache_path}"
log.error(error_msg)
raise IIBError(error_msg)


@retry(
before_sleep=before_sleep_log(log, logging.WARNING),
reraise=True,
Expand Down
109 changes: 108 additions & 1 deletion tests/test_workers/test_tasks/test_opm_operations.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
# SPDX-License-Identifier: GPL-3.0-or-later
import os.path
import pytest
import textwrap

from unittest import mock

Expand Down Expand Up @@ -166,7 +167,9 @@ def test_opm_migrate(

@pytest.mark.parametrize("dockerfile", (None, 'index.Dockerfile'))
@mock.patch('iib.workers.tasks.utils.run_cmd')
def test_opm_generate_dockerfile(mock_run_cmd, tmpdir, dockerfile):
@mock.patch('iib.workers.tasks.opm_operations.generate_cache_locally')
@mock.patch('iib.workers.tasks.opm_operations.insert_cache_into_dockerfile')
def test_opm_generate_dockerfile(mock_icid, mock_gcl, mock_run_cmd, tmpdir, dockerfile):
index_db_file = os.path.join(tmpdir, 'database/index.db')
fbc_dir = os.path.join(tmpdir, 'catalogs')

Expand All @@ -192,6 +195,9 @@ def create_dockerfile(*args, **kwargs):
with open(df_path, 'r') as f:
assert any(line.find('/var/lib/iib/_hidden/do.not.edit.db') != -1 for line in f.readlines())

mock_icid.assert_called_once_with(df_path, mock.ANY)
mock_gcl.assert_called_once_with(tmpdir, fbc_dir, mock.ANY)


@pytest.mark.parametrize("set_index_db_file", (False, True))
@mock.patch('iib.workers.tasks.utils.run_cmd')
Expand Down Expand Up @@ -512,3 +518,104 @@ def test_deprecate_bundles_fbc(
binary_image="some:image",
dockerfile_name='index.Dockerfile',
)


@mock.patch('iib.workers.tasks.utils.run_cmd')
@mock.patch('iib.workers.tasks.opm_operations.os.path.isdir', return_value=True)
def test_generate_cache_locally(mock_isdir, mock_cmd, tmpdir):
fbc_dir = os.path.join(tmpdir, 'catalogs')
local_cache_path = os.path.join(tmpdir, 'cache')
cmd = [
'opm',
'serve',
os.path.abspath(fbc_dir),
f'--cache-dir={local_cache_path}',
'--cache-only',
'--termination-log',
'/dev/null',
]

opm_operations.generate_cache_locally(tmpdir, fbc_dir, local_cache_path)

mock_cmd.assert_called_once_with(
cmd, {'cwd': tmpdir}, exc_msg='Failed to generate cache for file-based catalog'
)


@mock.patch('iib.workers.tasks.utils.run_cmd')
def test_generate_cache_locally_failed(mock_cmd, tmpdir):
fbc_dir = os.path.join(tmpdir, 'catalogs')
local_cache_path = os.path.join(tmpdir, 'cache')
cmd = [
'opm',
'serve',
os.path.abspath(fbc_dir),
f'--cache-dir={local_cache_path}',
'--cache-only',
'--termination-log',
'/dev/null',
]

with pytest.raises(IIBError, match='Cannot find generated cache at .+'):
opm_operations.generate_cache_locally(tmpdir, fbc_dir, local_cache_path)
mock_cmd.assert_called_once_with(
cmd, {'cwd': tmpdir}, exc_msg='Failed to generate cache for file-based catalog'
)


def test_insert_cache_into_dockerfile(tmpdir):
local_cache_dir = tmpdir.mkdir('cache')
local_cache_path = os.path.join(tmpdir, 'cache')
generated_dockerfile = local_cache_dir.join('catalog.Dockerfile')

dockerfile_template = textwrap.dedent(
"""\
ADD /configs
RUN something-else
{run_command}
COPY . .
"""
)

generated_dockerfile.write(
dockerfile_template.format(
run_command=(
'RUN ["/bin/opm", "serve", "/configs", "--cache-dir=/tmp/cache", "--cache-only"]'
)
)
)

opm_operations.insert_cache_into_dockerfile(generated_dockerfile, local_cache_path)

assert generated_dockerfile.read_text('utf-8') == dockerfile_template.format(
run_command=f'COPY {local_cache_path} /tmp/cache'
)


def test_insert_cache_into_dockerfile_no_matching_line(tmpdir):
local_cache_dir = tmpdir.mkdir('cache')
local_cache_path = os.path.join(tmpdir, 'cache')
generated_dockerfile = local_cache_dir.join('catalog.Dockerfile')

dockerfile_template = textwrap.dedent(
"""\
ADD /configs
RUN something-else
COPY . .
"""
)

generated_dockerfile.write(dockerfile_template)
with pytest.raises(IIBError, match='Dockerfile edit to insert locally built cache failed.'):
opm_operations.insert_cache_into_dockerfile(generated_dockerfile, local_cache_path)


def test_verify_cache_insertion_edit_dockerfile():
input_list = ['ADD /configs', 'COPY . .' 'COPY a/b /tmp/cache']
opm_operations.verify_cache_insertion_edit_dockerfile(input_list, 'a/b')


def test_verify_cache_insertion_edit_dockerfile_failed():
input_list = ['ADD /configs', 'COPY . .' 'RUN something']
with pytest.raises(IIBError, match='Dockerfile edit to insert locally built cache failed.'):
opm_operations.verify_cache_insertion_edit_dockerfile(input_list, 'a/b')