Fix handling of tmpfs-mode for tmpfs creation in compat mode

[rhatdan]

The permissions on disk were wrong since we were not converting to
octal.

Fixes: #13108

[NO NEW TESTS NEEDED] Since we don't currently test using the docker
client

Signed-off-by: Daniel J Walsh [email protected]

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: rhatdan

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [rhatdan]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[mheon]

LGTM

…

On Tue, Mar 8, 2022 at 09:00 openshift-ci[bot] ***@***.***> wrote: [APPROVALNOTIFIER] This PR is *APPROVED* This pull-request has been approved by: *rhatdan <#13455#>* The full list of commands accepted by this bot can be found here <https://go.k8s.io/bot-commands?repo=containers%2Fpodman>. The pull request process is described here <https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process> Needs approval from an approver in each of these files: - OWNERS <https://github.com/containers/podman/blob/main/OWNERS> [rhatdan] Approvers can indicate their approval by writing /approve in a comment Approvers can cancel approval by writing /approve cancel in a comment — Reply to this email directly, view it on GitHub <#13455 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AB3AOCCEOOWM4O36K7TSVMDU65MOLANCNFSM5QGNVUTQ> . Triage notifications on the go with GitHub Mobile for iOS <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>. You are receiving this because you are subscribed to this thread.Message ID: ***@***.***>

[rhatdan]

@nalind @jwhonce Any idea why the tests is failing. If I run the inspect with Docker, everything looks perfect.

[TomSweeneyRedHat]

Changes LGTM
@edsantiago did the test fedora-35 test just time out? I don't see any error except this at the tail end:

[+0136s] ok 1050 [70-short-names] DELETE images/localhost/foo:tag : status=200
[+0136s] ok 1051 [70-short-names] DELETE containers/b98466f8386b19521d2c2a5fad045b2a5b02d595ada8956e1a086ba4f9331795 : status=204
[+0137s] ok 1052 [70-short-names] DELETE images/localhost/bar:latest : status=200
[+0137s] rootdir: /tmp/test-apiv2.tmp.Fq6omo
[+0140s] 1..1052
[+0140s] make: *** [Makefile:628: localapiv2] Error 1
[14:07:13] END - [+0140s] total duration since 2022-03-08T14:07:13

[edsantiago]

It's a real error. Look at the colorized log.

[EDIT: by "error" I mean "Dan fixed a real bug, and the test itself was actually broken"]

[rhatdan]

Yes it is a real error but I can not understand how it is happening. When I run all of the tests with DOCKER_HOST docker, everything works correctly, If I inspect the container it looks correct. This only seems to be happening with the API.
This looks like another conversion error, and maybe docker converts it back to correct.

DOCKER_HOST=unix:///run/user/3267/podman/podman.sock docker run -it --mount=type=tmpfs,dst=/xxx,tmpfs-size=1G,tmpfs-mode=755 debian ls -ld /xxx
drwxr-xr-x. 2 root root 40 Mar  8 17:08 /xxx
$ DOCKER_HOST=unix:///run/user/3267/podman/podman.sock docker inspect --format '{{.HostConfig.Tmpfs}}' d1e695e4643f
map[/xxx:size=1073741824,mode=755,rw,rprivate,nosuid,nodev,tmpcopyup]

[edsantiago]

Oh! Sorry, the bug is here:

podman/test/apiv2/20-containers.at

Line 397 in a08e4e5

payload='{"Mounts":[{"Type":"tmpfs","Target":"/mnt/scratch","TmpfsOptions":{"SizeBytes":1024,"Mode":755}}]}'

The 755 is decimal, which is wrong. I don't know how to fix it, though: changing it to 0755 yields a JSON error. This is a problem for a JSON expert.

[edsantiago]

sigh

index cc5eda88e..49f8fb3fc 100644
--- a/test/apiv2/20-containers.at
+++ b/test/apiv2/20-containers.at
@@ -394,7 +394,8 @@ t GET containers/$cid/json 200 \
   .Config.Healthcheck.Retries=3
 
 # compat api: Test for mount options support
-payload='{"Mounts":[{"Type":"tmpfs","Target":"/mnt/scratch","TmpfsOptions":{"SizeBytes":1024,"Mode":755}}]}'
+# Sigh, JSON can't handle octal. 0755(octal) = 493(decimal)
+payload='{"Mounts":[{"Type":"tmpfs","Target":"/mnt/scratch","TmpfsOptions":{"SizeBytes":1024,"Mode":493}}]}'
 t POST containers/create Image=$IMAGE HostConfig="$payload" 201 .Id~[0-9a-f]\\{64\\}
 cid=$(jq -r '.Id' <<<"$output")
 t GET containers/$cid/json 200 \

[edsantiago]

Or, final thought for the day: maybe there's a way to make this code smarter about recognizing its input?

podman/pkg/specgenutil/volumes.go

Lines 431 to 435 in a08e4e5

	case "tmpfs-mode":
	if len(kv) == 1 {
	return newMount, errors.Wrapf(optionArgError, kv[0])
	}
	newMount.Options = append(newMount.Options, fmt.Sprintf("mode=%s", kv[1]))

[ashley-cui]

/lgtm