Deprecate `--local` flag (HMS-3792) #423

kingsleyzissou · 2024-05-07T13:50:05Z

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 podman machine and won't know about it.

This commit deprecates the --local flag and puts the requirement on the user to ensure that the container is in local storage before initiating the build.

cgwalters

Overall LGTM just two nits

bib/cmd/bootc-image-builder/main.go

mvo5

Thanks for this! Looks good, one tiny idea/suggestion inline.

test/test_build.py

kingsleyzissou · 2024-05-07T15:37:54Z

Arghhh of course this breaks tls-verify, how do we want to handle that? We aren't resolving containers from remote registries anymore, so maybe we want to disable this too

achilleas-k

The joy and simplicity of a mostly-red PR :)

bib/cmd/bootc-image-builder/main.go

mvo5

Thank you!

kingsleyzissou · 2024-05-08T10:19:18Z

flake8 failed on line length. Think it should be all good now 🤞

kingsleyzissou · 2024-05-08T15:31:53Z

Okay it's finally green now. Removing the flag also broke the cross-arch integration tests :/
Simple PR but had so many knock-on effects

mvo5

Thank you

mvo5

One more note/question but could be followup (just defensive)

test/test_build.py

ondrejbudai

I love this! :)

Marking this as request changes, because I want to first talk to the podman desktop folks if this is alright for them, and I want to figure out if we can somehow have bib releases, so users know what version they run, and thus which flags are supported and what's the default behaviour (git SHAs aren't very friendly in this case unfortunately).

ondrejbudai · 2024-09-17T09:15:58Z

Thanks! 👍

achilleas-k · 2024-09-17T17:45:44Z

Merge conflicts in the queue

jpace121 · 2024-10-13T19:12:37Z

Hi! So I'm currently using bootc-image-builder and am purposely not using --local, and being forced to use --local would break the tool for me.

More specifically, on my build computer I set the graphroot in storage.conf to a directory on a separate very large SSD from /, since the SSD I use for / is somewhat small. This means I don't have any images in /var/lib/containers/storage, and I don't want to; I don't have enough storage there. bootc-image-builder hard codes looking in /var/lib/containers/storage when looking for/building manifests though. To workaround this, I tried mounting the host's graphroot in the bootc-image-builder container under /var/lib/containers/storage, but that didn't work because I guess there is some db in that directory that is aware of what directory its in, and when the directories don't line up, podman get grumpy.

If you remove the local option, would you be willing to also find the places where the graphroot is assumed to be /var/lib/containers/storage and provide an option to override it?

EDIT: Thanks for this great tool and bootc in general, it really is a great way to build the custom images I want for my application, while still being able to take advantage of all the benefits of a full Linux Distro.

achilleas-k · 2024-10-14T07:41:47Z

@jpace121 I think your use case can be solved by mounting both into the bootc-image-builder container.

podman run -v /var/lib/containers:/var/lib/containers -v /path/to/large/ssd/container/storage:/path/to/large/ssd/container/storage ...

Can you see if that works?

The /var/lib/containers mount should make BIB pick up the storage.conf and point it to the large container storage, which will be at the expected location inside the container.

If you remove the local option, would you be willing to also find the places where the graphroot is assumed to be /var/lib/containers/storage and provide an option to override it?

If the path that graphroot points to isn't mounted in the container, there's not much we can do from inside the container itself to reach it automatically. Unless I'm misunderstanding the suggestion.

github-actions · 2024-11-14T04:10:26Z

This PR is stale because it has been open 30 days with no activity. Remove "Stale" label or comment or this will be closed in 7 days.

github-actions · 2024-11-21T04:10:37Z

This PR was closed because it has been stalled for 30+7 days with no activity.

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 deprecates the `--local` flag and warns users when it is passed to the CLI so that this won't break things for anyone who might already be using the flag. This change means that the user will have to ensure that the container is pulled to the local container store before initiating the build. Co-authored-by: Ondřej Budai <[email protected]>

Ensure that the containers have been copied into local storage for all test cases. We need to explicitly pull the container into local containers storage with the correct arch otherwise cross-arch building fails. The helper function uses the host-arch as a fallback when no target arch is provided.

Since all containers are coming from local storage and require the user to pull in the container before-hand, we can disable the `--tls-verify` flag. The containers will not be resolved from a remote registry but rather from the local container store.

cgwalters reviewed May 7, 2024

View reviewed changes

bib/cmd/bootc-image-builder/main.go Outdated Show resolved Hide resolved

bib/cmd/bootc-image-builder/main.go Outdated Show resolved Hide resolved

mvo5 reviewed May 7, 2024

View reviewed changes

test/test_build.py Outdated Show resolved Hide resolved

kingsleyzissou force-pushed the deprecate-local branch 2 times, most recently from 996a9ff to 0f0f46f Compare May 7, 2024 15:59

achilleas-k previously approved these changes May 7, 2024

View reviewed changes

achilleas-k reviewed May 7, 2024

View reviewed changes

bib/cmd/bootc-image-builder/main.go Show resolved Hide resolved

kingsleyzissou dismissed achilleas-k’s stale review via 53956d3 May 8, 2024 09:16

kingsleyzissou force-pushed the deprecate-local branch from 0f0f46f to 53956d3 Compare May 8, 2024 09:16

mvo5 previously approved these changes May 8, 2024

View reviewed changes

kingsleyzissou dismissed mvo5’s stale review via 4356c1e May 8, 2024 10:18

