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.

Sign up for GitHub

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

Introduce a ObsPackage class for grouping Container Images #1815

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
Draft

Introduce a ObsPackage class for grouping Container Images #1815

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
4531ad9
Add new "container" class MultiBuildObsPackage to replace ContainerCrate
dcermak Oct 8, 2024
2a6a483
Remove ContainerCrate in favor of ObsPackage
dcermak Oct 8, 2024
a12656f
Stop using SERVICE_TEMPLATE and use ObsPackage._service_file_contents…
dcermak Oct 8, 2024
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
46 changes: 0 additions & 46 deletions src/bci_build/containercrate.py
View file
Open in desktop

This file was deleted.

74 changes: 33 additions & 41 deletions src/bci_build/package/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,18 +21,17 @@
from bci_build.container_attributes import PackageType
from bci_build.container_attributes import ReleaseStage
from bci_build.container_attributes import SupportLevel
from bci_build.containercrate import ContainerCrate
from bci_build.os_version import ALL_OS_LTSS_VERSIONS
from bci_build.os_version import RELEASED_OS_VERSIONS
from bci_build.os_version import OsVersion
from bci_build.package.obs_package import ObsPackage
from bci_build.registry import ApplicationCollectionRegistry
from bci_build.registry import Registry
from bci_build.registry import publish_registry
from bci_build.service import Service
from bci_build.templates import DOCKERFILE_TEMPLATE
from bci_build.templates import INFOHEADER_TEMPLATE
from bci_build.templates import KIWI_TEMPLATE
from bci_build.templates import SERVICE_TEMPLATE
from bci_build.util import write_to_file

_BASH_SET: str = "set -euo pipefail"
Expand Down Expand Up @@ -135,24 +134,17 @@ def _build_tag_prefix(os_version: OsVersion) -> str:
return "bci"


@dataclass
class BaseContainerImage(abc.ABC):
@dataclass(kw_only=True)
class BaseContainerImage(ObsPackage):
"""Base class for all Base Containers."""

#: Name of this image. It is used to generate the build tags, i.e. it
#: defines under which name this image is published.
name: str

#: The SLE service pack to which this package belongs
os_version: OsVersion

#: Human readable name that will be inserted into the image title and description
pretty_name: str

#: Optional a package_name, used for creating the package name on OBS or IBS in
# ``devel:BCI:SLE-15-SP$ver`` (on OBS) or ``SUSE:SLE-15-SP$ver:Update:BCI`` (on IBS)
package_name: str | None = None

#: Epoch to use for handling os_version downgrades
os_epoch: int | None = None

Expand Down Expand Up @@ -210,9 +202,6 @@ class BaseContainerImage(abc.ABC):
#: build flavors to produce for this container variant
build_flavor: str | None = None

#: create that this container is part of
crate: ContainerCrate = None

#: Add any replacements via `obs-service-replace_using_package_version
#: <https://github.com/openSUSE/obs-service-replace_using_package_version>`_
#: that are used in this image into this list.
Expand Down Expand Up @@ -316,6 +305,8 @@ def publish_registry(self) -> Registry:
return self._publish_registry

def __post_init__(self) -> None:
super().__post_init__()

self.pretty_name = self.pretty_name.strip()

if not self.package_name:
Expand All @@ -329,11 +320,6 @@ def __post_init__(self) -> None:
"Cannot specify both a custom_end and a config.sh script! Use just config_sh_script."
)

if self.build_recipe_type is None:
self.build_recipe_type = (
BuildType.KIWI if self.os_version == OsVersion.SP3 else BuildType.DOCKER
)

