Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Switch the API implementation to use the local APIs everywhere we can. #1544

Merged
merged 3 commits into from
Jun 14, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion cmd/minikube/cmd/config/open.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ var addonsOpenCmd = &cobra.Command{
}
addonName := args[0]
//TODO(r2d4): config should not reference API, pull this out
api, err := machine.NewAPIClient(GetClientType())
api, err := machine.NewAPIClient()
if err != nil {
fmt.Fprintf(os.Stderr, "Error getting client: %s\n", err)
os.Exit(1)
Expand Down
10 changes: 1 addition & 9 deletions cmd/minikube/cmd/config/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ import (

"github.com/docker/machine/libmachine/drivers"
"github.com/pkg/errors"
"github.com/spf13/viper"
"k8s.io/minikube/pkg/minikube/assets"
"k8s.io/minikube/pkg/minikube/cluster"
"k8s.io/minikube/pkg/minikube/config"
Expand Down Expand Up @@ -82,13 +81,6 @@ func SetBool(m config.MinikubeConfig, name string, val string) error {
return nil
}

func GetClientType() machine.ClientType {
if viper.GetBool(useVendoredDriver) {
return machine.ClientTypeLocal
}
return machine.ClientTypeRPC
}

func EnableOrDisableAddon(name string, val string) error {

enable, err := strconv.ParseBool(val)
Expand All @@ -97,7 +89,7 @@ func EnableOrDisableAddon(name string, val string) error {
}

//TODO(r2d4): config package should not reference API, pull this out
api, err := machine.NewAPIClient(GetClientType())
api, err := machine.NewAPIClient()
if err != nil {
fmt.Fprintf(os.Stderr, "Error getting client: %s\n", err)
os.Exit(1)
Expand Down
2 changes: 1 addition & 1 deletion cmd/minikube/cmd/dashboard.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ var dashboardCmd = &cobra.Command{
Short: "Opens/displays the kubernetes dashboard URL for your local cluster",
Long: `Opens/displays the kubernetes dashboard URL for your local cluster`,
Run: func(cmd *cobra.Command, args []string) {
api, err := machine.NewAPIClient(clientType)
api, err := machine.NewAPIClient()
if err != nil {
fmt.Fprintf(os.Stderr, "Error getting client: %s\n", err)
os.Exit(1)
Expand Down
2 changes: 1 addition & 1 deletion cmd/minikube/cmd/delete.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ var deleteCmd = &cobra.Command{
associated files.`,
Run: func(cmd *cobra.Command, args []string) {
fmt.Println("Deleting local Kubernetes cluster...")
api, err := machine.NewAPIClient(clientType)
api, err := machine.NewAPIClient()
if err != nil {
fmt.Fprintf(os.Stderr, "Error getting client: %s\n", err)
os.Exit(1)
Expand Down
2 changes: 1 addition & 1 deletion cmd/minikube/cmd/env.go
Original file line number Diff line number Diff line change
Expand Up @@ -286,7 +286,7 @@ var dockerEnvCmd = &cobra.Command{
Long: `sets up docker env variables; similar to '$(docker-machine env)'`,
Run: func(cmd *cobra.Command, args []string) {

api, err := machine.NewAPIClient(clientType)
api, err := machine.NewAPIClient()
if err != nil {
fmt.Fprintf(os.Stderr, "Error getting client: %s\n", err)
os.Exit(1)
Expand Down
2 changes: 1 addition & 1 deletion cmd/minikube/cmd/ip.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ var ipCmd = &cobra.Command{
Short: "Retrieve the IP address of the running cluster.",
Long: `Retrieves the IP address of the running cluster, and writes it to STDOUT.`,
Run: func(cmd *cobra.Command, args []string) {
api, err := machine.NewAPIClient(clientType)
api, err := machine.NewAPIClient()
if err != nil {
fmt.Fprintf(os.Stderr, "Error getting client: %s\n", err)
os.Exit(1)
Expand Down
2 changes: 1 addition & 1 deletion cmd/minikube/cmd/logs.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ var logsCmd = &cobra.Command{
Short: "Gets the logs of the running localkube instance, used for debugging minikube, not user code",
Long: `Gets the logs of the running localkube instance, used for debugging minikube, not user code.`,
Run: func(cmd *cobra.Command, args []string) {
api, err := machine.NewAPIClient(clientType)
api, err := machine.NewAPIClient()
if err != nil {
fmt.Fprintf(os.Stderr, "Error getting client: %s\n", err)
os.Exit(1)
Expand Down
2 changes: 1 addition & 1 deletion cmd/minikube/cmd/mount.go
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ var mountCmd = &cobra.Command{
if glog.V(1) {
debugVal = 1 // ufs.StartServer takes int debug param
}
api, err := machine.NewAPIClient(clientType)
api, err := machine.NewAPIClient()
if err != nil {
fmt.Fprintf(os.Stderr, "Error getting client: %s\n", err)
os.Exit(1)
Expand Down
5 changes: 0 additions & 5 deletions cmd/minikube/cmd/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ import (
"k8s.io/minikube/cmd/util"
"k8s.io/minikube/pkg/minikube/config"
"k8s.io/minikube/pkg/minikube/constants"
"k8s.io/minikube/pkg/minikube/machine"
"k8s.io/minikube/pkg/minikube/notify"
)

Expand All @@ -56,7 +55,6 @@ const (

var (
enableUpdateNotification = true
clientType machine.ClientType
)

var viperWhiteList = []string{
Expand Down Expand Up @@ -95,9 +93,6 @@ Please use --v=3 to show libmachine logs, and --v=7 for debug level libmachine l
`)
}

//TODO(r2d4): config should not reference API
clientType = configCmd.GetClientType()

logDir := pflag.Lookup("log_dir")
if !logDir.Changed {
logDir.Value.Set(constants.MakeMiniPath("logs"))
Expand Down
2 changes: 1 addition & 1 deletion cmd/minikube/cmd/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ var serviceCmd = &cobra.Command{
}

svc := args[0]
api, err := machine.NewAPIClient(clientType)
api, err := machine.NewAPIClient()
if err != nil {
fmt.Fprintf(os.Stderr, "Error getting client: %s\n", err)
os.Exit(1)
Expand Down
2 changes: 1 addition & 1 deletion cmd/minikube/cmd/service_list.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ var serviceListCmd = &cobra.Command{
Short: "Lists the URLs for the services in your local cluster",
Long: `Lists the URLs for the services in your local cluster`,
Run: func(cmd *cobra.Command, args []string) {
api, err := machine.NewAPIClient(clientType)
api, err := machine.NewAPIClient()
if err != nil {
fmt.Fprintf(os.Stderr, "Error getting client: %s\n", err)
os.Exit(1)
Expand Down
2 changes: 1 addition & 1 deletion cmd/minikube/cmd/ssh.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ var sshCmd = &cobra.Command{
Short: "Log into or run a command on a machine with SSH; similar to 'docker-machine ssh'",
Long: "Log into or run a command on a machine with SSH; similar to 'docker-machine ssh'",
Run: func(cmd *cobra.Command, args []string) {
api, err := machine.NewAPIClient(clientType)
api, err := machine.NewAPIClient()
if err != nil {
fmt.Fprintf(os.Stderr, "Error getting client: %s\n", err)
os.Exit(1)
Expand Down
2 changes: 1 addition & 1 deletion cmd/minikube/cmd/start.go
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ assumes you have already installed one of the VM drivers: virtualbox/vmwarefusio
}

func runStart(cmd *cobra.Command, args []string) {
api, err := machine.NewAPIClient(clientType)
api, err := machine.NewAPIClient()
if err != nil {
fmt.Fprintf(os.Stderr, "Error getting client: %s\n", err)
os.Exit(1)
Expand Down
2 changes: 1 addition & 1 deletion cmd/minikube/cmd/status.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ var statusCmd = &cobra.Command{
Short: "Gets the status of a local kubernetes cluster",
Long: `Gets the status of a local kubernetes cluster.`,
Run: func(cmd *cobra.Command, args []string) {
api, err := machine.NewAPIClient(clientType)
api, err := machine.NewAPIClient()
if err != nil {
fmt.Fprintf(os.Stderr, "Error getting client: %s\n", err)
os.Exit(1)
Expand Down
2 changes: 1 addition & 1 deletion cmd/minikube/cmd/stop.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ var stopCmd = &cobra.Command{
itself, leaving all files intact. The cluster can be started again with the "start" command.`,
Run: func(cmd *cobra.Command, args []string) {
fmt.Println("Stopping local Kubernetes cluster...")
api, err := machine.NewAPIClient(clientType)
api, err := machine.NewAPIClient()
if err != nil {
fmt.Fprintf(os.Stderr, "Error getting client: %s\n", err)
os.Exit(1)
Expand Down
71 changes: 31 additions & 40 deletions pkg/minikube/machine/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,53 +49,23 @@ import (

type driverGetter func([]byte) (drivers.Driver, error)

type ClientType int
type clientFactory interface {
NewClient(string, string) libmachine.API
}

type localClientFactory struct{}

func (*localClientFactory) NewClient(storePath, certsDir string) libmachine.API {
return &LocalClient{
certsDir: certsDir,
storePath: storePath,
Filestore: persist.NewFilestore(storePath, certsDir, certsDir),
}
}

type rpcClientFactory struct{}

func (*rpcClientFactory) NewClient(storePath, certsDir string) libmachine.API {
func NewRPCClient(storePath, certsDir string) libmachine.API {
c := libmachine.NewClient(storePath, certsDir)
c.SSHClientType = ssh.Native
return c
}

var clientFactories = map[ClientType]clientFactory{
// ClientTypeNative: &nativeClientFactory{},
ClientTypeLocal: &localClientFactory{},
ClientTypeRPC: &rpcClientFactory{},
}

const (
ClientTypeLocal ClientType = iota
ClientTypeRPC

// ClientTypeNative
)

// Gets a new client depending on the clientType specified
// defaults to the libmachine client
func NewAPIClient(clientType ClientType) (libmachine.API, error) {
// NewAPIClient gets a new client.
func NewAPIClient() (libmachine.API, error) {
storePath := constants.GetMinipath()
certsDir := constants.MakeMiniPath("certs")
newClientFactory, ok := clientFactories[clientType]
if !ok {
return nil, fmt.Errorf("No implementation for API client type %d", clientType)
}

return newClientFactory.NewClient(storePath, certsDir), nil
return &LocalClient{
certsDir: certsDir,
storePath: storePath,
Filestore: persist.NewFilestore(storePath, certsDir, certsDir),
legacyClient: NewRPCClient(storePath, certsDir),
}, nil
}

func getDriver(driverName string, rawDriver []byte) (drivers.Driver, error) {
Expand Down Expand Up @@ -131,9 +101,15 @@ type LocalClient struct {
certsDir string
storePath string
*persist.Filestore
legacyClient libmachine.API
}

func (api *LocalClient) NewHost(driverName string, rawDriver []byte) (*host.Host, error) {
// If not should get Driver, use legacy
if _, ok := driverMap[driverName]; !ok {
return api.legacyClient.NewHost(driverName, rawDriver)
}

driver, err := getDriver(driverName, rawDriver)
if err != nil {
return nil, errors.Wrap(err, "Error getting driver")
Expand Down Expand Up @@ -168,6 +144,11 @@ func (api *LocalClient) Load(name string) (*host.Host, error) {
return nil, errors.Wrap(err, "Error loading host from store")
}

// If not should get Driver, use legacy
if _, ok := driverMap[h.DriverName]; !ok {
return api.legacyClient.Load(name)
}

h.Driver, err = getDriver(h.DriverName, h.RawDriver)
if err != nil {
return nil, errors.Wrap(err, "Error loading driver from host")
Expand All @@ -176,9 +157,19 @@ func (api *LocalClient) Load(name string) (*host.Host, error) {
return h, nil
}

func (api *LocalClient) Close() error { return nil }
func (api *LocalClient) Close() error {
if api.legacyClient != nil {
return api.legacyClient.Close()
}
return nil
}

func (api *LocalClient) Create(h *host.Host) error {

if _, ok := driverMap[h.Driver.DriverName()]; !ok {
return api.legacyClient.Create(h)
}

steps := []struct {
name string
f func() error
Expand Down
1 change: 0 additions & 1 deletion pkg/minikube/machine/client_darwin.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ import (

var driverMap = map[string]driverGetter{
"vmwarefusion": getVMWareFusionDriver,
"xhyve": getXhyveDriver,
"virtualbox": getVirtualboxDriver,
}

Expand Down
8 changes: 0 additions & 8 deletions pkg/minikube/machine/client_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,18 +28,10 @@ import (
)

var driverMap = map[string]driverGetter{
"kvm": getKVMDriver,
"virtualbox": getVirtualboxDriver,
"none": getNoneDriver,
}

func getKVMDriver(rawDriver []byte) (drivers.Driver, error) {
return nil, errors.New(`
The KVM driver is not included in minikube yet. Please follow the direction at
https://github.com/kubernetes/minikube/blob/master/DRIVERS.md#kvm-driver
`)
}

func getNoneDriver(rawDriver []byte) (drivers.Driver, error) {
var driver drivers.Driver
driver = &none.Driver{}
Expand Down
42 changes: 4 additions & 38 deletions pkg/minikube/machine/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -116,8 +116,10 @@ func TestGetDriver(t *testing.T) {
}

func TestLocalClientNewHost(t *testing.T) {
f := clientFactories[ClientTypeLocal]
c := f.NewClient("", "")
c, err := NewAPIClient()
if err != nil {
t.Fatal(err)
}

var tests = []struct {
description string
Expand Down Expand Up @@ -162,42 +164,6 @@ func TestLocalClientNewHost(t *testing.T) {
}
}

func TestNewAPIClient(t *testing.T) {
var tests = []struct {
description string
clientType ClientType
err bool
}{
{
description: "Client type local",
clientType: ClientTypeLocal,
},
{
description: "Client type RPC",
clientType: ClientTypeRPC,
},
{
description: "Incorrect client type",
clientType: -1,
err: true,
},
}

for _, test := range tests {
test := test
t.Run(test.description, func(t *testing.T) {
t.Parallel()
_, err := NewAPIClient(test.clientType)
if err != nil && !test.err {
t.Errorf("Unexpected error: %s", err)
}
if err == nil && test.err {
t.Errorf("No error returned, but expected err")
}
})
}
}

func makeTempDir() string {
tempDir, err := ioutil.TempDir("", "minipath")
if err != nil {
Expand Down