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

Remove OPM usage for Dockerfile creation #716

Merged
merged 1 commit into from
Sep 11, 2024
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
Remove OPM usage for Dockerfile creation
Refers to CLOUDDST-23891
  • Loading branch information
yashvardhannanavati committed Sep 11, 2024
commit a7d7817653a9742cd12d57cd27a1b98a9cddf4c6
4 changes: 2 additions & 2 deletions iib/workers/tasks/build.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@
deprecate_bundles,
Opm,
verify_operators_exists,
opm_generate_dockerfile,
create_dockerfile,
opm_validate,
)
from iib.workers.tasks.utils import (
Expand Down Expand Up @@ -1119,7 +1119,7 @@ def handle_rm_request(
# validate fbc config
opm_validate(fbc_dir_path)

opm_generate_dockerfile(
create_dockerfile(
fbc_dir=fbc_dir_path,
base_dir=temp_dir,
index_db=index_db_path,
Expand Down
4 changes: 2 additions & 2 deletions iib/workers/tasks/build_merge_index_image.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
from iib.workers.tasks.opm_operations import (
opm_registry_add_fbc,
opm_migrate,
opm_generate_dockerfile,
create_dockerfile,
deprecate_bundles_fbc,
opm_index_add,
deprecate_bundles,
Expand Down Expand Up @@ -368,7 +368,7 @@ def handle_merge_request(
if os.path.isfile(dockerfile_path):
log.info('Removing previously generated dockerfile.')
os.remove(dockerfile_path)
opm_generate_dockerfile(
create_dockerfile(
fbc_dir=fbc_dir,
base_dir=temp_dir,
index_db=index_db_file,
Expand Down
93 changes: 28 additions & 65 deletions iib/workers/tasks/opm_operations.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import socket
import subprocess
import tempfile
import textwrap
import time
from typing import Callable, List, Optional, Set, Tuple, Union
from packaging.version import Version
Expand Down Expand Up @@ -589,7 +590,7 @@ def deprecate_bundles_fbc(
fbc_dir, _ = opm_migrate(index_db_file, base_dir)
# we should keep generating Dockerfile here
# to have the same behavior as we run `opm index deprecatetruncate` with '--generate' option
opm_generate_dockerfile(
create_dockerfile(
fbc_dir=fbc_dir,
base_dir=base_dir,
index_db=index_db_file,
Expand Down Expand Up @@ -637,15 +638,15 @@ def opm_migrate(
return fbc_dir_path, None


def opm_generate_dockerfile(
def create_dockerfile(
fbc_dir: str,
base_dir: str,
index_db: str,
binary_image: str,
dockerfile_name: Optional[str] = None,
) -> str:
"""
Generate Dockerfile using opm command and adding index.db to hidden location.
Create Dockerfile and adding index.db to hidden location.
:param str fbc_dir: directory containing file-based catalog (JSON or YAML files).
:param str base_dir: base directory where Dockerfile should be created.
Expand All @@ -656,8 +657,6 @@ def opm_generate_dockerfile(
:raises: IIBError when Dockerfile was not generated
:rtype: str
"""
from iib.workers.tasks.utils import run_cmd

# we do not want to continue if Dockerfile already exists
dockerfile_name_opm_default = f"{os.path.basename(fbc_dir)}.Dockerfile"
tmp_dockerfile_name = dockerfile_name or dockerfile_name_opm_default
Expand All @@ -671,33 +670,35 @@ def opm_generate_dockerfile(
)
return dockerfile_path

cmd = [
Opm.opm_version,
'generate',
'dockerfile',
os.path.abspath(fbc_dir),
'--binary-image',
binary_image,
]

log.info('Generating Dockerfile with binary image %s' % binary_image)
run_cmd(cmd, {'cwd': base_dir}, exc_msg='Failed to generate Dockerfile for file-based catalog')

# check if opm command generated Dockerfile successfully
log.info('Creating Dockerfile with binary image %s' % binary_image)
dockerfile_path_opm_default = os.path.join(base_dir, dockerfile_name_opm_default)
if not os.path.isfile(dockerfile_path_opm_default):
error_msg = f"Cannot find generated Dockerfile at {dockerfile_path_opm_default}"
log.error(error_msg)
raise IIBError(error_msg)
with open(dockerfile_path_opm_default, 'w') as dockerfile:
dockerfile.write(
textwrap.dedent(
f"""\
FROM {binary_image}
# Configure the entrypoint and command
ENTRYPOINT ["/bin/opm"]
CMD ["serve", "/configs", "--cache-dir=/tmp/cache"]
# Copy declarative config root and cache into image
ADD catalog /configs
COPY --chown=1001:0 cache /tmp/cache
# Set DC-specific label for the location of the DC root directory
# in the image
LABEL operators.operatorframework.io.index.configs.v1=/configs
"""
xDaile marked this conversation as resolved.
Show resolved Hide resolved
)
)

# we should rename Dockerfile generated by opm if `dockerfile_name` parameter is set
if dockerfile_name:
if os.path.exists(dockerfile_path):
log.info('Rewriting Dockerfile %s with newly generated by opm.', dockerfile_path)
os.rename(dockerfile_path_opm_default, dockerfile_path)

insert_cache_into_dockerfile(dockerfile_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 @@ -707,44 +708,6 @@ def opm_generate_dockerfile(
return dockerfile_path


def verify_cache_insertion_edit_dockerfile(file_list: list) -> None:
"""
Verify Dockerfile edit to insert local cache was successful.
:param list file_list: Generated dockerfile as a list.
:raises: IIBError when the Dockerfile edit is unsuccessful.
"""
copied_cache_found = False
match_str = 'COPY --chown=1001:0 cache /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) -> None:
"""
Insert built cache into the Dockerfile.
:param str dockerfile_path: path to the generated Dockerfile.
"""
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"]',
'COPY --chown=1001:0 cache /tmp/cache',
)

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())


@create_port_filelocks(port_purposes=["opm_pprof_port"])
def generate_cache_locally(
base_dir: str,
Expand Down Expand Up @@ -923,7 +886,7 @@ def opm_registry_add_fbc(
fbc_dir, _ = opm_migrate(index_db=index_db_file, base_dir=base_dir)
# we should keep generating Dockerfile here
# to have the same behavior as we run `opm index add` with '--generate' option
opm_generate_dockerfile(
create_dockerfile(
fbc_dir=fbc_dir,
base_dir=base_dir,
index_db=index_db_file,
Expand Down Expand Up @@ -1055,7 +1018,7 @@ def opm_create_empty_fbc(
# Migrate the index to FBC
fbc_dir, _ = opm_migrate(index_db=index_db_path, base_dir=temp_dir)

opm_generate_dockerfile(
create_dockerfile(
fbc_dir=fbc_dir,
base_dir=temp_dir,
index_db=index_db_path,
Expand Down Expand Up @@ -1145,7 +1108,7 @@ def opm_registry_add_fbc_fragment(
)

log.info("Dockerfile generated from %s", from_index_configs_dir)
opm_generate_dockerfile(
create_dockerfile(
fbc_dir=from_index_configs_dir,
base_dir=temp_dir,
index_db=index_db_path,
Expand Down
2 changes: 1 addition & 1 deletion tests/test_workers/test_tasks/test_build.py
Original file line number Diff line number Diff line change
Expand Up @@ -1064,7 +1064,7 @@ def test_handle_rm_request(
@mock.patch('iib.workers.tasks.build.get_catalog_dir')
@mock.patch('iib.workers.tasks.build.merge_catalogs_dirs')
@mock.patch('iib.workers.tasks.build.generate_cache_locally')
@mock.patch('iib.workers.tasks.build.opm_generate_dockerfile')
@mock.patch('iib.workers.tasks.build.create_dockerfile')
@mock.patch('os.rename')
@mock.patch('iib.workers.tasks.opm_operations.Opm.set_opm_version')
@mock.patch('iib.workers.tasks.opm_operations.Opm.get_opm_version_number')
Expand Down
4 changes: 2 additions & 2 deletions tests/test_workers/test_tasks/test_build_merge_index_image.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
@mock.patch('iib.workers.tasks.utils.opm_registry_serve')
@mock.patch('iib.workers.tasks.build_merge_index_image._update_index_image_pull_spec')
@mock.patch('iib.workers.tasks.build._verify_index_image')
@mock.patch('iib.workers.tasks.build_merge_index_image.opm_generate_dockerfile')
@mock.patch('iib.workers.tasks.build_merge_index_image.create_dockerfile')
@mock.patch('iib.workers.tasks.build._get_index_database')
@mock.patch('iib.workers.tasks.build_merge_index_image.opm_migrate')
@mock.patch('iib.workers.tasks.build_merge_index_image.opm_registry_add_fbc')
Expand Down Expand Up @@ -190,7 +190,7 @@ def side_effect(*args, base_dir, **kwargs):
@mock.patch('iib.workers.tasks.utils.opm_serve_from_index')
@mock.patch('iib.workers.tasks.build_merge_index_image._update_index_image_pull_spec')
@mock.patch('iib.workers.tasks.build._verify_index_image')
@mock.patch('iib.workers.tasks.build_merge_index_image.opm_generate_dockerfile')
@mock.patch('iib.workers.tasks.build_merge_index_image.create_dockerfile')
@mock.patch('iib.workers.tasks.build._get_index_database')
@mock.patch('iib.workers.tasks.build_merge_index_image.opm_migrate')
@mock.patch('iib.workers.tasks.build_merge_index_image.opm_registry_add_fbc')
Expand Down
Loading