Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Add a flag to build the image from a file #1469

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

Add a flag to build the image from a file #1469

wants to merge 6 commits into from

Conversation

KilianHanich
Copy link

Hello,

this PR is about #1397. It adds two flags:

  • --build to specify a build-context which is then passed to podman build
  • --build-tag to optionally specify how the built image should be named; it passes the argument to podman build --tag

I changed the files under doc to reflect the changed and added a few bats test cases.

I can add more if these are wanted, but I currently don't think more are needed for this feature.

@softwarefactory-project-zuul softwarefactory-project-zuul
Copy link

Build failed.
https://softwarefactory-project.io/zuul/t/local/buildset/04aaaad3091b45748c692c3d8d7475e8

✔️ unit-test SUCCESS in 5m 00s
❌ unit-test-migration-path-for-coreos-toolbox FAILURE in 2m 57s
✔️ unit-test-restricted SUCCESS in 3m 54s
❌ system-test-fedora-rawhide FAILURE in 35m 37s
❌ system-test-fedora-39 FAILURE in 33m 51s
❌ system-test-fedora-38 FAILURE in 36m 00s

@KilianHanich
Copy link
Author

Would be nice to know how this error wasn't caught by building it locally, but whatever.

@softwarefactory-project-zuul softwarefactory-project-zuul
Copy link

Build failed.
https://softwarefactory-project.io/zuul/t/local/buildset/e93ff974735f48d6ac1e3893675b7e87

✔️ unit-test SUCCESS in 4m 52s
✔️ unit-test-migration-path-for-coreos-toolbox SUCCESS in 3m 35s
✔️ unit-test-restricted SUCCESS in 3m 54s
❌ system-test-fedora-rawhide FAILURE in 35m 48s
❌ system-test-fedora-39 FAILURE in 34m 02s
❌ system-test-fedora-38 FAILURE in 34m 25s

@KilianHanich
Copy link
Author

Ok, I have no idea what the problem with this error is.

It's calling "podman images" that fails, but it works on my machine for some reason.

@softwarefactory-project-zuul softwarefactory-project-zuul
Copy link

Build failed.
https://softwarefactory-project.io/zuul/t/local/buildset/629f647fc7fb4d8ea59bfeb7ebb221ff

✔️ unit-test SUCCESS in 4m 48s
✔️ unit-test-migration-path-for-coreos-toolbox SUCCESS in 3m 32s
✔️ unit-test-restricted SUCCESS in 3m 55s
❌ system-test-fedora-rawhide FAILURE in 41m 10s
❌ system-test-fedora-39 FAILURE in 33m 48s
❌ system-test-fedora-38 FAILURE in 34m 50s

@softwarefactory-project-zuul softwarefactory-project-zuul
Copy link

Build failed.
https://softwarefactory-project.io/zuul/t/local/buildset/1030f61da6e34eeca05dccca9e8075de

✔️ unit-test SUCCESS in 4m 55s
✔️ unit-test-migration-path-for-coreos-toolbox SUCCESS in 3m 44s
✔️ unit-test-restricted SUCCESS in 3m 54s
❌ system-test-fedora-rawhide TIMED_OUT in 1h 20m 25s
❌ system-test-fedora-39 FAILURE in 34m 59s
❌ system-test-fedora-38 FAILURE in 34m 57s

@softwarefactory-project-zuul softwarefactory-project-zuul
Copy link

Build failed.
https://softwarefactory-project.io/zuul/t/local/buildset/ca7ad37c057d493b944c20ebd2c0b50c

✔️ unit-test SUCCESS in 5m 14s
✔️ unit-test-migration-path-for-coreos-toolbox SUCCESS in 3m 31s
✔️ unit-test-restricted SUCCESS in 4m 05s
❌ system-test-fedora-rawhide TIMED_OUT in 1h 20m 25s
❌ system-test-fedora-39 FAILURE in 34m 00s
❌ system-test-fedora-38 FAILURE in 33m 59s

@softwarefactory-project-zuul softwarefactory-project-zuul
Copy link

Build failed.
https://softwarefactory-project.io/zuul/t/local/buildset/2a56c374c8654e8d995dec00cf5c8134

✔️ unit-test SUCCESS in 5m 05s
✔️ unit-test-migration-path-for-coreos-toolbox SUCCESS in 3m 10s
✔️ unit-test-restricted SUCCESS in 4m 04s
❌ system-test-fedora-rawhide TIMED_OUT in 1h 20m 20s
❌ system-test-fedora-39 FAILURE in 35m 12s
❌ system-test-fedora-38 FAILURE in 34m 47s

debarshiray
debarshiray reviewed Apr 9, 2024
View reviewed changes
Copy link
Member

@debarshiray debarshiray left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wow, thanks for picking this up again.

One quick question: other than the discussion on the original issue, did you see the discussion on the previously attempted pull request?

@KilianHanich
Copy link
Author

One quick question: other than the discussion on the original issue, did you see the discussion on the previously attempted pull request?

Yes, I did.

On the one hand, yes, filesystems are racy and one could ofc guard against stuff.

On the other hand, one just passed a path to the directory with the Containerfile in and guarding against it can be quite annoying.

