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

stages/ostree.deploy.container: allow deploying from container #1402

Merged
merged 4 commits into from
Oct 24, 2023

Conversation

dustymabe
Copy link
Contributor

@dustymabe dustymabe commented Oct 12, 2023

The ostree.deploy stage now accepts either a ostree input:

      - type: org.osbuild.ostree.deploy
        options:
          osname: fedora-coreos
          remote: fedora
          mounts:
            - /boot
            - /boot/efi
          kernel_opts:
            - rw
            - console=tty0
            - console=ttyS0
            - ignition.platform.id=qemu
            - '$ignition_firstboot'
        inputs:
          commits:
            type: org.osbuild.ostree
            origin: org.osbuild.source
            mpp-resolve-ostree-commits:
              commits:
                - ref: fedora/x86_64/coreos/stable
                  remote:
                    url: https://kojipkgs.fedoraproject.org/ostree/repo/

or a containers input:

      - type: org.osbuild.ostree.deploy.container
        options:
          osname: fedora-coreos
          target_imgref: ostree-remote-registry:fedora:quay.io/fedora/fedora-coreos:stable
          mounts:
            - /boot
            - /boot/efi
          kernel_opts:
            - rw
            - console=tty0
            - console=ttyS0
            - ignition.platform.id=qemu
            - '$ignition_firstboot'
        inputs:
          images:
            type: org.osbuild.containers
            origin: org.osbuild.source
            mpp-resolve-images:
              images:
                - source: registry.gitlab.com/redhat/services/products/image-builder/ci/images/fedora-coreos
                  tag: stable

In the containers input case we will run ostree container image deploy wheras for ostree commits input we will continue to execute ostree admin deploy.

Co-authored-by: Dusty Mabe dusty@dustymabe.com and Luke Yang @lukewarmtemp

@dustymabe
Copy link
Contributor Author

dustymabe commented Oct 12, 2023

Marking as draft since this builds on top of #1399 (IOW only review the last two commits).

@dustymabe dustymabe force-pushed the dusty-ostree-deploy-container branch from 43fb6c6 to 290de7c Compare October 12, 2023 20:29
@dustymabe dustymabe force-pushed the dusty-ostree-deploy-container branch 2 times, most recently from defb5bf to ae894fa Compare October 13, 2023 19:47
@cgwalters
Copy link
Contributor

One thing we will lose vs current FCOS-and-derivatives is https://github.com/coreos/fedora-coreos-tracker/blob/main/internals/README-internals.md#aleph-version right?

