Merge pull request #7360 from tstromberg/wrong-url

Correct assumptions for forwarded hostname & IP handling

cmd/minikube/cmd/docker-env.go

@@ -150,7 +150,7 @@ var dockerEnvCmd = &cobra.Command{
 
 		var err error
 		port := constants.DockerDaemonPort
-		if driver.IsKIC(driverName) {
+		if driver.NeedsPortForward(driverName) {
 			port, err = oci.ForwardedPort(driverName, cname, port)
 			if err != nil {
 				exit.WithCodeT(exit.Failure, "Error getting port binding for '{{.driver_name}} driver: {{.error}}", out.V{"driver_name": driverName, "error": err})
@@ -161,7 +161,7 @@ var dockerEnvCmd = &cobra.Command{
 			EnvConfig: sh,
 			profile:   cname,
 			driver:    driverName,
-			hostIP:    co.CP.ForwardedIP.String(),
+			hostIP:    co.CP.IP.String(),
 			port:      port,
 			certsDir:  localpath.MakeMiniPath("certs"),
 			noProxy:   noProxy,

cmd/minikube/cmd/ip.go

@@ -29,6 +29,6 @@ var ipCmd = &cobra.Command{
 	Long:  `Retrieves the IP address of the running cluster, and writes it to STDOUT.`,
 	Run: func(cmd *cobra.Command, args []string) {
 		co := mustload.Running(ClusterFlagValue())
-		out.Ln(co.CP.ForwardedIP.String())
+		out.Ln(co.CP.IP.String())
 	},
 }

cmd/minikube/cmd/start.go

@@ -192,7 +192,7 @@ func initKubernetesFlags() {
 	startCmd.Flags().String(featureGates, "", "A set of key=value pairs that describe feature gates for alpha/experimental features.")
 	startCmd.Flags().String(dnsDomain, constants.ClusterDNSDomain, "The cluster dns domain name used in the kubernetes cluster")
 	startCmd.Flags().Int(apiServerPort, constants.APIServerPort, "The apiserver listening port")
-	startCmd.Flags().String(apiServerName, constants.APIServerName, "The apiserver name which is used in the generated certificate for kubernetes.  This can be used if you want to make the apiserver available from outside the machine")
+	startCmd.Flags().String(apiServerName, constants.APIServerName, "The authoritative apiserver hostname for apiserver certificates and connectivity. This can be used if you want to make the apiserver available from outside the machine")
 	startCmd.Flags().StringArrayVar(&apiServerNames, "apiserver-names", nil, "A set of apiserver names which are used in the generated certificate for kubernetes.  This can be used if you want to make the apiserver available from outside the machine")
 	startCmd.Flags().IPSliceVar(&apiServerIPs, "apiserver-ips", nil, "A set of apiserver IP Addresses which are used in the generated certificate for kubernetes.  This can be used if you want to make the apiserver available from outside the machine")
 }

cmd/minikube/cmd/status.go

@@ -32,7 +32,6 @@ import (
 	"k8s.io/minikube/pkg/minikube/bootstrapper/bsutil/kverify"
 	"k8s.io/minikube/pkg/minikube/cluster"
 	"k8s.io/minikube/pkg/minikube/config"
-	"k8s.io/minikube/pkg/minikube/constants"
 	"k8s.io/minikube/pkg/minikube/driver"
 	"k8s.io/minikube/pkg/minikube/exit"
 	"k8s.io/minikube/pkg/minikube/kubeconfig"
@@ -187,33 +186,18 @@ func status(api libmachine.API, cc config.ClusterConfig, n config.Node) (*Status
 	}
 
 	// We have a fully operational host, now we can check for details
-	ip, err := cluster.GetHostDriverIP(api, name)
-	if err != nil {
-		glog.Errorln("Error host driver ip status:", err)
-		st.APIServer = state.Error.String()
+	if _, err := cluster.GetHostDriverIP(api, name); err != nil {
+		glog.Errorf("failed to get driver ip: %v", err)
+		st.Host = state.Error.String()
 		return st, err
 	}
 
-	port, err := kubeconfig.Port(name)
-	if err != nil {
-		glog.Warningf("unable to get port: %v", err)
-		port = constants.APIServerPort
-	}
-
-	st.Kubeconfig = Misconfigured
+	st.Kubeconfig = Configured
 	if !controlPlane {
 		st.Kubeconfig = Irrelevant
 		st.APIServer = Irrelevant
 	}
 
-	if st.Kubeconfig != Irrelevant {
-		ok, err := kubeconfig.IsClusterInConfig(ip, cc.Name)
-		glog.Infof("%s is in kubeconfig at ip %s: %v (err=%v)", name, ip, ok, err)
-		if ok {
-			st.Kubeconfig = Configured
-		}
-	}
-
 	host, err := machine.LoadHost(api, name)
 	if err != nil {
 		return st, err
@@ -234,8 +218,24 @@ func status(api libmachine.API, cc config.ClusterConfig, n config.Node) (*Status
 		st.Kubelet = stk.String()
 	}
 
-	if st.APIServer != Irrelevant {
-		sta, err := kverify.APIServerStatus(cr, ip, port)
+	// Early exit for regular nodes
+	if !controlPlane {
+		return st, nil
+	}
+
+	hostname, _, port, err := driver.ControlPaneEndpoint(&cc, &n, host.DriverName)
+	if err != nil {
+		glog.Errorf("forwarded endpoint: %v", err)
+		st.Kubeconfig = Misconfigured
+	} else {
+		err := kubeconfig.VerifyEndpoint(cc.Name, hostname, port)
+		if err != nil {
+			glog.Errorf("kubeconfig endpoint: %v", err)
+			st.Kubeconfig = Misconfigured
+		}
+	}
+
+	sta, err := kverify.APIServerStatus(cr, hostname, port)
 	glog.Infof("%s apiserver status = %s (err=%v)", name, stk, err)
 
 	if err != nil {
@@ -244,7 +244,6 @@ func status(api libmachine.API, cc config.ClusterConfig, n config.Node) (*Status
 	} else {
 		st.APIServer = sta.String()
 	}
-	}
 
 	return st, nil
 }

cmd/minikube/cmd/update-context.go

@@ -33,17 +33,15 @@ var updateContextCmd = &cobra.Command{
 	Run: func(cmd *cobra.Command, args []string) {
 		cname := ClusterFlagValue()
 		co := mustload.Running(cname)
-		ip := co.CP.ForwardedIP
 
-		// ??? For KIC, should we also update the port ???
-		updated, err := kubeconfig.UpdateIP(ip, cname, kubeconfig.PathFromEnv())
+		updated, err := kubeconfig.UpdateEndpoint(cname, co.CP.Hostname, co.CP.Port, kubeconfig.PathFromEnv())
 		if err != nil {
 			exit.WithError("update config", err)
 		}
 		if updated {
-			out.T(out.Celebrate, "{{.cluster}} IP has been updated to point at {{.ip}}", out.V{"cluster": cname, "ip": ip})
+			out.T(out.Celebrate, `"{{.context}}" context has been updated to point to {{.hostname}}:{{.port}}`, out.V{"context": cname, "hostname": co.CP.Hostname, "port": co.CP.Port})
 		} else {
-			out.T(out.Meh, "{{.cluster}} IP was already correctly configured for {{.ip}}", out.V{"cluster": cname, "ip": ip})
+			out.T(out.Meh, `No changes required for the "{{.context}}" context`, out.V{"context": cname})
 		}
 
 	},

pkg/drivers/none/none.go

@@ -18,7 +18,6 @@ package none
 
 import (
 	"fmt"
-	"net"
 	"os/exec"
 
 	"github.com/docker/machine/libmachine/drivers"
@@ -126,20 +125,14 @@ func (d *Driver) GetURL() (string, error) {
 
 // GetState returns the state that the host is in (running, stopped, etc)
 func (d *Driver) GetState() (state.State, error) {
-	glog.Infof("GetState called")
-	ip, err := d.GetIP()
-	if err != nil {
-		return state.Error, err
-	}
-
-	port, err := kubeconfig.Port(d.BaseDriver.MachineName)
+	hostname, port, err := kubeconfig.Endpoint(d.BaseDriver.MachineName)
 	if err != nil {
 		glog.Warningf("unable to get port: %v", err)
 		port = constants.APIServerPort
 	}
 
 	// Confusing logic, as libmachine.Stop will loop until the state == Stopped
-	ast, err := kverify.APIServerStatus(d.exec, net.ParseIP(ip), port)
+	ast, err := kverify.APIServerStatus(d.exec, hostname, port)
 	if err != nil {
 		return ast, err
 	}

pkg/minikube/bootstrapper/bootstrapper.go

@@ -17,7 +17,6 @@ limitations under the License.
 package bootstrapper
 
 import (
-	"net"
 	"time"
 
 	"k8s.io/minikube/pkg/minikube/bootstrapper/images"
@@ -47,7 +46,7 @@ type Bootstrapper interface {
 	LogCommands(config.ClusterConfig, LogOptions) map[string]string
 	SetupCerts(config.KubernetesConfig, config.Node) error
 	GetKubeletStatus() (string, error)
-	GetAPIServerStatus(net.IP, int) (string, error)
+	GetAPIServerStatus(string, int) (string, error)
 }
 
 const (

pkg/minikube/bootstrapper/bsutil/kverify/kverify.go

@@ -183,7 +183,7 @@ func WaitForSystemPods(r cruntime.Manager, bs bootstrapper.Bootstrapper, cfg con
 }
 
 // WaitForHealthyAPIServer waits for api server status to be running
-func WaitForHealthyAPIServer(r cruntime.Manager, bs bootstrapper.Bootstrapper, cfg config.ClusterConfig, cr command.Runner, client *kubernetes.Clientset, start time.Time, ip string, port int, timeout time.Duration) error {
+func WaitForHealthyAPIServer(r cruntime.Manager, bs bootstrapper.Bootstrapper, cfg config.ClusterConfig, cr command.Runner, client *kubernetes.Clientset, start time.Time, hostname string, port int, timeout time.Duration) error {
 	glog.Infof("waiting for apiserver healthz status ...")
 	hStart := time.Now()
 
@@ -197,7 +197,7 @@ func WaitForHealthyAPIServer(r cruntime.Manager, bs bootstrapper.Bootstrapper, c
 			time.Sleep(kconst.APICallRetryInterval * 5)
 		}
 
-		status, err := apiServerHealthz(net.ParseIP(ip), port)
+		status, err := apiServerHealthz(hostname, port)
 		if err != nil {
 			glog.Warningf("status: %v", err)
 			return false, nil
@@ -254,7 +254,7 @@ func announceProblems(r cruntime.Manager, bs bootstrapper.Bootstrapper, cfg conf
 }
 
 // APIServerStatus returns apiserver status in libmachine style state.State
-func APIServerStatus(cr command.Runner, ip net.IP, port int) (state.State, error) {
+func APIServerStatus(cr command.Runner, hostname string, port int) (state.State, error) {
 	glog.Infof("Checking apiserver status ...")
 
 	pid, err := apiServerPID(cr)
@@ -267,7 +267,7 @@ func APIServerStatus(cr command.Runner, ip net.IP, port int) (state.State, error
 	rr, err := cr.RunCmd(exec.Command("sudo", "egrep", "^[0-9]+:freezer:", fmt.Sprintf("/proc/%d/cgroup", pid)))
 	if err != nil {
 		glog.Warningf("unable to find freezer cgroup: %v", err)
-		return apiServerHealthz(ip, port)
+		return apiServerHealthz(hostname, port)
 
 	}
 	freezer := strings.TrimSpace(rr.Stdout.String())
@@ -275,13 +275,13 @@ func APIServerStatus(cr command.Runner, ip net.IP, port int) (state.State, error
 	fparts := strings.Split(freezer, ":")
 	if len(fparts) != 3 {
 		glog.Warningf("unable to parse freezer - found %d parts: %s", len(fparts), freezer)
-		return apiServerHealthz(ip, port)
+		return apiServerHealthz(hostname, port)
 	}
 
 	rr, err = cr.RunCmd(exec.Command("sudo", "cat", path.Join("/sys/fs/cgroup/freezer", fparts[2], "freezer.state")))
 	if err != nil {
 		glog.Errorf("unable to get freezer state: %s", rr.Stderr.String())
-		return apiServerHealthz(ip, port)
+		return apiServerHealthz(hostname, port)
 	}
 
 	fs := strings.TrimSpace(rr.Stdout.String())
@@ -289,12 +289,12 @@ func APIServerStatus(cr command.Runner, ip net.IP, port int) (state.State, error
 	if fs == "FREEZING" || fs == "FROZEN" {
 		return state.Paused, nil
 	}
-	return apiServerHealthz(ip, port)
+	return apiServerHealthz(hostname, port)
 }
 
 // apiServerHealthz hits the /healthz endpoint and returns libmachine style state.State
-func apiServerHealthz(ip net.IP, port int) (state.State, error) {
-	url := fmt.Sprintf("https://%s/healthz", net.JoinHostPort(ip.String(), fmt.Sprint(port)))
+func apiServerHealthz(hostname string, port int) (state.State, error) {
+	url := fmt.Sprintf("https://%s/healthz", net.JoinHostPort(hostname, fmt.Sprint(port)))
 	glog.Infof("Checking apiserver healthz at %s ...", url)
 	// To avoid: x509: certificate signed by unknown authority
 	tr := &http.Transport{

pkg/minikube/bootstrapper/kubeadm/kubeadm.go

@@ -39,7 +39,6 @@ import (
 	"k8s.io/client-go/kubernetes"
 	kconst "k8s.io/kubernetes/cmd/kubeadm/app/constants"
 	"k8s.io/minikube/pkg/drivers/kic"
-	"k8s.io/minikube/pkg/drivers/kic/oci"
 	"k8s.io/minikube/pkg/kapi"
 	"k8s.io/minikube/pkg/minikube/assets"
 	"k8s.io/minikube/pkg/minikube/bootstrapper"
@@ -103,8 +102,8 @@ func (k *Bootstrapper) GetKubeletStatus() (string, error) {
 }
 
 // GetAPIServerStatus returns the api-server status
-func (k *Bootstrapper) GetAPIServerStatus(ip net.IP, port int) (string, error) {
-	s, err := kverify.APIServerStatus(k.c, ip, port)
+func (k *Bootstrapper) GetAPIServerStatus(hostname string, port int) (string, error) {
+	s, err := kverify.APIServerStatus(k.c, hostname, port)
 	if err != nil {
 		return state.Error.String(), err
 	}
@@ -316,20 +315,6 @@ func (k *Bootstrapper) StartCluster(cfg config.ClusterConfig) error {
 	return k.init(cfg)
 }
 
-func (k *Bootstrapper) controlPlaneEndpoint(cfg config.ClusterConfig) (string, int, error) {
-	cp, err := config.PrimaryControlPlane(&cfg)
-	if err != nil {
-		return "", 0, err
-	}
-
-	if driver.IsKIC(cfg.Driver) {
-		ip := oci.DefaultBindIPV4
-		port, err := oci.ForwardedPort(cfg.Driver, cfg.Name, cp.Port)
-		return ip, port, err
-	}
-	return cp.IP, cp.Port, nil
-}
-
 // client sets and returns a Kubernetes client to use to speak to a kubeadm launched apiserver
 func (k *Bootstrapper) client(ip string, port int) (*kubernetes.Clientset, error) {
 	if k.k8sClient != nil {
@@ -371,17 +356,17 @@ func (k *Bootstrapper) WaitForNode(cfg config.ClusterConfig, n config.Node, time
 		return err
 	}
 
-	ip, port, err := k.controlPlaneEndpoint(cfg)
+	hostname, _, port, err := driver.ControlPaneEndpoint(&cfg, &n, cfg.Driver)
 	if err != nil {
 		return err
 	}
 
-	client, err := k.client(ip, port)
+	client, err := k.client(hostname, port)
 	if err != nil {
 		return errors.Wrap(err, "get k8s client")
 	}
 
-	if err := kverify.WaitForHealthyAPIServer(cr, k, cfg, k.c, client, start, ip, port, timeout); err != nil {
+	if err := kverify.WaitForHealthyAPIServer(cr, k, cfg, k.c, client, start, hostname, port, timeout); err != nil {
 		return err
 	}
 
@@ -392,13 +377,13 @@ func (k *Bootstrapper) WaitForNode(cfg config.ClusterConfig, n config.Node, time
 }
 
 // needsReset returns whether or not the cluster needs to be reconfigured
-func (k *Bootstrapper) needsReset(conf string, ip string, port int, client *kubernetes.Clientset, version string) bool {
+func (k *Bootstrapper) needsReset(conf string, hostname string, port int, client *kubernetes.Clientset, version string) bool {
 	if rr, err := k.c.RunCmd(exec.Command("sudo", "diff", "-u", conf, conf+".new")); err != nil {
 		glog.Infof("needs reset: configs differ:\n%s", rr.Output())
 		return true
 	}
 
-	st, err := kverify.APIServerStatus(k.c, net.ParseIP(ip), port)
+	st, err := kverify.APIServerStatus(k.c, hostname, port)
 	if err != nil {
 		glog.Infof("needs reset: apiserver error: %v", err)
 		return true
@@ -447,19 +432,24 @@ func (k *Bootstrapper) restartCluster(cfg config.ClusterConfig) error {
 		glog.Errorf("failed to create compat symlinks: %v", err)
 	}
 
-	ip, port, err := k.controlPlaneEndpoint(cfg)
+	cp, err := config.PrimaryControlPlane(&cfg)
+	if err != nil {
+		return errors.Wrap(err, "primary control plane")
+	}
+
+	hostname, _, port, err := driver.ControlPaneEndpoint(&cfg, &cp, cfg.Driver)
 	if err != nil {
 		return errors.Wrap(err, "control plane")
 	}
 
-	client, err := k.client(ip, port)
+	client, err := k.client(hostname, port)
 	if err != nil {
 		return errors.Wrap(err, "getting k8s client")
 	}
 
 	// If the cluster is running, check if we have any work to do.
 	conf := bsutil.KubeadmYamlPath
-	if !k.needsReset(conf, ip, port, client, cfg.KubernetesConfig.KubernetesVersion) {
+	if !k.needsReset(conf, hostname, port, client, cfg.KubernetesConfig.KubernetesVersion) {
 		glog.Infof("Taking a shortcut, as the cluster seems to be properly configured")
 		return nil
 	}
@@ -499,7 +489,7 @@ func (k *Bootstrapper) restartCluster(cfg config.ClusterConfig) error {
 		return errors.Wrap(err, "apiserver healthz")
 	}
 
-	if err := kverify.WaitForHealthyAPIServer(cr, k, cfg, k.c, client, time.Now(), ip, port, kconst.DefaultControlPlaneTimeout); err != nil {
+	if err := kverify.WaitForHealthyAPIServer(cr, k, cfg, k.c, client, time.Now(), hostname, port, kconst.DefaultControlPlaneTimeout); err != nil {
 		return errors.Wrap(err, "apiserver health")
 	}
 

pkg/minikube/driver/driver.go

@@ -19,6 +19,7 @@ package driver
 import (
 	"fmt"
 	"os"
+	"runtime"
 	"sort"
 	"strings"
 
@@ -128,6 +129,12 @@ func NeedsRoot(name string) bool {
 	return name == None || name == Podman
 }
 
+// NeedsPortForward returns true if driver is unable provide direct IP connectivity
+func NeedsPortForward(name string) bool {
+	// Docker for Desktop
+	return IsKIC(name) && (runtime.GOOS == "darwin" || runtime.GOOS == "windows")
+}
+
 // HasResourceLimits returns true if driver can set resource limits such as memory size or CPU count.
 func HasResourceLimits(name string) bool {
 	return !(name == None || name == Podman)

pkg/minikube/driver/endpoint.go

+/*
+Copyright 2020 The Kubernetes Authors All rights reserved.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package driver
+
+import (
+	"net"
+
+	"k8s.io/minikube/pkg/drivers/kic/oci"
+	"k8s.io/minikube/pkg/minikube/config"
+	"k8s.io/minikube/pkg/minikube/constants"
+)
+
+// ControlPaneEndpoint returns the location where callers can reach this cluster
+func ControlPaneEndpoint(cc *config.ClusterConfig, cp *config.Node, driverName string) (string, net.IP, int, error) {
+	if NeedsPortForward(driverName) {
+		port, err := oci.ForwardedPort(cc.Driver, cc.Name, cp.Port)
+		hostname := oci.DefaultBindIPV4
+		ip := net.ParseIP(hostname)
+
+		// https://github.com/kubernetes/minikube/issues/3878
+		if cc.KubernetesConfig.APIServerName != constants.APIServerName {
+			hostname = cc.KubernetesConfig.APIServerName
+		}
+		return hostname, ip, port, err
+	}
+
+	// https://github.com/kubernetes/minikube/issues/3878
+	hostname := cp.IP
+	if cc.KubernetesConfig.APIServerName != constants.APIServerName {
+		hostname = cc.KubernetesConfig.APIServerName
+	}
+	return hostname, net.ParseIP(cp.IP), cp.Port, nil
+}

pkg/minikube/kubeconfig/context_test.go

@@ -24,7 +24,8 @@ import (
 )
 
 func TestDeleteContext(t *testing.T) {
-	fn := tempFile(t, fakeKubeCfg)
+	// See kubeconfig_test
+	fn := tempFile(t, kubeConfigWithoutHTTPS)
 	if err := DeleteContext("la-croix", fn); err != nil {
 		t.Fatal(err)
 	}

pkg/minikube/kubeconfig/kubeconfig.go

@@ -19,7 +19,6 @@ package kubeconfig
 import (
 	"fmt"
 	"io/ioutil"
-	"net"
 	"net/url"
 	"os"
 	"path/filepath"
@@ -35,51 +34,27 @@ import (
 	"k8s.io/minikube/pkg/util/lock"
 )
 
-// IsClusterInConfig verifies the ip stored in kubeconfig.
-func IsClusterInConfig(ip net.IP, clusterName string, configPath ...string) (bool, error) {
+// VerifyEndpoint verifies the IP:port stored in kubeconfig.
+func VerifyEndpoint(contextName string, hostname string, port int, configPath ...string) error {
 	path := PathFromEnv()
 	if configPath != nil {
 		path = configPath[0]
 	}
-	if ip == nil {
-		return false, fmt.Errorf("error, empty ip passed")
-	}
-	kip, err := extractIP(clusterName, path)
-	if err != nil {
-		return false, err
-	}
-	if kip.Equal(ip) {
-		return true, nil
-	}
-	// Kubeconfig IP misconfigured
-	return false, nil
-
-}
 
-// Port returns the Port number stored for minikube in the kubeconfig specified
-func Port(clusterName string, configPath ...string) (int, error) {
-	path := PathFromEnv()
-	if configPath != nil {
-		path = configPath[0]
-	}
-	cfg, err := readOrNew(path)
-	if err != nil {
-		return 0, errors.Wrap(err, "Error getting kubeconfig status")
+	if hostname == "" {
+		return fmt.Errorf("empty IP")
 	}
-	cluster, ok := cfg.Clusters[clusterName]
-	if !ok {
-		return 0, errors.Errorf("Kubeconfig does not have a record of the machine cluster")
-	}
-	kurl, err := url.Parse(cluster.Server)
+
+	gotHostname, gotPort, err := Endpoint(contextName, path)
 	if err != nil {
-		return constants.APIServerPort, nil
+		return errors.Wrap(err, "extract IP")
 	}
-	_, kport, err := net.SplitHostPort(kurl.Host)
-	if err != nil {
-		return constants.APIServerPort, nil
+
+	if hostname != gotHostname || port != gotPort {
+		return fmt.Errorf("got: %s:%d, want: %s:%d", gotHostname, gotPort, hostname, port)
 	}
-	port, err := strconv.Atoi(kport)
-	return port, err
+
+	return nil
 }
 
 // PathFromEnv gets the path to the first kubeconfig
@@ -98,65 +73,58 @@ func PathFromEnv() string {
 	return constants.KubeconfigPath
 }
 
-// extractIP returns the IP address stored for minikube in the kubeconfig specified
-func extractIP(machineName string, configPath ...string) (net.IP, error) {
+// Endpoint returns the IP:port address stored for minikube in the kubeconfig specified
+func Endpoint(contextName string, configPath ...string) (string, int, error) {
 	path := PathFromEnv()
 	if configPath != nil {
 		path = configPath[0]
 	}
 	apiCfg, err := readOrNew(path)
 	if err != nil {
-		return nil, errors.Wrap(err, "Error getting kubeconfig status")
+		return "", 0, errors.Wrap(err, "read")
 	}
-	cluster, ok := apiCfg.Clusters[machineName]
+	cluster, ok := apiCfg.Clusters[contextName]
 	if !ok {
-		return nil, errors.Errorf("Kubeconfig does not have a record of the machine cluster")
+		return "", 0, errors.Errorf("%q does not appear in %s", contextName, path)
 	}
-	kurl, err := url.Parse(cluster.Server)
+
+	glog.Infof("found %q server: %q", contextName, cluster.Server)
+	u, err := url.Parse(cluster.Server)
 	if err != nil {
-		return net.ParseIP(cluster.Server), nil
+		return "", 0, errors.Wrap(err, "url parse")
 	}
-	kip, _, err := net.SplitHostPort(kurl.Host)
+
+	port, err := strconv.Atoi(u.Port())
 	if err != nil {
-		return net.ParseIP(kurl.Host), nil
+		return "", 0, errors.Wrap(err, "atoi")
 	}
-	ip := net.ParseIP(kip)
-	return ip, nil
-}
 
-// UpdateIP overwrites the IP stored in kubeconfig with the provided IP.
-func UpdateIP(ip net.IP, machineName string, configPath ...string) (bool, error) {
-	path := PathFromEnv()
-	if configPath != nil {
-		path = configPath[0]
-	}
+	return u.Hostname(), port, nil
+}
 
-	if ip == nil {
-		return false, fmt.Errorf("error, empty ip passed")
+// UpdateEndpoint overwrites the IP stored in kubeconfig with the provided IP.
+func UpdateEndpoint(contextName string, hostname string, port int, path string) (bool, error) {
+	if hostname == "" {
+		return false, fmt.Errorf("empty ip")
 	}
 
-	kip, err := extractIP(machineName, path)
-	if err != nil {
-		return false, err
-	}
-	if kip.Equal(ip) {
+	err := VerifyEndpoint(contextName, hostname, port, path)
+	if err == nil {
 		return false, nil
 	}
-	kport, err := Port(machineName, path)
-	if err != nil {
-		return false, err
-	}
+	glog.Infof("verify returned: %v", err)
+
 	cfg, err := readOrNew(path)
 	if err != nil {
-		return false, errors.Wrap(err, "Error getting kubeconfig status")
+		return false, errors.Wrap(err, "read")
 	}
-	// Safe to lookup server because if field non-existent getIPFromKubeconfig would have given an error
-	cfg.Clusters[machineName].Server = "https://" + ip.String() + ":" + strconv.Itoa(kport)
+
+	cfg.Clusters[contextName].Server = "https://" + hostname + ":" + strconv.Itoa(port)
 	err = writeToFile(cfg, path)
 	if err != nil {
-		return false, err
+		return false, errors.Wrap(err, "write")
 	}
-	// Kubeconfig IP reconfigured
+
 	return true, nil
 }
 

pkg/minikube/kubeconfig/kubeconfig_test.go

@@ -18,7 +18,6 @@ package kubeconfig
 
 import (
 	"io/ioutil"
-	"net"
 	"os"
 	"path/filepath"
 	"strconv"
@@ -30,7 +29,7 @@ import (
 	"k8s.io/client-go/tools/clientcmd"
 )
 
-var fakeKubeCfg = []byte(`
+var kubeConfigWithoutHTTPS = []byte(`
 apiVersion: v1
 clusters:
 - cluster:
@@ -52,7 +51,7 @@ users:
     client-key: /home/la-croix/apiserver.key
 `)
 
-var fakeKubeCfg2 = []byte(`
+var kubeConfig192 = []byte(`
 apiVersion: v1
 clusters:
 - cluster:
@@ -74,12 +73,34 @@ users:
     client-key: /home/la-croix/apiserver.key
 `)
 
-var fakeKubeCfg3 = []byte(`
+var kubeConfigLocalhost = []byte(`
 apiVersion: v1
 clusters:
 - cluster:
     certificate-authority: /home/la-croix/apiserver.crt
-    server: https://192.168.1.1:8443
+    server: https://127.0.0.1:8443
+  name: minikube
+contexts:
+- context:
+    cluster: la-croix
+    user: la-croix
+  name: la-croix
+current-context: la-croix
+kind: Config
+preferences: {}
+users:
+- name: la-croix
+  user:
+    client-certificate: /home/la-croix/apiserver.crt
+    client-key: /home/la-croix/apiserver.key
+`)
+
+var kubeConfigLocalhost12345 = []byte(`
+apiVersion: v1
+clusters:
+- cluster:
+    certificate-authority: /home/la-croix/apiserver.crt
+    server: https://127.0.0.1:12345
   name: minikube
 contexts:
 - context:
@@ -120,7 +141,7 @@ func TestUpdate(t *testing.T) {
 		{
 			description: "add to kube config",
 			cfg:         setupCfg,
-			existingCfg: fakeKubeCfg,
+			existingCfg: kubeConfigWithoutHTTPS,
 		},
 		{
 			description: "use config env var",
@@ -136,7 +157,7 @@ func TestUpdate(t *testing.T) {
 				CertificateAuthority: "/home/apiserver.crt",
 				KeepContext:          true,
 			},
-			existingCfg: fakeKubeCfg,
+			existingCfg: kubeConfigWithoutHTTPS,
 		},
 	}
 
@@ -176,54 +197,72 @@ func TestUpdate(t *testing.T) {
 	}
 }
 
-func TestIsClusterInConfig(t *testing.T) {
+func TestVerifyEndpoint(t *testing.T) {
 
 	var tests = []struct {
 		description string
-		ip          net.IP
+		hostname    string
+		port        int
 		existing    []byte
 		err         bool
 		status      bool
 	}{
 		{
-			description: "empty ip",
-			ip:          nil,
-			existing:    fakeKubeCfg,
+			description: "empty hostname",
+			hostname:    "",
+			port:        8443,
+			existing:    kubeConfigWithoutHTTPS,
 			err:         true,
 		},
 		{
 			description: "no minikube cluster",
-			ip:          net.ParseIP("192.168.10.100"),
-			existing:    fakeKubeCfg,
+			hostname:    "192.168.10.100",
+			port:        8443,
+			existing:    kubeConfigWithoutHTTPS,
 			err:         true,
 		},
 		{
-			description: "exactly matching ip",
-			ip:          net.ParseIP("192.168.10.100"),
-			existing:    fakeKubeCfg2,
+			description: "exactly matching hostname/port",
+			hostname:    "192.168.10.100",
+			port:        8443,
+			existing:    kubeConfig192,
 			status:      true,
 		},
 		{
-			description: "different ips",
-			ip:          net.ParseIP("192.168.10.100"),
-			existing:    fakeKubeCfg3,
+			description: "different hostnames",
+			hostname:    "192.168.10.100",
+			port:        8443,
+			existing:    kubeConfigLocalhost,
+			err:         true,
+		},
+		{
+			description: "different hostname",
+			hostname:    "",
+			port:        8443,
+			existing:    kubeConfigLocalhost,
+			err:         true,
+		},
+		{
+			description: "different ports",
+			hostname:    "127.0.0.1",
+			port:        84430,
+			existing:    kubeConfigLocalhost,
+			err:         true,
 		},
 	}
 
 	for _, test := range tests {
+		test := test
 		t.Run(test.description, func(t *testing.T) {
 			t.Parallel()
 			configFilename := tempFile(t, test.existing)
-			statusActual, err := IsClusterInConfig(test.ip, "minikube", configFilename)
+			err := VerifyEndpoint("minikube", test.hostname, test.port, configFilename)
 			if err != nil && !test.err {
 				t.Errorf("Got unexpected error: %v", err)
 			}
 			if err == nil && test.err {
 				t.Errorf("Expected error but got none: %v", err)
 			}
-			if test.status != statusActual {
-				t.Errorf("Expected status %t, but got %t", test.status, statusActual)
-			}
 		})
 
 	}
@@ -233,45 +272,58 @@ func TestUpdateIP(t *testing.T) {
 
 	var tests = []struct {
 		description string
-		ip          net.IP
+		hostname    string
+		port        int
 		existing    []byte
 		err         bool
 		status      bool
 		expCfg      []byte
 	}{
 		{
-			description: "empty ip",
-			ip:          nil,
-			existing:    fakeKubeCfg2,
+			description: "empty hostname",
+			hostname:    "",
+			port:        8443,
+			existing:    kubeConfig192,
 			err:         true,
-			expCfg:      fakeKubeCfg2,
+			expCfg:      kubeConfig192,
 		},
 		{
 			description: "no minikube cluster",
-			ip:          net.ParseIP("192.168.10.100"),
-			existing:    fakeKubeCfg,
+			hostname:    "192.168.10.100",
+			port:        8080,
+			existing:    kubeConfigWithoutHTTPS,
 			err:         true,
-			expCfg:      fakeKubeCfg,
+			expCfg:      kubeConfigWithoutHTTPS,
 		},
 		{
 			description: "same IP",
-			ip:          net.ParseIP("192.168.10.100"),
-			existing:    fakeKubeCfg2,
-			expCfg:      fakeKubeCfg2,
+			hostname:    "192.168.10.100",
+			port:        8443,
+			existing:    kubeConfig192,
+			expCfg:      kubeConfig192,
 		},
 		{
 			description: "different IP",
-			ip:          net.ParseIP("192.168.10.100"),
-			existing:    fakeKubeCfg3,
+			hostname:    "127.0.0.1",
+			port:        8443,
+			existing:    kubeConfig192,
 			status:      true,
-			expCfg:      fakeKubeCfg2,
+			expCfg:      kubeConfigLocalhost,
+		},
+		{
+			description: "different port",
+			hostname:    "127.0.0.1",
+			port:        12345,
+			existing:    kubeConfigLocalhost,
+			status:      true,
+			expCfg:      kubeConfigLocalhost12345,
 		},
 	}
 	for _, test := range tests {
 		t.Run(test.description, func(t *testing.T) {
 			t.Parallel()
 			configFilename := tempFile(t, test.existing)
-			statusActual, err := UpdateIP(test.ip, "minikube", configFilename)
+			statusActual, err := UpdateEndpoint("minikube", test.hostname, test.port, configFilename)
 			if err != nil && !test.err {
 				t.Errorf("Got unexpected error: %v", err)
 			}
@@ -336,37 +388,42 @@ func TestNewConfig(t *testing.T) {
 	}
 }
 
-func Test_extractIP(t *testing.T) {
+func Test_Endpoint(t *testing.T) {
 
 	var tests = []struct {
 		description string
 		cfg         []byte
-		ip          net.IP
+		hostname    string
+		port        int
 		err         bool
 	}{
 		{
 			description: "normal IP",
-			cfg:         fakeKubeCfg2,
-			ip:          net.ParseIP("192.168.10.100"),
+			cfg:         kubeConfig192,
+			hostname:    "192.168.10.100",
+			port:        8443,
 		},
 		{
 			description: "no minikube cluster",
-			cfg:         fakeKubeCfg,
+			cfg:         kubeConfigWithoutHTTPS,
 			err:         true,
 		},
 	}
 	for _, test := range tests {
 		t.Run(test.description, func(t *testing.T) {
 			configFilename := tempFile(t, test.cfg)
-			ip, err := extractIP("minikube", configFilename)
+			hostname, port, err := Endpoint("minikube", configFilename)
 			if err != nil && !test.err {
 				t.Errorf("Got unexpected error: %v", err)
 			}
 			if err == nil && test.err {
 				t.Errorf("Expected error but got none: %v", err)
 			}
-			if !ip.Equal(test.ip) {
-				t.Errorf("IP returned: %s does not match ip given: %s", ip, test.ip)
+			if hostname != test.hostname {
+				t.Errorf("got hostname = %q, want hostname = %q", hostname, test.hostname)
+			}
+			if port != test.port {
+				t.Errorf("got port = %q, want port = %q", port, test.port)
 			}
 		})
 	}

pkg/minikube/mustload/mustload.go

@@ -26,7 +26,6 @@ import (
 	"github.com/docker/machine/libmachine/host"
 	"github.com/docker/machine/libmachine/state"
 	"github.com/golang/glog"
-	"k8s.io/minikube/pkg/drivers/kic/oci"
 	"k8s.io/minikube/pkg/minikube/bootstrapper/bsutil/kverify"
 	"k8s.io/minikube/pkg/minikube/command"
 	"k8s.io/minikube/pkg/minikube/config"
@@ -45,11 +44,18 @@ type ClusterController struct {
 }
 
 type ControlPlane struct {
+	// Host is the libmachine host object
 	Host *host.Host
+	// Node is our internal control object
 	Node *config.Node
+	// Runner provides command execution
 	Runner command.Runner
-	ForwardedIP   net.IP
-	ForwardedPort int
+	// Hostname is the host-accesible target for the apiserver
+	Hostname string
+	// Port is the host-accessible port for the apiserver
+	Port int
+	// IP is the host-accessible IP for the control plane
+	IP net.IP
 }
 
 // Partial is a cmd-friendly way to load a cluster which may or may not be running
@@ -112,24 +118,9 @@ func Running(name string) ClusterController {
 		exit.WithError("Unable to get command runner", err)
 	}
 
-	ipStr, err := host.Driver.GetIP()
+	hostname, ip, port, err := driver.ControlPaneEndpoint(cc, &cp, host.DriverName)
 	if err != nil {
-		exit.WithError("Unable to get driver IP", err)
-	}
-
-	ip := net.ParseIP(ipStr)
-	if ip == nil {
-		exit.WithCodeT(exit.Software, fmt.Sprintf("Unable to parse driver IP: %q", ipStr))
-	}
-
-	cpIP := cp.IP
-	cpPort := cp.Port
-	if driver.IsKIC(host.DriverName) {
-		cpIP = oci.DefaultBindIPV4
-		cpPort, err = oci.ForwardedPort(cc.Driver, cc.Name, cp.Port)
-		if err != nil {
-			exit.WithError("Unable to get forwarded port", err)
-		}
+		exit.WithError("Unable to get forwarded endpoint", err)
 	}
 
 	return ClusterController{
@@ -139,8 +130,9 @@ func Running(name string) ClusterController {
 			Runner:   cr,
 			Host:     host,
 			Node:     &cp,
-			ForwardedIP:   net.ParseIP(cpIP),
-			ForwardedPort: cpPort,
+			Hostname: hostname,
+			IP:       ip,
+			Port:     port,
 		},
 	}
 }
@@ -149,7 +141,7 @@ func Running(name string) ClusterController {
 func Healthy(name string) ClusterController {
 	co := Running(name)
 
-	as, err := kverify.APIServerStatus(co.CP.Runner, co.CP.ForwardedIP, co.CP.ForwardedPort)
+	as, err := kverify.APIServerStatus(co.CP.Runner, co.CP.Hostname, co.CP.Port)
 	if err != nil {
 		out.T(out.FailureType, `Unable to get control plane status: {{.error}}`, out.V{"error": err})
 		exitTip("delete", name, exit.Unavailable)

pkg/minikube/node/start.go

@@ -34,7 +34,6 @@ import (
 	"golang.org/x/sync/errgroup"
 	cmdcfg "k8s.io/minikube/cmd/minikube/cmd/config"
 	"k8s.io/minikube/pkg/addons"
-	"k8s.io/minikube/pkg/drivers/kic/oci"
 	"k8s.io/minikube/pkg/minikube/bootstrapper"
 	"k8s.io/minikube/pkg/minikube/bootstrapper/images"
 	"k8s.io/minikube/pkg/minikube/cluster"
@@ -273,26 +272,9 @@ func setupKubeconfig(h *host.Host, cc *config.ClusterConfig, n *config.Node, clu
 }
 
 func apiServerURL(h host.Host, cc config.ClusterConfig, n config.Node) (string, error) {
-	hostname := ""
-	port := n.Port
-	var err error
-	if driver.IsKIC(h.DriverName) {
-		// for kic drivers we use 127.0.0.1 instead of node IP,
-		// because of Docker on MacOs limitations for reaching to container's IP.
-		hostname = oci.DefaultBindIPV4
-		port, err = oci.ForwardedPort(h.DriverName, h.Name, port)
+	hostname, _, port, err := driver.ControlPaneEndpoint(&cc, &n, h.DriverName)
 	if err != nil {
-			return "", errors.Wrap(err, "host port binding")
-		}
-	} else {
-		hostname, err = h.Driver.GetIP()
-		if err != nil {
-			return "", errors.Wrap(err, "get ip")
-		}
-	}
-
-	if cc.KubernetesConfig.APIServerName != constants.APIServerName {
-		hostname = cc.KubernetesConfig.APIServerName
+		return "", err
 	}
 	return fmt.Sprintf("https://" + net.JoinHostPort(hostname, strconv.Itoa(port))), nil
 }

test/integration/addons_test.go

@@ -173,6 +173,10 @@ func validateRegistryAddon(ctx context.Context, t *testing.T, profile string) {
 		t.Errorf("expected curl response be %q, but got *%s*", want, rr.Stdout.String())
 	}
 
+	if NeedsPortForward() {
+		t.Skipf("Unable to complete rest of the test due to connectivity assumptions")
+	}
+
 	// Test from outside the cluster
 	rr, err = Run(t, exec.CommandContext(ctx, Target(), "-p", profile, "ip"))
 	if err != nil {

test/integration/cert_options_test.go

+// +build integration
+
+/*
+Copyright 2016 The Kubernetes Authors All rights reserved.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package integration
+
+import (
+	"context"
+	"os/exec"
+	"testing"
+)
+
+func TestCertOptions(t *testing.T) {
+	if NoneDriver() {
+		t.Skip("skipping: none driver does not support ssh or bundle docker")
+	}
+	MaybeParallel(t)
+
+	profile := UniqueProfileName("cert-options")
+	ctx, cancel := context.WithTimeout(context.Background(), Minutes(30))
+	defer CleanupWithLogs(t, profile, cancel)
+
+	// Use the most verbose logging for the simplest test. If it fails, something is very wrong.
+	args := append([]string{"start", "-p", profile, "--memory=1900", "--apiserver-ips=127.0.0.1,192.168.15.15", "--apiserver-names=localhost,www.google.com", "--apiserver-port=8555"}, StartArgs()...)
+
+	// We can safely override --apiserver-name with
+	if NeedsPortForward() {
+		args = append(args, "--apiserver-name=localhost")
+	}
+
+	rr, err := Run(t, exec.CommandContext(ctx, Target(), args...))
+	if err != nil {
+		t.Errorf("failed to start minikube with args: %q : %v", rr.Command(), err)
+	}
+
+	// test that file written from host was read in by the pod via cat /mount-9p/written-by-host;
+	rr, err = Run(t, exec.CommandContext(ctx, "kubectl", "--context", profile, "version"))
+	if err != nil {
+		t.Errorf("failed to get kubectl version. args %q : %v", rr.Command(), err)
+	}
+
+}

test/integration/functional_test.go

@@ -335,8 +335,9 @@ func validateDashboardCmd(ctx context.Context, t *testing.T, profile string) {
 
 	resp, err := retryablehttp.Get(u.String())
 	if err != nil {
-		t.Fatalf("failed to http get %q : %v", u.String(), err)
+		t.Fatalf("failed to http get %q: %v\nresponse: %+v", u.String(), err, resp)
 	}
+
 	if resp.StatusCode != http.StatusOK {
 		body, err := ioutil.ReadAll(resp.Body)
 		if err != nil {
@@ -617,6 +618,10 @@ func validateServiceCmd(ctx context.Context, t *testing.T, profile string) {
 		t.Errorf("expected 'service list' to contain *hello-node* but got -%q-", rr.Stdout.String())
 	}
 
+	if NeedsPortForward() {
+		t.Skipf("test is broken for port-forwarded drivers: https://github.com/kubernetes/minikube/issues/7383")
+	}
+
 	// Test --https --url mode
 	rr, err = Run(t, exec.CommandContext(ctx, Target(), "-p", profile, "service", "--namespace=default", "--https", "--url", "hello-node"))
 	if err != nil {
@@ -831,7 +836,7 @@ func validateUpdateContextCmd(ctx context.Context, t *testing.T, profile string)
 		t.Errorf("failed to run minikube update-context: args %q: %v", rr.Command(), err)
 	}
 
-	want := []byte("IP was already correctly configured")
+	want := []byte("No changes")
 	if !bytes.Contains(rr.Stdout.Bytes(), want) {
 		t.Errorf("update-context: got=%q, want=*%q*", rr.Stdout.Bytes(), want)
 	}

test/integration/main.go

@@ -20,6 +20,7 @@ import (
 	"flag"
 	"fmt"
 	"os"
+	"runtime"
 	"strings"
 	"testing"
 	"time"
@@ -73,6 +74,12 @@ func KicDriver() bool {
 	return strings.Contains(*startArgs, "--driver=docker") || strings.Contains(*startArgs, "--vm-driver=docker") || strings.Contains(*startArgs, "--vm-driver=podman") || strings.Contains(*startArgs, "driver=podman")
 }
 
+// NeedsPortForward returns access to endpoints with this driver needs port forwarding
+// (Docker on non-Linux platforms requires ports to be forwarded to 127.0.0.1)
+func NeedsPortForward() bool {
+	return KicDriver() && (runtime.GOOS == "windows" || runtime.GOOS == "darwin")
+}
+
 // CanCleanup returns if cleanup is allowed
 func CanCleanup() bool {
 	return *cleanup