So, we get a few questions:

  • How likely is it to actually be a problem?
  • Does this project want the complexity from it? It would require quite a bit more changes to properly do it.
  • This option assumes that the Containerfile is at the root of the build context. If the other files in there actually matter but the Containerfile itself changed (or is removed), we don't have any guarantee that these files didn't change either. So, what would such a guard actually guard against? If it's malicious, one can do it just as well via these files. And if it isn't, one has no guarantee that the other files wouldn't be broken (for the build) too.
  • Shouldn't this problem be delegated to better builders? (e.g. podman-build, buildah etc.)

Personally I don't think dealing with that is worth it. But ofc if you want that, I would implement it.

@KilianHanich
Copy link
Author

On a different note, could it be that podman can't pull something in the tests? Because that would explain the failures...

@KilianHanich
Copy link
Author

I think I finally figured out what fails.
podman build fails thanks to SELinux...

To be exact, podman outputs at step 6 (RUN rm /etc/rpm/macros.image-language-conf):
/bin/sh: error while loading shared libraries: /lib64/libc.so.6: cannot apply additional memory protection after relocation: Permission denied
The AVC has as scontext system_u:system_r:container_t:s0:c123,c347 and as tcontext unconfined_u:object_r:cache_home_t:s0.

@debarshiray Do you have an idea about how to fix this? I am not really well versed when it comes to SELinux.

@boolzy boolzy mentioned this pull request Jun 3, 2024
Open
@KilianHanich
create-build: added --build option
3bd6c26 
uses the given path as the build context forwarded to podman build

afterwards it extracts the name of the image and sets it to satisfy other
assumption in existing code

the test builds a Fedora 39 toolbx and checks if the image is successfully
tagged as localhost/fedora-toolbox

Signed-off-by: Kilian Hanich <khanich.opensource@gmx.de>
@KilianHanich
doc: document the added --build option for the create subcommand
2fd6b30 
Signed-off-by: Kilian Hanich <khanich.opensource@gmx.de>
@KilianHanich
create-build-tag: added --build-tag option
2ca688b 
tags the built image of --build by forwarding the value to --tag of
podman build

added a test case with and without a repository
registry.fedoraproject.org was choosen as repository because podman doesn't
let one chose arbitrarily

Signed-off-by: Kilian Hanich <khanich.opensource@gmx.de>
@KilianHanich
doc: document the added --build-tag option for the create subcommand
b868db5 
Signed-off-by: Kilian Hanich <khanich.opensource@gmx.de>
@KilianHanich
added missing parameter
6e58a0d 
Signed-off-by: Kilian Hanich <khanich.opensource@gmx.de>
@softwarefactory-project-zuul softwarefactory-project-zuul
Copy link

Build failed.
https://softwarefactory-project.io/zuul/t/local/buildset/3da50194cc5d435ca01d8f1e0abf683e

✔️ unit-test SUCCESS in 7m 00s
✔️ unit-test-migration-path-for-coreos-toolbox SUCCESS in 3m 28s
✔️ unit-test-restricted SUCCESS in 5m 37s
❌ system-test-fedora-rawhide FAILURE in 43m 38s
❌ system-test-fedora-40 FAILURE in 35m 58s
❌ system-test-fedora-39 FAILURE in 35m 49s
❌ system-test-fedora-38 FAILURE in 34m 19s

@KilianHanich
fixing bugs around the build feature
b56a4cc 
image to build in the tests is also moved to Fedora 38 as the Fedora 39
file doesn't work at the time of writing

Signed-off-by: Kilian Hanich <khanich.opensource@gmx.de>
@KilianHanich
Copy link
Author

Don't approve of it yet, please.

@softwarefactory-project-zuul softwarefactory-project-zuul
Copy link

Build failed.
https://softwarefactory-project.io/zuul/t/local/buildset/64d89cec14d04070aabae258897fb5ef

✔️ unit-test SUCCESS in 6m 56s
✔️ unit-test-migration-path-for-coreos-toolbox SUCCESS in 3m 29s
✔️ unit-test-restricted SUCCESS in 5m 50s
✔️ system-test-fedora-rawhide SUCCESS in 43m 18s
❌ system-test-fedora-40 FAILURE in 35m 42s
❌ system-test-fedora-39 FAILURE in 35m 19s
❌ system-test-fedora-38 FAILURE in 34m 28s

@KilianHanich
Copy link
Author

Ok, so, there is still one problem here with the build pipeline:

Since bats create a temporary image repository under a tmpdir which are labeled with e.g. unconfined_u:object_r:cache_home_t:s0 which SELinux doesn't like since podman is labeled as system_u:system_r:container_t:s0: with some Multi-Category-Security label.

I tried to locally fix this by running XDG_CACHE_HOME=$HOME/podmanreg bats test/system to make the test suit place the stuff it works on somewhere else and gets a different label. That didn't work since it still can't access files with type user_home_t.

This implementation so far works with the default placement of the images and with SELinux in permissive mode. But I am unsure where to go from here.

I also tested this by creating a VM and set XDG_CACHE_HOME to $HOME/.local/share and creating a symlink from ~/.local/share/toolbx/system-test-storage to ~/.local/share/containers (I didn't want to loose my actual toolboxes after all and cleaned up the contents of ~/.local/share/containers). That worked.

So, the remaining issue is one with the pipeline and I don't know how to proceed with that. @debarshiray, do you have an idea?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@debarshiray debarshiray debarshiray left review comments

@HarryMichal HarryMichal Awaiting requested review from HarryMichal HarryMichal is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@KilianHanich @debarshiray