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,18 +218,33 @@ 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)
-		glog.Infof("%s apiserver status = %s (err=%v)", name, stk, err)
+	// 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.Errorln("Error apiserver status:", err)
-			st.APIServer = state.Error.String()
-		} else {
-			st.APIServer = sta.String()
+			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 {
+		glog.Errorln("Error apiserver status:", err)
+		st.APIServer = state.Error.String()
+	} 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,34 +267,34 @@ 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())
 	glog.Infof("apiserver freezer: %q", freezer)
 	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())
 	glog.Infof("freezer state: %q", fs)
 	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)