if not self.maintainer:
self.maintainer = (
"openSUSE (https://www.opensuse.org/)"
Expand Down Expand Up @@ -364,12 +350,6 @@ def prepare_template(self) -> None:

pass

@property
@abc.abstractmethod
def uid(self) -> str:
"""unique identifier of this image, either its name or ``$name-$tag_version``."""
pass

@property
@abc.abstractmethod
def oci_version(self) -> str:
Expand Down Expand Up @@ -1046,12 +1026,34 @@ def kiwi_additional_tags(self) -> str | None:

return ",".join(extra_tags) if extra_tags else None

async def write_files_to_folder(self, dest: str) -> list[str]:
@property
def services(self) -> tuple[Service, ...]:
if not self.replacements_via_service:
return ()

if self.build_recipe_type == BuildType.DOCKER:
if self.build_flavor:
default_file_name = f"Dockerfile.{self.build_flavor}"
else:
default_file_name = "Dockerfile"
elif self.build_recipe_type == BuildType.KIWI:
default_file_name = f"{self.package_name}.kiwi"
else:
raise ValueError(f"invalid build recipe type: {self.build_recipe_type}")

return tuple(
replacement.to_service(default_file_name)
for replacement in self.replacements_via_service
)

async def write_files_to_folder(
self, dest: str, *, with_service_file: bool = True
) -> list[str]:
"""Writes all files required to build this image into the destination folder and
returns the filenames (not full paths) that were written to the disk.

"""
files = ["_service"]
files = ["_service"] if with_service_file else []
tasks = []

self.prepare_template()
Expand Down Expand Up @@ -1097,18 +1099,8 @@ async def write_file_to_dest(fname: str, contents: str | bytes) -> None:
False
), f"got an unexpected build_recipe_type: '{self.build_recipe_type}'"

if self.build_flavor:
dfile = "Dockerfile"
tasks.append(write_file_to_dest(dfile, self.crate.default_dockerfile()))
files.append(dfile)

mname = "_multibuild"
tasks.append(write_file_to_dest(mname, self.crate.multibuild(self)))
files.append(mname)

tasks.append(
write_file_to_dest("_service", SERVICE_TEMPLATE.render(image=self))
)
if with_service_file:
tasks.append(self._write_service_file(dest))

changes_file_name = self.package_name + ".changes"
if not (Path(dest) / changes_file_name).exists():
Expand Down Expand Up @@ -1474,7 +1466,7 @@ def generate_disk_size_constraints(size_gb: int) -> str:
from .rust import RUST_CONTAINERS # noqa: E402
from .spack import SPACK_CONTAINERS # noqa: E402

