-
Notifications
You must be signed in to change notification settings - Fork 183
build via container cleanups #4316
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
build via container cleanups #4316
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request introduces a series of cleanups and fixes for the container-based build process. The changes are well-motivated and improve the robustness and maintainability of the build scripts.
Key improvements include:
- Fixing how the
NAMEvariable is sourced from argument files to handle non-standard bash syntax. - Cleaning up override repository metadata more reliably to prevent build errors.
- Removing a hardcoded repository path in
import_oci_archiveby passing it as an argument, which resolves an issue with transienttmp/directories. - Generating the OCI manifest from the ociarchive directly to correctly handle manifest lists.
My review includes one suggestion for src/cmd-build-with-buildah to use a more robust method for checking file existence. Overall, these are solid improvements.
Operate on the ociarchive directly rather than args.srcimg because otherwise the manifest we fetch with skopeo *could* be a manifest list. Fixes coreos#4310
Somehow hardcoding `tmp/repo` here isn't actually working. Here's
what happens if I blow away my `tmp/` and then attempt to do another
operation (like `cosa osbuild qemu`):
```
Wrote: ostree-unverified-image:oci-archive:/srv/tmp/cosa-import-op0cg6_c/out.ociarchive => e202ee3cb8a2a8d814bd2b325cdef5500459fd2381d4fa766cd15ab730ae0e80
2273 metadata, 8746 content objects imported; 1.2 GB content written
1471 metadata, 1534 content objects imported; 133.6 MB content written
Imported OCI image as build 42.20250821.dev.0
Pruning build 44.20250826.dev.0
Deleting 5 blob refs
[coreos-assembler]$
[coreos-assembler]$ rm -rf tmp/*
[coreos-assembler]$ cosa osbuild qemu
Config commit: e5f88e72120b3f89cf2c4d81b45bbe791d9bd79d
Using manifest: /srv/src/config/manifest.yaml
Will build qemu
Extracting e202ee3cb8a2a8d814bd2b325cdef5500459fd2381d4fa766cd15ab730ae0e80
layers already present: 0; layers needed: 66 (975.9 MB)
3725 metadata, 21742 content objects imported; 1.7 GB content written 3
2603 metadata, 10508 content objects imported; 250.5 MB content written
error: No such metadata object e202ee3cb8a2a8d814bd2b325cdef5500459fd2381d4fa766cd15ab730ae0e80.commit
Traceback (most recent call last):
File "<string>", line 10, in <module>
cmdlib.import_ostree_commit(workdir, builddir, buildmeta, extract_json=('1' == '1'))
~~~~~~~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/usr/lib/coreos-assembler/cosalib/cmdlib.py", line 362, in import_ostree_commit
extract_image_json(workdir, commit)
~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^
File "/usr/lib/coreos-assembler/cosalib/cmdlib.py", line 272, in extract_image_json
raise Exception("Failed to extract image.json")
Exception: Failed to extract image.json
failed to execute cmd-osbuild: exit status 1
```
Let's just have the caller explicitly tell us the path to the repo
we want to import into is.
If there are no rpms then let's clean up the repo metadata if it exists which should prevent cosmetic warts like: ``` Updating and loading repositories: overrides ???% | 0.0 B/s | -1.0 B | ? >>> Curl error (37): Could not read a file:// file for file:///run/src/overrides >>> Usable URL not found ``` Or fatal ones like: ``` error: Installing packages: importing RPMs: failed to open /run/src/overrides/rpm/systemd-258~rc3-2.fc44.x86_64.rpm ```
In [1] we added `DESCRIPTION=Fedora CoreOS testing-devel` to build-args.conf, but this isn't exactly proper bash syntax. Bash would want `Fedora CoreOS testing-devel` to be in quotes, but `podman`/`docker` don't really support that in env file [2] [3] and I assume args file is the same. If I added quotes there I get "org.opencontainers.image.description": "\"Fedora CoreOS branched\"". [1] coreos/fedora-coreos-config@90109c4 [2] containers/podman#9446 [3] docker/for-linux#1208
1104bc4 to
7bbb24a
Compare
|
PR was previously approved and I applied the code review comment. Merging. |
A few cleanups I've been carrying locally for a while. See individual commit messages: