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

Add support for otk-defined distributions (COMPOSER-2299) #797

Open
wants to merge 14 commits into
base: main
Choose a base branch
from

Conversation

achilleas-k
Copy link
Member

This PR adds the initial (but not complete) support for defining distributions using otk yaml files. I've been experimenting with multiple ideas for how to do this and I think this is the nicest to work with while keeping disruption of current internal distro definitions to a minimum.

There's still a lot more to do, but I wanted to get this in front of people to discuss design choices before moving on with the full solution. Meanwhile, I intend to start converting image types to otk files to evaluate the integration.

Notable design choices worth discussing

  • Distributions are defined under an otk root directory (which will be configurable and we could support multiple paths). Distributions, architectures, and image types are discovered using the directory structure: <distro ID>/<architecture>/<image type name>.yaml.
  • Each distribution must provide a properties.yaml in its root (<distro ID>/properties.yaml) which defines values required by osbuild-composer. Currently this includes name (which can differ from the distro ID), os_version, release_version, and runner.
    • Note that the current values might not all be necessary, but we will probably discover that we need some global distro properties like this to be defined somewhere. For example, module_platform_id is definitely not needed since otk is taking care of depsolving for us. But we will almost certainly need a way to define the canonical distro name in order to do host distro selection.
  • Blueprint customizations are added via an entrypoint yaml file which includes the image type entrypoint (more on this below).

Keep in mind that while we transition to a fully otk-defined state, we will need to keep the instantiation and serialisation flow of the internal manifests and a lot of the interfaces associated with that. Once we finish transitioning, a lot of this should become simpler.

Blueprint customizations

When compiling an otk manifest, an otk-customizations.yaml is created in a temporary directory which includes all the Blueprint customizations and an otk.include to the target image type entrypoint. The customizations file is used as the entrypoint to the compile command.

For example, given the following Blueprint:

[customizations]
hostname = "hal-9000"

[customizations.kernel]
append = "debug"

[customizations.locale]
keyboard = "uk"

when compiling a Fedora 40 x86-64 qcow2, an otk-customizations.yaml is created with:

otk.define.customizations:
  hostname: "hal-9000"
  kernel_append: "debug"
  keyboard: "uk"

otk.include: `/otkroot/fedora-40/x86_64/qcow2.yaml`

(where /otkroot is the root where we define our otk distros)
and the manifest is compiled with:

otk compile --target osbuild /path/to/otk-customizations.yaml

The qcow2 yaml (or the files it includes) should then be properly handling these values. For example:

otk.target.osbuild:
  pipelines:
    - name: os
      stages:
        - otk.op.if-set:
          what: "${keyboard}"
          then:
            - type: org.osbuild.keymap
              options:
                keymap: "${keyboard}"

One issue with this approach however is that this puts customizations at the lowest priority since they're defined first, so any defines that follow will override the value. One way around this issue is prefixing these or namespacing them under an object that is treated with higher priority (e.g. modifications.keyboard), which is something we already discussed in the otk specifications.

Make the distro field of the manifest.Manifest struct private and make
sure it's set using the manifest.New() constructor.

The intention is to make manifest.Manifest an interface and getting rid
of private fields is the first step towards this.
manifest.Manifest is now an interface with one implementation called
IntManifest (Internal Manifest), which is the previous Manifest type.

All functions that interact with pointers to the old concrete Manifest
type now interact with the interface instead.
The OTKManifest is a second implementation of the manifest.Manifest
interface that will be used to generate manifests from otk yaml files.
The type only implements the Serialize() method.  None of the other
methods are important (at least for now).  The content spec methods in
particular are not needed since otk will handle content resolution
(depsolving, container and ostree commit resolves) itself.
The directory structure follows the assumptions of the test_otk_distro.
New package that loads a distribution from otk yaml files.
Distribution-level properties, like name, codename, versions, etc, are
loaded from a file called 'properties.yaml' in the root of the
distribution's directory.  An error is returned if this file is missing
or if any of the required properties are undefined/empty.

The distro creates a distro->arch->imagetype hierarchy based on the
contents of the filesystem tree.  It assumes a directory structure
`<distroid>/<archname>/<imagetypename>.yaml`.  For example:

fedora-40/
  properties.yaml
  x86_64/
    qcow2.yaml
    ami.yaml
  aarch64/
    qcow2.yaml

The top-level directory is used as the unique distro ID, whereas the
name is defined in the properties.yaml.  The distinction is useful for
separating the on-disk, unique ID of a distro from the display name.
Test that the distro is loaded with the expected properties, arches, and
image types.
Add the DistroFactory() function to otkdistro, which will enable loading
distros defined using otk files alongside the internally defined ones
using the same mechanism.

The root of the otk path is hard-coded to "./otk", but we will have to
make this configurable.
With this addition, for every file found in the repository configuration
path (for this project, "test/data/repositories/" by default), the
distrofactory will try to load an otk distro at "otk/<distroid>/".
When initialising an OTKManifest, add the Blueprint to it as well, so we
can create variables (defines, modifications) based on it before
compiling.
New otk package for managing otk-related things.
The otkCustomizationsFile type handles creating an otk file with defines
for customizations and a single otk.include.  The purpose is to use this
to create a 'customizations.yaml' file that 'otk.define's all the
variables represented by a blueprint then 'otk.include' the file we want
to compile, effectively setting variables defined by the user in the
compilation of the manifest.

Currently it only writes a few simple customizations as a proof of
concept, but it will grow to cover all potential customizations.
When serializing an OTKManifest, create a customizations file that
includes 'otk.define's to represent the options of a blueprint, then
'otk.include' the original entrypoint to generate a manifest with the
user customizations included.
Define the expected file contents as a map[string]interface{} and
deserialize the file contents before comparing for convenience and nicer
error output (compared to strings).
Add tests for otk serialization with some simple blueprint
customizations.

The entrypoint contents and expected serialized manifests are defined as
map[string]interface for convenience and nicer error output (compared to
strings).
Remove test/otk/fakedistro and instead create it when running the unit
test.  This will make it easier to evolve the test to cover more
features.
@achilleas-k
Copy link
Member Author

achilleas-k commented Jul 16, 2024

Oh nice. Tests are failing now because we depend on otk 😆

I'll probably mock the otk calls for testing the Serialize() code path.

@achilleas-k achilleas-k changed the title Add support for otk-defined distributions Add support for otk-defined distributions (COMPOSER-2299) Jul 16, 2024
Copy link

This PR is stale because it has been open 30 days with no activity. Remove "Stale" label or comment or this will be closed in 7 days.

@github-actions github-actions bot added the Stale label Aug 16, 2024
@mvo5 mvo5 removed the Stale label Aug 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants