Skip to content
This repository has been archived by the owner on Jan 21, 2020. It is now read-only.

Multiplex instance plugins #360

Merged
merged 6 commits into from
Jan 13, 2017

Conversation

chungers
Copy link
Contributor

To better support different resource types (see #290), this PR introduces multiplexing a single RPC endpoint for instance plugin onto multiple, typed instance spi objects. This is done at the RPC server and client level so that we can preserve the current instance SPI while supporting new resource types other than compute (which up to now the instance plugin SPI implies):

  • The naming of a plugin now supports an optional / to denote a base and a subtype:
name := $lookupname[/$subtype]

For instance, existing plugin name of instance-vagrant still works just fine, but it is now possible to have plugin references such as aws/subnet and aws/vpc. This means that there is a single RPC server named aws (as socket file) and it hosts multiple instance plugin spi objects, one implementing the SPI for provisioning a subnet and one for vpc.

  • With this convention, the existing instance SPI can be generalized for all resource types, including compute -- which is a special case of this where the subtype could be explicitly defined as compute by a plugin that can support different types of resources.
  • Usage: see rpc_multi_test.go for example usage. In summary, you can bind multiple instance plugin spi implementations to a single RPC endpoint like so:
server, err := rpc_server.StartPluginAtPath("aws", PluginServerWithTypes(
		map[string]instance.Plugin{
			"subnet": &impl1,
                        "vpc": &impl2,
                })

and for client, looking up is the same. Given a JSON like

{
   "Plugin" : "aws/subnet",
   "Properties" : ...
}

The plugin discovery now takes the name format into account and connects to the single RPC endpoint (aws) but routes the client's method calls to the typed / bound SPI object in the server (subnet).

  • Essentially this design removes the need to introduce a 'type' into the existing Instance SPI methods just so that the plugin code does the switch/case in its own implementation. Instead, the switch/case by type is done by the RPC transport. This simplifies the development of resource typed instance plugins and avoids near duplication of SPI just to support different resource types.

@GordonTheTurtle
Copy link

Please sign your commits following these rules:
https://github.com/docker/docker/blob/master/CONTRIBUTING.md#sign-your-work
The easiest way to do this is to amend the last commit:

$ git clone -b "multiplex-instance" [email protected]:chungers/infrakit.git somewhere
$ cd somewhere
$ git rebase -i HEAD~842354749768
editor opens
change each 'pick' to 'edit'
save the file and quit
$ git commit --amend -s --no-edit
$ git rebase --continue # and repeat the amend for each commit
$ git push -f

Amending updates the existing PR. You DO NOT need to open a new one.

@codecov-io
Copy link

codecov-io commented Jan 13, 2017

Current coverage is 65.02% (diff: 79.31%)

Merging #360 into master will decrease coverage by 0.14%

@@             master       #360   diff @@
==========================================
  Files            44         45     +1   
  Lines          2110       2153    +43   
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
+ Hits           1375       1400    +25   
- Misses          596        608    +12   
- Partials        139        145     +6   

Powered by Codecov. Last update 275105d...62f602e

@nwt
Copy link
Contributor

nwt commented Jan 13, 2017

Looks good to me. I don't think you meant to include pkg/rpc/client/client.test, though.

Signed-off-by: David Chung <[email protected]>
Copy link
Contributor

@FrenchBen FrenchBen left a comment

Choose a reason for hiding this comment

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

LGTM but doc updates should probably be made as well to describe this change.

@chungers
Copy link
Contributor Author

@FrenchBen - I will add docs on this, but for now, there are no typed instance plugins and everything still works as before. I will add another PR to consider doing the same thing to Flavors too. This is because it turns out Flavor has a pretty ready example -- the swarm flavor. The current swarm flavor uses a field inside its config to decide if it's for a swarm manager or worker. If we use the same approach, then we can easily have swarm/manager and swarm/worker for plugin names and that makes it obvious right away without having to look into the nested JSON config itself.

@chungers chungers merged commit 5a6c860 into docker-archive:master Jan 13, 2017
@chungers chungers deleted the multiplex-instance branch January 13, 2017 22:25
chungers pushed a commit to chungers/infrakit that referenced this pull request Sep 30, 2017
Added note for diag as an AWS only feature
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants