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.

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

testing CI: DO NOT MERGE #3287

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
3 participants
@justincormack
Copy link
Collaborator

justincormack commented Feb 11, 2019

Signed-off-by: Justin Cormack justin.cormack@docker.com

- What I did

- How I did it

- How to verify it

- Description for the changelog

- A picture of a cute animal (not mandatory but encouraged)

DO NOT MERGE
Signed-off-by: Justin Cormack <justin.cormack@docker.com>

@justincormack justincormack force-pushed the justincormack:donotmerge branch from c447f8c to 48aa694 Feb 11, 2019

@justincormack

This comment has been minimized.

Copy link
Collaborator Author

justincormack commented Feb 11, 2019

Hmm, didn't pass CI.

@ijc

This comment has been minimized.

Copy link
Collaborator

ijc commented Feb 11, 2019

seems legit:

[+ 45m 26s] [STDERR  ] 2019-02-11T12:42:14.957054949Z: time="2019-02-11T12:40:44Z" level=warning msg="ignored xattr user.key in archive" error="operation not supported" 
[+ 45m 26s] [STDERR  ] 2019-02-11T12:42:14.957068447Z: --- FAIL: TestTarWithXattr (0.00s)
[+ 45m 26s] [STDERR  ] 2019-02-11T12:42:14.957081050Z:     --- FAIL: TestTarWithXattr/WithXattrsUser (0.00s)
[+ 45m 26s] [STDERR  ] 2019-02-11T12:42:14.957094160Z:     	tar_test.go:945: Validation failed: file xattrs expect to be value, actually get 
[+ 45m 26s] [STDERR  ] 2019-02-11T12:42:14.957107170Z: FAIL
[+ 45m 26s] [STDERR  ] 2019-02-11T12:42:14.957120123Z: FAIL	github.com/containerd/containerd/archive	0.136s
[+ 45m 26s] [STDERR  ] 2019-02-11T12:42:14.957133332Z: ok  	github.com/containerd/containerd/metadata	10.035s
[+ 45m 26s] [STDERR  ] 2019-02-11T12:42:14.957145730Z: ok  	github.com/containerd/containerd/mount	0.089s
[+ 45m 26s] [STDERR  ] 2019-02-11T12:42:14.957168609Z: ok  	github.com/containerd/containerd/snapshots/btrfs	10.374s
[+ 45m 26s] [STDERR  ] 2019-02-11T12:42:14.957181457Z: ok  	github.com/containerd/containerd/snapshots/native	5.652s
[+ 45m 26s] [STDERR  ] 2019-02-11T12:42:14.957197595Z: ok  	github.com/containerd/containerd/snapshots/overlay	7.543s
[+ 45m 26s] [STDERR  ] 2019-02-11T12:42:14.957209771Z: containerd test suite FAILED
[+ 45m 26s] [STDERR  ] 2019-02-11T12:42:14.957230358Z: make: *** [Makefile:154: root-test] Error 1
[+ 45m 26s] [STDERR  ] 2019-02-11T12:42:14.957244148Z: [  112.553375] sd 0:0:0:0: [sda] Synchronizing SCSI cache
[+ 45m 26s] [STDERR  ] 2019-02-11T12:42:14.957256356Z: [  112.559043] sd 0:0:0:0: [sda] Stopping disk
[+ 45m 26s] [STDERR  ] 2019-02-11T12:42:14.957283155Z: [  112.564715] ACPI: Preparing to enter system sleep state S5
[+ 45m 26s] [STDERR  ] 2019-02-11T12:42:14.957304547Z: [  112.570568] reboot: Power down
[+ 45m 26s] [STDERR  ] 2019-02-11T12:42:14.975143330Z: + clean_up
[+ 45m 26s] [STDERR  ] 2019-02-11T12:42:14.975209604Z: + rm -rf containerd-cmdline containerd-initrd.img containerd-kernel containerd-state
[+ 45m 26s] [FAIL    ] linuxkit.packages.containerd 193.65s
@ijc

This comment has been minimized.

