Skip to content

Conversation

@selansen
Copy link
Contributor

@selansen selansen commented Jun 21, 2018

This is follow up of #2174.
I fixed all test case failures.

Description:
The use of Client() on v2 plugins is being deprecated so that we can be more flexible on the protocol used for plugins.

This means checking specifically if the plugin implements the Client() *plugins.Client interface for V1 plugins, and for v2 plugins building a the client manually.

cpuguy83 added 3 commits June 22, 2018 23:51
Signed-off-by: Brian Goff <cpuguy83@gmail.com>
(cherry picked from commit aae1b0e)
Signed-off-by: selansen <elango.siva@docker.com>
The use of `Client()` on v2 plugins is being deprecated so that we can
be more flexible on the protocol used for plugins.

This means checking specifically if the plugin implements the
`Client() *plugins.Client` interface for V1 plugins, and for v2 plugins
building a the client manually.

Signed-off-by: Brian Goff <cpuguy83@gmail.com>
(cherry picked from commit 45824a2)
Signed-off-by: selansen <elango.siva@docker.com>
Most of the libcontainer imports was just for a single test to marshal a
simple type, meanwhile this caused all kinds of transient imports that
are not really needed.

Signed-off-by: Brian Goff <cpuguy83@gmail.com>
(cherry picked from commit a07a1ee)
Signed-off-by: selansen <elango.siva@docker.com>
@selansen selansen changed the title New plugin intf [WIP] New plugin intf Jun 23, 2018
@selansen selansen force-pushed the new_plugin_intf branch 5 times, most recently from 41b041c to 88323a8 Compare June 25, 2018 17:23
@selansen selansen changed the title [WIP] New plugin intf Use new plugin interfaces provided by plugin pkg Jun 25, 2018
@selansen selansen force-pushed the new_plugin_intf branch 3 times, most recently from e7cc7dd to 3879845 Compare June 26, 2018 03:10
@selansen
Copy link
Contributor Author

Ping @fcrisciani @euanh

Dockerfile Outdated
github.com/gordonklaus/ineffassign \
github.com/client9/misspell/cmd/misspell \
honnef.co/go/tools/cmd/gosimple
honnef.co/go/tools/cmd/gosimple

Choose a reason for hiding this comment

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

remove the extra space here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed it

vendor.conf Outdated
github.com/deckarep/golang-set ef32fa3046d9f249d399f98ebaf9be944430fd1d

github.com/docker/docker 162ba6016def672690ee4a1f3978368853a1e149
github.com/docker/docker 162ba6016def672690ee4a1f3978368853a1e149

Choose a reason for hiding this comment

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

remove extra space

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed it

return nil
}

func getPluginClient(p plugingetter.CompatPlugin) (*plugins.Client, error) {

Choose a reason for hiding this comment

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

isn't this function declared twice in the same remote package?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes this has been declared twice but in different packages.
github.com/docker/libnetwork/IPAMS/remote
github.com/docker/libnetwork/drivers/remote
I thought its same package initially and wasted my time figuring out why it throws error when I remove one API definition :(

Signed-off-by: Brian Goff <cpuguy83@gmail.com>
(cherry picked from commit 8856c1e)
Signed-off-by: selansen <elango.siva@docker.com>
@selansen
Copy link
Contributor Author

Error seems to be due to cloud provider :)..not coz our PR !!!
a vm: instance not ready yet: unassigned
We had an unexpected error preparing a VM for this build, potentially due to our infrastructure or cloud provider. Please retry the build in a few minutes

@cpuguy83
Copy link
Member

LGTM

Copy link

@fcrisciani fcrisciani left a comment

Choose a reason for hiding this comment

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

LGTM

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants