Skip to content

Commit

Permalink
bib: make build the default subcommand
Browse files Browse the repository at this point in the history
Prior this commit, the bootc-image-builder container image had
a custom entrypoint that hardcoded the use of the build subcommand. This
meant that if a user wanted to use a different subcommand, they had to
overwrite the entrypoint.

This commit changes the cobra code in bib to fallback to build if no
subcommand was given. This is slighly ugly, but it allows us to remove
the custom entrypoint, streamlining the use of subcommands. Let's see
an example of calling the version subcommand:

Before:
podman run --rm -it --entrypoint=/usr/bin/bootc-image-builder \
    quay.io/centos-bootc/bootc-image-builder:latest version

After:
sudo podman run --rm -it \
    quay.io/centos-bootc/bootc-image-builder:latest version

Kudos to https://github.com/IKukhta for his code from
spf13/cobra#823 (comment)
  • Loading branch information
ondrejbudai authored and mvo5 committed Aug 19, 2024
1 parent b20b596 commit ac382a2
Show file tree
Hide file tree
Showing 7 changed files with 16 additions and 28 deletions.
3 changes: 1 addition & 2 deletions Containerfile
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,9 @@ COPY ./group_osbuild-osbuild-fedora.repo /etc/yum.repos.d/
COPY ./package-requires.txt .
RUN grep -vE '^#' package-requires.txt | xargs dnf install -y && rm -f package-requires.txt && dnf clean all
COPY --from=builder /build/bin/* /usr/bin/
COPY entrypoint.sh /
COPY bib/data /usr/share/bootc-image-builder

ENTRYPOINT ["/entrypoint.sh"]
ENTRYPOINT ["/usr/bin/bootc-image-builder"]
VOLUME /output
WORKDIR /output
VOLUME /store
Expand Down
3 changes: 1 addition & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -522,8 +522,7 @@ The contents of the file `$(pwd)/wheel-passwordless-sudo` should be

Please report bugs to the [Bug Tracker](https://github.com/osbuild/bootc-image-builder/issues) and include instructions to reproduce and the output of:
```
$ sudo podman run --rm -it --entrypoint=/usr/bin/bootc-image-builder \
quay.io/centos-bootc/bootc-image-builder:latest version
$ sudo podman run --rm -it quay.io/centos-bootc/bootc-image-builder:latest version
```

## 📊 Project
Expand Down
12 changes: 11 additions & 1 deletion bib/cmd/bootc-image-builder/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -623,7 +623,6 @@ func run() error {
buildCmd.Flags().String("output", ".", "artifact output directory")
buildCmd.Flags().String("progress", "text", "type of progress bar to use")
buildCmd.Flags().String("store", "/store", "osbuild store for intermediate pipeline trees")

// flag rules
for _, dname := range []string{"output", "store", "rpmmd"} {
if err := buildCmd.MarkFlagDirname(dname); err != nil {
Expand All @@ -635,6 +634,17 @@ func run() error {
}
buildCmd.MarkFlagsRequiredTogether("aws-region", "aws-bucket", "aws-ami-name")

// If no subcommand is given, assume the user wants to use the build subcommand
// See https://github.com/spf13/cobra/issues/823#issuecomment-870027246
// which cannot be used verbatim because the arguments for "build" like
// "quay.io" will create an "err != nil". Ideally we could check err
// for something like cobra.UnknownCommandError but cobra just gives
// us an error string
_, _, err := rootCmd.Find(os.Args[1:])
if err != nil && !slices.Contains([]string{"help", "completion"}, os.Args[1]) {
args := append([]string{buildCmd.Use}, os.Args[1:]...)
rootCmd.SetArgs(args)
}
return rootCmd.Execute()
}

Expand Down
3 changes: 1 addition & 2 deletions devel/Containerfile
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,8 @@ COPY --from=osbuild-builder /build/rpmbuild/RPMS/noarch/*.rpm /rpms/
COPY ./package-requires.txt .
RUN grep -vE '^#' package-requires.txt | xargs dnf install -y && rm -f package-requires.txt && dnf install -y /rpms/*.rpm && dnf clean all
COPY --from=bib-builder /build/bin/bootc-image-builder /usr/bin/bootc-image-builder
COPY entrypoint.sh /entrypoint.sh

ENTRYPOINT ["/entrypoint.sh"]
ENTRYPOINT ["/usr/bin/bootc-image-builder"]
VOLUME /output
WORKDIR /output
VOLUME /store
Expand Down
5 changes: 0 additions & 5 deletions entrypoint.sh

This file was deleted.

16 changes: 2 additions & 14 deletions test/test_manifest.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ def find_image_size_from(manifest_str):
def test_manifest_smoke(build_container, tc):
output = subprocess.check_output([
*testutil.podman_run_common,
"--entrypoint=/usr/bin/bootc-image-builder",
build_container,
"manifest",
*tc.bib_rootfs_args(),
Expand All @@ -53,7 +52,6 @@ def test_manifest_smoke(build_container, tc):
def test_iso_manifest_smoke(build_container, tc):
output = subprocess.check_output([
*testutil.podman_run_common,
"--entrypoint=/usr/bin/bootc-image-builder",
build_container,
"manifest",
"--type=anaconda-iso", f"{tc.container_ref}",
Expand Down Expand Up @@ -82,7 +80,6 @@ def test_manifest_disksize(tmp_path, build_container, tc):
print(f"using {container_tag}")
manifest_str = subprocess.check_output([
*testutil.podman_run_common,
"--entrypoint", "/usr/bin/bootc-image-builder",
build_container,
"manifest", "--local",
*tc.bib_rootfs_args(),
Expand All @@ -102,7 +99,6 @@ def test_manifest_local_checks_containers_storage_errors(build_container):
"podman", "run", "--rm",
"--privileged",
"--security-opt", "label=type:unconfined_t",
"--entrypoint=/usr/bin/bootc-image-builder",
build_container,
"manifest", "--local", "arg-not-used",
], check=False, stdout=subprocess.PIPE, stderr=subprocess.PIPE, encoding="utf8")
Expand All @@ -121,7 +117,6 @@ def test_manifest_local_checks_containers_storage_works(tmp_path, build_containe
with make_container(tmp_path) as container_tag:
subprocess.run([
*testutil.podman_run_common,
"--entrypoint=/usr/bin/bootc-image-builder",
build_container,
"manifest", "--local",
*tc.bib_rootfs_args(),
Expand All @@ -141,7 +136,6 @@ def test_manifest_cross_arch_check(tmp_path, build_container):
with pytest.raises(subprocess.CalledProcessError) as exc:
subprocess.run([
*testutil.podman_run_common,
"--entrypoint=/usr/bin/bootc-image-builder",
build_container,
"manifest", "--target-arch=aarch64",
"--local", f"localhost/{container_tag}"
Expand All @@ -165,7 +159,6 @@ def test_manifest_rootfs_respected(build_container, tc):
# TODO: derive container and fake "bootc install print-configuration"?
output = subprocess.check_output([
*testutil.podman_run_common,
"--entrypoint=/usr/bin/bootc-image-builder",
build_container,
"manifest", f"{tc.container_ref}",
])
Expand All @@ -183,7 +176,6 @@ def test_manifest_rootfs_override(build_container):

output = subprocess.check_output([
*testutil.podman_run_common,
"--entrypoint=/usr/bin/bootc-image-builder",
build_container,
"manifest", "--rootfs", "btrfs", f"{container_ref}",
])
Expand Down Expand Up @@ -216,7 +208,6 @@ def test_manifest_user_customizations_toml(tmp_path, build_container):
output = subprocess.check_output([
*testutil.podman_run_common,
"-v", f"{config_toml_path}:/config.toml:ro",
"--entrypoint=/usr/bin/bootc-image-builder",
build_container,
"manifest", f"{container_ref}",
])
Expand All @@ -243,7 +234,6 @@ def test_manifest_installer_customizations(tmp_path, build_container):
output = subprocess.check_output([
*testutil.podman_run_common,
"-v", f"{config_toml_path}:/config.toml:ro",
"--entrypoint=/usr/bin/bootc-image-builder",
build_container,
"manifest", "--type=anaconda-iso", f"{container_ref}",
])
Expand Down Expand Up @@ -297,7 +287,6 @@ def test_mount_ostree_error(tmpdir_factory, build_container):
subprocess.check_output([
*testutil.podman_run_common,
"-v", f"{output_path}:/output",
"--entrypoint=/usr/bin/bootc-image-builder",
build_container,
"manifest", f"{container_ref}",
"--config", "/output/config.json",
Expand All @@ -317,8 +306,9 @@ def test_manifest_checks_build_container_is_bootc(build_container, container_ref
def check_image_ref():
subprocess.check_output([
*testutil.podman_run_common,
f'--entrypoint=["/usr/bin/bootc-image-builder", "manifest", "{container_ref}"]',
build_container,
"manifest",
container_ref,
], stderr=subprocess.PIPE, encoding="utf8")
if should_error:
with pytest.raises(subprocess.CalledProcessError) as exc:
Expand All @@ -333,7 +323,6 @@ def test_manifest_target_arch_smoke(build_container, tc):
# TODO: actually build an image too
output = subprocess.check_output([
*testutil.podman_run_common,
"--entrypoint=/usr/bin/bootc-image-builder",
build_container,
"manifest",
*tc.bib_rootfs_args(),
Expand Down Expand Up @@ -386,7 +375,6 @@ def test_manifest_anaconda_module_customizations(tmpdir_factory, build_container
output = subprocess.check_output([
*testutil.podman_run_common,
"-v", f"{output_path}:/output",
"--entrypoint=/usr/bin/bootc-image-builder",
build_container,
"manifest",
"--config", "/output/config.json",
Expand Down
2 changes: 0 additions & 2 deletions test/test_opts.py
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,6 @@ def test_bib_help_hides_config(tmp_path, container_storage, build_fake_container
"--security-opt", "label=type:unconfined_t",
"-v", f"{container_storage}:/var/lib/containers/storage",
"-v", f"{output_path}:/output",
"--entrypoint=/usr/bin/bootc-image-builder",
build_fake_container,
"manifest", "--help",
], check=True, capture_output=True, text=True)
Expand Down Expand Up @@ -159,7 +158,6 @@ def test_bib_version(tmp_path, container_storage, build_fake_container):
"--security-opt", "label=type:unconfined_t",
"-v", f"{container_storage}:/var/lib/containers/storage",
"-v", f"{output_path}:/output",
"--entrypoint=/usr/bin/bootc-image-builder",
build_fake_container,
"version",
], check=True, capture_output=True, text=True)
Expand Down

0 comments on commit ac382a2

Please sign in to comment.