Skip to content
This repository has been archived by the owner on Jun 20, 2024. It is now read-only.

Add two functions required by ipamapi "contract" #1985

Merged
merged 1 commit into from
Feb 18, 2016
Merged

Conversation

bboreham
Copy link
Contributor

Fixes #1984

@bboreham bboreham added this to the 1.4.5 milestone Feb 18, 2016
@rade
Copy link
Member

rade commented Feb 18, 2016

I notice we have

    DiscoverNew(discover *api.DiscoveryNotification) error
    DiscoverDelete(delete *api.DiscoveryNotification) error

in our (network) Driver interface. And have discoverNew and discoverDelete methods on the listener that call them. But they are not wired into the http method handling.

Cruft?

@bboreham
Copy link
Contributor Author

Well, having them in the interface is plausible, since they were in Docker's Driver interface (refactored by moby/libnetwork@94e4e39)

Not wiring them into the http mux seems like a mistake.

Also the spelling error in their log-lines.

@bboreham
Copy link
Contributor Author

Following moby/libnetwork#908 (comment) I guess this 'fix' is now a mistake.

@rade
Copy link
Member

rade commented Feb 18, 2016

quite. let's see how that resolves itself in the end. we should probably also check/ask about the network driver api.

@mavenugo
Copy link

@bboreham sorry about the breakage. We are recommending for plugins to use https://github.com/docker/go-plugins-helpers/ for any docker plugin development. Can you please migrate to using these APIs instead of depending the libnetwork/ipamapi layer ?

@rade
Copy link
Member

rade commented Feb 18, 2016

@mavenugo sure, can do!

@rade
Copy link
Member

rade commented Feb 18, 2016

I am merging this as is since switching the dependencies is too much of a leap for a patch release.

I have raised #1986 to fix this properly.

rade added a commit that referenced this pull request Feb 18, 2016
Add two functions required by ipamapi "contract"

Fixes #1984
@rade rade merged commit 5e761a6 into 1.4 Feb 18, 2016
@bboreham
Copy link
Contributor Author

@mavenugo where exactly was this publicised/recommended? I missed it.

@rade
Copy link
Member

rade commented Feb 19, 2016

@mavenugo AFAICT, go-plugins-helpers does not allow one to serve multiple APIs, e.g. networking and ipam, on the same listener. This is something the plugin API spec permits and we depend on. Did I miss something in my reading of the plugins-helpers code?

@rade rade deleted the issues/1984-discovery branch March 19, 2016 16:30
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants