Update Docker Personality to support Docker API 1.25 [full ci] #3756

sflxn · 2017-01-27T23:51:43Z

Updated vendored sources to support Docker 1.13
Updated all the backend interfaces within
vic/lib/apiservers/backends
Added hollowed out swarm.go and backends/exec/SwarmBackend.go
Updated main.go
Added robot scripts for docker secret, checkpoint, deploy, node,
plugin, service, stack, swarm.

resolves #3415

vburenin · 2017-01-30T16:35:16Z

lib/apiservers/engine/backends/cache/image_cache.go

@@ -24,8 +24,8 @@ import (

 	log "github.com/Sirupsen/logrus"

-	"github.com/docker/distribution/digest"
-	derr "github.com/docker/docker/errors"
+	// "github.com/docker/distribution/digest"


vburenin · 2017-01-30T16:36:52Z

lib/apiservers/engine/backends/container_proxy.go

@@ -84,7 +85,7 @@ type VicContainerProxy interface {
 	CommitContainerHandle(handle, containerID string, waitTime int32) error
 	StreamContainerLogs(name string, out io.Writer, started chan struct{}, showTimestamps bool, followLogs bool, since int64, tailLines int64) error

-	Stop(vc *viccontainer.VicContainer, name string, seconds int, unbound bool) error
+	Stop(vc *viccontainer.VicContainer, name string, seconds *int, unbound bool) error


why pointer?

This is Docker's change.

vburenin · 2017-01-30T16:41:15Z

lib/apiservers/engine/backends/container_proxy.go

@@ -711,11 +712,20 @@ func (c *ContainerProxy) AttachStreams(ctx context.Context, vc *viccontainer.Vic
 	plClient, transport := c.createNewAttachClientWithTimeouts(attachConnectTimeout, 0, attachAttemptTimeout)
 	defer transport.Close()

+	keys := []byte{}


in this case it cases memory allocation. Try to use "var keys []byte"

vburenin · 2017-01-30T16:42:19Z

lib/apiservers/engine/backends/endpoint.go

@@ -129,3 +129,9 @@ func (e *endpoint) Address() *net.IPNet {
 func (e *endpoint) AddressIPv6() *net.IPNet {
 	return nil
 }
+
+// DisableService removes a managed contianer's endpoints from the load balancer


This comment is definitely invalid. Could you fix it?

vburenin · 2017-01-30T16:42:42Z

lib/apiservers/engine/backends/executor/SwarmBackend.go

+import (
+	"io"
+	"time"
+


remove empty line here.

vburenin · 2017-01-31T16:18:53Z

Let me know when you need review.

sflxn · 2017-02-01T02:08:17Z

@vburenin Sure thing. I still need to get networking fully working and write robot scripts to verify we don't croak on swarm, checkpoint, build, and plugin calls.

mdubya66

I approve this vendor rev

vburenin · 2017-02-10T23:00:58Z

lib/apiservers/engine/backends/checkpoint.go

@@ -0,0 +1,40 @@
+// Copyright 2016 VMware, Inc. All Rights Reserved.


it is 2017 outside, at least it was so yesterday ;)

good catch :)

hickeng · 2017-02-10T23:18:28Z

lib/apiservers/engine/backends/executor/SwarmBackend.go

+}
+
+func (b SwarmBackend) CreateManagedNetwork(clustertypes.NetworkCreateRequest) error {
+	return nil


Am I correct in assuming that we have to make this a noop instead an "unsupported" error?
Same question for the other operations.

Docker called it a "backend," but it's not a REST router backend. The actual REST router backend is in lib/apiservers/engine/backends/swarm.go.

hickeng · 2017-02-10T23:23:48Z

lib/apiservers/engine/backends/cache/image_cache.go

@@ -183,10 +182,10 @@ func (ic *ICache) Get(idOrRef string) (*metadata.ImageConfig, error) {
 	return copyImageConfig(config), nil
 }

-func (ic *ICache) getImageByDigest(digest digest.Digest) *metadata.ImageConfig {
+func (ic *ICache) getImageByDigest(digest string) *metadata.ImageConfig {


is this change to signature needed - from just this commit it looks like the original code cast it to string where needed and it's got to be a string type anyway given the comparison on line 167

I cannot remember why I did this. I remember it was to get around a build issue, but that was before 1.13 officially got released. With the updated vendored files, the old signature works so I reverted it back.

hickeng

Didn't notice any issues from a scan - would like someone else to look through it as well. I've not confirmed function.

hmahmood · 2017-02-13T21:58:10Z

Does this fix #3712 too?

hmahmood · 2017-02-13T21:58:53Z

nvm ... I guess you left the networking part out.

sflxn · 2017-02-14T01:19:53Z

@hmahmood Yeah, I left it out. Before fully hooking up the cluster management code, I wanted to see if minimally hooking it up would pass our integration tests so we can avoid unnecessary work.

jzt · 2017-02-14T01:26:16Z

lib/apiservers/engine/backends/plugins.go

+package backends
+
+import (
+    "fmt"


minor: indent is short

jzt · 2017-02-14T01:26:51Z

lib/apiservers/engine/backends/sandbox.go

@@ -20,6 +20,7 @@ import (
 	"github.com/docker/docker/pkg/stringid"
 	"github.com/docker/libnetwork"
 	"github.com/docker/libnetwork/types"
+	// "k8s.io/minikube/pkg/minikube/notify"


Not sure if you left this on purpose, just bringing it to your attention :)

jzt · 2017-02-14T01:27:53Z

tests/test-cases/Group1-Docker-Commands/1-32-Docker-Plugin.robot

+    Should Be Equal As Integers  ${rc}  1
+    Should Contain  ${output}  does not yet support plugins
+
+#Docker plugin create


Do we still need/want this test? Could we maybe pass & warn or something?

Fixed that test.

jzt · 2017-02-14T01:28:19Z

cmd/docker/main.go

@@ -1,4 +1,4 @@
-// Copyright 2016-2017 VMware, Inc. All Rights Reserved.
+// Copyright 2016 VMware, Inc. All Rights Reserved.


Looks like you accidentally removed the copyright

jzt · 2017-02-14T01:28:39Z

lib/apiservers/engine/backends/cache/image_cache.go

@@ -166,6 +166,7 @@ func (ic *ICache) Get(idOrRef string) (*metadata.ImageConfig, error) {
 	var config *metadata.ImageConfig
 	if imgDigest != "" {
 		config = ic.getImageByDigest(imgDigest)
+		// config = ic.getImageByDigest(string(imgDigest))


Commented code

jzt · 2017-02-14T01:29:17Z

lib/apiservers/engine/backends/image.go

-	"github.com/docker/engine-api/types/registry"
+	"github.com/docker/docker/api/types"
+	"github.com/docker/docker/api/types/backend"
+	// "github.com/docker/docker/api/types/container"


commented import

jzt · 2017-02-14T01:32:12Z

lib/apiservers/engine/backends/cache/image_cache.go

@@ -184,6 +185,7 @@ func (ic *ICache) Get(idOrRef string) (*metadata.ImageConfig, error) {
 }

 func (ic *ICache) getImageByDigest(digest digest.Digest) *metadata.ImageConfig {
+// func (ic *ICache) getImageByDigest(digest string) *metadata.ImageConfig {


commented code

jzt · 2017-02-14T01:32:51Z

LGTM

emlin

LGTM

The following are vendored source changes. The term rolled up means the nested vendored version of a repo within VIC's top level vendored sources were removed in favor of VIC's to avoid golang's namespace collision. For example, vic/vendor/github.com/docker/distribution vic/vendor/github.com/docker/docker/vendor/github.com/docker/distribution In the above example, the second repo was removed. 1. Updated github.com/docker/docker to 1.13. 2. Updated github.com/docker/distribution to 28602af35aceda2f8d571bad7ca37a54cf0250bc -- rolled up ditribution in docker/docker 3. Updated github.com/docker/libtrust to 9cbd2a1374f46905c68a4eb3694a130610adc62a -- rolled up libtrust in docker/docker -- rolled up libtrust in docker/distribution 4. Updated github.com/docker/swarmkit to 335561b66a44bf214224afe879e4368204e7fa45 -- rolled up swarmkit in docker/docker 5. Updated github.com/Sirupsen/logrus to v0.11.0 -- rolled up logrus in docker/docker -- rolled up logrus in docker/swarmkit 6. Updated google.golang.org/grpc to v1.0.2 -- rolled up in docker/docker -- rolled up in docker/swarmkit 7. github.com/golang/protobuf to 1f49d83d9aa00e6ce4fc8258c71cc7786aec968a 8. github.com/vishvananda/netns to 604eaf189ee867d8c147fafc28def2394e878d25 -- rolled up in docker/docker 9. github.com/vishvananda/netlink to 482f7a52b758233521878cb6c5904b6bd63f3457 -- rolled up in docker/docker 10. Rolled up go-connections in docker/docker 11. Rolled up github.com/golang/x/net/context in docker/docker and docker/swarmkit

1. Updated all the backend interfaces within vic/lib/apiservers/backends 2. Added hollowed out swarm.go and backends/exec/SwarmBackend.go 3. Updated main.go 4. Added robot scripts for docker secret, checkpoint, deploy, node, plugin, service, stack, swarm.

mhagen-vmware

lgtm

vmwclabot added the cla-not-required label Jan 27, 2017

sflxn changed the title ~~[WIP] Update Docker Personality to support Docker API 1.25 [skip ci]~~ [WIP] Update Docker Personality to support Docker API 1.25 [full ci] Jan 28, 2017

sflxn force-pushed the update_api_1.25 branch from 72e0271 to f8478b7 Compare January 28, 2017 00:51

stuclem mentioned this pull request Jan 30, 2017

Document support for Docker API version > 1.23 #3419

Closed

vburenin reviewed Jan 30, 2017

View reviewed changes

lib/apiservers/engine/backends/executor/SwarmBackend.go Outdated

import (

"io"

"time"

Copy link

Contributor

vburenin Jan 30, 2017

Choose a reason for hiding this comment

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

remove empty line here.

sflxn force-pushed the update_api_1.25 branch 4 times, most recently from d981394 to fd90e5a Compare January 30, 2017 22:58

sflxn mentioned this pull request Feb 1, 2017

Update docker personality to accept API versions > 1.23 #3415

Closed

mdubya66 approved these changes Feb 1, 2017

View reviewed changes

sflxn force-pushed the update_api_1.25 branch 5 times, most recently from df24972 to 8a4a503 Compare February 3, 2017 20:31

sflxn force-pushed the update_api_1.25 branch 2 times, most recently from 3521f72 to b74444d Compare February 10, 2017 21:46

sflxn changed the title ~~[WIP] Update Docker Personality to support Docker API 1.25 [full ci]~~ Update Docker Personality to support Docker API 1.25 [full ci] Feb 10, 2017

sflxn requested a review from hickeng February 10, 2017 22:32

vburenin reviewed Feb 10, 2017

View reviewed changes

sflxn force-pushed the update_api_1.25 branch from b74444d to 70f254f Compare February 10, 2017 23:17

hickeng reviewed Feb 10, 2017

View reviewed changes

hickeng approved these changes Feb 10, 2017

View reviewed changes

sflxn requested a review from jzt February 10, 2017 23:50

sflxn force-pushed the update_api_1.25 branch from 70f254f to 571af5e Compare February 13, 2017 20:13

jzt reviewed Feb 14, 2017

View reviewed changes

jzt approved these changes Feb 14, 2017

View reviewed changes

sflxn force-pushed the update_api_1.25 branch 3 times, most recently from 428ddc2 to 4f6dbfd Compare February 14, 2017 03:39

sflxn requested review from emlin and mhagen-vmware February 14, 2017 03:40

emlin approved these changes Feb 14, 2017

View reviewed changes

sflxn added 2 commits February 14, 2017 10:25

sflxn force-pushed the update_api_1.25 branch from 4f6dbfd to 64f6dcc Compare February 14, 2017 18:29

mhagen-vmware approved these changes Feb 14, 2017

View reviewed changes

sflxn merged commit d5a66ca into vmware:master Feb 14, 2017

hmahmood mentioned this pull request Feb 14, 2017

Research the implication of Swarm coupling in 1.13 for VIC #3712

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update Docker Personality to support Docker API 1.25 [full ci] #3756

Update Docker Personality to support Docker API 1.25 [full ci] #3756

sflxn commented Jan 27, 2017 •

edited

Loading

vburenin Jan 30, 2017

vburenin Jan 30, 2017

sflxn Jan 30, 2017

vburenin Jan 30, 2017

vburenin Jan 30, 2017

vburenin Jan 30, 2017

vburenin commented Jan 31, 2017

sflxn commented Feb 1, 2017

mdubya66 left a comment

vburenin Feb 10, 2017

sflxn Feb 10, 2017

hickeng Feb 10, 2017 •

edited

Loading

sflxn Feb 10, 2017

hickeng Feb 10, 2017

sflxn Feb 10, 2017

hickeng left a comment

hmahmood commented Feb 13, 2017 •

edited

Loading

hmahmood commented Feb 13, 2017

sflxn commented Feb 14, 2017

jzt Feb 14, 2017

sflxn Feb 14, 2017

jzt Feb 14, 2017

jzt Feb 14, 2017

sflxn Feb 14, 2017

jzt Feb 14, 2017

jzt Feb 14, 2017

jzt Feb 14, 2017

jzt Feb 14, 2017

jzt commented Feb 14, 2017 •

edited by mdubya66

Loading

emlin left a comment

mhagen-vmware left a comment

		@@ -0,0 +1,40 @@
		// Copyright 2016 VMware, Inc. All Rights Reserved.

		@@ -1,4 +1,4 @@
		// Copyright 2016-2017 VMware, Inc. All Rights Reserved.
		// Copyright 2016 VMware, Inc. All Rights Reserved.

Update Docker Personality to support Docker API 1.25 [full ci] #3756

Update Docker Personality to support Docker API 1.25 [full ci] #3756

Conversation

sflxn commented Jan 27, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vburenin commented Jan 31, 2017

sflxn commented Feb 1, 2017

mdubya66 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hickeng Feb 10, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hickeng left a comment

Choose a reason for hiding this comment

hmahmood commented Feb 13, 2017 • edited Loading

hmahmood commented Feb 13, 2017

sflxn commented Feb 14, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jzt commented Feb 14, 2017 • edited by mdubya66 Loading

emlin left a comment

Choose a reason for hiding this comment

mhagen-vmware left a comment

Choose a reason for hiding this comment

sflxn commented Jan 27, 2017 •

edited

Loading

hickeng Feb 10, 2017 •

edited

Loading

hmahmood commented Feb 13, 2017 •

edited

Loading

jzt commented Feb 14, 2017 •

edited by mdubya66

Loading