ALL_CONTAINER_IMAGE_NAMES: dict[str, BaseContainerImage] = {
ALL_CONTAINER_IMAGE_NAMES: dict[str, ObsPackage] = {
f"{bci.uid}-{bci.os_version.pretty_print.lower()}": bci
for bci in (
*BASE_CONTAINERS,
Expand Down Expand Up @@ -1521,7 +1513,7 @@ def generate_disk_size_constraints(size_gb: int) -> str:

SORTED_CONTAINER_IMAGE_NAMES = sorted(
ALL_CONTAINER_IMAGE_NAMES,
key=lambda bci: f"{ALL_CONTAINER_IMAGE_NAMES[bci].os_version}-{ALL_CONTAINER_IMAGE_NAMES[bci].name}",
key=lambda bci: f"{ALL_CONTAINER_IMAGE_NAMES[bci].os_version}-{ALL_CONTAINER_IMAGE_NAMES[bci].uid}",
Copy link
Member

Choose a reason for hiding this comment

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

that change seems unrelated?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It is required, because ObsPackageBase doesn't have a name property

)


Expand Down
49 changes: 30 additions & 19 deletions src/bci_build/package/apache_tomcat.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@

from bci_build.container_attributes import PackageType
from bci_build.container_attributes import SupportLevel
from bci_build.containercrate import ContainerCrate
from bci_build.os_version import CAN_BE_LATEST_OS_VERSION
from bci_build.os_version import OsVersion
from bci_build.package import DOCKERFILE_RUN
Expand All @@ -13,6 +12,7 @@
from bci_build.package import Package
from bci_build.package import Replacement
from bci_build.package import _build_tag_prefix
from bci_build.package.obs_package import MultiBuildObsPackage
from bci_build.registry import publish_registry

# last version needs to be the newest
Expand Down Expand Up @@ -51,14 +51,11 @@ def _get_sac_supported_until(
)


TOMCAT_CONTAINERS = [
ApplicationStackContainer(
def _create_tomcat_container(
os_version: OsVersion, tomcat_ver: str, jre_version: int
) -> ApplicationStackContainer:
return ApplicationStackContainer(
name="apache-tomcat",
package_name=(
f"apache-tomcat-{tomcat_ver.partition('.')[0]}-image"
if os_version.is_tumbleweed
else f"sac-apache-tomcat-{tomcat_ver.partition('.')[0]}-image"
),
_publish_registry=publish_registry(os_version, app_collection=True),
pretty_name="Apache Tomcat",
custom_description=(
Expand Down Expand Up @@ -125,15 +122,29 @@ def _get_sac_supported_until(
entrypoint_user="tomcat",
logo_url="https://tomcat.apache.org/res/images/tomcat.png",
)
for tomcat_ver, os_version, jre_version in (
("10.1", OsVersion.TUMBLEWEED, 23),
("10.1", OsVersion.TUMBLEWEED, 21),
("10.1", OsVersion.TUMBLEWEED, 17),
("9", OsVersion.TUMBLEWEED, 17),
("10.1", OsVersion.SP6, 21),
("10.1", OsVersion.SP6, 17),
# (10.1, OsVersion.SP7, 21),
)
]

TOMCAT_CRATE = ContainerCrate(TOMCAT_CONTAINERS)

TOMCAT_CONTAINERS: list[MultiBuildObsPackage | ApplicationStackContainer] = [
MultiBuildObsPackage.from_bcis(
Comment on lines +127 to +128
Copy link
Member

Choose a reason for hiding this comment

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

I guess this comes down to a matter of personal taste, but looking at this, this makes the _multibuild handling explicit over hiding the complexity in an abstraction. Code wise, this is larger and maintenance effort is higher (you need to manually group bcis into multibuilds accordingly). the "package name is unclear" issue seems a non issue for me because in the previous implementation, the package name is determined by the container, the containercrate just generates an extra _multibuild file for the cases where it's needed, in the places where it's needed, and that's all. How often is it needed to dig into the implementation of multibuild generation to fix a bug? I'd argue, almost never. how often do we need to update the multibuildobspackage mapping whenever a flavor or a variant is added? each and every time. Leaky abstractions are imho worse than no abstractions at all, and we could simply have an "extra_files" that containers the _multibuild file and be done with this overall. It's even more explicit than this construct.

for me, keeping the custom, repetitive boilerplate in the container definitions at a minimum should be a goal of the dockerfile generator. this implementation turns the inner guts outside, so you constantly have to deal with the can of worms for every container that you want to convert to multibuild (and I can see that we have several candidates for that, e.g. the openjdks, potentially the pythons and other language containers).

bcis=[
_create_tomcat_container(os_version, tomcat_ver, jre_version)
for tomcat_ver, os_version, jre_version in (
("10.1", OsVersion.TUMBLEWEED, 23),
("10.1", OsVersion.TUMBLEWEED, 21),
("10.1", OsVersion.TUMBLEWEED, 17),
)
],
package_name="apache-tomcat-10-image",
),
_create_tomcat_container(OsVersion.TUMBLEWEED, "9", 17),
Copy link
Member

Choose a reason for hiding this comment

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

this should also be a multibuildobspackage, imho.

MultiBuildObsPackage.from_bcis(
package_name="sac-apache-tomcat-image",
bcis=[
_create_tomcat_container(os_version, tomcat_ver, jre_version)
for tomcat_ver, os_version, jre_version in (
("10.1", OsVersion.SP6, 21),
("10.1", OsVersion.SP6, 17),
)
],
),
]
Loading
Loading