quadlet API: fix tar install when non-quadlet file sorts first - #28696
Conversation
github-actions
Bot
added
the
kind/api-change
|
@simonbrauner , if you want to take a look at this after what we discussed today, I decided to touch the api function instead of the QuadletInstall function |
| quadlet_5=quadlet-test-5-$(cat /proc/sys/kernel/random/uuid).container | ||
| containerfile_1=quadlet-test-containerfile-1-$(cat /proc/sys/kernel/random/uuid).Containerfile | ||
| containerfile_1=quadlet-test-containerfile-1-$(cat /proc/sys/kernel/random/uuid) | ||
| configfile_1=quadlet-test-configfile-1-$(cat /proc/sys/kernel/random/uuid).conf |
There was a problem hiding this comment.
Is this file removed on cleanup?
| tar --format=posix -C "$TMPD" -cvf "$TMPD/$quadlet_7$containerfile_3.tar" "$quadlet_7" "$containerfile_3" &> /dev/null | ||
|
|
||
| t POST "libpod/quadlets" "$TMPD/$quadlet_7$containerfile_3.tar" 200 \ | ||
| '.InstalledQuadlets|length=2' \ |
|
|
||
| t GET "libpod/quadlets/$quadlet_7/file" 200 | ||
| is "$output" "$quadlet_7_content" "quadlet-7 should be installed" | ||
| is "$(cat "$quadlet_install_dir/$containerfile_3")" "$containerfile_3_content" "containerfile_3 should be installed" |
There was a problem hiding this comment.
Should ' $ configfile_1 ' also be checked?
There was a problem hiding this comment.
fixing your comments, thanks
719b8e2 to
9f7216b
Compare
|
[NON-BLOCKING] Packit jobs failed. @containers/packit-build please check. Everyone else, feel free to ignore. |
|
I'm not sure whether it makes more sense to fix this in
Otherwise LGTM |
|
@inknos, can you please rebase on main and resolve conflicts? Thanks. |
|
a bunch of changes might be needed after the application PR merged. maybe this is not even required anymore. checking |
QuadletInstall expects the quadlet file to be first in the file list, The API handler passes files in filepath.Walk order (lexicographic). Install would fail if a file like "Containerfile" comes before the quadlet file. Signed-off-by: Nicola Sella <nsella@redhat.com>
9f7216b to
10cc702
Compare
|
@Honny1 should be fine for another round of review now |
Honny1
left a comment
Honny1
left a comment
There was a problem hiding this comment.
LGTM
PTAL @podman-container-tools/podman-reviewers @podman-container-tools/podman-maintainers
|
LGTM |
PR podman-container-tools#28860 requires non-quadlet files (e.g a Containerfile with no recognized extension) to be passed as part of a named application. Update tests added in podman-container-tools#28696 to adhere to this requirment. Signed-off-by: Danish Prakash <contact@danishpraka.sh>
4 participants
QuadletInstall expects the quadlet file to be first in the file list, The API handler passes files in filepath.Walk order (lexicographic). Install would fail if a file like "Containerfile" comes before the quadlet file.
Checklist
Ensure you have completed the following checklist for your pull request to be reviewed:
commits. (
git commit -s). (If needed, usegit commit -s --amend). The author email must matchthe sign-off email address. See CONTRIBUTING.md
for more information.
Fixes: #00000in commit message (if applicable)make validatepr(format/lint checks)Noneif no user-facing changes)Does this PR introduce a user-facing change?