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

Quadlet - Use = sign when setting the pull arg for build #24643

Merged
merged 1 commit into from
Nov 22, 2024
+36 −2
Merged

Quadlet - Use = sign when setting the pull arg for build #24643

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion pkg/systemd/quadlet/quadlet.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1397,10 +1397,15 @@ func ConvertBuild(build *parser.UnitFile, unitsInfoMap map[string]*UnitInfo, isU
podman := createBasePodmanCommand(build, BuildGroup)
podman.add("build")

// The `--pull` flag has to be handled separately and the `=` sign must be present
// See https://github.com/containers/podman/issues/24599 for details
if val, ok := build.Lookup(BuildGroup, KeyPull); ok && len(val) > 0 {
podman.addf("--pull=%s", val)
}

stringKeys := map[string]string{
KeyArch: "--arch",
KeyAuthFile: "--authfile",
KeyPull: "--pull",
KeyTarget: "--target",
KeyVariant: "--variant",
}
Expand Down
2 changes: 1 addition & 1 deletion test/e2e/quadlet/pull.build
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
## assert-podman-final-args-regex /.*/podman-e2e-.*/subtest-.*/quadlet
## assert-podman-args "--tag" "localhost/imagename"
## assert-podman-args "--pull" "never"
## assert-podman-args "--pull=never"

[Build]
ImageTag=localhost/imagename
Expand Down
27 changes: 27 additions & 0 deletions test/system/252-quadlet.bats
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1699,4 +1699,31 @@ EOF
fi
done < <(parse_table "${dropin_files}")
}

# Following issue: https://github.com/containers/podman/issues/24599
# Make sure future changes do not break
@test "quadlet - build with pull" {
Copy link
Member

Choose a reason for hiding this comment

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

This is going to cause flakes. buildah cannot run in parallel, hence podman build cannot run in parallel: containers/buildah#5674

In my foolish youth, I overoptimistically added the ci:parallel tag to this entire file. That will apply here, which will cause this test to flake.

Proposed solution(?): override file_tags, or move this test into a separate file. I don't know which is better. To override, it looks like you can redefine file_tags, then redefine it again below this test. That's barfy. So is a separate file. There might be better solutions that I can't think of right now.

Copy link
Member

Choose a reason for hiding this comment

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

Will this cause flakes even where there no layers? The Containerfile is just FROM xxx so is this already enough to trigger flakes?

Copy link
Member

Choose a reason for hiding this comment

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

I honestly don't know. I've never dived deeply enough into that failure. Maybe it's OK to leave this as it is, and play the wait-and-see game.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@edsantiago I rather not have to split the files. But, I don't mind clearing the flag and setting it again around the test. Do you think it's safer? No point in running later to fix it

Copy link
Member

Choose a reason for hiding this comment

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

I'm really sorry... I don't know. I've never actually tried the change-file-tags trick. It seems worth a try. It also might be okay to merge as-is and see if it flakes or not: that might provide insight into the nature of the buildah-parallel bug.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So, if you're OK with it, I'll keep it this way and we'll see what happens. Thanks

local quadlet_tmpdir=$PODMAN_TMPDIR/quadlets

mkdir $quadlet_tmpdir

local container_file_path=$quadlet_tmpdir/Containerfile
cat >$container_file_path << EOF
FROM $IMAGE
EOF

local image_tag=quay.io/i-$(safename):$(random_string)
local quadlet_file=$PODMAN_TMPDIR/pull_$(safename).build
cat >$quadlet_file << EOF
[Build]
ImageTag=$image_tag
File=$container_file_path
Pull=never
EOF

run_quadlet "$quadlet_file"
service_setup $QUADLET_SERVICE_NAME "wait"

run_podman rmi -i $image_tag
}
# vim: filetype=sh
2 changes: 2 additions & 0 deletions test/system/helpers.systemd.bash
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,8 @@ quadlet_to_service_name() {
suffix="-image"
elif [ "$extension" == "pod" ]; then
suffix="-pod"
elif [ "$extension" == "build" ]; then
suffix="-build"
fi

echo "$filename$suffix.service"
Expand Down