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

otk-gen-partiton-table: generate strictly internal representation for now #2

Merged

Conversation

mvo5
Copy link

@mvo5 mvo5 commented May 28, 2024

As a starting point generate internal vars from the partition
table. This means in the otk manifest one woudl write:

otk.define.defaults:
  filesystems:
    otk.external.osbuild.gen_partition_table:
      total_size: 10G

and it would get replaced with:

otk.define.defaults:
  filesystems:
    internal_partition_table:
      ...:
    kernel_opts_list:
      - foo

and then our "partition_table_make_osbuild_stage" would consume
the "internal_partition_table" and we would declare that those
are really (for now) just meant for the tools and otk manifests
should not try to do anything with them (beside passing to the
companion tool).

Later we could consider a nicer internal representation?

mvo5 added 2 commits May 28, 2024 14:30
… now

As a starting point generate internal vars from the partition
table. This means in the otk manifest one woudl write:
```
otk.define.defaults:
  filesystems:
    otk.external.osbuild.gen_partition_table:
      total_size: 10G
```
and it would get replaced with:
```
otk.define.defaults:
  filesystems:
    internal_partition_table:
      ...:
    kernel_opts_list:
      - foo
```
and then our "partition_table_make_osbuild_stage" would consume
the "internal_partition_table" and we would declare that those
are really (for now) just meant for the tools and otk manifests
should not try to do anything with them (beside passing to the
companion tool).

Later we could consider a nicer internal representation?
@mvo5 mvo5 changed the title Cmd/partition table mvo otk-gen-partiton-table: generate strictly internal representation for now May 28, 2024
@achilleas-k
Copy link
Owner

Does this mean that the internal partition table representations live in the external tool instead of the otks?

@mvo5
Copy link
Author

mvo5 commented May 28, 2024

Does this mean that the internal partition table representations live in the external tool instead of the otks?

Kind of, yes. So my thinking is (and please challenge it!) that the partitioning is done by two externals:

  1. producer of defines
  2. consumer of defines that generates stage(s)

The producer of defines would generate a tree like:

otk.define.defaults:
 filesystem:
  otk.external.gen_partition_table:
{
  "kernel_command_line": "root=UUID=....",
   "mount_point": {
      "root": {
        "uuid": "12345",
        ...
       },
       "boot": ...
       "boot-efi": ...
    }
  },
  "internal": { "partition_table" ...
}

We would then use the "filesystems.kernel_command_line" in places where we need reference the kernel command line, like:

  kernel_opts_list:
    otk.op.join:
      values:
        - - "console=tty0"
          - "console=ttyS0,115200n8"
          - "net.ifnames=0"
        - ${filesystem.kernel_opts_list}
        - ${kernel_opts_list}

And also:

 type: org.osbuild.grub2
  options:
  root_fs_uuid: ${filesystem.root.uuid}
  boot_fs_uuid: ${filesystem.boot.uuid}
...

we will have to make sure somehow (either by convention or lint or code) that some of these values are readonly to avoid doing something like:

otk.define.yolo:
 filesystem:
   root:
     uuid: "yolo"

we could wrap stuff that really should not be changed into something like:

  "read_only": {
    "kernel_command_line": "root=UUID=....",
    "mount_point": ...
  },
  "internal": { "partition_table" ...
}

and if you do:

otk.define.yolo:
 filesystem:
  read_only:
   mount_point:
    root: "yolo"

you have been warned? But maybe I'm overthinking it.

