Skip to content

Commit

Permalink
bib: deprecate the --local flag
Browse files Browse the repository at this point in the history
Pulling container images before building would break in the case of
authenticated images on podman machine, since the auth file lives on the
host and not podman machine and won't know about it.

This commit silently deprecates the `--local` flag, meaning the flag can
still be passed by the user, without any effects and without warning the
user. This, however, does put the requirement on the user to ensure that
the container is in local storage before initiating the build.
  • Loading branch information
kingsleyzissou committed May 8, 2024
1 parent 99d4e4d commit 04d0c2e
Show file tree
Hide file tree
Showing 2 changed files with 7 additions and 21 deletions.
22 changes: 2 additions & 20 deletions bib/cmd/bootc-image-builder/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,6 @@ func manifestFromCobra(cmd *cobra.Command, args []string) ([]byte, *mTLSConfig,
rpmCacheRoot, _ := cmd.Flags().GetString("rpmmd")
targetArch, _ := cmd.Flags().GetString("target-arch")
tlsVerify, _ := cmd.Flags().GetBool("tls-verify")
localStorage, _ := cmd.Flags().GetBool("local")
rootFs, _ := cmd.Flags().GetString("rootfs")

if targetArch != "" && arch.FromString(targetArch) != arch.Current() {
Expand All @@ -193,10 +192,8 @@ func manifestFromCobra(cmd *cobra.Command, args []string) ([]byte, *mTLSConfig,
}
// TODO: add "target-variant", see https://github.com/osbuild/bootc-image-builder/pull/139/files#r1467591868

if localStorage {
if err := setup.ValidateHasContainerStorageMounted(); err != nil {
return nil, nil, fmt.Errorf("local storage not working, did you forget -v /var/lib/containers/storage:/var/lib/containers/storage? (%w)", err)
}
if err := setup.ValidateHasContainerStorageMounted(); err != nil {
return nil, nil, fmt.Errorf("could not access container storage, did you forget -v /var/lib/containers/storage:/var/lib/containers/storage? (%w)", err)
}

buildType, err := NewBuildType(imgTypes)
Expand All @@ -209,21 +206,6 @@ func manifestFromCobra(cmd *cobra.Command, args []string) ([]byte, *mTLSConfig,
return nil, nil, fmt.Errorf("cannot read config: %w", err)
}

// If --local wasn't given, always pull the container.
// If the user mount a container storage inside bib (without --local), the code will try to pull
// a newer version of the container even if an older one is already present. This doesn't match
// how `podman run`` behaves by default, but it matches the bib's behaviour before the switch
// to using containers storage in all code paths happened.
// We might want to change this behaviour in the future to match podman.
if !localStorage {
logrus.Infof("Pulling image %s (arch=%s)\n", imgref, cntArch)
if output, err := exec.Command("podman", "pull", "--arch", cntArch.String(), fmt.Sprintf("--tls-verify=%v", tlsVerify), imgref).CombinedOutput(); err != nil {
return nil, nil, fmt.Errorf("failed to pull container image: %w\n%s", err, output)
}
} else {
logrus.Debug("Using local container")
}

cntSize, err := getContainerSize(imgref)
if err != nil {
return nil, nil, fmt.Errorf("cannot get container size: %w", err)
Expand Down
6 changes: 5 additions & 1 deletion test/test_manifest.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ def test_manifest_smoke(build_container, testcase_ref):
"podman", "run", "--rm",
"--privileged",
"--security-opt", "label=type:unconfined_t",
"-v", "/var/lib/containers/storage:/var/lib/containers/storage",
f'--entrypoint=["/usr/bin/bootc-image-builder", "manifest", "{container_ref}"]',
build_container,
])
Expand Down Expand Up @@ -90,7 +91,8 @@ def test_manifest_local_checks_containers_storage_errors(build_container):
build_container,
], check=False, stdout=subprocess.PIPE, stderr=subprocess.PIPE, encoding="utf8")
assert res.returncode == 1
err = 'local storage not working, did you forget -v /var/lib/containers/storage:/var/lib/containers/storage?'
err = 'could not access container storage,' + \
'did you forget -v /var/lib/containers/storage:/var/lib/containers/storage?'
assert err in res.stderr


Expand Down Expand Up @@ -156,6 +158,7 @@ def test_manifest_rootfs_respected(build_container, testcase_ref):
"podman", "run", "--rm",
"--privileged",
"--security-opt", "label=type:unconfined_t",
"-v", "/var/lib/containers/storage:/var/lib/containers/storage",
f'--entrypoint=["/usr/bin/bootc-image-builder", "manifest", "{container_ref}"]',
build_container,
])
Expand All @@ -177,6 +180,7 @@ def test_manifest_rootfs_override(build_container):
"podman", "run", "--rm",
"--privileged",
"--security-opt", "label=type:unconfined_t",
"-v", "/var/lib/containers/storage:/var/lib/containers/storage",
f'--entrypoint=["/usr/bin/bootc-image-builder", "manifest",\
"--rootfs", "ext4", "{container_ref}"]',
build_container,
Expand Down

0 comments on commit 04d0c2e

Please sign in to comment.