cmd: make bootc-image-builder a multi-call binary of ibcli (HMS-9808) #374
Conversation
mvo5
force-pushed
the
merge-bib-multicall
branch
from
6ba4502 to
d84ece8
Compare
The existing code was checking if dnf could be run with --version to detect if dnf is installed at all. That is wrong, dnf can fail when it is just run with --version, e.g.: ``` Failed to open log file: /var/log/hawkey.log 4.14.0 ``` But errors like this should be surfaced to the user (which the next call to dnf will do). So instead of running dnf just run: ``` sh -c "command -v dnf" ``` to check if its available. This will fix the test failures in osbuild/image-builder-cli#374 Note that the detection is covered by an existing test so no new test needs adding.
The existing code was checking if dnf could be run with --version to detect if dnf is installed at all. That is wrong, dnf can fail when it is just run with --version, e.g.: ``` Failed to open log file: /var/log/hawkey.log 4.14.0 ``` But errors like this should be surfaced to the user (which the next call to dnf will do). So instead of running dnf just run: ``` sh -c "command -v dnf" ``` to check if its available. This will fix the test failures in osbuild/image-builder-cli#374 Note that the detection is covered by an existing test so no new test needs adding.
mvo5
force-pushed
the
merge-bib-multicall
branch
from
d84ece8 to
8d4a5cb
Compare
|
So like with
Do you mean there is a Nice. |
That is correct. Its is more lines than I would like but a lot is cobra setup which
Yeah, the goal would be that the bib containerfile essentially just does a |
Maybe we do not need a symlink but just a variable to specify the target, because we only distribute bib in a container, do we? But this is fine, symlink is sort of expected in these scenarios. |
The existing code was checking if dnf could be run with --version to detect if dnf is installed at all. That is wrong, dnf can fail when it is just run with --version, e.g.: ``` Failed to open log file: /var/log/hawkey.log 4.14.0 ``` But errors like this should be surfaced to the user (which the next call to dnf will do). So instead of running dnf just run: ``` sh -c "command -v dnf" ``` to check if its available. This will fix the test failures in osbuild/image-builder-cli#374 Note that the detection is covered by an existing test so no new test needs adding.
Makefile
Outdated
| .PHONY: build | ||
| build: $(BUILDDIR)/bin/ ## build the binary from source | ||
| go build -ldflags="-X main.version=${VERSION}" -o $<image-builder ./cmd/image-builder/ | ||
| (cd ./cmd/image-builder && ln -sf image-builder bootc-image-builder) |
There was a problem hiding this comment.
@lzap you mean this symlink here in your comment in #374 (comment) ? If so the idea behind this one was that it would be nice to provide the bib binary everywhere where we have ibcli so that its easier for users to switch - but thinking about this again maybe its not such a good ideas as we want people to move to bib and by making it too easy to just stick with bib we defeat this goal. Idk, happy to remove this again and reconsider.
There was a problem hiding this comment.
Yes, I meant this link.
Don't we only support bib via its container? Then introducing this link is actually a step back if we distribute it with CLI RPM and we might accidentally acquire more users of bib who were previously not exposed. That is why I thought just building differently named binary would be enough. In fact, if bib is only consumed via podman, then we could just introduce an environment variable because the name of the binary does not matter as it is never directly used it is docker or podman what is typed in.
There was a problem hiding this comment.
Yeah, I think this link here should go away for the reasons you outline.
mvo5
force-pushed
the
merge-bib-multicall
branch
3 times, most recently
from
f8c277c to
14d57d8
Compare
mvo5
requested review from
croissanne and
lzap
and removed request for
a team
avitova
left a comment
avitova
left a comment
There was a problem hiding this comment.
I swear I had a different issue I found, so I started reviewing, but then I understood how this works and found out it is not an issue. So here I submit a review with a typo I found.
schutzbot
changed the title
schutzbot
added
the
🌟 best practice
Co-Authored-By: Michael Vogt <[email protected]>
awscloud.NewDefault() already supports authentication via AWS_ACCESS_KEY_ID, credential file etc. No need for special cases or special command line options here.
In the future it will very likely be useful for us to have other verbs. For now, this is effectively an implementation detail though because the entrypoint is changed to always pass `build`. (Actually right now this fixes the problem that the `completion` verb was being masked) Signed-off-by: Colin Walters <[email protected]>
It's just ugly to have bits of this in shell script in this way; it's not complex code and doing it in Go helps keep the development environment simpler. Signed-off-by: Colin Walters <[email protected]>
With the merge of osbuild/images#1886 the handling of the lorax templates changed. Ideally we would follow images here closer and use the distro YAML loader to load the metadata from images:data/distrodefs. But as a quick fix this just duplicates the logic we had before.
Workaround to prevent ISOs from failing to boot when used as disk images in combination with UEFI [1] [1]: osbuild/images#1947 (comment) Signed-off-by: Simon de Vlieger <[email protected]>
In images the Lorax templates are a struct since 0.206.0 [1]. [1]: osbuild/images#1949 Signed-off-by: Simon de Vlieger <[email protected]>
This reverts commit 7343bca07349f1d17be3b5d1130cc33f039ff7cb. When a new release of images is merged that contains [1] we can push this revert through after verification that the problem remains gone. [1]: osbuild/images#1949
This is a regression test to ensure the commandline handling of aws upload is correct. Its a followup for osbuild/bootc-image-builder#1030
The NewAnacondaContainerInstaller got renamed to NewAnacondaContainerInstallerLegacy.
This commit adds support for the new `bootc-installer` image type that will take a bootc container and create an ISO out of it. It also adds a new `--installer-payload-ref` option so that the user can specify a different payload container to install. See osbuild/images#1906 for details. This is the equivalent of osbuild#341 for bootc-image-builder and allows us to build these kinds of images with bib now too.
When adding new image types like `vhd` or the coming `gcp` I noticed that we currently need to modify two different places. This package consolidates all knowledge about image types, their exports and if they are ISO or disk into a single package. This should make it easier to expand the supported image types.
This commit uses the new `imagetypes` package and replaces the use of `BuildType` with the new `ImageTypes` to determine what is available, valid, what to export and if it's an ISO.
This commit adds a new image type `gce` that contains a tar file with the raw image inside. This can then be imported into GCE.
We want to move into a world where we build the anaconda image from bootc containers instead of our current RPM based construction [0] so lets mark the rpm based installer ISO image types as legacy. This should make it easy to support a potential new `bootc-iso` or `bootc-installer` image type [0] while still supporting the legacy mode for a while.
This commit exposes the new `ova` image type and adds a basic smoke test.
This commit adds support for the new `bootc-installer` image type that will take a bootc container and create an ISO out of it. It also adds a new `--installer-payload-ref` option so that the user can specify a different payload container to install. See osbuild/images#1906 for details. This is the equivalent of osbuild#341 for bootc-image-builder and allows us to build these kinds of images with bib now too.
Just use os.WriteFile() intead of reimplementing it.
Drop legacy_iso.go and use the images library to build the image.
This commit move the cmd/bootc-image-builder code into cmd/image-builder and changes `main()` so that bootc-image-builder becomes a multi-call binary. When image-builder is called with argv[0] as bootc-image-builder it will use the cobra commandline and code from bootc-image-builder. There is still a bit of duplication in the manifest generation and building between bib_main.go and main.go but that is hard to avoid if we want the "new" bib to be as close as possible to the existing one (e.g. we do things like "canChownInPath()" in bib which we do not need in ibcli). One important difference between the two is how `anaconda-iso` image types for bootc handle the repositories. The `bib` personality stays the same as the orignal, i.e. it will use the container repositories for depsolving. This require some complicated mTLS handling that does not map well into ibcli. Here we do not do that and instead use our own repos for the detected distro. I.e. if the detected distro is fedora we build the (rpm) installer from out fedora repos. We can strive to unify this and add (not so nice) code into ibcli to support this *or* we just warn in ibcli if bootc with anaconda-iso is used that this behaves differently from bib and that people should use bib if they need the old behavior (I would prefer that). I hope then we can phase out this rpm based bootc anaconda-iso in favor of the container based bootc-installer type.
mvo5
force-pushed
the
merge-bib-multicall
branch
from
14d57d8 to
7d151ef
Compare
20 participants
[this is now used in https://github.com/osbuild/bootc-image-builder/pull/1157 and green there]
This PR move the bootc-image-builder code into
cmd/image-builder and changes
main()so that image-builderbecomes a multi-call binary. When image-builder is called with
argv[0] as bootc-image-builder it will use the cobra commandline
and code from bootc-image-builder.
There is still a bit of duplication in the manifest generation
and building between bib_main.go and main.go but that is hard
to avoid if we want the "new" bib to be as close as possible
to the existing one (e.g. we do things like "canChownInPath()"
in bib which we do not need in ibcli).
One important difference between the two is how
anaconda-isoimage types for bootc handle the repositories. The
bibpersonalitystays the same as the orignal, i.e. it will use the container
repositories for depsolving. This require some complicated mTLS
handling that does not map well into ibcli. Here we do not do
that and instead use our own repos for the detected distro. I.e.
if the detected distro is fedora we build the (rpm) installer from
out fedora repos. We can strive to unify this and add (not so nice)
code into ibcli to support this or we just warn in ibcli if
bootc with anaconda-iso is used that this behaves differently
from bib and that people should use bib if they need the old
behavior (I would prefer that). I hope then we can phase out
this rpm based bootc anaconda-iso in favor of the container
based bootc-installer type.
This is osbuild/bootc-image-builder#1131
but directly in ibcli instead of doing the bib intermediate step.
The advantage is that we can "freeze" the bib repo and if we
run into issues with the transition have the last known good
bib code. Once we are happy we just delete all go files in the
bib repo and it just becomes a repo that builds the container
for bib.
It has many commits because it imports the full history of bib/cmd/bootc-image-builder.
/jira-epic HMS-8839
JIRA: HMS-9808