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

Fix parsing of Tmpfs field in compat create #9512

Merged
merged 1 commit into from
Mar 2, 2021
+31 −1
Merged

Fix parsing of Tmpfs field in compat create #9512

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
11 changes: 10 additions & 1 deletion cmd/podman/common/create_opts.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -311,6 +311,15 @@ func ContainerCreateToContainerCLIOpts(cc handlers.CreateContainerConfig, cgroup
netInfo.CNINetworks = []string{string(cc.HostConfig.NetworkMode)}
}

parsedTmp := make([]string, 0, len(cc.HostConfig.Tmpfs))
for path, options := range cc.HostConfig.Tmpfs {
finalString := path
if options != "" {
finalString += ":" + options
}
parsedTmp = append(parsedTmp, finalString)
}

// Note: several options here are marked as "don't need". this is based
// on speculation by Matt and I. We think that these come into play later
// like with start. We believe this is just a difference in podman/compat
Expand Down Expand Up @@ -367,7 +376,7 @@ func ContainerCreateToContainerCLIOpts(cc handlers.CreateContainerConfig, cgroup
StorageOpt: stringMaptoArray(cc.HostConfig.StorageOpt),
Sysctl: stringMaptoArray(cc.HostConfig.Sysctls),
Systemd: "true", // podman default
TmpFS: stringMaptoArray(cc.HostConfig.Tmpfs),
TmpFS: parsedTmp,
TTY: cc.Config.Tty,
User: cc.Config.User,
UserNS: string(cc.HostConfig.UsernsMode),
Expand Down
21 changes: 21 additions & 0 deletions test/apiv2/44-mounts.at
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
# -*- sh -*-

podman pull $IMAGE &>/dev/null

# Test various HostConfig options
tmpfs_name="/mytmpfs"
t POST containers/create?name=hostconfig_test '"Image":"'$IMAGE'","Cmd":["df"],"HostConfig":{"TmpFs":{"'$tmpfs_name'":"rw"}}' 201 \
Copy link
Member

Choose a reason for hiding this comment

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

If for some reason you need to resubmit, I think a better test would be ["df","-P","'$tmpfs_name'"] and include a status-code check in the wait. The reason is that line 20 bothers me a lot: it will pass on /mytmpfs but also on /mytmpfsrw or /mytmpfs=rw because I have no sane way to test for EOL. Explicitly dfing the desired filesystem is a safer test. But this is absolutely not worth re-pushing for.

.Id~[0-9a-f]\\{64\\}
cid=$(jq -r '.Id' <<<"$output")

# Prior to #9512, the tmpfs would be called '/mytmpfs=rw', with the '=rw'
t GET containers/${cid}/json 200 \
.HostConfig.Tmpfs[\"${tmpfs_name}\"]~rw,

# Run the container, verify output
t POST containers/${cid}/start '' 204
t POST containers/${cid}/wait '' 200
t GET containers/${cid}/logs?stdout=true 200

like "$(<$WORKDIR/curl.result.out)" ".* ${tmpfs_name}" \
Copy link
Member

Choose a reason for hiding this comment

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

Likewise, if you have to re-push for other reasons, I think a comment here would be helpful:

# /logs returns application/octet-stream, which our test helper saves in an outfile
# rather than returning in $output. That's why we can't test this directly in the /logs
# test above; instead, we rely on knowing the path to the stored results.

Again, not worth a re-push for this alone.

"'df' output includes tmpfs name"