Merge pull request #2730 from dgageot/close-api

Close api

commands/commands.go

@@ -10,7 +10,6 @@ import (
 	"github.com/docker/machine/commands/mcndirs"
 	"github.com/docker/machine/libmachine"
 	"github.com/docker/machine/libmachine/crashreport"
-	"github.com/docker/machine/libmachine/drivers/rpc"
 	"github.com/docker/machine/libmachine/host"
 	"github.com/docker/machine/libmachine/log"
 	"github.com/docker/machine/libmachine/mcnutils"
@@ -95,7 +94,8 @@ func runAction(actionName string, c CommandLine, api libmachine.API) error {
 
 func fatalOnError(command func(commandLine CommandLine, api libmachine.API) error) func(context *cli.Context) {
 	return func(context *cli.Context) {
-		api := libmachine.NewClient(mcndirs.GetBaseDir())
+		api := libmachine.NewClient(mcndirs.GetBaseDir(), mcndirs.GetMachineCertDir())
+		defer api.Close()
 
 		if context.GlobalBool("native-ssh") {
 			api.SSHClientType = ssh.Native
@@ -114,8 +114,6 @@ func fatalOnError(command func(commandLine CommandLine, api libmachine.API) erro
 		mcnutils.GithubAPIToken = api.GithubAPIToken
 		ssh.SetDefaultClient(api.SSHClientType)
 
-		defer rpcdriver.CloseDrivers()
-
 		if err := command(&contextCommandLine{context}, api); err != nil {
 			log.Fatal(err)
 		}

commands/create.go

@@ -14,7 +14,6 @@ import (
 
 	"github.com/codegangsta/cli"
 	"github.com/docker/machine/commands/mcndirs"
-	"github.com/docker/machine/drivers/errdriver"
 	"github.com/docker/machine/libmachine"
 	"github.com/docker/machine/libmachine/auth"
 	"github.com/docker/machine/libmachine/drivers"
@@ -152,12 +151,7 @@ func cmdCreateInner(c CommandLine, api libmachine.API) error {
 	}
 
 	driverName := c.String("driver")
-	driver, err := api.NewPluginDriver(driverName, rawDriver)
+	h, err := api.NewHost(driverName, rawDriver)
-	if err != nil {
-		return fmt.Errorf("Error loading driver %q: %s", driverName, err)
-	}
-
-	h, err := api.NewHost(driver)
 	if err != nil {
 		return fmt.Errorf("Error getting new host: %s", err)
 	}
@@ -209,7 +203,7 @@ func cmdCreateInner(c CommandLine, api libmachine.API) error {
 	// driverOpts is the actual data we send over the wire to set the
 	// driver parameters (an interface fulfilling drivers.DriverOptions,
 	// concrete type rpcdriver.RpcFlags).
-	mcnFlags := driver.GetCreateFlags()
+	mcnFlags := h.Driver.GetCreateFlags()
 	driverOpts := getDriverOpts(c, mcnFlags)
 
 	if err := h.Driver.SetConfigFromFlags(driverOpts); err != nil {
@@ -285,13 +279,9 @@ func cmdCreateOuter(c CommandLine, api libmachine.API) error {
 		return fmt.Errorf("Error attempting to marshal bare driver data: %s", err)
 	}
 
-	driver, err := api.NewPluginDriver(driverName, rawDriver)
+	h, err := api.NewHost(driverName, rawDriver)
 	if err != nil {
-		return fmt.Errorf("Error loading driver %q: %s", driverName, err)
+		return err
-	}
-
-	if _, ok := driver.(*errdriver.Driver); ok {
-		return errdriver.NotLoadable{Name: driverName}
 	}
 
 	// TODO: So much flag manipulation and voodoo here, it seems to be
@@ -299,7 +289,7 @@ func cmdCreateOuter(c CommandLine, api libmachine.API) error {
 	//
 	// mcnFlags is the data we get back over the wire (type mcnflag.Flag)
 	// to indicate which parameters are available.
-	mcnFlags := driver.GetCreateFlags()
+	mcnFlags := h.Driver.GetCreateFlags()
 
 	// This bit will actually make "create" display the correct flags based
 	// on the requested driver.
@@ -315,10 +305,6 @@ func cmdCreateOuter(c CommandLine, api libmachine.API) error {
 		}
 	}
 
-	if serialDriver, ok := driver.(*drivers.SerialDriver); ok {
-		driver = serialDriver.Driver
-	}
-
 	return c.Application().Run(os.Args)
 }
 

libmachine/drivers/rpc/client_driver.go

@@ -6,6 +6,8 @@ import (
 	"sync"
 	"time"
 
+	"io"
+
 	"github.com/docker/machine/libmachine/drivers"
 	"github.com/docker/machine/libmachine/drivers/plugin/localbinary"
 	"github.com/docker/machine/libmachine/log"
@@ -16,10 +18,25 @@ import (
 
 var (
 	heartbeatInterval = 5 * time.Second
-	openedDrivers     = []*RPCClientDriver{}
-	openedDriversLock = &sync.Mutex{}
 )
 
+type RPCClientDriverFactory interface {
+	NewRPCClientDriver(driverName string, rawDriver []byte) (*RPCClientDriver, error)
+	io.Closer
+}
+
+type DefaultRPCClientDriverFactory struct {
+	openedDrivers     []*RPCClientDriver
+	openedDriversLock sync.Locker
+}
+
+func NewRPCClientDriverFactory() RPCClientDriverFactory {
+	return &DefaultRPCClientDriverFactory{
+		openedDrivers:     []*RPCClientDriver{},
+		openedDriversLock: &sync.Mutex{},
+	}
+}
+
 type RPCClientDriver struct {
 	plugin          localbinary.DriverPlugin
 	heartbeatDoneCh chan bool
@@ -86,19 +103,21 @@ func NewInternalClient(rpcclient *rpc.Client) *InternalClient {
 	}
 }
 
-func CloseDrivers() {
+func (f *DefaultRPCClientDriverFactory) Close() error {
-	openedDriversLock.Lock()
+	f.openedDriversLock.Lock()
-	defer openedDriversLock.Unlock()
+	defer f.openedDriversLock.Unlock()
 
-	for _, openedDriver := range openedDrivers {
+	for _, openedDriver := range f.openedDrivers {
 		if err := openedDriver.close(); err != nil {
 			log.Warnf("Error closing a plugin driver: %s", err)
 		}
 	}
-	openedDrivers = []*RPCClientDriver{}
+	f.openedDrivers = []*RPCClientDriver{}
+
+	return nil
 }
 
-func NewRPCClientDriver(driverName string, rawDriver []byte) (*RPCClientDriver, error) {
+func (f *DefaultRPCClientDriverFactory) NewRPCClientDriver(driverName string, rawDriver []byte) (*RPCClientDriver, error) {
 	mcnName := ""
 
 	p, err := localbinary.NewPlugin(driverName)
@@ -129,9 +148,9 @@ func NewRPCClientDriver(driverName string, rawDriver []byte) (*RPCClientDriver,
 		heartbeatDoneCh: make(chan bool),
 	}
 
-	openedDriversLock.Lock()
+	f.openedDriversLock.Lock()
-	openedDrivers = append(openedDrivers, c)
+	f.openedDrivers = append(f.openedDrivers, c)
-	openedDriversLock.Unlock()
+	f.openedDriversLock.Unlock()
 
 	var serverVersion int
 	if err := c.Client.Call(GetVersionMethod, struct{}{}, &serverVersion); err != nil {

libmachine/examples/vbox_create.go

@@ -13,7 +13,8 @@ import (
 func main() {
 	log.SetDebug(true)
 
-	client := libmachine.NewClient("/tmp/automatic")
+	client := libmachine.NewClient("/tmp/automatic", "/tmp/automatic/certs")
+	defer client.Close()
 
 	hostName := "myfunhost"
 
@@ -27,12 +28,7 @@ func main() {
 		log.Fatal(err)
 	}
 
-	pluginDriver, err := client.NewPluginDriver("virtualbox", data)
+	h, err := client.NewHost("virtualbox", data)
-	if err != nil {
-		log.Fatal(err)
-	}
-
-	h, err := client.NewHost(pluginDriver)
 	if err != nil {
 		log.Fatal(err)
 	}

libmachine/libmachine.go

@@ -4,11 +4,16 @@ import (
 	"fmt"
 	"path/filepath"
 
+	"io"
+
+	"github.com/docker/machine/drivers/errdriver"
 	"github.com/docker/machine/libmachine/auth"
 	"github.com/docker/machine/libmachine/cert"
 	"github.com/docker/machine/libmachine/check"
 	"github.com/docker/machine/libmachine/crashreport"
 	"github.com/docker/machine/libmachine/drivers"
+	"github.com/docker/machine/libmachine/drivers/plugin/localbinary"
+	"github.com/docker/machine/libmachine/drivers/rpc"
 	"github.com/docker/machine/libmachine/engine"
 	"github.com/docker/machine/libmachine/host"
 	"github.com/docker/machine/libmachine/log"
@@ -22,38 +27,49 @@ import (
 )
 
 type API interface {
-	persist.Store
+	io.Closer
-	persist.PluginDriverFactory
+	NewHost(driverName string, rawDriver []byte) (*host.Host, error)
-	NewHost(drivers.Driver) (*host.Host, error)
 	Create(h *host.Host) error
+	persist.Store
 }
 
 type Client struct {
-	*persist.PluginStore
+	certsDir       string
 	IsDebug        bool
 	SSHClientType  ssh.ClientType
 	GithubAPIToken string
+	*persist.Filestore
+	clientDriverFactory rpcdriver.RPCClientDriverFactory
 }
 
-func NewClient(storePath string) *Client {
+func NewClient(storePath, certsDir string) *Client {
-	certsDir := filepath.Join(storePath, ".docker", "machine", "certs")
 	return &Client{
+		certsDir:            certsDir,
 		IsDebug:             false,
 		SSHClientType:       ssh.External,
-		PluginStore:   persist.NewPluginStore(storePath, certsDir, certsDir),
+		Filestore:           persist.NewFilestore(storePath, certsDir, certsDir),
+		clientDriverFactory: rpcdriver.NewRPCClientDriverFactory(),
 	}
 }
 
-func (api *Client) NewHost(driver drivers.Driver) (*host.Host, error) {
+func (api *Client) NewHost(driverName string, rawDriver []byte) (*host.Host, error) {
-	certDir := filepath.Join(api.Path, "certs")
+	driver, err := api.clientDriverFactory.NewRPCClientDriver(driverName, rawDriver)
+	if err != nil {
+		return nil, err
+	}
 
-	hostOptions := &host.Options{
+	return &host.Host{
+		ConfigVersion: version.ConfigVersion,
+		Name:          driver.GetMachineName(),
+		Driver:        driver,
+		DriverName:    driver.DriverName(),
+		HostOptions: &host.Options{
 			AuthOptions: &auth.Options{
-			CertDir:          certDir,
+				CertDir:          api.certsDir,
-			CaCertPath:       filepath.Join(certDir, "ca.pem"),
+				CaCertPath:       filepath.Join(api.certsDir, "ca.pem"),
-			CaPrivateKeyPath: filepath.Join(certDir, "ca-key.pem"),
+				CaPrivateKeyPath: filepath.Join(api.certsDir, "ca-key.pem"),
-			ClientCertPath:   filepath.Join(certDir, "cert.pem"),
+				ClientCertPath:   filepath.Join(api.certsDir, "cert.pem"),
-			ClientKeyPath:    filepath.Join(certDir, "key.pem"),
+				ClientKeyPath:    filepath.Join(api.certsDir, "key.pem"),
 				ServerCertPath:   filepath.Join(api.GetMachinesDir(), "server.pem"),
 				ServerKeyPath:    filepath.Join(api.GetMachinesDir(), "server-key.pem"),
 			},
@@ -67,15 +83,33 @@ func (api *Client) NewHost(driver drivers.Driver) (*host.Host, error) {
 				Image:    "swarm:latest",
 				Strategy: "spread",
 			},
+		},
+	}, nil
+}
+
+func (api *Client) Load(name string) (*host.Host, error) {
+	h, err := api.Filestore.Load(name)
+	if err != nil {
+		return nil, err
 	}
 
-	return &host.Host{
+	d, err := api.clientDriverFactory.NewRPCClientDriver(h.DriverName, h.RawDriver)
-		ConfigVersion: version.ConfigVersion,
+	if err != nil {
-		Name:          driver.GetMachineName(),
+		// Not being able to find a driver binary is a "known error"
-		Driver:        driver,
+		if _, ok := err.(localbinary.ErrPluginBinaryNotFound); ok {
-		DriverName:    driver.DriverName(),
+			h.Driver = errdriver.NewDriver(h.DriverName)
-		HostOptions:   hostOptions,
+			return h, nil
-	}, nil
+		}
+		return nil, err
+	}
+
+	if h.DriverName == "virtualbox" {
+		h.Driver = drivers.NewSerialDriver(d)
+	} else {
+		h.Driver = d
+	}
+
+	return h, nil
 }
 
 // Create is the wrapper method which covers all of the boilerplate around
@@ -108,7 +142,6 @@ func (api *Client) Create(h *host.Host) error {
 }
 
 func (api *Client) performCreate(h *host.Host) error {
-
 	if err := h.Driver.Create(); err != nil {
 		return fmt.Errorf("Error in driver during machine creation: %s", err)
 	}
@@ -150,7 +183,6 @@ func (api *Client) performCreate(h *host.Host) error {
 	}
 
 	return nil
-
 }
 
 func sendCrashReport(err error, api *Client, host *host.Host) {
@@ -161,3 +193,7 @@ func sendCrashReport(err error, api *Client, host *host.Host) {
 		crashreport.Send(err, "api.performCreate", host.DriverName, "Create")
 	}
 }
+
+func (api *Client) Close() error {
+	return api.clientDriverFactory.Close()
+}

libmachine/libmachinetest/fake_api.go

@@ -16,7 +16,11 @@ func (api *FakeAPI) NewPluginDriver(string, []byte) (drivers.Driver, error) {
 	return nil, nil
 }
 
-func (api *FakeAPI) NewHost(driver drivers.Driver) (*host.Host, error) {
+func (api *FakeAPI) Close() error {
+	return nil
+}
+
+func (api *FakeAPI) NewHost(driverName string, rawDriver []byte) (*host.Host, error) {
 	return nil, nil
 }
 

libmachine/persist/plugin_store.go

@@ -1,60 +0,0 @@
-package persist
-
-import (
-	"github.com/docker/machine/drivers/errdriver"
-	"github.com/docker/machine/libmachine/drivers"
-	"github.com/docker/machine/libmachine/drivers/plugin/localbinary"
-	"github.com/docker/machine/libmachine/drivers/rpc"
-	"github.com/docker/machine/libmachine/host"
-)
-
-type PluginDriverFactory interface {
-	NewPluginDriver(driverName string, rawDriver []byte) (drivers.Driver, error)
-}
-
-type RPCPluginDriverFactory struct{}
-
-type PluginStore struct {
-	*Filestore
-	PluginDriverFactory
-}
-
-func (factory RPCPluginDriverFactory) NewPluginDriver(driverName string, rawDriver []byte) (drivers.Driver, error) {
-	d, err := rpcdriver.NewRPCClientDriver(driverName, rawDriver)
-	if err != nil {
-		// Not being able to find a driver binary is a "known error"
-		if _, ok := err.(localbinary.ErrPluginBinaryNotFound); ok {
-			return errdriver.NewDriver(driverName), nil
-		}
-		return nil, err
-	}
-
-	if driverName == "virtualbox" {
-		return drivers.NewSerialDriver(d), nil
-	}
-
-	return d, nil
-}
-
-func NewPluginStore(path, caCertPath, caPrivateKeyPath string) *PluginStore {
-	return &PluginStore{
-		Filestore:           NewFilestore(path, caCertPath, caPrivateKeyPath),
-		PluginDriverFactory: RPCPluginDriverFactory{},
-	}
-}
-
-func (ps PluginStore) Load(name string) (*host.Host, error) {
-	h, err := ps.Filestore.Load(name)
-	if err != nil {
-		return nil, err
-	}
-
-	d, err := ps.NewPluginDriver(h.DriverName, h.RawDriver)
-	if err != nil {
-		return nil, err
-	}
-
-	h.Driver = d
-
-	return h, nil
-}