Copy link
Collaborator

ijc commented Feb 11, 2019

From https://github.com/containerd/containerd/releases/tag/v1.2.3:

fix in Tar xattrs to restore compatibility with older container images containerd/containerd#2953

... sounds awfully suspect to me...

@justincormack

This comment has been minimized.

Copy link
Collaborator Author

justincormack commented Feb 11, 2019

Yeah, does seem suspect...

@ijc

This comment has been minimized.

Copy link
Collaborator

ijc commented Feb 11, 2019

https://github.com/containerd/containerd/blob/master/archive/tar.go#L414 is the line producing the error, but it is ancient not new in v1.2.3.

Actual thing which (I reckon) broke us is containerd/containerd@2244a20 but that's just making xattrs work at all -- so I think the core issue is the setxattr failing on our kernels. I've no idea why that should be. Maybe it's the underlying fs?

@justincormack

This comment has been minimized.

Copy link
Collaborator Author

justincormack commented Feb 11, 2019

Oh, maybe the fs doesn't have userxattr mount option? Is it s tmpfs?

@ijc

This comment has been minimized.

Copy link
Collaborator

ijc commented Feb 11, 2019

I think it might be an overlayfs since it run in this context:

FROM scratch
COPY --from=mirror /out/ /
COPY --from=mirror /go/src/github.com/containerd/containerd /go/src/github.com/containerd/containerd/
ENV GOPATH=/go
WORKDIR $GOPATH/src/github.com/containerd/containerd
@justincormack

This comment has been minimized.

Copy link
Collaborator Author

justincormack commented Feb 11, 2019

Hmm, well if docker build doesnt run with userxattr I wouldn't be surprised... but maybe it depends on the underlying fs for overlay.

@ijc

This comment has been minimized.

Copy link
Collaborator

ijc commented Feb 11, 2019

I mean it runs in a container create via that Dockerfile snippet -- it's actually running as a linuxkit service built from the resulting container.

@ijc

This comment has been minimized.

Copy link
Collaborator

ijc commented Feb 12, 2019

linuxkit source has no mention of userxattr so I suppose it doesn't pass that.

Looking at my running (non-Linuxkit) laptop system, /proc/mounts doesn't suggest Docker does either mind.

I tried a quick hack in the run.sh in the test-containerd:

mkdir -p /var/lib/go/src/github.com/containerd/
cp -r $GOPATH/src/github.com/containerd/containerd /var/lib/go/src/github.com/containerd/containerd
export GOPATH=/var/lib/go
cd /var/lib/go/src/github.com/containerd/containerd
mount -o remount,user_xattr /var/lib

(i.e. moving the source off the overlay fs onto the persistent disk and ensuring user xattrs were on) the result was the same though.

I also reproduced with linuxkit build examples/sshd.yaml then logging in and:

