Merge pull request #6885 from medyagh/docker_env_restart

warn to update docker-env on restart

pkg/drivers/kic/kic.go

@@ -136,10 +136,10 @@ func (d *Driver) prepareSSH() error {
 
 // DriverName returns the name of the driver
 func (d *Driver) DriverName() string {
-	if d.NodeConfig.OCIBinary == "podman" {
-		return "podman"
+	if d.NodeConfig.OCIBinary == oci.Podman {
+		return oci.Podman
 	}
-	return "docker"
+	return oci.Docker
 }
 
 // GetIP returns an IP or hostname that this host is available at
@@ -227,6 +227,9 @@ func (d *Driver) GetState() (state.State, error) {
 
 // Kill stops a host forcefully, including any containers that we are managing.
 func (d *Driver) Kill() error {
+	if err := oci.PointToHostDockerDaemon(); err != nil {
+		return errors.Wrap(err, "point host docker daemon")
+	}
 	cmd := exec.Command(d.NodeConfig.OCIBinary, "kill", d.MachineName)
 	if err := cmd.Run(); err != nil {
 		return errors.Wrapf(err, "killing kic node %s", d.MachineName)
@@ -236,6 +239,10 @@ func (d *Driver) Kill() error {
 
 // Remove will delete the Kic Node Container
 func (d *Driver) Remove() error {
+	if err := oci.PointToHostDockerDaemon(); err != nil {
+		return errors.Wrap(err, "point host docker daemon")
+	}
+
 	if _, err := oci.ContainerID(d.OCIBinary, d.MachineName); err != nil {
 		log.Warnf("could not find the container %s to remove it.", d.MachineName)
 	}
@@ -253,13 +260,14 @@ func (d *Driver) Remove() error {
 
 // Restart a host
 func (d *Driver) Restart() error {
+	if err := oci.PointToHostDockerDaemon(); err != nil {
+		return errors.Wrap(err, "point host docker daemon")
+	}
 	s, err := d.GetState()
 	if err != nil {
 		return errors.Wrap(err, "get kic state")
 	}
 	switch s {
-	case state.Paused:
-		return d.Unpause()
 	case state.Stopped:
 		return d.Start()
 	case state.Running, state.Error:
@@ -275,18 +283,12 @@ func (d *Driver) Restart() error {
 	return fmt.Errorf("restarted not implemented for kic state %s yet", s)
 }
 
-// Unpause a kic container
-func (d *Driver) Unpause() error {
-	cmd := exec.Command(d.NodeConfig.OCIBinary, "unpause", d.MachineName)
-	if err := cmd.Run(); err != nil {
-		return errors.Wrapf(err, "unpausing %s", d.MachineName)
-	}
-	return nil
-}
-
 // Start a _stopped_ kic container
 // not meant to be used for Create().
 func (d *Driver) Start() error {
+	if err := oci.PointToHostDockerDaemon(); err != nil {
+		return errors.Wrap(err, "point host docker daemon")
+	}
 	s, err := d.GetState()
 	if err != nil {
 		return errors.Wrap(err, "get kic state")
@@ -304,6 +306,9 @@ func (d *Driver) Start() error {
 
 // Stop a host gracefully, including any containers that we are managing.
 func (d *Driver) Stop() error {
+	if err := oci.PointToHostDockerDaemon(); err != nil {
+		return errors.Wrap(err, "point host docker daemon")
+	}
 	cmd := exec.Command(d.NodeConfig.OCIBinary, "stop", d.MachineName)
 	if err := cmd.Run(); err != nil {
 		return errors.Wrapf(err, "stopping %s", d.MachineName)

pkg/drivers/kic/oci/network.go

@@ -21,6 +21,7 @@ import (
 	"net"
 	"os/exec"
 	"runtime"
+	"strconv"
 	"strings"
 
 	"github.com/golang/glog"
@@ -77,3 +78,82 @@ func dockerGatewayIP() (net.IP, error) {
 	glog.Infof("got host ip for mount in container by inspect docker network: %s", ip.String())
 	return ip, nil
 }
+
+// HostPortBinding will return port mapping for a container using cli.
+// example : HostPortBinding("docker", "minikube", "22")
+// will return the docker assigned port:
+// 32769, nil
+// only supports TCP ports
+func HostPortBinding(ociBinary string, ociID string, contPort int) (int, error) {
+	if err := PointToHostDockerDaemon(); err != nil {
+		return 0, errors.Wrap(err, "point host docker daemon")
+	}
+	var out []byte
+	var err error
+	if ociBinary == Podman {
+		//podman inspect -f "{{range .NetworkSettings.Ports}}{{if eq .ContainerPort "80"}}{{.HostPort}}{{end}}{{end}}"
+		cmd := exec.Command(ociBinary, "inspect", "-f", fmt.Sprintf("{{range .NetworkSettings.Ports}}{{if eq .ContainerPort %s}}{{.HostPort}}{{end}}{{end}}", fmt.Sprint(contPort)), ociID)
+		out, err = cmd.CombinedOutput()
+		if err != nil {
+			return 0, errors.Wrapf(err, "get host-bind port %d for %q, output %s", contPort, ociID, out)
+		}
+	} else {
+		cmd := exec.Command(ociBinary, "inspect", "-f", fmt.Sprintf("'{{(index (index .NetworkSettings.Ports \"%d/tcp\") 0).HostPort}}'", contPort), ociID)
+		out, err = cmd.CombinedOutput()
+		if err != nil {
+			return 0, errors.Wrapf(err, "get host-bind port %d for %q, output %s", contPort, ociID, out)
+		}
+	}
+
+	o := strings.TrimSpace(string(out))
+	o = strings.Trim(o, "'")
+	p, err := strconv.Atoi(o)
+	if err != nil {
+		return p, errors.Wrapf(err, "convert host-port %q to number", p)
+	}
+	return p, nil
+}
+
+// ContainerIPs returns ipv4,ipv6, error of a container by their name
+func ContainerIPs(ociBinary string, name string) (string, string, error) {
+	if ociBinary == Podman {
+		return podmanConttainerIP(name)
+	}
+	return dockerContainerIP(name)
+}
+
+// podmanConttainerIP returns ipv4, ipv6 of container or error
+func podmanConttainerIP(name string) (string, string, error) {
+	cmd := exec.Command(Podman, "inspect",
+		"-f", "{{.NetworkSettings.IPAddress}}",
+		name)
+	out, err := cmd.CombinedOutput()
+	if err != nil {
+		return "", "", errors.Wrapf(err, "podman inspect ip %s", name)
+	}
+	output := strings.TrimSpace(string(out))
+	if err == nil && output == "" { // podman returns empty for 127.0.0.1
+		return DefaultBindIPV4, "", nil
+	}
+	return output, "", nil
+}
+
+// dockerContainerIP returns ipv4, ipv6 of container or error
+func dockerContainerIP(name string) (string, string, error) {
+	if err := PointToHostDockerDaemon(); err != nil {
+		return "", "", errors.Wrap(err, "point host docker daemon")
+	}
+	// retrieve the IP address of the node using docker inspect
+	lines, err := inspect(Docker, name, "{{range .NetworkSettings.Networks}}{{.IPAddress}},{{.GlobalIPv6Address}}{{end}}")
+	if err != nil {
+		return "", "", errors.Wrap(err, "inspecting NetworkSettings.Networks")
+	}
+	if len(lines) != 1 {
+		return "", "", errors.Errorf("IPs output should only be one line, got %d lines", len(lines))
+	}
+	ips := strings.Split(lines[0], ",")
+	if len(ips) != 2 {
+		return "", "", errors.Errorf("container addresses should have 2 values, got %d values: %+v", len(ips), ips)
+	}
+	return ips[0], ips[1], nil
+}

pkg/drivers/kic/oci/oci.go

@@ -20,7 +20,6 @@ import (
 	"context"
 	"os"
 	"path/filepath"
-	"strconv"
 	"time"
 
 	"bufio"
@@ -210,85 +209,6 @@ func Copy(ociBinary string, ociID string, targetDir string, fName string) error
 	return nil
 }
 
-// HostPortBinding will return port mapping for a container using cli.
-// example : HostPortBinding("docker", "minikube", "22")
-// will return the docker assigned port:
-// 32769, nil
-// only supports TCP ports
-func HostPortBinding(ociBinary string, ociID string, contPort int) (int, error) {
-	if err := PointToHostDockerDaemon(); err != nil {
-		return 0, errors.Wrap(err, "point host docker daemon")
-	}
-	var out []byte
-	var err error
-	if ociBinary == Podman {
-		//podman inspect -f "{{range .NetworkSettings.Ports}}{{if eq .ContainerPort "80"}}{{.HostPort}}{{end}}{{end}}"
-		cmd := exec.Command(ociBinary, "inspect", "-f", fmt.Sprintf("{{range .NetworkSettings.Ports}}{{if eq .ContainerPort %s}}{{.HostPort}}{{end}}{{end}}", fmt.Sprint(contPort)), ociID)
-		out, err = cmd.CombinedOutput()
-		if err != nil {
-			return 0, errors.Wrapf(err, "get host-bind port %d for %q, output %s", contPort, ociID, out)
-		}
-	} else {
-		cmd := exec.Command(ociBinary, "inspect", "-f", fmt.Sprintf("'{{(index (index .NetworkSettings.Ports \"%d/tcp\") 0).HostPort}}'", contPort), ociID)
-		out, err = cmd.CombinedOutput()
-		if err != nil {
-			return 0, errors.Wrapf(err, "get host-bind port %d for %q, output %s", contPort, ociID, out)
-		}
-	}
-
-	o := strings.TrimSpace(string(out))
-	o = strings.Trim(o, "'")
-	p, err := strconv.Atoi(o)
-	if err != nil {
-		return p, errors.Wrapf(err, "convert host-port %q to number", p)
-	}
-	return p, nil
-}
-
-// ContainerIPs returns ipv4,ipv6, error of a container by their name
-func ContainerIPs(ociBinary string, name string) (string, string, error) {
-	if ociBinary == Podman {
-		return podmanConttainerIP(name)
-	}
-	return dockerContainerIP(name)
-}
-
-// podmanConttainerIP returns ipv4, ipv6 of container or error
-func podmanConttainerIP(name string) (string, string, error) {
-	cmd := exec.Command(Podman, "inspect",
-		"-f", "{{.NetworkSettings.IPAddress}}",
-		name)
-	out, err := cmd.CombinedOutput()
-	if err != nil {
-		return "", "", errors.Wrapf(err, "podman inspect ip %s", name)
-	}
-	output := strings.TrimSpace(string(out))
-	if err == nil && output == "" { // podman returns empty for 127.0.0.1
-		return DefaultBindIPV4, "", nil
-	}
-	return output, "", nil
-}
-
-// dockerContainerIP returns ipv4, ipv6 of container or error
-func dockerContainerIP(name string) (string, string, error) {
-	if err := PointToHostDockerDaemon(); err != nil {
-		return "", "", errors.Wrap(err, "point host docker daemon")
-	}
-	// retrieve the IP address of the node using docker inspect
-	lines, err := inspect(Docker, name, "{{range .NetworkSettings.Networks}}{{.IPAddress}},{{.GlobalIPv6Address}}{{end}}")
-	if err != nil {
-		return "", "", errors.Wrap(err, "inspecting NetworkSettings.Networks")
-	}
-	if len(lines) != 1 {
-		return "", "", errors.Errorf("IPs output should only be one line, got %d lines", len(lines))
-	}
-	ips := strings.Split(lines[0], ",")
-	if len(ips) != 2 {
-		return "", "", errors.Errorf("container addresses should have 2 values, got %d values: %+v", len(ips), ips)
-	}
-	return ips[0], ips[1], nil
-}
-
 // ContainerID returns id of a container name
 func ContainerID(ociBinary string, nameOrID string) (string, error) {
 	if err := PointToHostDockerDaemon(); err != nil {
@@ -461,7 +381,7 @@ func listContainersByLabel(ociBinary string, label string) ([]string, error) {
 func PointToHostDockerDaemon() error {
 	p := os.Getenv(constants.MinikubeActiveDockerdEnv)
 	if p != "" {
-		glog.Infof("shell is pointing to docker inside minikube. will unset to use host")
+		glog.Infof("shell is pointing to dockerd inside minikube. will unset to use host")
 	}
 
 	for i := range constants.DockerDaemonEnvs {

pkg/minikube/driver/driver.go

@@ -85,7 +85,7 @@ func Supported(name string) bool {
 	return false
 }
 
-// IsKIC checks if the driver is a kubernetes in continer
+// IsKIC checks if the driver is a kubernetes in container
 func IsKIC(name string) bool {
 	return name == Docker || name == Podman
 }

pkg/minikube/machine/fix.go

@@ -19,6 +19,7 @@ package machine
 import (
 	"fmt"
 	"math"
+	"os"
 	"strconv"
 	"strings"
 	"time"
@@ -32,6 +33,7 @@ import (
 	"github.com/golang/glog"
 	"github.com/pkg/errors"
 	"k8s.io/minikube/pkg/minikube/config"
+	"k8s.io/minikube/pkg/minikube/constants"
 	"k8s.io/minikube/pkg/minikube/driver"
 	"k8s.io/minikube/pkg/minikube/out"
 	"k8s.io/minikube/pkg/util/retry"
@@ -68,6 +70,9 @@ func fixHost(api libmachine.API, mc config.ClusterConfig) (*host.Host, error) {
 		return h, errors.Wrap(err, "Error loading existing host. Please try running [minikube delete], then run [minikube start] again.")
 	}
 
+	// check if need to re-run docker-env
+	maybeWarnAboutEvalEnv(mc.Driver, mc.Name)
+
 	s, err := h.Driver.GetState()
 	if err != nil || s == state.Stopped || s == state.None {
 		// If virtual machine does not exist due to user interrupt cancel(i.e. Ctrl + C), recreate virtual machine
@@ -145,6 +150,26 @@ func fixHost(api libmachine.API, mc config.ClusterConfig) (*host.Host, error) {
 	return h, ensureSyncedGuestClock(h, mc.Driver)
 }
 
+// maybeWarnAboutEvalEnv wil warn user if they need to re-eval their docker-env, podman-env
+// because docker changes the allocated bind ports after restart https://github.com/kubernetes/minikube/issues/6824
+func maybeWarnAboutEvalEnv(drver string, name string) {
+	if !driver.IsKIC(drver) {
+		return
+	}
+	p := os.Getenv(constants.MinikubeActiveDockerdEnv)
+	if p == "" {
+		return
+	}
+	out.T(out.Notice, "Noticed that you are using minikube docker-env:")
+	out.T(out.WarningType, `After minikube restart the dockerd ports might have changed. To ensure docker-env works properly.
+Please re-eval the docker-env command:
+
+	'minikube -p {{.profile_name}} docker-env'
+
+	`, out.V{"profile_name": name})
+
+}
+
 // ensureGuestClockSync ensures that the guest system clock is relatively in-sync
 func ensureSyncedGuestClock(h hostRunner, drv string) error {
 	if !driver.IsVM(drv) {

pkg/minikube/registry/drvs/docker/docker.go

@@ -63,6 +63,10 @@ func status() registry.State {
 		return registry.State{Error: err, Installed: false, Healthy: false, Fix: "Docker is required.", Doc: "https://minikube.sigs.k8s.io/docs/reference/drivers/docker/"}
 	}
 
+	if err := oci.PointToHostDockerDaemon(); err != nil {
+		return registry.State{Error: err, Installed: true, Healthy: false, Fix: "Failed to point to dockerd. Please make sure DOCKER_HOST environment variable is pointing to your installed dockerd."}
+	}
+
 	// Allow no more than 3 seconds for docker info
 	ctx, cancel := context.WithTimeout(context.Background(), 3*time.Second)
 	defer cancel()