Issue #368 Making OCI and Docker daemon transport work on Windows #379

hferentschik · 2017-11-09T19:15:44Z

Signed-off-by: Hardy Ferentschik hardy@hibernate.org

runcom · 2017-11-09T19:18:34Z

I have unfortunately no way to verify this on windows. Changes make sense though

hferentschik · 2017-11-09T19:19:24Z

docker/daemon/daemon_src.go

-const temporaryDirectoryForBigFiles = "/var/tmp" // Do not use the system default of os.TempDir(), usually /tmp, because with systemd it could be a tmpfs.
+var temporaryDirectoryForBigFiles string
+
+func init() {


Instead of doing the same thing in multiple places, I was wondering whether there should be a shared util function GetTemporaryDirectoryForBigFiles somewhere. AFAICT, there is no obvious location yet for these type of utility method. Should I create one? And if so where?

Yes, please, move this into a single implementation.

Probably a subpackage in the internal hierarchy, something like containers/image/internal/tmpdir.

I wouldn’t be opposed to making this a public subpackage under containers/image/pkg/ either, it could be useful for other software; OTOH having such a containers-independent utility under containers/image feels a little weird.

Ok. I think I go for the internal/tmpdir

hferentschik · 2017-11-09T19:23:03Z

oci/layout/oci_transport.go

 	// This is necessary to prevent directory paths returned by PolicyConfigurationNamespaces
 	// from being ambiguous with values of PolicyConfigurationIdentity.
-	if strings.Contains(resolved, ":") {
+	if strings.Contains(resolved, ":") && runtime.GOOS != "windows" {


I am not so sure about this one. Obviously, on Windows, there will be a ':' in the path. Just adding this condition works for my use cases. I am not sure what the comment tries to say in terms of PolicyConfigurationNamespaces and PolicyConfigurationIdentity. I need some guidance here on whether a simple OS check is sufficient here or whether something else needs to happen. Maybe it can at least be a first step solution, since not allowing colon ':' in the path stops this to be working on Windows right in the beginning.

The original comment is no longer applicable after 4eb8db3 (and should be updated, along with ValidatePolicyConfigurationIdentity), but this restriction is still somewhat necessary, because the same ambiguity exists in ParseReference:

/path:busybox means dir="/path", image="busybox"

/path and /path: means dir="/path", image=""

/path:busybox:latest means dir="/path", image="busybox:latest";

Note that dir="/path:busybox", image="latest" is impossible to express in the string form consumed by ParseReference, because it would be parsed the other way. Giving up on ParseReference for this path and allowing dir="/path:busybox" only via NewReference would also be problematic because StringWithinTransport() must return a valid input for ParseReference.

I think it would be very reasonable to accept a : on Windows (perhaps, to be more strict, only in the second position, but the output of explicitfilepath.ResolvePathToFullyExplicit is supposed to be a valid path, and : is reserved on Windows, so it should not appear anywhere but after a drive letter.)

The flip side of that, though, is that ParseReference should likewise accept paths starting with a drive letter and a colon, and parse that as a part of the dir , not as the separator starting image.

ValidatePolicyConfigurationIdentity should also be updated - to treat the input only as a directory, the way PolicyConfigurationIdentity formats it now, and to reject : on non-Windows only.

I am aware of the problems of ParseReference. In fact, I am not using ParseReference and use NewReference directly. That works fine when using the API programmatically. It is more an issue in the case of Skopeo or other potential tools which want to create a reference from a single string.

As I mentioned before, I think the use of ':' is unfortunate in this case. I suggested back then to use a different separator like '@' or similar.

I feel the ParseReference problems is a bigger issue which I'd like to defer to another issue. WDYT?

In moving this along, is a OS specific check ok, together with an adjustment to ValidatePolicyConfigurationIdentity?

I feel the ParseReference problems is a bigger issue which I'd like to defer to another issue. WDYT?

I’d much prefer to keep of oci_transport.go consistent. Otherwise there’s no telling which of the alternatives will be updated in the future to make the code consistent, i.e. what use case will break.

An OS-specific check determining the treatment of : is OK with me.

Another way to put it, if you really don’t want to update ParseReference etc. for Windows, I guess I can live with that, but then you’d have to document the code clearly and explicitly enough to keep your work working in the future.

@mtrmac another unrelated question. There is one more problem I am seeing which is not addressed in this pull request yet. Could you have a look at

image/copy/copy.go

Line 290 in 6e47653

osErr := fmt.Errorf("image operating system %q cannot be used on %q", c.OS, runtime.GOOS)

.

As you know I am exporting a bunch of images from a Docker daemon (running in a VM), store them in an OCI tree on a Windows host and then at some stage re-import into a Docker daemon. This works fine for macOS and Linux. On Windows, I get the error which is referenced above. Effectively:

image operating system linux cannot be used on windows

src.OCIConfig() seems to return linux, I would expect Windows though. Any ideas? I have locally commented out the call to checkImageDestinationForCurrentRuntimeOS, since I find it questionable either way. Moving forward though, I was wondering how to solve this? If there is no bug in src.OCIConfig() , I was wondering whether it would make sense to add something to SystemContext which allows to skip the check for ImageDestinationForCurrentRuntimeOS. WDYT?

src.OCIConfig() seems to return linux, I would expect Windows though

The image was created by a dockerd on Linux, which is what is recorded inside the image’s config. It does not matter what OS is accessing the image or where the image was copied, it is designed to run on Linux.

I was wondering whether it would make sense to add something to SystemContext which allows to skip the check for ImageDestinationForCurrentRuntimeOS. WDYT?

Yes; I have implemented that already in 8074c38 for #346 .

Yes; I have implemented that already in 8074c38 for #346 .

LOL :-) Seems to be exactly what I need. Except it does not seem to be merged yet :-(
I will for now focus on the things we discussed so far and leave the destination check for #346. It would be great to get this merged though. Anything I can do do get #346 merged?

Anything I can do do get #346 merged?

It needs a review by @runcom at least; any extra reviews would be welcome as well.

@mtrmac any chance you mean ValidatePolicyConfigurationScope? There seems to be no ValidatePolicyConfigurationIdentity.

Yes, sorry.

If you mean ValidatePolicyConfigurationScope, what is the expected input to this function. The same as to _ ParseReference_? Something like <dir>:<image>?

I am still struggling a bit with these policies and scopes. Not quite sure how everything fits together in this area of the code.

References: A way for the user to refer to a single image in text, basically a way to express a types.ImageReference. There may be multiple ways to refer to a single underlying image, either by definition (docker://busybox vs. docker://busybox:latest), or by the environment (dir:pathtodir vs. dir:symlinktodir). Parsed by Transport.ParseReference, formatted by ImageReference.StringWithinTransport (i.e. the output of this must be consumable by ParseReference and be interpreted as the same underlying image).

Policy configuration scopes: A way for the user to define a hierarchical namespace of images. The same underlying image must have exactly one, canonical, “identity” (which is why symlinks are resolved in NewReference—but note that the identity fundamentally depends on the environment, i.e. the contents of the symlink), and PolicyConfigurationNamespaces returns the namespaces which contain that image (i.e. typically the analogue of the set of its parent directories / paths / hostnames etc.). The values are formatted by PolicyConfigurationIdentity and PolicyConfigurationNamespaces. There is no method to parse the string back into a Go value (strings in config files are compared for equality with the outputs of these two methods), but there is a ValidatePolicyConfigurationScope which should allow the output of PolicyConfiguration{Identity,Scope} and preferably little or nothing else, to allow detecting errors in the configuration.

hferentschik · 2017-11-09T19:25:38Z

@mtrmac, @runcom - I am trying to sort out some Windows incompatibilities which stop us to move forward with integrating containers/image into Minishift. See also minishift/minishift#1589. With the proposed changes, we can get things to work for all major OS types.

hferentschik · 2017-11-09T19:29:55Z

@runcom

I have unfortunately no way to verify this on windows. Changes make sense though

Sure. It would be nice to get more CI system involved and build not only on Linux, but also on other OS types. Minsihift uses CircleCI and AppVeyor for building/testing on macOS resp. Windows.

For now it might suffice that we are testing this change as part of our integration of containers/image into Minishift - minishift/minishift#1589.

It would be awesome to get this in, so that we can go ahead on our side as well.

mtrmac · 2017-11-13T15:47:27Z

oci/layout/oci_dest.go

+
+	// Chmod is not supported on Windows
+	if runtime.GOOS != "windows" {
+		if err := blobFile.Chmod(0644); err != nil {


How does Chmod fail here? Reading go/src/os/file_posix.go and go/src/syscall/syscall_windows.go it seems to be translated to GetFileAttributes+SetFileAttributes modifying the readonly attribute, which seems plausible.

There is a difference between os.Chmod and os.File.Chmod. Go figure. See also here google/skicka#77. As a workaround, we can also try to use os.Chmod at this point.

Oops, missed that. The code is fine, please just add a more detailed explanation, something like

// On POSIX systems, blobFile was created with mode 0600, so we need to make it readable. // On Windows, the “permissions of newly created files” argument to syscall.Open is // ignored and the file is already readable; besides, blobFile.Chmod, i.e. syscall.Fchmod, // always fails on Windows.

mtrmac · 2017-11-13T15:50:46Z

oci/layout/oci_dest.go

 	}
+
+	// need to explicitly close the file, since a rename won't otherwise not work on Windows
+	blobFile.Close()


Per https://golang.org/pkg/io/#Closer

The behavior of Close after the first call is undefined.

i.e. the defer … blobFile.Close() must be deactivated, similarly to the succeeded boolean.

hmm, one still would like defer to kick in until I can explicitly close. I'll take a look.

mtrmac · 2017-11-13T15:53:29Z

docker/daemon/daemon_src.go

-const temporaryDirectoryForBigFiles = "/var/tmp" // Do not use the system default of os.TempDir(), usually /tmp, because with systemd it could be a tmpfs.
+var temporaryDirectoryForBigFiles string
+
+func init() {


Yes, please, move this into a single implementation.

Probably a subpackage in the internal hierarchy, something like containers/image/internal/tmpdir.

I wouldn’t be opposed to making this a public subpackage under containers/image/pkg/ either, it could be useful for other software; OTOH having such a containers-independent utility under containers/image feels a little weird.

mtrmac

Thanks for working on this.

Are the "/" references throughout oci_transport.go correct, or should they more generically refer to a separator? Especially the check for absolute paths by a leading '/'

mtrmac · 2017-11-13T16:15:21Z

oci/layout/oci_transport.go

 	// This is necessary to prevent directory paths returned by PolicyConfigurationNamespaces
 	// from being ambiguous with values of PolicyConfigurationIdentity.
-	if strings.Contains(resolved, ":") {
+	if strings.Contains(resolved, ":") && runtime.GOOS != "windows" {


The original comment is no longer applicable after 4eb8db3 (and should be updated, along with ValidatePolicyConfigurationIdentity), but this restriction is still somewhat necessary, because the same ambiguity exists in ParseReference:

/path:busybox means dir="/path", image="busybox"

/path and /path: means dir="/path", image=""

/path:busybox:latest means dir="/path", image="busybox:latest";

Note that dir="/path:busybox", image="latest" is impossible to express in the string form consumed by ParseReference, because it would be parsed the other way. Giving up on ParseReference for this path and allowing dir="/path:busybox" only via NewReference would also be problematic because StringWithinTransport() must return a valid input for ParseReference.

I think it would be very reasonable to accept a : on Windows (perhaps, to be more strict, only in the second position, but the output of explicitfilepath.ResolvePathToFullyExplicit is supposed to be a valid path, and : is reserved on Windows, so it should not appear anywhere but after a drive letter.)

The flip side of that, though, is that ParseReference should likewise accept paths starting with a drive letter and a colon, and parse that as a part of the dir , not as the separator starting image.

ValidatePolicyConfigurationIdentity should also be updated - to treat the input only as a directory, the way PolicyConfigurationIdentity formats it now, and to reject : on non-Windows only.

hferentschik · 2017-11-14T16:09:25Z

@mtrmac I pushed an update for everything except oci_transport.go. I will look into the latter tomorrow.

mtrmac · 2017-11-14T16:11:47Z

@mtrmac I pushed an update for everything except oci_transport.go.

Thanks, looks good.

hferentschik · 2017-11-16T11:30:30Z

@mtrmac I rebased the pull request and changed ParseReference as well as ValidatePolicyConfigurationScope to handle Windows as well. I was tempted to refactor further, but decided against, since I did not want to change any semantics/tests for the non-Windows case. WDYT?

cc @runcom

hferentschik · 2017-11-16T11:47:36Z

copy/copy.go

-	if err := checkImageDestinationForCurrentRuntimeOS(src, dest); err != nil {
-		return err
-	}
+	// Disabling checkImageDestinationForCurrentRuntimeOS since the implementation is basing decision on whether there is a match on the current OS


@mtrmac I commented this check out. I think the current implementation is not correct and w/o the ability to control this behaviour in some way, I don't think it is really useful.

I don't think that the OS of the host can be used to determine whether a copy can occur. The destination of the copy might be remote. Or am I missing something?

dest already can choose whether this check make sense, via ImageDestination.MustMatchRuntimeOS; and this check is actually useful for copying to local docker-daemon:.

Maybe a specific transport should change its value of MustMatchRuntimeOS? Perhaps docker-daemon: when connecting to a remote host?

dest already can choose whether this check make sense,

Right, but it is hardcoded. In the case of the Docker daemon daemon_dest.go. Given that a Docker daemon can also be remote, having MustMatchRuntimeOS() always return true is preventing me to copy an image from a Windows host to a Docker daemon, even though it should be possible.

and this check is actually useful for copying to local docker-daemon:.

Maybe.

Maybe a specific transport should change its value of MustMatchRuntimeOS? Perhaps docker-daemon: when connecting to a remote host?

Right. So one solution would be to me daemonImageDestination.MustMatchRuntimeOS() stateful.
In the simplest case one could just use dockerclient.DefaultDockerHost. If the default is used, we return true, if the SystemContext explicitly specifies a value for DockerDaemonHost we return false (unless it maybe matches dockerclient.DefaultDockerHost). What I want to avoid is to make any decisions based on the IP/host specified in ctx.DockerDaemonHost. Maybe something like 127.0.0.1 or localhost would still make sense, but everything else is not easily verifiable.

WDYT? Make daemonImageDestination.MustMatchRuntimeOS() stateful?

@mtrmac Any feedback regarding the other changes in this pull request? Are we good on these?

@mtrmac I made MustMatchRuntimeOS() conditional. WDYT?

@mtrmac I made MustMatchRuntimeOS() conditional. WDYT?

Looks good. As you say, some of the cases are difficult to determine, and most users shouldn’t hit them. If someone does hits them,. we might need to make the tests a bit more precise and change the semantics; hopefully that won’t affect any previously working users.

hferentschik · 2017-11-20T14:04:16Z

@mtrmac, @runcom What do you think about this current state of the pull request?

mtrmac · 2017-11-20T22:35:03Z

oci/layout/oci_transport.go

-	// (Note: we do allow "/:someimage", a bit ridiculous but why refuse it?)
-	if scope == "/" {
-		return errors.New(`Invalid scope "/": Use the generic default scope ""`)
+	err := oci.ValidateScope(scope, dir)


Now that this helper exists, it could contain all of ValidatePolicyConfigurationScope, i.e. including the filepath.Clean check.

mtrmac · 2017-11-20T22:39:15Z

oci/layout/oci_transport.go

-		}
+	dir, image := oci.SplitDirAndImage(scope)
+	if image != nil && !oci.ValidImageName(*image) {
+		return errors.Errorf("Invalid image %s", image)


The output of PolicyConfigurationIdentity is now a path to a file/dir, and PolicyConfigurationNamespaces are parent directories; i.e. the valid values do not contain an image name any more. So, the input to ValidatePolicyConfigurationScope should not be treated as a path+image pair, but only as a path (and validated the same way resolved is validated in NewReference, i.e. 0/1 colon characters).

(To be fair, the original is also incorrect.)

… see the other new comment; NewReference maybe does not need refuse colons in resolved at all; in which case this should do just the check for absolute paths which are not /.

Ahh, that would make things easier indeed. Needs a bit of refactoring ....

mtrmac · 2017-11-20T22:46:39Z

oci/layout/oci_transport.go

+		return NewReference(dir, "")
 	}
-	return NewReference(dir, image)
+	return NewReference(dir, *image)


This would be simpler if SplitDirAndImage returned a "" instead of nil if the image value is missing.

I know. But as said, I wanted to keep the existing code behaving exactly how it was. And for that I need to differentiate between "" and nil. If you are ok to return "" and that an existing test gets changed, then yes.

I know. But as said, I wanted to keep the existing code behaving exactly how it was. And for that I need to differentiate between "" and nil.

All ParseReference does now is call SplitDirAndImage, and replace a nil with "". The difference is immediately erased.

Or are you talking (with the references to “existing test” about ValidatePolicyConfigurationScope)? That one does treat a trailing : and no : differently — but ValidatePolicyConfigurationScope should nowadays not expect a :image in the input at all, so that will disappear.

mtrmac · 2017-11-20T22:47:45Z

oci/archive/oci_transport.go

-
-	if len(sep) == 2 {
-		image = sep[1]
+	dir, image := oci.SplitDirAndImage(reference)


Nitpick: please use file in oci/archive, dir in oci/layout. Or something like path for both.

mtrmac · 2017-11-20T22:48:37Z

oci/archive/oci_transport.go

 	// from being ambiguous with values of PolicyConfigurationIdentity.
 	if strings.Contains(resolved, ":") {
 		return nil, errors.Errorf("Invalid OCI reference %s:%s: path %s contains a colon", file, image, resolved)
 	}


This should contain the same Windows-specific check as in oci/layout. Or preferably move that to another helper in the shared subpackage.

mtrmac · 2017-11-20T22:53:37Z

oci/oci.go

+// SplitDirAndImage tries to split the provided OCI reference into the OCI directory and image.
+// The image is returned as string pointer, allowing for nil being returned, if no image part could be extraced from the reference.
+// Neither directory nor image parts are validated at this stage.
+func SplitDirAndImage(reference string) (string, *string) {


Because this is shared between oci/archive and oci/layout, use something generic like path, not dir, throughout, please.

mtrmac · 2017-11-20T23:06:08Z

oci/oci.go

+	if len(groups) != 3 {
+		return reference, nil
+	}
+	return groups[1], &groups[2]


In non-Windows it is possible to use relative paths, e.g. oci:somedir:imagename. This seems to return a non-nil image only if the input is an absolute path; is that intentional?

AFAICT relative paths could be allowed without ambiguity:

c:\dir\… cannot mean “the relative path .\c:\dir” because c: is an invalid directory name

c:\dir… cannot mean “the image \dir… in the relative path c” because \dir is an invalid image name.

The troublesome case is c:dir, which could mean “the relative path dir on the current directory of c:” or “the image dir in the relative path c”, but at least https://blogs.msdn.microsoft.com/oldnewthing/20101011-00/?p=12563 says this concept does not exist in Windows anymore.

The above could be non-blocking, left to someone in the future to improve, because the Windows support is new; more troublesome is that this parses somedir:busybox:latest as path somedir:busybox:latest, missing image, which would then fail validation in NewReference; and, worse, somedir:busybox as path somedir:busybox, no image, which would succeed but be inconsistent with the POSIX version.

This is still outstanding I think. Or are you fine with oci:somedir:busybox just failing?

and, worse, somedir:busybox as path somedir:busybox, no image, which would succeed but be inconsistent with the POSIX version.

Sorry, that would pass validation but somedir:busybox is an invalid path on Windows, so it would fail either way.

So, overall, with this PR relative paths with images just fail; that’s clearly not worse than before this PR, so non-blocking for me.

mtrmac · 2017-11-20T23:06:53Z

oci/oci.go

+
+	// Refuse also "/", otherwise "/" and "" would have the same semantics,
+	// and "" could be unexpectedly shadowed by the "/" entry.
+	// (Note: we do allow "/:someimage", a bit ridiculous but why refuse it?)


The parenthesized note is no longer relevant now that identities/scopes do not contain the image name.

I'll remove it

mtrmac · 2017-11-20T23:10:11Z

oci/oci.go

+	if runtime.GOOS == "windows" {
+		return validateScopeWindows(scope, dir)
+	}
+	return validateScopeNonWindows(scope, dir)


Can the two variants share code via filepath.IsAbs? (That one also handles UNC paths, but that seems to be compatible enough with our formats.)

Potentially. I guess you are talking about just calling "filepath.IsAbs" w/o a OS specific switch. There would still be

if scope == "/" { return errors.New(`Invalid scope "/": Use the generic default scope ""`) }

to take care of which is non Windows specific (unless you suggest we drop this).

Another disadvantage of using filepath.IsAbs is that one cannot easily write tests for Windows paths. In the current code I can test validateScopeWindows even w/o running the test suite on Windows. Once filepath.IsAbs is used that is not the case anymore.

I am undecided. I see pro and cons for both and tempted to prefer the current code to start with.

Sure, the non-Windows testing is a valid concern. I suppose we don’t need to care about UNC paths at least for now.

mtrmac · 2017-11-20T23:12:32Z

oci/oci_test.go

+		{`D:\:apline`, `D:\`, ""},
+		{"C:", "C:", "Invalid scope 'C:'. Must be an absolute path"},
+		{"E", "E", "Invalid scope 'E'. Must be an absolute path"},
+		{"", "", "Invalid scope ''. Must be an absolute path"},


(ValidatePolicyConfigurationScope is never called with the "" value, so this case is unnecessary. Doesn’t hurt though.)

mtrmac · 2017-11-21T00:40:42Z

oci/layout/oci_transport.go

+	} else {
+		if strings.Contains(resolved, ":") {
+			return nil, errors.Errorf("Invalid OCI reference %s:%s: path %s contains a colon", dir, image, resolved)
+		}


One more thing, prompted by containers/skopeo#455 :

Nowadays we don’t treat : in policy scopes specially — but we do treat : specially in the non-canonicalized input path, as output in StringWithinTransport and re-parsed in ParseReference. So, actually, this check for colons should, if I am not mistaken again, validate dir, not resolved.

(We originally didn’t validate dir because image was not allowed to contain colons, so dir_with_colons:image_without_colons was not ambiguous. After image was allowed colons, this became ambiguous and we should have started refusing such dir values at that time.

Conceptually independently, but at at about the same time, image was dropped from PolicyConfiguration*, and the resolved check was no longer necessary.)

ahh, so check is against the unresolved path. ok

hferentschik · 2017-11-23T21:26:56Z

@mtrmac, I pushed an update to the pull request incorporating your feedback. Hopefully, I understood all your comments correctly. Turns out things are getting easier and the code looks more regular which is usually a good sign. WDYT?

mtrmac · 2017-11-23T21:34:28Z

oci/archive/oci_transport.go

+	if err != nil {
+		return err
 	}
 	return nil


(This could be just return internal.ValidateScope(scope). Applies to both transports.)

mtrmac · 2017-11-23T21:38:18Z

oci/internal/oci_util.go

+// SplitDirAndImage tries to split the provided OCI reference into the OCI directory and image.
+// The image is returned as string pointer, allowing for nil being returned, if no image part could be extraced from the reference.
+// Neither directory nor image parts are validated at this stage.
+func SplitDirAndImage(reference string) (string, string) {


Please rename this dir to path, throughout; and same for the comments.

The comment about a string pointer is no longer true.

mtrmac · 2017-11-23T21:42:13Z

oci/oci.go

+	if len(groups) != 3 {
+		return reference, nil
+	}
+	return groups[1], &groups[2]


This is still outstanding I think. Or are you fine with oci:somedir:busybox just failing?

mtrmac · 2017-11-23T21:44:48Z

oci/internal/oci_util.go

+	return nil
+}
+
+// ValidateScope takes the OCI directory and validates its scope.


“validates a policy configuration scope for an OCI transport” or something like that; for oci-archive this can be just an absolute path to a file.

mtrmac · 2017-11-23T21:46:49Z

oci/archive/oci_transport.go

-		return nil, errors.Errorf("Invalid OCI reference %s:%s: path %s contains a colon", file, image, resolved)
+
+	err = internal.ValidateOCIPath(file)
+	if err != nil {


(This could be if err := … err != nil {. Applies to both transports.)

sure. Personally, I don't like this style of the if statement, but fair enough

mtrmac · 2017-11-23T21:51:43Z

oci/oci.go

+	if len(groups) != 3 {
+		return reference, nil
+	}
+	return groups[1], &groups[2]


and, worse, somedir:busybox as path somedir:busybox, no image, which would succeed but be inconsistent with the POSIX version.

Sorry, that would pass validation but somedir:busybox is an invalid path on Windows, so it would fail either way.

So, overall, with this PR relative paths with images just fail; that’s clearly not worse than before this PR, so non-blocking for me.

mtrmac

Nice.

Please make the SplitDirAndImage name generic as well. Other than that, a few absolutely non-blocking nitpicks.

The question of accepting relative paths is still outstanding; but if you prefer merging the PR now without making that possible, that’s fine with me, as long as it is a conscious decision.

…indows Signed-off-by: Hardy Ferentschik <hardy@hibernate.org>

hferentschik · 2017-11-26T11:14:57Z

@mtrmac pushed an update.

The question of accepting relative paths is still outstanding; but if you prefer merging the PR now without making that possible, that’s fine with me, as long as it is a conscious decision.

I'd like to get this merged, since this allows Minishift to continue.

hferentschik · 2017-11-27T10:17:49Z

@runcom, @mtrmac - what do you think, can we merge this now?

mtrmac · 2017-11-28T14:20:32Z

👍

The question of accepting relative paths is still outstanding; but if you prefer merging the PR now without making that possible, that’s fine with me, as long as it is a conscious decision.

I'd like to get this merged, since this allows Minishift to continue.

OK. Thanks!

@runcom PTAL.

hferentschik · 2017-11-29T15:21:19Z

@runcom not to seem impatient, but I have two pull requests on the Minishift side depending on this :-) :

Issue #952 Implementing image caching using OCI based image cache minishift/minishift#1685
Issue #1408 Using a global cache for all profiles minishift/minishift#1716

runcom · 2017-11-29T15:25:41Z

lgtm

hferentschik · 2017-11-29T15:27:36Z

thanks :-)

hferentschik commented Nov 9, 2017

View reviewed changes

hferentschik mentioned this pull request Nov 9, 2017

[WIP] Issue #1575 Switching to containers/image OCI transport for image caching minishift/minishift#1589

Closed

mtrmac reviewed Nov 13, 2017

View reviewed changes

hferentschik force-pushed the issue-368-pr branch from fbe9a83 to ba6d26d Compare November 14, 2017 16:06

hferentschik mentioned this pull request Nov 15, 2017

Provide caching of docker images across VM instances minishift/minishift#952

Closed

hferentschik force-pushed the issue-368-pr branch from ba6d26d to 590c9d4 Compare November 16, 2017 11:28

hferentschik commented Nov 16, 2017

View reviewed changes

hferentschik force-pushed the issue-368-pr branch 2 times, most recently from 6b5a848 to b4368a4 Compare November 20, 2017 08:45

mtrmac reviewed Nov 20, 2017

View reviewed changes

mtrmac mentioned this pull request Nov 21, 2017

OCI image transport broken again? containers/skopeo#455

Closed

mtrmac reviewed Nov 21, 2017

View reviewed changes

hferentschik force-pushed the issue-368-pr branch from b4368a4 to e254395 Compare November 23, 2017 21:25

mtrmac reviewed Nov 23, 2017

View reviewed changes

hferentschik force-pushed the issue-368-pr branch 2 times, most recently from c6b9a47 to ab3e0f9 Compare November 25, 2017 07:27

Issue containers#368 Making OCI and Docker daemon transport work on W…

48c04db

…indows Signed-off-by: Hardy Ferentschik <hardy@hibernate.org>

hferentschik force-pushed the issue-368-pr branch from ab3e0f9 to 48c04db Compare November 26, 2017 11:13

runcom merged commit 8b24210 into containers:master Nov 29, 2017

mtrmac mentioned this pull request Dec 6, 2017

Windows incompatibilities in Docker daemon and OCI transport #368

Closed

Issue #368 Making OCI and Docker daemon transport work on Windows #379

Issue #368 Making OCI and Docker daemon transport work on Windows #379

Uh oh!

Conversation

hferentschik commented Nov 9, 2017

Uh oh!

runcom commented Nov 9, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hferentschik commented Nov 9, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hferentschik commented Nov 9, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mtrmac left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hferentschik commented Nov 14, 2017

Uh oh!

mtrmac commented Nov 14, 2017

Uh oh!

hferentschik commented Nov 16, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hferentschik commented Nov 20, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

hferentschik commented Nov 9, 2017 •

edited

Loading

hferentschik commented Nov 16, 2017 •

edited

Loading