The "consumer" from (2) would consumer the "internal" representation of the generated values and we would make sure that they are always in sync (which is natural because they would just use (initially) the "naked" disk.PartitionTable serialized.

Does this make sense?

@achilleas-k
Copy link
Owner

we will have to make sure somehow (either by convention or lint or code) that some of these values are readonly

Is this specific to externals or a general otk mechanism?

@achilleas-k
Copy link
Owner

achilleas-k commented May 28, 2024

The "consumer" from (2) would consumer the "internal" representation of the generated values and we would make sure that they are always in sync (which is natural because they would just use (initially) the "naked" disk.PartitionTable serialized.

Does this make sense?

I'm not sure. At first I thought I understood which side you were calling the "consumer" and which the "producer" but then I realised both interpretation make sense from different points of view, so I'm lost.
I'm going to assume the producer of defines is an otk manifest (or otk itself, depending on how you look at it) and the consumer is the external, given the part below.

The producer of defines would generate a tree like:

otk.define.defaults:
 filesystem:
  otk.external.gen_partition_table:

How concretely are we speaking right now? I mean, is the external here being called with no options? Do we want that?

Here's how I'm thinking about this. There's a few levels that a partition table generator can operate on (in terms of abstraction). At the most abstract level, there's everything we support in the blueprints, so a partitioning mode + extra mountpoints. Though even that requires we have a base table with a default root filesystem and information on whether we need /boot or EFI. So the generator should at least take as input:

  • A partitioning mode (lvm, btrfs, raw, lvm+luks).
  • A base filesystem type (for / for non-btrfs).
  • Extra mountpoints (with sizes and potentially fileystem types).

But I think for this generator to be useful for our purposes (and the purposes of a distro maintainer), it should support taking an almost full description of a partition table and filling in the gaps. So, the boring stuff, like UUIDs, starts, and offsets, should be handled by the generator, but everything else I think should be defined in the inputs:

filesystem:
  otk.external.gen_partition_table:
    options:
      uefi: 
        size: "1 GiB"
      bios: true
      type: "gpt"
      size: "10 GiB"
    partitions:
      - name: root
        mountpoint: "/"
        label: "root"
        size: "7 GiB"
        type: "ext4"
      - name: home
        mountpoint: "/home"
        label: "home"
        size: "2 GiB"
        type: "ext4"

I think this is the simplest input we can get away with. The bios and uefi flags make the generator add the BIOS boot and EFI partitions respectively, and things like UUIDs, starts, and offsets are set automatically. The mount options and fstab options are set to the defaults, though we can support them being added from the input. Same for sector size.

In this scenario, a btrfs input would look like:

filesystem:
  otk.external.gen_partition_table:
    options:
      uefi: 
        size: "1 GiB"
      bios: true
      type: "gpt"
      size: "10 GiB"
    partitions:
      disk:
        size: "9 GiB"
        type: "btrfs"
        subvolumes:
          - name: root
            path: "/subvol/root"
            mountpoint: "/"
            label: "root"
            size: "7 GiB"
            type: "btrfs" (or None)
          - name: home
            path: "/subvol/home"
            mountpoint: "/home"
            label: "home"
            size: "2 GiB"
            type: "btrfs" (or None)

and lvm would be something like:

filesystem:
  otk.external.gen_partition_table:
    options:
      uefi: 
        size: "1 GiB"
      bios: true
      type: "gpt"
      size: "10 GiB"
    partitions:
      disk:
        size: "9 GiB"
        type: "lvm"
        volume-groups:
          vgname:
            logical-volumes:
              - name: root
                mountpoint: "/"
                size: "7 GiB"
                type: "ext4"
              - name: 
                mountpoint: "/home"
                size: "2 GiB"
                type: "ext4"

I think anything more abstract than this and we take away too much power from the image definitions. We need this level of granularity (if not more), otherwise we're putting too much information into the generator, which I'm not sure I'm comfortable with.

Perhaps I'm too familiar with the current state though where each distribution defines its own partition tables almost entirely and the disk package is just responsible for filling in the blanks and sector alignment.

The output can certainly be adapted to fit what we need in the stages. We're certainly going to need all the information that the internal representation defines, but we might get away with flattening some things (for example, the distinction between a partition and the "payload" filesystem on the partition).

EDIT: Converted partition listings in examples to arrays (from objects) and changed uefi to a size from a boolean.

EDIT2: moved top-level stuff under options:

@achilleas-k
Copy link
Owner

achilleas-k commented May 28, 2024

Also, keep in mind, there's a lot of situations (in my example at least) where we choose to use objects/maps instead of arrays. We should avoid this because the order matters for a lot of things. The order of the input for one defines how the disk is laid out and that should be something the manifest should be able to define. Then there's the order of things in the output. The generator will return things to otk that it will have to use to define osbuild mounts, and those must be done in a certain order too (alphabetical ordering by mountpoint turns out to be a good way to order them). Given what we have now in otk, I doubt we'll be able to manipulate the output to do the ordering in the manifest generator, so the partition table generator should be aware of this.

@mvo5
Copy link
Author

mvo5 commented May 29, 2024

Fwiw, the "consumer" is this example above would be something like:

otk.target.osbuild:
pipeline:
 disk:
  otk.external.make_truncate_stage:
   partition_table: ${filesystems.internal.partition_table}

@achilleas-k
Copy link
Owner

achilleas-k commented May 29, 2024

otk.external.make_mkfs_stages: ${filesystems.const.partition_table}

org.osbuild.copy:
  options:
    source: "mount://root/"
    destination: "tree:///"
  otk.external.make_stage_devices_mounts: ${filesystems.const.partition_table}

This is very much aligned with how things are done in images now. Naming conventions (make_ vs gen_) can be decided later.

Explanation of const here: osbuild/otk#101

@achilleas-k achilleas-k merged commit 43d5393 into achilleas-k:cmd/partition-table May 29, 2024
1 check passed
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.

2 participants