We are actually planning potentially grow at least test-level dependencies on this for OCP as part of bootimage update work. (A while ago I did openshift/machine-config-operator#2994 which is currently just informative)

I think we'll just have to keep this in mind? But maybe it would make sense to have an aleph stage.

Incidentally, bootc has its own aleph which has less stuff basically for the same reason as here - the input is a container image.

@dustymabe
Copy link
Contributor Author

I think we'll just have to keep this in mind? But maybe it would make sense to have an aleph stage.

Indeed. I would look at this PR as the start. There's a whole host of other things that need to be tidied up or added (one example is a bootupd stage) that will get unlocked after this merges.

@cgwalters
Copy link
Contributor

Indeed. I would look at this PR as the start. There's a whole host of other things that need to be tidied up or added (one example is a bootupd stage) that will get unlocked after this merges.

Not to go too far off but...as I've been thinking about things here I would like to consider that we add a bootc install-to-filesystem stage actually. That would drive us to embed the default base partitioning configuration inside the OS image, which is what I think we want long term.

(If for example I want to change the default for my OS installs from xfs to ext4 or so, that'd just be flipping https://github.com/containers/bootc/blob/c9f224cbb2d3fc3040410125c234c30e8b322cb9/lib/src/install/00-defaults.toml#L3 in a drop-in in my container build)

The reason I mention this is because bootc install-to-filesystem will also take care of the bootupd stuff too.

stages/org.osbuild.ostree.deploy Outdated Show resolved Hide resolved
stages/org.osbuild.ostree.deploy Outdated Show resolved Hide resolved
@dustymabe
Copy link
Contributor Author

Not to go too far off but...as I've been thinking about things here I would like to consider that we add a bootc install-to-filesystem stage actually.

I'd like to keep the scope of this to be small if we can and focus on the EPIC we're currently working on. Maybe open an issue to this for the next EPIC in this OSBuild series?

@hh
Copy link

hh commented Oct 17, 2023

Yeah this merged! #1399

Ref #1402 (comment)

Is there anything we can do to test or move this forward? Happy to help!

@dustymabe dustymabe force-pushed the dusty-ostree-deploy-container branch from ae894fa to f742e5e Compare October 18, 2023 02:02
@dustymabe
Copy link
Contributor Author

Did a new upload here with some fixes and rebased on top of latest main branch.

I don't know if we want the last commit in this patchset to merge in (cc @achilleas-k).

stages/org.osbuild.ostree.deploy Outdated Show resolved Hide resolved
stages/org.osbuild.ostree.deploy Show resolved Hide resolved
stages/org.osbuild.ostree.deploy Outdated Show resolved Hide resolved
@achilleas-k
Copy link
Member

Looking at the stage in its final form here I'm starting to wonder if it would be a better idea to split it into a separate one org.osbuild.ostree.container.deploy. That way we could avoid conditional behaviour based on input type, ref being needed only for one input type, the whole target and verification discussion above, etc. A separate stage would have some minor duplicate code (the main() part of the current stage) but the freedom to redefine the schema would let us communicate the purpose of all the options more easily, like having a target-imgref option instead of remote.

@achilleas-k
Copy link
Member

@dustymabe had a chat with some people from the team and we've agreed that it makes more sense in terms of osbuild's design for this to be a separate stage. Like I mentioned yesterday:

  • It gets rid of automagic behaviour that switches based on input type, which isn't something we've done before and is hard to communicate.
  • The easiest stages to work with are generally the ones that are thin wrappers around the command they shell out to.
  • The stage options for the two cases are different enough that it makes it simpler to verify with two separate schemas, rather than having things like oneOf, inter-option dependencies, and options with duplicate meaning.

Also, as a correction to my previous comment, the name for the new stage should be org.osbuild.ostree.deploy.container. That way, the stage gets namespaced under the deploy command, making it clear that it's associated (or an alternative to) the original org.osbuild.ostree.deploy stage.

@dustymabe
Copy link
Contributor Author

Sound good @achilleas-k, we'll try to accommodate.

@dustymabe
Copy link
Contributor Author

OK. We added a new push here that creates a new org.osbuild.ostree.deploy.container stage.

@dustymabe dustymabe changed the title stages/ostree.deploy: accept containers input to deploy stages/ostree.deploy.container: allow deploying from container Oct 20, 2023
@dustymabe dustymabe force-pushed the dusty-ostree-deploy-container branch from 961d070 to 1e8776a Compare October 20, 2023 15:29
@achilleas-k achilleas-k self-requested a review October 20, 2023 17:16
Copy link
Member

@achilleas-k achilleas-k left a comment

Choose a reason for hiding this comment

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

This is great. It's much easier to read and understand the stage now that it's its own thing.
I have a couple of inline comments but also one more general one: since the org.osbuild.ostree.deploy stage isn't affected by this PR, can you revert the changes to it to avoid any confusion?
I'd also like the git history to be cleaned up to only have the introduction of the new stage and tests.

Thanks!

stages/org.osbuild.ostree.deploy.container Outdated Show resolved Hide resolved
stages/org.osbuild.ostree.deploy.container Outdated Show resolved Hide resolved
@dustymabe
Copy link
Contributor Author

I have a couple of inline comments but also one more general one: since the org.osbuild.ostree.deploy stage isn't affected by this PR, can you revert the changes to it to avoid any confusion?

My preference is that we keep the changes to the deploy stage primarily to keep the deploy and deploy.container stages diff-able. Right now it's really easy to vimdiff the two files and see exactly where they differ.

I honestly prefer keeping the code in a single stage, but if we have to break them out preserving this diff-able ability I think makes it easier to manage. WDYT?

I'd also like the git history to be cleaned up to only have the introduction of the new stage and tests.

Yes. We can simplify things here. The reason we didn't squash to begin with: I was thinking maybe we keep the git history around in case we change our minds in the future and want the deploy stage to accept ostree or container as input.

achilleas-k added a commit to achilleas-k/images that referenced this pull request Oct 30, 2023
Creates an org.osbuild.ostree.deploy.container stage.
Validates the options with a regular expression that matches the schema.
Takes a container as input.  Validates the length of the input
references to be exactly 1.

See osbuild/osbuild#1402
achilleas-k added a commit to achilleas-k/images that referenced this pull request Nov 3, 2023
Creates an org.osbuild.ostree.deploy.container stage.
Validates the options with a regular expression that matches the schema.
Takes a container as input.  Validates the length of the input
references to be exactly 1.

See osbuild/osbuild#1402
achilleas-k added a commit to achilleas-k/images that referenced this pull request Nov 3, 2023
Creates an org.osbuild.ostree.deploy.container stage.
Validates the options with a regular expression that matches the schema.
Takes a container as input.  Validates the length of the input
references to be exactly 1.

See osbuild/osbuild#1402
achilleas-k added a commit to achilleas-k/images that referenced this pull request Nov 6, 2023
Creates an org.osbuild.ostree.deploy.container stage.
Validates the options with a regular expression that matches the schema.
Takes a container as input.  Validates the length of the input
references to be exactly 1.

See osbuild/osbuild#1402
achilleas-k added a commit to achilleas-k/images that referenced this pull request Nov 6, 2023
A new image functions called coreosImage() that requires a container
source and creates a qcow2 image from a CoreOS ostree container.
Based on the example manifest added in osbuild/osbuild#1402
achilleas-k added a commit to achilleas-k/images that referenced this pull request Nov 7, 2023
Creates an org.osbuild.ostree.deploy.container stage.
Validates the options with a regular expression that matches the schema.
Takes a container as input.  Validates the length of the input
references to be exactly 1.

See osbuild/osbuild#1402
achilleas-k added a commit to achilleas-k/images that referenced this pull request Nov 7, 2023
A new image functions called coreosImage() that requires a container
source and creates a qcow2 image from a CoreOS ostree container.
Based on the example manifest added in osbuild/osbuild#1402
@ericcurtin
Copy link
Contributor

ericcurtin commented Nov 8, 2023

So testing this via a Silverblue Asahi Remix I'm working on, it fails at this point:

⏱  Duration: 0s
org.osbuild.ostree.deploy.container: db4a447eda1413d32197f49e4fbcbb619e2283d0b73bb6fafdd218509f42d6e1 {
  "osname": "silverblue-asahi",
  "target_imgref": "ostree-remote-registry:quay.io/ecurtin/silverblue-asahi:39",
  "mounts": [
    "/boot",
    "/boot/efi"
  ],
  "kernel_opts": [
    "rw",
    "console=tty0",
    "console=ttyS0",
    "ignition.platform.id=qemu",
    "$ignition_firstboot"
  ]
}
/usr/lib/tmpfiles.d/journal-nocow.conf:26: Failed to replace specifiers in '/var/log/journal/%m': No such file or directory
/usr/lib/tmpfiles.d/systemd.conf:23: Failed to replace specifiers in '/run/log/journal/%m': No such file or directory
/usr/lib/tmpfiles.d/systemd.conf:25: Failed to replace specifiers in '/run/log/journal/%m': No such file or directory
/usr/lib/tmpfiles.d/systemd.conf:26: Failed to replace specifiers in '/run/log/journal/%m/*.journal*': No such file or directory
/usr/lib/tmpfiles.d/systemd.conf:29: Failed to replace specifiers in '/var/log/journal/%m': No such file or directory
/usr/lib/tmpfiles.d/systemd.conf:30: Failed to replace specifiers in '/var/log/journal/%m/system.journal': No such file or directory
/usr/lib/tmpfiles.d/systemd.conf:32: Failed to replace specifiers in '/var/log/journal/%m': No such file or directory
/usr/lib/tmpfiles.d/systemd.conf:33: Failed to replace specifiers in '/var/log/journal/%m/system.journal': No such file or directory
ostree container image deploy --imgref=ostree-unverified-image:dir:/tmp/tmpae4toj1l/image --stateroot=silverblue-asahi --target-imgref=ostree-remote-registry:quay.io/ecurtin/silverblue-asahi:39 --karg=rw --karg=console=tty0 --karg=console=ttyS0 --karg=ignition.platform.id=qemu --karg=$ignition_firstboot --sysroot=/run/osbuild/tree
error: Performing deployment: Deploying tree: opendir(var): No such file or directory
Traceback (most recent call last):
  File "/run/osbuild/bin/org.osbuild.ostree.deploy.container", line 139, in <module>
    r = main(stage_args["tree"],
        ^^^^^^^^^^^^^^^^^^^^^^^^
  File "/run/osbuild/bin/org.osbuild.ostree.deploy.container", line 134, in main
    ostree_container_deploy(tree, inputs, osname, target_imgref, kopts)
  File "/run/osbuild/bin/org.osbuild.ostree.deploy.container", line 108, in ostree_container_deploy
    ostree.cli("container", "image", "deploy",
  File "/run/osbuild/lib/osbuild/util/ostree.py", line 200, in cli
    subprocess.run(["ostree"] + args,
  File "/usr/lib64/python3.11/subprocess.py", line 571, in run
    raise CalledProcessError(retcode, process.args,
subprocess.CalledProcessError: Command '['ostree', 'container', 'image', 'deploy', '--imgref=ostree-unverified-image:dir:/tmp/tmpae4toj1l/image', '--stateroot=silverblue-asahi', '--target-imgref=ostree-remote-registry:quay.io/ecurtin/silverblue-asahi:39', '--karg=rw', '--karg=console=tty0', '--karg=console=ttyS0', '--karg=ignition.platform.id=qemu', '--karg=$ignition_firstboot', '--sysroot=/run/osbuild/tree']' returned non-zero exit status 1.

⏱  Duration: 17s

Failed

it's probably more correct to report this in the ostree space, but just leaving it here for awareness.

@cgwalters
Copy link
Contributor

error: Performing deployment: Deploying tree: opendir(var): No such file or directory
ostreedev/ostree#3073
➡️ your stateroot doesn't exist

(BTW, I would just use default for osname/stateroot in general)

@ericcurtin
Copy link
Contributor

Thanks I'll try that.

achilleas-k added a commit to achilleas-k/images that referenced this pull request Nov 8, 2023
A new image functions called coreosImage() that requires a container
source and creates a qcow2 image from a CoreOS ostree container.
Based on the example manifest added in osbuild/osbuild#1402
achilleas-k added a commit to achilleas-k/images that referenced this pull request Nov 9, 2023
Creates an org.osbuild.ostree.deploy.container stage.
Validates the options with a regular expression that matches the schema.
Takes a container as input.  Validates the length of the input
references to be exactly 1.

See osbuild/osbuild#1402
achilleas-k added a commit to achilleas-k/images that referenced this pull request Nov 9, 2023
A new image functions called coreosImage() that requires a container
source and creates a qcow2 image from a CoreOS ostree container.
Based on the example manifest added in osbuild/osbuild#1402
achilleas-k added a commit to achilleas-k/images that referenced this pull request Nov 10, 2023
Creates an org.osbuild.ostree.deploy.container stage.
Validates the options with a regular expression that matches the schema.
Takes a container as input.  Validates the length of the input
references to be exactly 1.

See osbuild/osbuild#1402
achilleas-k added a commit to achilleas-k/images that referenced this pull request Nov 10, 2023
A new image functions called coreosImage() that requires a container
source and creates a qcow2 image from a CoreOS ostree container.
Based on the example manifest added in osbuild/osbuild#1402
achilleas-k added a commit to achilleas-k/images that referenced this pull request Nov 20, 2023
Creates an org.osbuild.ostree.deploy.container stage.
Validates the options with a regular expression that matches the schema.
Takes a container as input.  Validates the length of the input
references to be exactly 1.

See osbuild/osbuild#1402
achilleas-k added a commit to achilleas-k/images that referenced this pull request Nov 20, 2023
A new image functions called coreosImage() that requires a container
source and creates a qcow2 image from a CoreOS ostree container.
Based on the example manifest added in osbuild/osbuild#1402
achilleas-k added a commit to achilleas-k/images that referenced this pull request Nov 24, 2023
Creates an org.osbuild.ostree.deploy.container stage.
Validates the options with a regular expression that matches the schema.
Takes a container as input.  Validates the length of the input
references to be exactly 1.

See osbuild/osbuild#1402
achilleas-k added a commit to achilleas-k/images that referenced this pull request Nov 24, 2023
Creates an org.osbuild.ostree.deploy.container stage.
Validates the options with a regular expression that matches the schema.
Takes a container as input.  Validates the length of the input
references to be exactly 1.

See osbuild/osbuild#1402
achilleas-k added a commit to achilleas-k/images that referenced this pull request Nov 27, 2023
Creates an org.osbuild.ostree.deploy.container stage.
Validates the options with a regular expression that matches the schema.
Takes a container as input.  Validates the length of the input
references to be exactly 1.

See osbuild/osbuild#1402
achilleas-k added a commit to achilleas-k/images that referenced this pull request Nov 27, 2023
Creates an org.osbuild.ostree.deploy.container stage.
Validates the options with a regular expression that matches the schema.
Takes a container as input.  Validates the length of the input
references to be exactly 1.

See osbuild/osbuild#1402
achilleas-k added a commit to achilleas-k/images that referenced this pull request Nov 27, 2023
Creates an org.osbuild.ostree.deploy.container stage.
Validates the options with a regular expression that matches the schema.
Takes a container as input.  Validates the length of the input
references to be exactly 1.

See osbuild/osbuild#1402
achilleas-k added a commit to achilleas-k/images that referenced this pull request Nov 27, 2023
Creates an org.osbuild.ostree.deploy.container stage.
Validates the options with a regular expression that matches the schema.
Takes a container as input.  Validates the length of the input
references to be exactly 1.

Also added tests for the validators for options and inputs.

See osbuild/osbuild#1402
achilleas-k added a commit to achilleas-k/images that referenced this pull request Nov 27, 2023
Creates an org.osbuild.ostree.deploy.container stage.
Validates the options with a regular expression that matches the schema.
Takes a container as input.  Validates the length of the input
references to be exactly 1.

Also added tests for the validators for options and inputs.

See osbuild/osbuild#1402
achilleas-k added a commit to achilleas-k/images that referenced this pull request Nov 27, 2023
Creates an org.osbuild.ostree.deploy.container stage.
Validates the options with a regular expression that matches the schema.
Takes a container as input.  Validates the length of the input
references to be exactly 1.

Also added tests for the validators for options and inputs.

See osbuild/osbuild#1402
github-merge-queue bot pushed a commit to osbuild/images that referenced this pull request Nov 28, 2023
Creates an org.osbuild.ostree.deploy.container stage.
Validates the options with a regular expression that matches the schema.
Takes a container as input.  Validates the length of the input
references to be exactly 1.

Also added tests for the validators for options and inputs.

See osbuild/osbuild#1402
achilleas-k added a commit to achilleas-k/images that referenced this pull request Nov 28, 2023
Creates an org.osbuild.ostree.deploy.container stage.
Validates the options with a regular expression that matches the schema.
Takes a container as input.  Validates the length of the input
references to be exactly 1.

Also added tests for the validators for options and inputs.

See osbuild/osbuild#1402
mvo5 pushed a commit to mvo5/images that referenced this pull request Dec 1, 2023
Creates an org.osbuild.ostree.deploy.container stage.
Validates the options with a regular expression that matches the schema.
Takes a container as input.  Validates the length of the input
references to be exactly 1.

See osbuild/osbuild#1402
mvo5 pushed a commit to mvo5/images that referenced this pull request Dec 1, 2023
A new image functions called coreosImage() that requires a container
source and creates a qcow2 image from a CoreOS ostree container.
Based on the example manifest added in osbuild/osbuild#1402
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

7 participants