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 proof of concept for building with osbuild #3643

Merged
merged 1 commit into from
Oct 28, 2023

Conversation

dustymabe
Copy link
Member

This is proof of concept code with many things hardcoded in the coreos.osbuild.mpp.yaml that need to become more dynamically defined. To use this you can update the image.yaml to add use-osbuild: true to force use of this for the cosa buildextend-qemu command.

src/image-default.yaml Outdated Show resolved Hide resolved
mkdir /root/osbuild && cd /root/osbuild

# Use osbuild from the dusty-fedora-coreos fork for now
git clone https://github.com/dustymabe/osbuild.git --branch dusty-fedora-coreos .
Copy link
Member

Choose a reason for hiding this comment

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

Seems better to do this in build.sh and statically install in cosa, no?

Copy link
Member Author

Choose a reason for hiding this comment

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

yes - I meant to open this PR as draft (switching now). I think the goal here is to get something out there to show people the approach and start discussion (also they can start hacking). I think before we merge this I'd want to get a few more PRs merged into OSBuild and then just bake those patches in here rather than doing a git-clone.

@dustymabe
Copy link
Member Author

The enablement PR merged osbuild/osbuild#1402

After tomorrow's release of OSBuild I should be able to modify this PR to just use the OSBuild from RPMs (along with a very small patch to enable running with our software in COSA as the buildroot) and mark it as ready for review.

@dustymabe
Copy link
Member Author

The enablement PR merged osbuild/osbuild#1402

Ironically enough the enablement PR enables deploying via container, which we don't leverage here just yet but tomorrow's release will include several of our recent PRs that we do need for this.

@dustymabe
Copy link
Member Author

ok we are now using the OSBuild RPMs with two patches.. One of them is in OSBuild upstream already, but not in a release, and the other will be obsoleted once we create a bootupd stage in OSBuild and start using that.

Comment on lines +26 to +27
# XXX: Dynamically set this size in the future
size: 4194304
Copy link
Member

Choose a reason for hiding this comment

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

Yes, in the future this will should be guess-timated from the input container image size, as is done in cosa today.

- type: org.osbuild.ignition
- type: org.osbuild.ostree.deploy
options:
osname: fedora-coreos
Copy link
Member

Choose a reason for hiding this comment

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

(Minor cleanup to do in future; have stateroot automatically use the one that is already present, assuming exactly one which is really the expected normal case)

Copy link
Member Author

Choose a reason for hiding this comment

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

you mean inside OSBuild?

options:
deployment:
osname: fedora-coreos
ref: ostree/1/1/0
Copy link
Member

Choose a reason for hiding this comment

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

This bit is actually poking into what I'd call an ostree implementation detail. Probably another case of "assume exactly one deployment and use its commit digest".

Copy link
Member Author

Choose a reason for hiding this comment

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

yeah, the reason I did this is because for the deploy-via-container case there is no easy ref: we could find to use. For what we are doing here, though, we can just use the ref we are already passing in and just use ref: $ref here.

I think what you are suggesting, though, is that OSBuild be modified to make specifying an osname/ref optional and just "do the right thing" in the most common case where there is only one deployment.

bootfs:
label: boot
uefi:
vendor: fedora
Copy link
Member

Choose a reason for hiding this comment

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

Why is this hardcoded versus auto-detecting? That's just going to completely break use cases of centos vs rhel vs alma etc.

I guess that gets fixed by bootupd implicitly...

Copy link
Member Author

Choose a reason for hiding this comment

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

Why is this hardcoded versus auto-detecting? That's just going to completely break use cases of centos vs rhel vs alma etc.

I'm mostly copying other examples here. Maybe we could ask over in https://matrix.to/#/#image-builder:fedoraproject.org ?

I guess that gets fixed by bootupd implicitly...

Correct. I think this stage would no longer be used by us.

This is proof of concept code with many things hardcoded in the
coreos.osbuild.mpp.yaml that need to become more dynamically defined.
To use this you can set the COSA_USE_OSBUILD env var to have a value.
COSA_USE_OSBUILD=1 should work just fine.
@dustymabe dustymabe force-pushed the dusty-coreos-osbuild branch from 68cef2b to 74f9b0b Compare October 27, 2023 02:16
@dustymabe
Copy link
Member Author

Tested this in staging again and it seems to be working. Lifting draft on this.

Note that no behavior changes here. It's opt-in and we'll opt-in in the pipeline using coreos/fedora-coreos-pipeline#921

@dustymabe dustymabe marked this pull request as ready for review October 27, 2023 03:00
@cgwalters cgwalters merged commit 5a1bd04 into coreos:main Oct 28, 2023
2 checks 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