containerd / continuity Goto Github PK

See a similar change from containerd: containerd/containerd#1580

Currently continuity store device numbers in a resource and could possibly use that to create a device on a system. However since devices and their numbers are system specific continuity should not store the numbers, but rather only store metadata that is non system specific. Continuity apply should not make any alterations in regards to devices but only check the metadata to ensure the referenced device exists.

#66 stubbed out support. We'll need to direct to the correct syscalls for this to work in in practice.

cc @estesp

This proposal enables specifying digest of a partial chunk of a file.

I'd like to integrate this into https://github.com/AkihiroSuda/filegrain

diff --git a/proto/manifest.proto b/proto/manifest.proto
index e2e110f..9363478 100644
--- a/proto/manifest.proto
+++ b/proto/manifest.proto
@@ -39,13 +39,9 @@ message Resource {
     // for regular files.
     uint64 size = 7;

-    // Digest specifies the content digest of the target file. Only valid for
-    // regular files. The strings are formatted in OCI style, i.e. <alg>:<encoded>.
-    // For detailed information about the format, please refer to OCI Image Spec:
-    // https://github.com/opencontainers/image-spec/blob/master/descriptor.md#digests-and-verification
-    // The digests are sorted in lexical order and implementations may choose
-    // which algorithms they prefer.
-    repeated string digest = 8;
+    // Digests specifies the content digest of the target file. Only valid for
+    // regular files.
+    repeated DigestEntry digests = 8;

     // Target defines the target of a hard or soft link. Absolute links start
     // with a slash and specify the resource relative to the bundle root.
@@ -85,7 +81,7 @@ message ADSEntry {
     // See also the description about the digest below.
     bytes data = 2;

-    // Digest is a CAS representation of the stream data.
+    // Digests is a CAS representation of the stream data.
     //
     // At least one of data or digest MUST be specified, and either one of them
     // SHOULD be specified.
@@ -93,5 +89,26 @@ message ADSEntry {
     // How to access the actual data using the digest is implementation-specific,
     // and implementations can choose not to implement digest.
     // So, digest SHOULD be used only when the stream data is large.
-    string digest = 3;
+    repeated DigestEntry digests = 3;
+}
+
+// DigestEntry encodes information of digest of a data stream.
+//
+// It is valid to compose multiple digest entries to represent the digest of a single stream.
+// e.g. [{digest=..,begin=0,end=1073741823},{digest=..,begin=1073741824,end=2147483647}].
+//
+// Each of entries SHOULD not overlap and SHOULD cover whole the stream.
+message DigestEntry {
+    // Digest strings are formatted in OCI style, i.e. <alg>:<encoded>.
+    // For detailed information about the format, please refer to OCI Image Spec:
+    // https://github.com/opencontainers/image-spec/blob/master/descriptor.md#digests-and-verification
+    // The digests are sorted in lexical order and implementations may choose
+    // which algorithms they prefer.
+    repeated string digest = 1;
+
+    // Begin and end specify the byte offsets of the chunk of the target data
+    // stream that corresponds to the digest.
+    uint64 begin = 2;
+
+    uint64 end = 3;
 }

My suggestion is:

1. Make sure resource forks should be represented as com.apple.ResourceFork xattr, not as ..namedfork/rsrc nor as AppleDouble nor as Alternative Data Stream
2. Make sure FinderInfo should be represented as com.apple.FinderInfo xattr, not as AppleDouble
3. Prohibit including AppleDouble files (_MACOSX and ._<original_filename>) in the manifest maybe?

message Resource {
  ..
  // Ads stores one or more alternate data streams for the target resource.
  map<string, bytes> ads = 13;
}

In current design, the representation of ADS is likely to become so huge, because it contains the full data of ADS.

So WDYT about adding CAS representation for ADS?
i.e.

message Resource {
  ..
  // Ads stores one or more alternate data streams for the target resource.
  map<string, bytes> ads = 13;
  // AdsDigests stores digests of one or more alternate data streams for the target resource.
  // How to access the actual data using the digest is not defined.
  // Intersection of Ads and AdsDigests SHOULD be an empty set.
  map<string, string> ads_digests = 14;
}

Continuity manifest should be able to be created from a tar stream or be able to generate a tar stream from a source directory and manifest.

The test code should not be using a custom resource type which in turn must be created into the Resource types used by manifests.

r, err := Declare(Resource{
    Kind: RegularFile,
    Path: "/foo",
})

And for tests

[]Resource{
    MustDeclare(Resource{
        Kind: RegularFile,
        Path: "/foo",
    }),
}

Referenced from #14 (comment)

Why do we have to support Windows? IT seems like a waste of time.

User may want to ignore the permission bits specified in the manifest during verification, for platform-specific reason.

e.g. Create a manifest on Unix and verify it on Windows, and vice versa

This could be implemented as a bitmask like this

$ continuity verify --ignore-mode-bits 0777

This will make it much easier to understand code the involves context.Context and the thing that manipulates the local filesystem.

The cp --reflink command uses the FICLONERANGE ioctl() command, but not copy_file_range(); in fact, no utilities use copy_file_range(), though it should be the default.

https://lwn.net/Articles/754506/

And there is also ioctl_fideduperange(2. really confusing.. :(

Right now we are testing 1.8, 1.9, 1.10, 1.11, and tip via Travis, across 3 platforms; so 15 travis runs for each PR. Not a big deal if we want to maintain compatibility back that far, but none of our other projects seem to have that breadth of coverage.

@containerd/containerd-maintainers

In order to fully reproduce a source file system we should include the modification and creation times in the manifest.

This project will become github.com/containerd/continuity as part of containerd/containerd#772. We'll have to do the following:

• Transfer to containerd org
• Update import paths (#72)

Dear containerd authors,

could you clarify the copyright for continuity, maybe like it was done in https://github.com/containerd/fifo/pull/15/files ?

Also, while grepping for the keyword 'Copyright', I found sysx/asm.s which has this mention:

// Copyright 2014 The Go Authors.  All rights reserved.
// Use of this source code is governed by a BSD-style
// license that can be found in the LICENSE file.

However the license file of continuity is the Apache license file, so maybe something should be changed here, like adding a LICENSE.BSD file, and referring to it in sysx/asm.s. Or something similar.

Thanks

Please consider defining a media type?
e.g. application/vnd.stevvooe.continuity.manifest.v1+pb (But I'm not sure what is the de facto standard of a suffix for protobuf. Seems undefined in RFC https://en.wikipedia.org/wiki/Media_type#Suffix )

We have cut these data structures but have yet to create implementations. We intend for continuity to have solid cross-platform support, so this needs to be taken care of.

The current implementation of continuity apply checks an existing directory against a Continuity manifest. A better model would involve specifying a source and destination, so that Continuity creates the target based on the manifest and data source. The data source could take multiple forms, such as a tar file, preexisting directory on disk, version control repository, etc.

Possible syntax:

continuity apply --source <dir|tar|etc.> <manifest> <target>

For the dir case, it would be more efficient to move files from the source directory instead of copying them. Perhaps this should be made optional, by splitting dir into copydir and movedir options. This could be further refined to support special features for copy-on-write filesystems such as btrfs.

(*driver).DeviceInfo is declared twice for the darwin platform, in driver_darwin.go and driver_unix.go:

src/github.com/stevvooe/continuity/driver_unix.go:105: (*driver).DeviceInfo redeclared in this block
	previous declaration at src/github.com/stevvooe/continuity/driver_darwin.go:5

Currently the copy functionality added in the fs package is just used for copying a directory and is called CopyDir. We should rename this to Copy and expand its abilities to support the filtering needed by buildkit and some of the performance optimizations from moby.

The sysx package is generally useful and provides the generators and framework needed for generating syscalls not yet in Go.

This package could be used to simply packages such as https://github.com/docker/docker/tree/master/pkg/system and in many times negate the need to use such packages.

Additionally we can add functionality for copy_file_range() (https://lwn.net/Articles/659523/) which could be useful for continuity but even more useful for other projects.

#82 (comment)

• should we use / separator on both Unix and Windows?
• "absolute path" does not contain drive letter (C:), right?

Background

Following on from containerd/containerd#1482 (comment)

Docker style volumes (those declared with VOLUME /path in a Dockerfile) have the property that they are initialised using the contents /path in underlying image. In order to produce a helper which can allow containerd clients to support such functionality a safe mechanism to duplicate a filesystem trees is required.

Safe here means at least:

• Must never dereference symbolic links (must recreate them as is, even if they dangle) including never traversing a directory symlink (since this could lead outside the root).
• Must not pull entire filesystem objects into RAM.
• Safe against directory hardlink loops.

Since continuity can safely and fully represent a filesystem tree extending it to allow duplicating that tree seems reasonable.

Proposal

I propose adding a single method to the existing Resource interface:

Copy(srcroot, dstroot string) error

This method will (given p := resource.Path()) duplicate srcroot/p to dstroot/p, preserving permissions and ownership. If the Resource implements the Xattrer interface then xattrs will also be preserved. Likewise if the Resource implements Hardlinkable then all hardlinks will be created. (similarly for any other interfaces I've missed or which are added in the future)

In normal expected usage srcroot would be the root of a Context. Therefore a new method will also be added to the Context interfaces:

Copy(dstroot string) error

This will BuildManifest(self), sort the result using ByPath and then iterate calling resource.Copy(self.path, dstroot) for each resource.

Variations

Copying Xattrer and Hardlinkable as responsiblity of caller

The support for Xattrer and Hardlinkable in each implementation of Resource seems likely to be quite similar, which likely implies the addition of a helper for each.

We could therefore consider excluding handling of those from the Resource.Copy method and instead push the responsibility back to the callers (so, Context.Copy here) using those same helpers.

Perhaps:

ApplyXattrs(xattrer Xattrer, dstroot, path string) error
ApplyHardlinks(hlable Hardlinkable, dstroot, path string) error

I lean towards requiring Resource.Copy to completely copy all known properties since this seems simpler. The downside is that callers cannot then opt out of copying certain properties.

Copyable interface

Rather than adding a new method to Resource we could add a new interface:

type Copyable interface {
    Copy(dstroot string) error
}

However since all Resources would have to support the method in order for the overall functionality to be reliably usable there doesn't seem much point.

I noticed some test files are missing, we need to add circle ci support to make sure each commit doesn't leave anything out.

We shouldn't be trying to maintain a system package and should only include changes which haven't been accepted upstream. The upstream package has much better support for a wider range of architectures which we could support.

https://github.com/gogo/protobuf/ has a number of benefits, including faster serialization and tighter control over generated code. We have used on a number of successful projects.

If someone wants to take this on, let me know.

Hi.
Cross-posting the bug, which is tracked during build progress for cri-o :

github.com/containerd/continuity/fs
mkdir -p $WORK/b407/
cd $WORK
/usr/local/bin/llvm-goc -fgo-importcfg=/dev/null -c -x c - -o /dev/null || true
mkdir -p $WORK/b407/importcfgroot/github.com/containerd/continuity
ln -s /home/oceanfish81/.cache/go-build/01/0149872ebed362b2db5d5cb500fe4c36dc53aa0eeaf43445d2a1d57f87797f22-d $WORK/b407/importcfgroot/github.com/containerd/continuity/libsysx.a
mkdir -p $WORK/b407/importcfgroot/github.com/pkg
ln -s /home/oceanfish81/.cache/go-build/5c/5c5c0c7d8f5175eae2b4d2a083bbfa586296699b6d6e847a6034fb77f0283e5b-d $WORK/b407/importcfgroot/github.com/pkg/liberrors.a
mkdir -p $WORK/b407/importcfgroot/github.com/sirupsen
ln -s /home/oceanfish81/.cache/go-build/92/92e22833f5517c1584404320f391ae330ef4ec12a7ee701ea0863735c4e5a39e-d $WORK/b407/importcfgroot/github.com/sirupsen/liblogrus.a
mkdir -p $WORK/b407/importcfgroot/golang.org/x/sync
ln -s /home/oceanfish81/.cache/go-build/b4/b4dade4823a139177beb6326f044ad405152a51915db5eff2d1f1100f377d538-d $WORK/b407/importcfgroot/golang.org/x/sync/liberrgroup.a
mkdir -p $WORK/b407/importcfgroot/golang.org/x/sys
ln -s /home/oceanfish81/.cache/go-build/79/79581a3b9031e51313108a803ec4be9cd5e789dcf66ef4dbe3af40cfbd47664a-d $WORK/b407/importcfgroot/golang.org/x/sys/libunix.a
cd /home/oceanfish81/cri-o/vendor/github.com/containerd/continuity/fs
/usr/local/bin/llvm-goc -c -O2 -g -m64 -fdebug-prefix-map=$WORK=/tmp/go-build -gno-record-gcc-switches -fgo-pkgpath=github.com/containerd/continuity/fs -o $WORK/b407/go.o -I $WORK/b407/importcfgroot ./copy.go ./copy_linux.go ./diff.go ./diff_unix.go ./dtype_linux.go ./du.go ./du_unix.go ./hardlink.go ./hardlink_unix.go ./path.go ./stat_linuxopenbsd.go ./time.go
github.com/containerd/continuity/fs
vendor/github.com/containerd/continuity/fs/dtype_linux.go:61:26:

error: reference to undefined identifier 'syscall.DT_UNKNOWN'

Is it possible to rule out the cause of this, by expressing the algorithm without the usage of a pre-compiled constant (to calculate in another way) ?

Ivan

It would be nice to be able to mount a continuity manifest via fuse via command:

continuity mount --fuse <mount> <manifest> <root>

This would only serve files identified by the manifest and only if their hash matched. We use --fuse to allow other kinds of mount implementations (imagine http, tar, etc.). Such a command should take the same options required to map uids, links, etc (not defined yet).

Update the generation process in sysx to take advantage of go generate

This if statement is not always triggered correctly because of a flaw in the logic above it.

If the size of the file was 0, eq will be false. This means that sameFile thinks that two empty files are different even though they are the same.

As a result, when you start a new container and immediately run ctr snapshot diff on it, you get a bunch of spurious "changed" empty files. This can be problematic when you're trying to understand if there are real changes in the running image.

#66 stubbed these implementations out. Calls will have to be routed to appropriate syscalls.

cc @estesp

On s390x the copy package fails if run inside a container because it fails to detect the existence of copy_file_range properly.

If ran on host

But if the same code is run under limited capabilities it produces EPERM instead(even though it doesn't seem to be allowed error for this syscall) and the copy fails.

--- FAIL: TestCopyDirectory (0.00s)
    copy_test.go:47: Copy test failed: operation not permitted
        copy file range failed
        github.com/containerd/continuity/fs.copyFileContent
        	/root/go/src/github.com/containerd/continuity/fs/copy_linux.go:79
        github.com/containerd/continuity/fs.CopyFile
        	/root/go/src/github.com/containerd/continuity/fs/copy.go:137
        github.com/containerd/continuity/fs.copyDirectory
        	/root/go/src/github.com/containerd/continuity/fs/copy.go:92
        github.com/containerd/continuity/fs.copyDirectory
        	/root/go/src/github.com/containerd/continuity/fs/copy.go:79
        github.com/containerd/continuity/fs.CopyDir
        	/root/go/src/github.com/containerd/continuity/fs/copy.go:40
        github.com/containerd/continuity/fs.testCopy
        	/root/go/src/github.com/containerd/continuity/fs/copy_test.go:97
        github.com/containerd/continuity/fs.TestCopyDirectory
        	/root/go/src/github.com/containerd/continuity/fs/copy_test.go:46
        testing.tRunner
        	/usr/local/go/src/testing/testing.go:827
        runtime.goexit
        	/usr/local/go/src/runtime/asm_s390x.s:782
        failed to copy files
        github.com/containerd/continuity/fs.copyDirectory
        	/root/go/src/github.com/containerd/continuity/fs/copy.go:93
        github.com/containerd/continuity/fs.copyDirectory
        	/root/go/src/github.com/containerd/continuity/fs/copy.go:79
        github.com/containerd/continuity/fs.CopyDir
        	/root/go/src/github.com/containerd/continuity/fs/copy.go:40
        github.com/containerd/continuity/fs.testCopy
        	/root/go/src/github.com/containerd/continuity/fs/copy_test.go:97
        github.com/containerd/continuity/fs.TestCopyDirectory
        	/root/go/src/github.com/containerd/continuity/fs/copy_test.go:46
        testing.tRunner
        	/usr/local/go/src/testing/testing.go:827
        runtime.goexit
        	/usr/local/go/src/runtime/asm_s390x.s:782
        failed to copy
        github.com/containerd/continuity/fs.testCopy
        	/root/go/src/github.com/containerd/continuity/fs/copy_test.go:98
        github.com/containerd/continuity/fs.TestCopyDirectory
        	/root/go/src/github.com/containerd/continuity/fs/copy_test.go:46
        testing.tRunner
        	/usr/local/go/src/testing/testing.go:827
        runtime.goexit
        	/usr/local/go/src/runtime/asm_s390x.s:782
FAIL

root@s390x-ubuntu-17:/home/ubuntu# uname -a
Linux s390x-ubuntu-17 4.4.0-130-generic #156-Ubuntu SMP Thu Jun 14 09:18:59 UTC 2018 s390x s390x s390x GNU/Linux

@estesp

Here is an example test suite from lua:

LINUX: https://github.com/justincormack/ljsyscall/blob/master/test/linux.lua
BSD: https://github.com/justincormack/ljsyscall/blob/master/test/bsd.lua

Courtesy @justincormack

Continuity currently captures device major/minor numbers, but doesn't apply or verify device nodes. Support for this should be added so that Continuity reproduces filesystems more accurately. If capturing devices in Continuity is not useful for a particular application, they should be filtered out with --ignore (see #26).

Continuity does not currently verify digests during the apply operation. It should be changed to verify them by default. In case the data comes from a trusted source, such as a content-addressable store, there should be a --disable-digest option to override this default and avoid unnecessary hashing.

verify should continue to always verify digests.

Continuity should capture ctime (file creation timestamp) and mtime (file modification timestamp) in manifests, and by default, it should apply and verify these properties.

In case a particular application of Continuity needs to be insensitive to creation or modification times, it should be possible to use --ignore-attr at verify or apply time to exclude these attributes (see #27).

Continuity has the potential to be used to store filesystem "diffs" in the same way tar files are used in Docker today. In order to support these diffs "whiteout" files should have first class support in the manifest. This will allow a portable way to use whiteouts between different copy on write file systems despite where it was originally generated. As a research item, how are whiteout files handled in the tar file diffs used by docker and are there any issues across filesystems (i.e export from AUFS import to overlay).

Similar to #28, named pipes should be supported in the application and verification steps.

related commit: 246e490

For the sake of completeness, Continuity manifests should include unix sockets. However, it is not generally useful for continuity to create these when applying a manifest. Thus, apply and verify should ignore unix sockets by default. A special flag should allow them to be applied and verified.

For encoding absolute symlinks, we typically remove the prefix of the continuity context root from the link target and store that as the resource target. When this gets "applied", the meaning of this absolute target becomes unclear in the context of chroot or linux filesystem namespaces. If we are inside a chroot or namespace, and applying the manifest to "/" (the root path of the chroot), it is unclear whether we have to write the full path, as it appears in the filesystem or just the path rooted at the context. It is also unclear whether this behaves differently for chroot and namespaces.

If this works correctly for both chroot and namespaces, we can likely avoid any complexity. If they need to be written differently for a given context, we need to provide the necessary hooks to allow the caller to adjust how absolute links are created.

Ideally, we'd find a nice solution to this, as options and flags that affect symlinks are notoriously finicky.

FreeBSD, Linux, and OS X each offer variants of this API. On OS X (at least), the ACL API is separate from extended attributes.

Relates to moby/moby#38758 / moby/moby#38404 (comment) / golang/go@a2a3dd0

Due to a change in Go 1.12, some file modes are no longer being returned as os. ModeDevice, but as os.ModeCharDevice. This caused pulling images to break on docker (moby/moby#38404 (comment)).

I'm not very familiar with this repository, but a quick search showed some similar uses in this package, so I suspect some of those will have to be updated accordingly.

I see there is a makefile but make does not work and go build gives me

go build
# github.com/stevvooe/continuity
./context.go:383: undefined: atomicWriteFile

The Go files under sysx have no build tags, but they seem to be intended for specific platforms. Notably, xattr_linux_arm64.go prevents continuity from building on Darwin:

sysx/xattr_linux_arm64.go:25: undefined: syscall.SYS_LLISTXATTR
sysx/xattr_linux_arm64.go:47: undefined: syscall.SYS_LREMOVEXATTR
sysx/xattr_linux_arm64.go:75: undefined: syscall.SYS_LSETXATTR
sysx/xattr_linux_arm64.go:103: undefined: syscall.SYS_LGETXATTR

cc @dmcgowan

There should be some flexibility in choosing what attributes are enforced or verified by continuity apply and continuity verify. For example, a user may wish to ignore ctime and mtime in continuity verify. Add a --ignore-attr flag for verify and apply providing control over which metadata attributes to ignore.

It should be possible to ignore files that should not be represented in the continuity manifest, such as certain device drivers. Support a --ignore flag to continuity build that allows files to be filtered out at build time.

https://github.com/stevvooe/continuity/blob/master/proto/manifest.proto

Since mode is currently defined as uint32, there is no way to omit mode from the manifest
(zero should be interpreted as zero, not nil)

Or maybe it should bbetter to clarify that mode is mandatory field and cannot be omitted, but I'm not sure it causes compatibility issue