(ns: sshd) linuxkit-4a444fac9d0f:~# apk add -U strace attr
fetch http://dl-cdn.alpinelinux.org/alpine/v3.8/main/x86_64/APKINDEX.tar.gz
fetch http://dl-cdn.alpinelinux.org/alpine/v3.8/community/x86_64/APKINDEX.tar.gz
(1/3) Installing libattr (2.4.47-r7)
(2/3) Installing attr (2.4.47-r7)
(3/3) Installing strace (4.22-r0)
Executing busybox-1.28.4-r1.trigger
OK: 19 MiB in 29 packages
(ns: sshd) linuxkit-4a444fac9d0f:~# touch foo
(ns: sshd) linuxkit-4a444fac9d0f:~# attr -s user.foo -V bar foo 
attr_set: Not supported
Could not set "user.foo" for foo
(ns: sshd) linuxkit-4a444fac9d0f:~# strace attr -s user.foo -V bar foo 
execve("/usr/bin/attr", ["attr", "-s", "user.foo", "-V", "bar", "foo"], 0x7ffced0963c8 /* 15 vars */) = 0
arch_prctl(ARCH_SET_FS, 0x7fb889818b88) = 0
set_tid_address(0x7fb889818bc0)         = 577
open("/etc/ld-musl-x86_64.path", O_RDONLY|O_CLOEXEC) = -1 ENOENT (No such file or directory)
open("/lib/libattr.so.1", O_RDONLY|O_CLOEXEC) = 3
fcntl(3, F_SETFD, FD_CLOEXEC)           = 0
fstat(3, {st_mode=S_IFREG|0644, st_size=18144, ...}) = 0
read(3, "\177ELF\2\1\1\0\0\0\0\0\0\0\0\0\3\0>\0\1\0\0\0\370\21\0\0\0\0\0\0"..., 960) = 960
mmap(NULL, 2117632, PROT_READ|PROT_EXEC, MAP_PRIVATE, 3, 0) = 0x7fb889383000
mmap(0x7fb889586000, 8192, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_FIXED, 3, 0x3000) = 0x7fb889586000
close(3)                                = 0
mprotect(0x7fb889586000, 4096, PROT_READ) = 0
mprotect(0x7fb889815000, 4096, PROT_READ) = 0
mprotect(0x5600439e4000, 4096, PROT_READ) = 0
lsetxattr("foo", "user.user.foo", "bar", 3, 0) = -1 EOPNOTSUPP (Not supported)
lsetxattr("foo", "user.user.foo", "bar", 3, 0) = -1 EOPNOTSUPP (Not supported)
writev(2, [{iov_base="", iov_len=0}, {iov_base="attr_set", iov_len=8}], 2attr_set) = 8
writev(2, [{iov_base="", iov_len=0}, {iov_base=":", iov_len=1}], 2:) = 1
writev(2, [{iov_base="", iov_len=0}, {iov_base=" ", iov_len=1}], 2 ) = 1
writev(2, [{iov_base="", iov_len=0}, {iov_base="Not supported", iov_len=13}], 2Not supported) = 13
writev(2, [{iov_base="", iov_len=0}, {iov_base="\n", iov_len=1}], 2
) = 1
writev(2, [{iov_base="Could not set \"user.foo\" for foo"..., iov_len=33}, {iov_base=NULL, iov_len=0}], 2Could not set "user.foo" for foo
) = 33
exit_group(1)                           = ?
+++ exited with 1 +++

That's a bit easier to experiment with at least.

(I see from the strace that the user. is implied with attr -s, hence I got user.user.foo, oops, the result is the same without the redundant prefix)

@ijc

This comment has been minimized.

Copy link
Collaborator

ijc commented Feb 12, 2019

more:

(ns: sshd) linuxkit-4a444fac9d0f:~# mount -o remount,user_xattr /
(ns: sshd) linuxkit-4a444fac9d0f:~#  attr -s foo -V bar foo 
attr_set: Not supported
Could not set "foo" for foo
(ns: sshd) linuxkit-4a444fac9d0f:~# df -h .
Filesystem                Size      Used Available Use% Mounted on
overlay                 492.1M      2.6M    489.5M   1% /
@ijc

This comment has been minimized.

Copy link
Collaborator

ijc commented Feb 12, 2019

# cat pr3287.yml
kernel:
  image: linuxkit/kernel:4.19.20
  cmdline: "console=ttyS0"
init:
  - linuxkit/init:4448c4b6d4308244160b71c423bc9df32bc180db
  - linuxkit/runc:a81d48a1568f41b1c2e048fe017dbd88c6a4bdcc
  - linuxkit/containerd:56e03cfa92d75d6eb5a7fc44742e50f427ba29a3
  - linuxkit/ca-certificates:v0.6
onboot:
  - name: sysctl
    image: linuxkit/sysctl:v0.6
  - name: rngd1
    image: linuxkit/rngd:v0.6
    command: ["/sbin/rngd", "-1"]
  - name: format
    image: linuxkit/format:v0.6
  - name: mount
    image: linuxkit/mount:v0.6
    command: ["/usr/bin/mountie", "/var/lib"]
services:
  - name: getty
    image: linuxkit/getty:2eb742cd7a68e14cf50577c02f30147bc406e478
    env:
     - INSECURE=true
  - name: rngd
    image: linuxkit/rngd:v0.6
  - name: dhcpcd
    image: linuxkit/dhcpcd:v0.6
  - name: sshd
    image: linuxkit/sshd:c4bc89cf0d66733c923ab9cb46198b599eb99320
    binds:
      - /var/lib:/persistent
      - /root/.ssh:/root/.ssh
      - /etc/resolv.conf:/etc/resolv.conf
      - /run:/run
      - /tmp:/tmp
      - /etc:/hostroot/etc
      - /usr/bin/ctr:/usr/bin/ctr
      - /usr/bin/runc:/usr/bin/runc
      - /containers:/containers
      - /var/log:/var/log
      - /var/lib/containerd:/var/lib/containerd
      - /dev:/dev
      - /sys:/sys
files:
  - path: root/.ssh/authorized_keys
    source: ~/.ssh/id_rsa.pub
    mode: "0600"
    optional: true
trust:
  org:
    - linuxkit
# linuxkit build pr3287.yml
...
# linuxkit run -publish 2222:22 -disk size=2G pr3287

then in the ssh session:

(ns: sshd) linuxkit-4e725ef9a167:~# df -h /persistent/
Filesystem                Size      Used Available Use% Mounted on
/dev/sda1                 1.9G      6.2M      1.8G   0% /persistent
(ns: sshd) linuxkit-4e725ef9a167:~# apk add -U attr strace
fetch http://dl-cdn.alpinelinux.org/alpine/v3.8/main/x86_64/APKINDEX.tar.gz
fetch http://dl-cdn.alpinelinux.org/alpine/v3.8/community/x86_64/APKINDEX.tar.gz
(1/3) Installing libattr (2.4.47-r7)
(2/3) Installing attr (2.4.47-r7)
(3/3) Installing strace (4.22-r0)
Executing busybox-1.28.4-r1.trigger
OK: 19 MiB in 29 packages
(ns: sshd) linuxkit-4e725ef9a167:~# touch /persistent/foo
(ns: sshd) linuxkit-4e725ef9a167:~# attr -s foo -V bar /persistent/foo 
Attribute "foo" set to a 3 byte value for /persistent/foo:
bar
(ns: sshd) linuxkit-4e725ef9a167:~# strace attr -s foo -V bar /persistent/foo 
execve("/usr/bin/attr", ["attr", "-s", "foo", "-V", "bar", "/persistent/foo"], 0x7ffcd1fdd818 /* 15 vars */) = 0
arch_prctl(ARCH_SET_FS, 0x7fc49bef5b88) = 0
set_tid_address(0x7fc49bef5bc0)         = 691
open("/etc/ld-musl-x86_64.path", O_RDONLY|O_CLOEXEC) = -1 ENOENT (No such file or directory)
open("/lib/libattr.so.1", O_RDONLY|O_CLOEXEC) = 3
fcntl(3, F_SETFD, FD_CLOEXEC)           = 0
fstat(3, {st_mode=S_IFREG|0644, st_size=18144, ...}) = 0
read(3, "\177ELF\2\1\1\0\0\0\0\0\0\0\0\0\3\0>\0\1\0\0\0\370\21\0\0\0\0\0\0"..., 960) = 960
mmap(NULL, 2117632, PROT_READ|PROT_EXEC, MAP_PRIVATE, 3, 0) = 0x7fc49ba60000
mmap(0x7fc49bc63000, 8192, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_FIXED, 3, 0x3000) = 0x7fc49bc63000
close(3)                                = 0
mprotect(0x7fc49bc63000, 4096, PROT_READ) = 0
mprotect(0x7fc49bef2000, 4096, PROT_READ) = 0
mprotect(0x555575fe5000, 4096, PROT_READ) = 0
lsetxattr("/persistent/foo", "user.foo", "bar", 3, 0) = 0
ioctl(1, TIOCGWINSZ, {ws_row=41, ws_col=149, ws_xpixel=2533, ws_ypixel=1312}) = 0
writev(1, [{iov_base="Attribute \"foo\" set to a 3 byte "..., iov_len=57}, {iov_base=":\n", iov_len=2}], 2Attribute "foo" set to a 3 byte value for /persistent/foo:
) = 59
writev(1, [{iov_base="bar", iov_len=3}, {iov_base="\n", iov_len=1}], 2bar
) = 4
exit_group(0)                           = ?

So that works, suggesting the issue is with overlay filesystems.

I'm not sure yet why my hack to the test case to move it to the persistent didn't work.

@ijc

This comment has been minimized.

Copy link
Collaborator

ijc commented Feb 13, 2019

I had a hypothesis overnight which is that the tests are using /tmp not the cwd.

(ns: sshd) linuxkit-4e725ef9a167:~# df -h /tmp
Filesystem                Size      Used Available Use% Mounted on
tmpfs                    98.4M         0     98.4M   0% /tmp
(ns: sshd) linuxkit-4e725ef9a167:~# touch /tmp/foo
(ns: sshd) linuxkit-4e725ef9a167:~# apk add -U attr strace
(1/3) Installing libattr (2.4.47-r7)
(2/3) Installing attr (2.4.47-r7)
(3/3) Installing strace (4.22-r0)
Executing busybox-1.28.4-r1.trigger
OK: 19 MiB in 29 packages
(ns: sshd) linuxkit-4e725ef9a167:~# attr -s foo -V bar /tmp/foo
attr_set: Not supported
Could not set "foo" for /tmp/foo
(ns: sshd) linuxkit-4e725ef9a167:~# mount -o remount,user_xattr /tmp
mount: /tmp: mount point not mounted or bad option.

I guess user xattrs and tmpfs don't mix. http://man7.org/linux/man-pages/man5/tmpfs.5.html seems to confirm:

  The tmpfs filesystem supports extended attributes (see xattr(7)), but
  user extended attributes are not permitted.

Although my local tmpfs(5) doesn't have those words.

I'm going to investigate whether the tests will obey $TMP/$TMPDIR etc as my first attempt to confirm/fix. If not then I will arrange for /tmp to be on the persistent disk for this container.

@ijc

This comment has been minimized.

Copy link
Collaborator

ijc commented Feb 13, 2019

BTW, I think a v1.2.4 of containerd is imminent (or already out, I didn't check my mail yet). I will bundle the fix for this issue in with the PR I will raise for that.

@ijc

This comment has been minimized.

Copy link
Collaborator

ijc commented Feb 13, 2019

[PASS ] linuxkit.packages.containerd 399.59s
from:

diff --git a/test/pkg/containerd/run.sh b/test/pkg/containerd/run.sh
index 23dc8d2b8..a4ea7b907 100755
--- a/test/pkg/containerd/run.sh
+++ b/test/pkg/containerd/run.sh
@@ -5,10 +5,17 @@ function failed {
        exit 1
 }
 
+set -x
+
 # Get these into the logs.
 git describe HEAD
 git rev-parse HEAD
 
+mkdir -p /var/lib/tmp
+chmod 1777 /var/lib/tmp
+# The unit tests need user_xattr support, which /tmp (a tmpfs) does not support.
+export TMPDIR=/var/lib/tmp
+
 # unset -race (does not work on alpine; see golang/go#14481)
 export TESTFLAGS=
 make root-test || failed

(Probably the chmod is not needed, but I added it because the test cycle is pretty slow...)

@ijc

This comment has been minimized.

Copy link
Collaborator

ijc commented Feb 13, 2019

chmod wasn't needed indeed.

I'm tracking containerd/containerd#3002, expect a new PR fixing the CI issue when 1.2.4 comes out (I think it should be today or tomorrow, if it drags on I might open a specific PR)

@ijc ijc referenced this pull request Feb 14, 2019

Merged

Containerd v1.2.4 #3290

@ijc

This comment has been minimized.

Copy link
Collaborator

ijc commented Feb 14, 2019

Fixed in #3290

@ijc ijc closed this Feb 14, 2019

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