kingsleyzissou force-pushed the deprecate-local branch from 53956d3 to 4356c1e Compare May 8, 2024 10:18

mvo5 previously approved these changes May 8, 2024

View reviewed changes

kingsleyzissou dismissed mvo5’s stale review via bc9b951 May 8, 2024 11:06

kingsleyzissou force-pushed the deprecate-local branch from 4356c1e to bc9b951 Compare May 8, 2024 11:06

mvo5 previously approved these changes May 8, 2024

View reviewed changes

kingsleyzissou dismissed mvo5’s stale review via cb769a3 May 8, 2024 14:30

kingsleyzissou force-pushed the deprecate-local branch from bc9b951 to cb769a3 Compare May 8, 2024 14:30

kingsleyzissou requested review from cgwalters, achilleas-k and mvo5 May 8, 2024 15:31

mvo5 previously approved these changes May 13, 2024

View reviewed changes

mvo5 reviewed May 13, 2024

View reviewed changes

test/test_build.py Outdated Show resolved Hide resolved

test/test_build.py Outdated Show resolved Hide resolved

ondrejbudai requested changes May 15, 2024

View reviewed changes

kingsleyzissou dismissed mvo5’s stale review via d20d867 May 15, 2024 22:41

kingsleyzissou force-pushed the deprecate-local branch 3 times, most recently from acc9c79 to e3d37c7 Compare May 15, 2024 23:17

achilleas-k enabled auto-merge September 17, 2024 17:42

achilleas-k added this pull request to the merge queue Sep 17, 2024

github-merge-queue bot removed this pull request from the merge queue due to a conflict with the base branch Sep 17, 2024

kingsleyzissou dismissed stale reviews from ondrejbudai, achilleas-k, and mvo5 via b159ca1 September 18, 2024 09:19

kingsleyzissou force-pushed the deprecate-local branch from be11011 to b159ca1 Compare September 18, 2024 09:19

kingsleyzissou force-pushed the deprecate-local branch 2 times, most recently from 6a95c3e to 22f4e9c Compare October 10, 2024 08:44

kingsleyzissou requested review from ondrejbudai, achilleas-k and mvo5 October 10, 2024 13:05

ondrejbudai mentioned this pull request Oct 30, 2024

bib/composer integration: add manifest job for osbuild processing (HMS-4843) osbuild/osbuild-composer#4452

Closed

4 tasks

github-actions bot added the Stale Issue or PR with no activity for extended period of time label Nov 14, 2024

github-actions bot closed this Nov 21, 2024

ondrejbudai removed the Stale Issue or PR with no activity for extended period of time label Nov 29, 2024

ondrejbudai reopened this Nov 29, 2024

ondrejbudai mentioned this pull request Nov 29, 2024

podman run has no --type parameter #735

Closed

mvo5 force-pushed the deprecate-local branch from 22f4e9c to cb3dcca Compare December 10, 2024 12:23

kingsleyzissou and others added 3 commits December 17, 2024 09:06

bib: disable --tls-verify flag

9c6b865

Since all containers are coming from local storage and require the user to pull in the container before-hand, we can disable the `--tls-verify` flag. The containers will not be resolved from a remote registry but rather from the local container store.

achilleas-k force-pushed the deprecate-local branch from cb3dcca to 9c6b865 Compare December 17, 2024 08:06

fixup! fix tests

83a8b09

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Deprecate `--local` flag (HMS-3792) #423

Deprecate `--local` flag (HMS-3792) #423

kingsleyzissou commented May 7, 2024 •

edited

Loading

cgwalters left a comment

mvo5 left a comment

kingsleyzissou commented May 7, 2024

achilleas-k left a comment

mvo5 left a comment

kingsleyzissou commented May 8, 2024

kingsleyzissou commented May 8, 2024

mvo5 left a comment

mvo5 left a comment

ondrejbudai left a comment

ondrejbudai commented Sep 17, 2024

achilleas-k commented Sep 17, 2024 •

edited

Loading

jpace121 commented Oct 13, 2024 •

edited

Loading

achilleas-k commented Oct 14, 2024

github-actions bot commented Nov 14, 2024

github-actions bot commented Nov 21, 2024

Deprecate --local flag (HMS-3792) #423

Are you sure you want to change the base?

Deprecate --local flag (HMS-3792) #423

Conversation

kingsleyzissou commented May 7, 2024 • edited Loading

cgwalters left a comment

Choose a reason for hiding this comment

mvo5 left a comment

Choose a reason for hiding this comment

kingsleyzissou commented May 7, 2024

achilleas-k left a comment

Choose a reason for hiding this comment

mvo5 left a comment

Choose a reason for hiding this comment

kingsleyzissou commented May 8, 2024

kingsleyzissou commented May 8, 2024

mvo5 left a comment

Choose a reason for hiding this comment

mvo5 left a comment

Choose a reason for hiding this comment

ondrejbudai left a comment

Choose a reason for hiding this comment

ondrejbudai commented Sep 17, 2024

achilleas-k commented Sep 17, 2024 • edited Loading

jpace121 commented Oct 13, 2024 • edited Loading

achilleas-k commented Oct 14, 2024

github-actions bot commented Nov 14, 2024

github-actions bot commented Nov 21, 2024

Deprecate `--local` flag (HMS-3792) #423

Deprecate `--local` flag (HMS-3792) #423

kingsleyzissou commented May 7, 2024 •

edited

Loading

achilleas-k commented Sep 17, 2024 •

edited

Loading

jpace121 commented Oct 13, 2024 •

edited

Loading