Skip to content

Remote plugins plumbing.#13222

Merged
crosbymichael merged 2 commits intomoby:masterfrom
calavera:plugins_infra
May 15, 2015
Merged

Remote plugins plumbing.#13222
crosbymichael merged 2 commits intomoby:masterfrom
calavera:plugins_infra

Conversation

@calavera
Copy link
Contributor

Package extracted from #13161 that includes only the plumbing to create rpc plugins.

See the next files to understand how to use it:

Adapter to hide rpc logic: https://github.com/docker/docker/pull/13161/files#diff-3df42339ab51e1cd0c14cccc2e4619ec
Interface to implement local and remote calls: https://github.com/docker/docker/pull/13161/files#diff-b880a48e8fc6e68e38502dcc73822c33
Proxy to perform the rpc calls: https://github.com/docker/docker/pull/13161/files#diff-51417af0fa9465a8b5415c0fbb16a099
Plugin memory storage for configured drivers: https://github.com/docker/docker/pull/13161/files#diff-54fddc8de3b5f9c246fa36477bfab894

Signed-off-by: David Calavera david.calavera@gmail.com

@calavera
Copy link
Contributor Author

/cc @mavenugo

Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like debug.

@LK4D4
Copy link
Contributor

LK4D4 commented May 14, 2015

Looks like needs some debug cleanups.

@mavenugo
Copy link
Contributor

@calavera Thanks. Also thanks for making it a pkg. Will get back to you shortly after a short PoC integration with libnetwork

@calavera
Copy link
Contributor Author

@LK4D4 fixed debug statements.

Copy link
Contributor

Choose a reason for hiding this comment

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

Debugf :)

Copy link
Member

Choose a reason for hiding this comment

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

commented code

Copy link
Contributor

Choose a reason for hiding this comment

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

Seems like we need another mutex in container :) Because this might be a problem.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think locking should be responsibility of the plugin. Let's leave that to the container that uses it.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, you're right. But this will be called under container.Lock() which we can't hold for long time.

mavenugo added a commit to mavenugo/docker that referenced this pull request May 15, 2015
Signed-off-by: Madhu Venugopal <madhu@docker.com>
mavenugo added a commit to mavenugo/docker that referenced this pull request May 15, 2015
Signed-off-by: Madhu Venugopal <madhu@docker.com>
calavera pushed a commit to calavera/docker that referenced this pull request May 15, 2015
Signed-off-by: Madhu Venugopal <madhu@docker.com>
(cherry picked from commit 7fc16f4)
Signed-off-by: David Calavera <david.calavera@gmail.com>
@calavera
Copy link
Contributor Author

@mavenugo I've incorporated your handle function to the PR.

@mavenugo
Copy link
Contributor

@calavera Thanks. appreciate it & its a LGTM for libnetwork remote driver integration.

Signed-off-by: Madhu Venugopal <madhu@docker.com>
@LK4D4
Copy link
Contributor

LK4D4 commented May 15, 2015

LGTM

1 similar comment
@crosbymichael
Copy link
Contributor

LGTM

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants