From 9cd42163b1679e9cf38f2816a40c92cd5b6ddaef Mon Sep 17 00:00:00 2001 From: Medya Gh Date: Mon, 21 Sep 2020 15:56:08 -0700 Subject: [PATCH 01/13] add network create funcs --- pkg/drivers/kic/oci/errors.go | 15 ++++ pkg/drivers/kic/oci/network_create.go | 125 ++++++++++++++++++++++++++ 2 files changed, 140 insertions(+) create mode 100644 pkg/drivers/kic/oci/network_create.go diff --git a/pkg/drivers/kic/oci/errors.go b/pkg/drivers/kic/oci/errors.go index 8c30201f978c..0889358bc0e9 100644 --- a/pkg/drivers/kic/oci/errors.go +++ b/pkg/drivers/kic/oci/errors.go @@ -39,12 +39,27 @@ var ErrWindowsContainers = &FailFastError{errors.New("docker container type is w // ErrCPUCountLimit is thrown when docker daemon doesn't have enough CPUs for the requested container var ErrCPUCountLimit = &FailFastError{errors.New("not enough CPUs is available for container")} +// ErrIPinUse is thrown when the container been given an IP used by another container +var ErrIPinUse = &FailFastError{errors.New("can't create with that IP, address already in use")} + // ErrExitedUnexpectedly is thrown when container is created/started without error but later it exists and it's status is not running anymore. var ErrExitedUnexpectedly = errors.New("container exited unexpectedly") // ErrDaemonInfo is thrown when docker/podman info is failing or not responding var ErrDaemonInfo = errors.New("daemon info not responding") +// ErrNetworkSubnetTaken is thrown when a subnet is taken by another network +var ErrNetworkSubnetTaken = errors.New("subnet is taken") + +// ErrNetworkNotFound is when given network was not found +var ErrNetworkNotFound = errors.New("kic network not found") + +// ErrNetworkGatewayTaken is when given network gatway is taken +var ErrNetworkGatewayTaken = errors.New("network gateway is taken") + +// ErrNetworkInUse is when trying to delete a network which is attached to another container +var ErrNetworkInUse = errors.New("can't delete network attached to a running container") + // LogContainerDebug will print relevant docker/podman infos after a container fails func LogContainerDebug(ociBin string, name string) string { rr, err := containerInspect(ociBin, name) diff --git a/pkg/drivers/kic/oci/network_create.go b/pkg/drivers/kic/oci/network_create.go new file mode 100644 index 000000000000..3df9660814a8 --- /dev/null +++ b/pkg/drivers/kic/oci/network_create.go @@ -0,0 +1,125 @@ +/* +Copyright 2019 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 oci + +import ( + "fmt" + "net" + "os/exec" + "strings" + + "github.com/golang/glog" + "github.com/pkg/errors" +) + +// DefaultSubnet subnet to be used on first cluster +const defaultSubnet = "192.168.39.0/24" + +// CreateNetwork creates a network returns gateway and error, minikube creates one network per cluster +func CreateNetwork(ociBin string, name string) (net.IP, error) { + if ociBin != Docker { + return nil, fmt.Errorf("%s network not implemented yet", ociBin) + } + return createDockerNetwork(name) +} + +func createDockerNetwork(name string) (net.IP, error) { + // check if the network already exists + subnet, gateway, err := dockerNetworkInspect(name) + if err == nil { + glog.Infof("Found existing network with subnet %s and gateway %s.", subnet, gateway) + return gateway, nil + } + // simple way to create networks, subnet is taken, try one bigger + attempt := 0 + _, subnet, err = net.ParseCIDR(defaultSubnet) + if err != nil { + return nil, errors.Wrapf(err, "parse default subnet %s", defaultSubnet) + } + + gateway, err = tryCreateDockerNetwork(subnet, name) + if err != nil { + if err != ErrNetworkSubnetTaken { + return nil, errors.Wrapf(err, "error creating network") + } + // try up to 13 times + // we can try up to 255 + for attempt < 13 { + attempt++ + glog.Infof("Couldn't create network %q at %q subnet will try again with a new subnet ...", name, subnet) + // increase 3nd digit by 10 each time + // 13 times adding 10 defaultSubnet "192.168.39.0/24" + // at most it will add up to 169 which is still less than max allowed 255 + // this is large enough to try more and not too small to not try enough + // can be tuned in the next iterations + subnet.IP.To4()[2] += 10 + gateway, err := tryCreateDockerNetwork(subnet, name) + if err == nil { + return gateway, nil + } + if err == ErrNetworkSubnetTaken { + continue + } + } + + } + return gateway, nil +} + +func tryCreateDockerNetwork(subnet *net.IPNet, name string) (net.IP, error) { + gateway := subnet.IP.To4() + gateway[3]++ // first ip for gateway + glog.Infof("attempt to create network %q with subnet: %s and gateway %s...", subnet, name, gateway) + // options documentation https://docs.docker.com/engine/reference/commandline/network_create/#bridge-driver-options + rr, err := runCmd(exec.Command(Docker, "network", "create", "--driver=bridge", fmt.Sprintf("--subnet=%s", subnet), fmt.Sprintf("--gateway=%s", gateway), "-o", "--ip-masq", "-o", "--icc", fmt.Sprintf("--label=%s=%s", CreatedByLabelKey, "true"), name)) + if err != nil { + if strings.Contains(rr.Output(), "Pool overlaps with other one on this address space") { + return nil, ErrNetworkSubnetTaken + } + if strings.Contains(rr.Output(), "failed to allocate gateway") && strings.Contains(rr.Output(), "Address already in use") { + return nil, ErrNetworkGatewayTaken + } + return nil, errors.Wrapf(err, "error creating network") + } + return gateway, nil +} + +// returns subnet and gate if exists +func dockerNetworkInspect(name string) (*net.IPNet, net.IP, error) { + rr, err := runCmd(exec.Command(Docker, "network", "inspect", name, "--format", "{{(index .IPAM.Config 0).Subnet}},{{(index .IPAM.Config 0).Gateway}}")) + if err != nil { + if strings.Contains(rr.Output(), "No such network:") { + return nil, nil, ErrNetworkNotFound + } + return nil, nil, err + } + // results looks like 172.17.0.0/16,172.17.0.1 + ips := strings.Split(strings.TrimSpace(rr.Stdout.String()), ",") + if len(ips) == 0 { + return nil, nil, fmt.Errorf("invalid network info") + } + + _, subnet, err := net.ParseCIDR(ips[0]) + if err != nil { + return nil, nil, errors.Wrapf(err, "parse subnet for %s", name) + } + var gateway net.IP + if len(ips) > 0 { + gateway = net.ParseIP(ips[1]) + } + return subnet, gateway, nil +} From f0f10d6135d7b98951369af6a2e4b37fb8b57926 Mon Sep 17 00:00:00 2001 From: Medya Gh Date: Mon, 21 Sep 2020 17:26:55 -0700 Subject: [PATCH 02/13] first version --- cmd/minikube/cmd/delete.go | 5 ++ cmd/minikube/cmd/mount.go | 2 +- hack/preload-images/generate.go | 1 + pkg/drivers/kic/kic.go | 32 ++++++++++ pkg/drivers/kic/kic_test.go | 59 +++++++++++++++++ pkg/drivers/kic/oci/network.go | 64 +++---------------- pkg/drivers/kic/oci/network_create.go | 71 +++++++++++++++++++++ pkg/drivers/kic/oci/oci.go | 9 +++ pkg/drivers/kic/oci/types.go | 3 + pkg/drivers/kic/types.go | 1 + pkg/minikube/cluster/ip.go | 6 +- pkg/minikube/node/start.go | 2 +- pkg/minikube/registry/drvs/docker/docker.go | 1 + pkg/minikube/registry/drvs/podman/podman.go | 1 + 14 files changed, 198 insertions(+), 59 deletions(-) create mode 100644 pkg/drivers/kic/kic_test.go diff --git a/cmd/minikube/cmd/delete.go b/cmd/minikube/cmd/delete.go index 6b50e4631620..b8ac93a56891 100644 --- a/cmd/minikube/cmd/delete.go +++ b/cmd/minikube/cmd/delete.go @@ -260,6 +260,11 @@ func deletePossibleKicLeftOver(cname string, driverName string) { glog.Warningf("error deleting volumes (might be okay).\nTo see the list of volumes run: 'docker volume ls'\n:%v", errs) } + errs = oci.DeleteAllNetworksByKIC() + if errs != nil { + glog.Warningf("error deleting left over networks (might be okay).\nTo see the list of netowrks: 'docker network ls'\n:%v", errs) + } + if bin == oci.Podman { // podman prune does not support --filter return diff --git a/cmd/minikube/cmd/mount.go b/cmd/minikube/cmd/mount.go index 8d268d1cc28f..f59b3ca4a781 100644 --- a/cmd/minikube/cmd/mount.go +++ b/cmd/minikube/cmd/mount.go @@ -110,7 +110,7 @@ var mountCmd = &cobra.Command{ var ip net.IP var err error if mountIP == "" { - ip, err = cluster.HostIP(co.CP.Host) + ip, err = cluster.HostIP(co.CP.Host, co.Config.Name) if err != nil { exit.Error(reason.IfHostIP, "Error getting the host IP address to use from within the VM", err) } diff --git a/hack/preload-images/generate.go b/hack/preload-images/generate.go index ec3de2ece506..e1d6061d8bd6 100644 --- a/hack/preload-images/generate.go +++ b/hack/preload-images/generate.go @@ -39,6 +39,7 @@ import ( func generateTarball(kubernetesVersion, containerRuntime, tarballFilename string) error { driver := kic.NewDriver(kic.Config{ + ClusterName: profile, KubernetesVersion: kubernetesVersion, ContainerRuntime: containerRuntime, OCIBinary: oci.Docker, diff --git a/pkg/drivers/kic/kic.go b/pkg/drivers/kic/kic.go index e296b327b83f..d63dfcd94343 100644 --- a/pkg/drivers/kic/kic.go +++ b/pkg/drivers/kic/kic.go @@ -37,6 +37,7 @@ import ( "k8s.io/minikube/pkg/minikube/constants" "k8s.io/minikube/pkg/minikube/cruntime" "k8s.io/minikube/pkg/minikube/download" + "k8s.io/minikube/pkg/minikube/out" "k8s.io/minikube/pkg/minikube/sysinit" "k8s.io/minikube/pkg/util/retry" ) @@ -65,6 +66,18 @@ func NewDriver(c Config) *Driver { return d } +// machineOrder returns the order of the container based on it is name +func machineOrder(machineName string) int { + // minikube-m02 + sp := strings.Split(machineName, "-") + m := strings.Trim(sp[len(sp)-1], "m") // m02 + i, err := strconv.Atoi(m) + if err != nil { + return 1 + } + return i +} + // Create a host using the driver's config func (d *Driver) Create() error { params := oci.CreateParams{ @@ -81,6 +94,20 @@ func (d *Driver) Create() error { APIServerPort: d.NodeConfig.APIServerPort, } + // one network bridge per cluster. + defaultNetwork := d.NodeConfig.ClusterName + if gateway, err := oci.CreateNetwork(d.OCIBinary, defaultNetwork); err != nil { + glog.Warningf("failed to create network: %v", err) + out.WarningT("Unable to create dedicated network, This might result in cluster IP change after restart.") + } else { + params.Network = defaultNetwork + ip := gateway.To4() + // calculate the container IP based on its machine order + ip[3] += byte(machineOrder(d.NodeConfig.MachineName)) + glog.Infof("calculated static IP %q for the %q container ", ip.String(), d.NodeConfig.MachineName) + params.IP = ip.String() + } + // control plane specific options params.PortMappings = append(params.PortMappings, oci.PortMapping{ ListenAddress: oci.DefaultBindIPV4, @@ -289,6 +316,11 @@ func (d *Driver) Remove() error { if id, err := oci.ContainerID(d.OCIBinary, d.MachineName); err == nil && id != "" { return fmt.Errorf("expected no container ID be found for %q after delete. but got %q", d.MachineName, id) } + + if err := oci.RemoveNetwork(d.NodeConfig.ClusterName); err != nil { + //TODO: Ingore error if this is a multinode cluster and first container is trying to delete network while other containers are attached to it + glog.Warningf("failed to remove network (which might be okay) %s: %v", d.NodeConfig.ClusterName, err) + } return nil } diff --git a/pkg/drivers/kic/kic_test.go b/pkg/drivers/kic/kic_test.go new file mode 100644 index 000000000000..380879183e44 --- /dev/null +++ b/pkg/drivers/kic/kic_test.go @@ -0,0 +1,59 @@ +/* +Copyright 2019 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 kic + +import ( + "testing" +) + +func TestMachineOrder(t *testing.T) { + testCases := []struct { + Name string + MachineName string + Want int + }{ + { + Name: "default", + MachineName: "minikube", + Want: 1}, + { + Name: "second-node", + MachineName: "minikube-m02", + Want: 2}, + { + Name: "dash-profile", + MachineName: "my-dashy-minikube", + Want: 1}, + + { + Name: "dash-profile-second-node", + MachineName: "my-dashy-minikube-m02", + Want: 2}, + { + Name: "michivious-user", + MachineName: "michivious-user-m02-m03", + Want: 3}, + } + + for _, tc := range testCases { + t.Run(tc.Name, func(t *testing.T) { + got := machineOrder(tc.MachineName) + if got != tc.Want { + t.Errorf("want order %q but got %q", tc.Want, got) + + } + }) + + } +} diff --git a/pkg/drivers/kic/oci/network.go b/pkg/drivers/kic/oci/network.go index ca9e53bdb627..e292cbad132a 100644 --- a/pkg/drivers/kic/oci/network.go +++ b/pkg/drivers/kic/oci/network.go @@ -31,17 +31,21 @@ import ( // RoutableHostIPFromInside returns the ip/dns of the host that container lives on // is routable from inside the container -func RoutableHostIPFromInside(ociBin string, containerName string) (net.IP, error) { +func RoutableHostIPFromInside(ociBin string, clusterName string, containerName string) (net.IP, error) { if ociBin == Docker { if runtime.GOOS == "linux" { - return dockerGatewayIP(containerName) + _, gateway, err := dockerNetworkInspect(clusterName) + if err != nil { + return gateway, errors.Wrap(err, "network inspect") + } + return gateway, nil } // for windows and mac, the gateway ip is not routable so we use dns trick. return digDNS(ociBin, containerName, "host.docker.internal") } if runtime.GOOS == "linux" { - return containerGatewayIP(ociBin, containerName) + return podmanGatewayIP(containerName) } return nil, fmt.Errorf("RoutableHostIPFromInside is currently only implemented for linux") @@ -59,57 +63,9 @@ func digDNS(ociBin, containerName, dns string) (net.IP, error) { return ip, nil } -// profileInContainers checks whether the profile is within the containers list -func profileInContainers(profile string, containers []string) bool { - for _, container := range containers { - if container == profile { - return true - } - } - return false -} - -// dockerGatewayIP gets the default gateway ip for the docker bridge on the user's host machine -// gets the ip from user's host docker -func dockerGatewayIP(profile string) (net.IP, error) { - var bridgeID string - rr, err := runCmd(exec.Command(Docker, "network", "ls", "--filter", "name=bridge", "--format", "{{.ID}}")) - if err != nil { - return nil, errors.Wrapf(err, "get network bridge") - } - networksOutput := strings.TrimSpace(rr.Stdout.String()) - networksSlice := strings.Fields(networksOutput) - // Look for the minikube container within each docker network - for _, net := range networksSlice { - // get all containers in the network - rs, err := runCmd(exec.Command(Docker, "network", "inspect", net, "-f", "{{range $k, $v := .Containers}}{{$v.Name}} {{end}}")) - if err != nil { - return nil, errors.Wrapf(err, "get containers in network") - } - containersSlice := strings.Fields(rs.Stdout.String()) - if profileInContainers(profile, containersSlice) { - bridgeID = net - break - } - } - - if bridgeID == "" { - return nil, errors.Errorf("unable to determine bridge network id from %q", networksOutput) - } - rr, err = runCmd(exec.Command(Docker, "network", "inspect", - "--format", "{{(index .IPAM.Config 0).Gateway}}", bridgeID)) - if err != nil { - return nil, errors.Wrapf(err, "inspect IP bridge network %q.", bridgeID) - } - - ip := net.ParseIP(strings.TrimSpace(rr.Stdout.String())) - glog.Infof("got host ip for mount in container by inspect docker network: %s", ip.String()) - return ip, nil -} - -// containerGatewayIP gets the default gateway ip for the container -func containerGatewayIP(ociBin, containerName string) (net.IP, error) { - rr, err := runCmd(exec.Command(ociBin, "container", "inspect", "--format", "{{.NetworkSettings.Gateway}}", containerName)) +// podmanGatewayIP gets the default gateway ip for the container +func podmanGatewayIP(containerName string) (net.IP, error) { + rr, err := runCmd(exec.Command(Podman, "container", "inspect", "--format", "{{.NetworkSettings.Gateway}}", containerName)) if err != nil { return nil, errors.Wrapf(err, "inspect gateway") } diff --git a/pkg/drivers/kic/oci/network_create.go b/pkg/drivers/kic/oci/network_create.go index 3df9660814a8..58f15066bd20 100644 --- a/pkg/drivers/kic/oci/network_create.go +++ b/pkg/drivers/kic/oci/network_create.go @@ -17,6 +17,8 @@ limitations under the License. package oci import ( + "bufio" + "bytes" "fmt" "net" "os/exec" @@ -123,3 +125,72 @@ func dockerNetworkInspect(name string) (*net.IPNet, net.IP, error) { } return subnet, gateway, nil } + +// RemoveNetwork removes a network +func RemoveNetwork(name string) error { + if !networkExists(name) { + return nil + } + rr, err := runCmd(exec.Command(Docker, "network", "remove", name)) + if err != nil { + if strings.Contains(rr.Output(), "No such network:") { + return ErrNetworkNotFound + } + // Error response from daemon: error while removing network: network mynet123 id f9e1c50b89feb0b8f4b687f3501a81b618252c9907bc20666e386d0928322387 has active endpoints + if strings.Contains(rr.Output(), "has active endpoints") { + return ErrNetworkInUse + } + } + + return err +} + +func networkExists(name string) bool { + if _, _, err := dockerNetworkInspect(name); err != nil { + if err == ErrNetworkNotFound { + return false + } + glog.Warningf("error inspecting network %s: %v", name, err) + return false + } + return true +} + +// returns all network names created by a label +func allNetworkByLabel(ociBin string, label string) ([]string, error) { + if ociBin != Docker { + return nil, fmt.Errorf("%s not supported", ociBin) + } + + // docker network ls --filter='label=created_by.minikube.sigs.k8s.io=true' --format '{{.Name}} + rr, err := runCmd(exec.Command(Docker, "network", "ls", fmt.Sprintf("--filter=label=%s", label), "--format", "{{.Name}}")) + if err != nil { + return nil, err + } + var lines []string + scanner := bufio.NewScanner(bytes.NewReader(rr.Stdout.Bytes())) + for scanner.Scan() { + lines = append(lines, strings.TrimSpace(scanner.Text())) + } + + return lines, nil +} + +// DeleteAllNetworksByKIC deletes all networks created by kic +func DeleteAllNetworksByKIC() []error { + var errs []error + ns, err := allNetworkByLabel(Docker, CreatedByLabelKey+"=true") + if err != nil { + return []error{errors.Wrap(err, "list all volume")} + } + for _, n := range ns { + err := RemoveNetwork(n) + if err != nil { + errs = append(errs, err) + } + } + if len(errs) > 0 { + return errs + } + return nil +} diff --git a/pkg/drivers/kic/oci/oci.go b/pkg/drivers/kic/oci/oci.go index 878189e38727..ac7c3ee87b06 100644 --- a/pkg/drivers/kic/oci/oci.go +++ b/pkg/drivers/kic/oci/oci.go @@ -169,6 +169,11 @@ func CreateContainerNode(p CreateParams) error { virtualization = "podman" // VIRTUALIZATION_PODMAN } if p.OCIBinary == Docker { + // to provide a static IP for docker + if p.Network != "" && p.IP != "" { + runArgs = append(runArgs, "--network", p.Network) + runArgs = append(runArgs, "--ip", p.IP) + } runArgs = append(runArgs, "--volume", fmt.Sprintf("%s:/var", p.Name)) // ignore apparmore github actions docker: https://github.com/kubernetes/minikube/issues/7624 runArgs = append(runArgs, "--security-opt", "apparmor=unconfined") @@ -285,6 +290,10 @@ func createContainer(ociBin string, image string, opts ...createOpt) error { if strings.Contains(rr.Output(), "Range of CPUs is from") && strings.Contains(rr.Output(), "CPUs available") { // CPUs available return ErrCPUCountLimit } + // example: docker: Error response from daemon: Address already in use. + if strings.Contains(rr.Output(), "Address already in use") { + return ErrIPinUse + } return err } diff --git a/pkg/drivers/kic/oci/types.go b/pkg/drivers/kic/oci/types.go index b2dad8de4ade..09a42d4da15c 100644 --- a/pkg/drivers/kic/oci/types.go +++ b/pkg/drivers/kic/oci/types.go @@ -43,6 +43,7 @@ const ( // CreateParams are parameters needed to create a container type CreateParams struct { + ClusterName string // cluster(profile name) that this container belongs to Name string // used for container name and hostname Image string // container image to use to create the node. ClusterLabel string // label the clusters we create using minikube so we can clean up @@ -56,6 +57,8 @@ type CreateParams struct { Envs map[string]string // environment variables to pass to the container ExtraArgs []string // a list of any extra option to pass to oci binary during creation time, for example --expose 8080... OCIBinary string // docker or podman + Network string // network to use for the containe + IP string // IP to assign for th container in the work } // createOpt is an option for Create diff --git a/pkg/drivers/kic/types.go b/pkg/drivers/kic/types.go index 5d138a0ea80a..67ab4da09e17 100644 --- a/pkg/drivers/kic/types.go +++ b/pkg/drivers/kic/types.go @@ -48,6 +48,7 @@ var ( // Config is configuration for the kic driver used by registry type Config struct { + ClusterName string // The cluster the container belongs to MachineName string // maps to the container name being created CPU int // Number of CPU cores assigned to the container Memory int // max memory in MB diff --git a/pkg/minikube/cluster/ip.go b/pkg/minikube/cluster/ip.go index 2dd4548020bd..caf34e2bb239 100644 --- a/pkg/minikube/cluster/ip.go +++ b/pkg/minikube/cluster/ip.go @@ -34,12 +34,12 @@ import ( ) // HostIP gets the ip address to be used for mapping host -> VM and VM -> host -func HostIP(host *host.Host) (net.IP, error) { +func HostIP(host *host.Host, clusterName string) (net.IP, error) { switch host.DriverName { case driver.Docker: - return oci.RoutableHostIPFromInside(oci.Docker, host.Name) + return oci.RoutableHostIPFromInside(oci.Docker, clusterName, host.Name) case driver.Podman: - return oci.RoutableHostIPFromInside(oci.Podman, host.Name) + return oci.RoutableHostIPFromInside(oci.Podman, clusterName, host.Name) case driver.KVM2: return net.ParseIP("192.168.39.1"), nil case driver.HyperV: diff --git a/pkg/minikube/node/start.go b/pkg/minikube/node/start.go index 6982f1013514..c00aa6cadca3 100644 --- a/pkg/minikube/node/start.go +++ b/pkg/minikube/node/start.go @@ -94,7 +94,7 @@ func Start(starter Starter, apiServer bool) (*kubeconfig.Settings, error) { showVersionInfo(starter.Node.KubernetesVersion, cr) // Add "host.minikube.internal" DNS alias (intentionally non-fatal) - hostIP, err := cluster.HostIP(starter.Host) + hostIP, err := cluster.HostIP(starter.Host, starter.Cfg.Name) if err != nil { glog.Errorf("Unable to get host IP: %v", err) } else if err := machine.AddHostAlias(starter.Runner, constants.HostAlias, hostIP); err != nil { diff --git a/pkg/minikube/registry/drvs/docker/docker.go b/pkg/minikube/registry/drvs/docker/docker.go index 38a325a6aa74..98d961e1d23d 100644 --- a/pkg/minikube/registry/drvs/docker/docker.go +++ b/pkg/minikube/registry/drvs/docker/docker.go @@ -61,6 +61,7 @@ func configure(cc config.ClusterConfig, n config.Node) (interface{}, error) { } return kic.NewDriver(kic.Config{ + ClusterName: cc.Name, MachineName: driver.MachineName(cc, n), StorePath: localpath.MiniPath(), ImageDigest: cc.KicBaseImage, diff --git a/pkg/minikube/registry/drvs/podman/podman.go b/pkg/minikube/registry/drvs/podman/podman.go index 166fd9e6d5a3..fdf912f5e237 100644 --- a/pkg/minikube/registry/drvs/podman/podman.go +++ b/pkg/minikube/registry/drvs/podman/podman.go @@ -73,6 +73,7 @@ func configure(cc config.ClusterConfig, n config.Node) (interface{}, error) { } return kic.NewDriver(kic.Config{ + ClusterName: cc.Name, MachineName: driver.MachineName(cc, n), StorePath: localpath.MiniPath(), ImageDigest: strings.Split(cc.KicBaseImage, "@")[0], // for podman does not support docker images references with both a tag and digest. From fa0afb220d0c1162e9b36d0ec9350414bfa8913e Mon Sep 17 00:00:00 2001 From: Medya Gh Date: Mon, 21 Sep 2020 17:52:06 -0700 Subject: [PATCH 03/13] lint --- pkg/drivers/kic/kic_test.go | 3 +++ pkg/drivers/kic/oci/network_create.go | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/pkg/drivers/kic/kic_test.go b/pkg/drivers/kic/kic_test.go index 380879183e44..d607bbc7f2dd 100644 --- a/pkg/drivers/kic/kic_test.go +++ b/pkg/drivers/kic/kic_test.go @@ -1,9 +1,12 @@ /* Copyright 2019 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. diff --git a/pkg/drivers/kic/oci/network_create.go b/pkg/drivers/kic/oci/network_create.go index 58f15066bd20..259efc7f35ed 100644 --- a/pkg/drivers/kic/oci/network_create.go +++ b/pkg/drivers/kic/oci/network_create.go @@ -1,5 +1,5 @@ /* -Copyright 2019 The Kubernetes Authors All rights reserved. +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. From 2766235a5c05406d333df73cb3ca929f1b252856 Mon Sep 17 00:00:00 2001 From: Medya Gh Date: Wed, 23 Sep 2020 17:08:56 -0700 Subject: [PATCH 04/13] try alternate if no network --- pkg/drivers/kic/oci/network.go | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/pkg/drivers/kic/oci/network.go b/pkg/drivers/kic/oci/network.go index e292cbad132a..2dcf22cc56cc 100644 --- a/pkg/drivers/kic/oci/network.go +++ b/pkg/drivers/kic/oci/network.go @@ -36,6 +36,10 @@ func RoutableHostIPFromInside(ociBin string, clusterName string, containerName s if runtime.GOOS == "linux" { _, gateway, err := dockerNetworkInspect(clusterName) if err != nil { + if errors.Is(err, ErrNetworkNotFound) { + glog.Infof("The container %s is not attached to a network, this could be because the cluster was created by an older than v.1.14 minikube, will try to get the IP using container gatway", containerName) + return containerGatewayIP(Docker, containerName) + } return gateway, errors.Wrap(err, "network inspect") } return gateway, nil @@ -43,9 +47,9 @@ func RoutableHostIPFromInside(ociBin string, clusterName string, containerName s // for windows and mac, the gateway ip is not routable so we use dns trick. return digDNS(ociBin, containerName, "host.docker.internal") } - + // podman if runtime.GOOS == "linux" { - return podmanGatewayIP(containerName) + return containerGatewayIP(Podman, containerName) } return nil, fmt.Errorf("RoutableHostIPFromInside is currently only implemented for linux") @@ -63,9 +67,9 @@ func digDNS(ociBin, containerName, dns string) (net.IP, error) { return ip, nil } -// podmanGatewayIP gets the default gateway ip for the container -func podmanGatewayIP(containerName string) (net.IP, error) { - rr, err := runCmd(exec.Command(Podman, "container", "inspect", "--format", "{{.NetworkSettings.Gateway}}", containerName)) +// containerGatewayIP gets the default gateway ip for the container +func containerGatewayIP(ociBin string, containerName string) (net.IP, error) { + rr, err := runCmd(exec.Command(ociBin, "container", "inspect", "--format", "{{.NetworkSettings.Gateway}}", containerName)) if err != nil { return nil, errors.Wrapf(err, "inspect gateway") } From f163cf9e15b38e165f3663c1c3562f1e910864a1 Mon Sep 17 00:00:00 2001 From: Medya Gh Date: Wed, 23 Sep 2020 20:03:11 -0700 Subject: [PATCH 05/13] update with new logic --- pkg/drivers/kic/kic.go | 6 ++--- pkg/drivers/kic/kic_test.go | 5 ++++ pkg/drivers/kic/oci/network_create.go | 37 ++++++++++++++------------- 3 files changed, 26 insertions(+), 22 deletions(-) diff --git a/pkg/drivers/kic/kic.go b/pkg/drivers/kic/kic.go index d63dfcd94343..9183fd609820 100644 --- a/pkg/drivers/kic/kic.go +++ b/pkg/drivers/kic/kic.go @@ -94,13 +94,11 @@ func (d *Driver) Create() error { APIServerPort: d.NodeConfig.APIServerPort, } - // one network bridge per cluster. - defaultNetwork := d.NodeConfig.ClusterName - if gateway, err := oci.CreateNetwork(d.OCIBinary, defaultNetwork); err != nil { + if gateway, err := oci.CreateNetwork(d.OCIBinary, d.NodeConfig.ClusterName); err != nil { glog.Warningf("failed to create network: %v", err) out.WarningT("Unable to create dedicated network, This might result in cluster IP change after restart.") } else { - params.Network = defaultNetwork + params.Network = d.NodeConfig.ClusterName ip := gateway.To4() // calculate the container IP based on its machine order ip[3] += byte(machineOrder(d.NodeConfig.MachineName)) diff --git a/pkg/drivers/kic/kic_test.go b/pkg/drivers/kic/kic_test.go index d607bbc7f2dd..a5bb6c9783cc 100644 --- a/pkg/drivers/kic/kic_test.go +++ b/pkg/drivers/kic/kic_test.go @@ -34,6 +34,11 @@ func TestMachineOrder(t *testing.T) { Name: "second-node", MachineName: "minikube-m02", Want: 2}, + { + Name: "funny", + MachineName: "hahaha", + Want: 1}, + { Name: "dash-profile", MachineName: "my-dashy-minikube", diff --git a/pkg/drivers/kic/oci/network_create.go b/pkg/drivers/kic/oci/network_create.go index 259efc7f35ed..04ba730a6102 100644 --- a/pkg/drivers/kic/oci/network_create.go +++ b/pkg/drivers/kic/oci/network_create.go @@ -29,7 +29,10 @@ import ( ) // DefaultSubnet subnet to be used on first cluster -const defaultSubnet = "192.168.39.0/24" +const defaultSubnetAddr = "192.168.39.0" + +// big enough for a cluster of 256 nodes +const defaultSubnetRange = 24 // CreateNetwork creates a network returns gateway and error, minikube creates one network per cluster func CreateNetwork(ociBin string, name string) (net.IP, error) { @@ -39,21 +42,17 @@ func CreateNetwork(ociBin string, name string) (net.IP, error) { return createDockerNetwork(name) } -func createDockerNetwork(name string) (net.IP, error) { +func createDockerNetwork(clusterName string) (net.IP, error) { // check if the network already exists - subnet, gateway, err := dockerNetworkInspect(name) + subnet, gateway, err := dockerNetworkInspect(clusterName) if err == nil { glog.Infof("Found existing network with subnet %s and gateway %s.", subnet, gateway) return gateway, nil } // simple way to create networks, subnet is taken, try one bigger attempt := 0 - _, subnet, err = net.ParseCIDR(defaultSubnet) - if err != nil { - return nil, errors.Wrapf(err, "parse default subnet %s", defaultSubnet) - } - - gateway, err = tryCreateDockerNetwork(subnet, name) + subnetAddr := defaultSubnetAddr + gateway, err = tryCreateDockerNetwork(subnetAddr, defaultSubnetRange, clusterName) if err != nil { if err != ErrNetworkSubnetTaken { return nil, errors.Wrapf(err, "error creating network") @@ -62,18 +61,20 @@ func createDockerNetwork(name string) (net.IP, error) { // we can try up to 255 for attempt < 13 { attempt++ - glog.Infof("Couldn't create network %q at %q subnet will try again with a new subnet ...", name, subnet) + glog.Infof("Couldn't create network %q at %q subnet will try again with a new subnet ...", clusterName, subnetAddr) // increase 3nd digit by 10 each time // 13 times adding 10 defaultSubnet "192.168.39.0/24" // at most it will add up to 169 which is still less than max allowed 255 // this is large enough to try more and not too small to not try enough // can be tuned in the next iterations - subnet.IP.To4()[2] += 10 - gateway, err := tryCreateDockerNetwork(subnet, name) + ip := net.ParseIP(subnetAddr).To4() + ip[2] += byte(9 + attempt) + + gateway, err = tryCreateDockerNetwork(ip.String(), defaultSubnetRange, clusterName) if err == nil { return gateway, nil } - if err == ErrNetworkSubnetTaken { + if errors.Is(err, ErrNetworkSubnetTaken) || errors.Is(err, ErrNetworkGatewayTaken) { continue } } @@ -82,12 +83,12 @@ func createDockerNetwork(name string) (net.IP, error) { return gateway, nil } -func tryCreateDockerNetwork(subnet *net.IPNet, name string) (net.IP, error) { - gateway := subnet.IP.To4() - gateway[3]++ // first ip for gateway - glog.Infof("attempt to create network %q with subnet: %s and gateway %s...", subnet, name, gateway) +func tryCreateDockerNetwork(subnetAddr string, subnetRange int, name string) (net.IP, error) { + gateway := net.ParseIP(subnetAddr) + gateway.To4()[3]++ // first ip for gateway + glog.Infof("attempt to create network %q with subnet: %s and gateway %s...", subnetAddr, name, gateway) // options documentation https://docs.docker.com/engine/reference/commandline/network_create/#bridge-driver-options - rr, err := runCmd(exec.Command(Docker, "network", "create", "--driver=bridge", fmt.Sprintf("--subnet=%s", subnet), fmt.Sprintf("--gateway=%s", gateway), "-o", "--ip-masq", "-o", "--icc", fmt.Sprintf("--label=%s=%s", CreatedByLabelKey, "true"), name)) + rr, err := runCmd(exec.Command(Docker, "network", "create", "--driver=bridge", fmt.Sprintf("--subnet=%s", fmt.Sprintf("%s/%d", subnetAddr, subnetRange)), fmt.Sprintf("--gateway=%s", gateway), "-o", "--ip-masq", "-o", "--icc", fmt.Sprintf("--label=%s=%s", CreatedByLabelKey, "true"), name)) if err != nil { if strings.Contains(rr.Output(), "Pool overlaps with other one on this address space") { return nil, ErrNetworkSubnetTaken From b7780ec815695a533062c3b656dc2d6cce07e58d Mon Sep 17 00:00:00 2001 From: Medya Gh Date: Mon, 28 Sep 2020 12:55:02 -0700 Subject: [PATCH 06/13] address review comments --- cmd/minikube/cmd/delete.go | 2 +- pkg/drivers/kic/kic.go | 14 +++++------ pkg/drivers/kic/kic_test.go | 4 ++-- pkg/drivers/kic/oci/network_create.go | 24 ++++++++++--------- pkg/drivers/kic/oci/types.go | 4 ++-- .../content/en/docs/handbook/vpn_and_proxy.md | 1 + 6 files changed, 25 insertions(+), 24 deletions(-) diff --git a/cmd/minikube/cmd/delete.go b/cmd/minikube/cmd/delete.go index b8ac93a56891..93ca41fd347e 100644 --- a/cmd/minikube/cmd/delete.go +++ b/cmd/minikube/cmd/delete.go @@ -262,7 +262,7 @@ func deletePossibleKicLeftOver(cname string, driverName string) { errs = oci.DeleteAllNetworksByKIC() if errs != nil { - glog.Warningf("error deleting left over networks (might be okay).\nTo see the list of netowrks: 'docker network ls'\n:%v", errs) + glog.Warningf("error deleting leftover networks (might be okay).\nTo see the list of networks: 'docker network ls'\n:%v", errs) } if bin == oci.Podman { diff --git a/pkg/drivers/kic/kic.go b/pkg/drivers/kic/kic.go index 9183fd609820..08eef23fd45d 100644 --- a/pkg/drivers/kic/kic.go +++ b/pkg/drivers/kic/kic.go @@ -66,8 +66,8 @@ func NewDriver(c Config) *Driver { return d } -// machineOrder returns the order of the container based on it is name -func machineOrder(machineName string) int { +// machineIndex returns the order of the container based on it is name +func machineIndex(machineName string) int { // minikube-m02 sp := strings.Split(machineName, "-") m := strings.Trim(sp[len(sp)-1], "m") // m02 @@ -95,14 +95,13 @@ func (d *Driver) Create() error { } if gateway, err := oci.CreateNetwork(d.OCIBinary, d.NodeConfig.ClusterName); err != nil { - glog.Warningf("failed to create network: %v", err) - out.WarningT("Unable to create dedicated network, This might result in cluster IP change after restart.") + out.WarningT("Unable to create dedicated network, This might result in cluster IP change after restart. {{.error}}", out.V{"error": err}) } else { params.Network = d.NodeConfig.ClusterName ip := gateway.To4() - // calculate the container IP based on its machine order - ip[3] += byte(machineOrder(d.NodeConfig.MachineName)) - glog.Infof("calculated static IP %q for the %q container ", ip.String(), d.NodeConfig.MachineName) + // calculate the container IP based on guessing the machine index + ip[3] += byte(machineIndex(d.NodeConfig.MachineName)) + glog.Infof("calculated static IP %q for the %q container", ip.String(), d.NodeConfig.MachineName) params.IP = ip.String() } @@ -316,7 +315,6 @@ func (d *Driver) Remove() error { } if err := oci.RemoveNetwork(d.NodeConfig.ClusterName); err != nil { - //TODO: Ingore error if this is a multinode cluster and first container is trying to delete network while other containers are attached to it glog.Warningf("failed to remove network (which might be okay) %s: %v", d.NodeConfig.ClusterName, err) } return nil diff --git a/pkg/drivers/kic/kic_test.go b/pkg/drivers/kic/kic_test.go index a5bb6c9783cc..47af3763feb0 100644 --- a/pkg/drivers/kic/kic_test.go +++ b/pkg/drivers/kic/kic_test.go @@ -20,7 +20,7 @@ import ( "testing" ) -func TestMachineOrder(t *testing.T) { +func TestMachineIndex(t *testing.T) { testCases := []struct { Name string MachineName string @@ -56,7 +56,7 @@ func TestMachineOrder(t *testing.T) { for _, tc := range testCases { t.Run(tc.Name, func(t *testing.T) { - got := machineOrder(tc.MachineName) + got := machineIndex(tc.MachineName) if got != tc.Want { t.Errorf("want order %q but got %q", tc.Want, got) diff --git a/pkg/drivers/kic/oci/network_create.go b/pkg/drivers/kic/oci/network_create.go index 04ba730a6102..bc31c9b55a1a 100644 --- a/pkg/drivers/kic/oci/network_create.go +++ b/pkg/drivers/kic/oci/network_create.go @@ -28,8 +28,9 @@ import ( "github.com/pkg/errors" ) -// DefaultSubnet subnet to be used on first cluster -const defaultSubnetAddr = "192.168.39.0" +// firstSubnetAddr subnet to be used on first kic cluster +// it is one octet more than the one used by KVM to avoid possible conflict +const firstSubnetAddr = "192.168.49.0" // big enough for a cluster of 256 nodes const defaultSubnetRange = 24 @@ -51,19 +52,19 @@ func createDockerNetwork(clusterName string) (net.IP, error) { } // simple way to create networks, subnet is taken, try one bigger attempt := 0 - subnetAddr := defaultSubnetAddr + subnetAddr := firstSubnetAddr gateway, err = tryCreateDockerNetwork(subnetAddr, defaultSubnetRange, clusterName) if err != nil { if err != ErrNetworkSubnetTaken { return nil, errors.Wrapf(err, "error creating network") } - // try up to 13 times - // we can try up to 255 - for attempt < 13 { + // try up to 20 times, third octet would go like 49,59 ,69,..., 239 + // max we could go is 255 we stop at 239 + for attempt < 20 { attempt++ glog.Infof("Couldn't create network %q at %q subnet will try again with a new subnet ...", clusterName, subnetAddr) - // increase 3nd digit by 10 each time - // 13 times adding 10 defaultSubnet "192.168.39.0/24" + // Find an open subnet by incrementing the 3rd octet by 10 for each try + // 13 times adding 10 firstSubnetAddr "192.168.49.0/24" // at most it will add up to 169 which is still less than max allowed 255 // this is large enough to try more and not too small to not try enough // can be tuned in the next iterations @@ -77,8 +78,8 @@ func createDockerNetwork(clusterName string) (net.IP, error) { if errors.Is(err, ErrNetworkSubnetTaken) || errors.Is(err, ErrNetworkGatewayTaken) { continue } + glog.Errorf("unexpected error while trying to create network, will try again anyways: %v", err) } - } return gateway, nil } @@ -90,7 +91,8 @@ func tryCreateDockerNetwork(subnetAddr string, subnetRange int, name string) (ne // options documentation https://docs.docker.com/engine/reference/commandline/network_create/#bridge-driver-options rr, err := runCmd(exec.Command(Docker, "network", "create", "--driver=bridge", fmt.Sprintf("--subnet=%s", fmt.Sprintf("%s/%d", subnetAddr, subnetRange)), fmt.Sprintf("--gateway=%s", gateway), "-o", "--ip-masq", "-o", "--icc", fmt.Sprintf("--label=%s=%s", CreatedByLabelKey, "true"), name)) if err != nil { - if strings.Contains(rr.Output(), "Pool overlaps with other one on this address space") { + // Pool overlaps with other one on this address space + if strings.Contains(rr.Output(), "Pool overlaps") { return nil, ErrNetworkSubnetTaken } if strings.Contains(rr.Output(), "failed to allocate gateway") && strings.Contains(rr.Output(), "Address already in use") { @@ -113,7 +115,7 @@ func dockerNetworkInspect(name string) (*net.IPNet, net.IP, error) { // results looks like 172.17.0.0/16,172.17.0.1 ips := strings.Split(strings.TrimSpace(rr.Stdout.String()), ",") if len(ips) == 0 { - return nil, nil, fmt.Errorf("invalid network info") + return nil, nil, fmt.Errorf("empty IP list parsed from: %q", rr.Output()) } _, subnet, err := net.ParseCIDR(ips[0]) diff --git a/pkg/drivers/kic/oci/types.go b/pkg/drivers/kic/oci/types.go index 09a42d4da15c..0e6de567f54c 100644 --- a/pkg/drivers/kic/oci/types.go +++ b/pkg/drivers/kic/oci/types.go @@ -57,8 +57,8 @@ type CreateParams struct { Envs map[string]string // environment variables to pass to the container ExtraArgs []string // a list of any extra option to pass to oci binary during creation time, for example --expose 8080... OCIBinary string // docker or podman - Network string // network to use for the containe - IP string // IP to assign for th container in the work + Network string // docker/podman network that the container will attach to + IP string // static IP to assign for th container in the cluster network } // createOpt is an option for Create diff --git a/site/content/en/docs/handbook/vpn_and_proxy.md b/site/content/en/docs/handbook/vpn_and_proxy.md index 3d430b10b46f..41174d9999b8 100644 --- a/site/content/en/docs/handbook/vpn_and_proxy.md +++ b/site/content/en/docs/handbook/vpn_and_proxy.md @@ -22,6 +22,7 @@ The NO_PROXY variable here is important: Without setting it, minikube may not be * **192.168.99.0/24**: Used by the minikube VM. Configurable for some hypervisors via `--host-only-cidr` * **192.168.39.0/24**: Used by the minikube kvm2 driver. +* **192.168.49.0/24**: Used by the minikube docker driver. * **10.96.0.0/12**: Used by service cluster IP's. Configurable via `--service-cluster-ip-range` One important note: If NO_PROXY is required by non-Kubernetes applications, such as Firefox or Chrome, you may want to specifically add the minikube IP to the comma-separated list, as they may not understand IP ranges ([#3827](https://github.com/kubernetes/minikube/issues/3827)). From 058adb775ca4ed4354937425d7db70bae4f7f185 Mon Sep 17 00:00:00 2001 From: Medya Gh Date: Mon, 28 Sep 2020 12:56:51 -0700 Subject: [PATCH 07/13] address review comments --- pkg/drivers/kic/oci/network_create.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/drivers/kic/oci/network_create.go b/pkg/drivers/kic/oci/network_create.go index bc31c9b55a1a..0525b616bb47 100644 --- a/pkg/drivers/kic/oci/network_create.go +++ b/pkg/drivers/kic/oci/network_create.go @@ -107,7 +107,7 @@ func tryCreateDockerNetwork(subnetAddr string, subnetRange int, name string) (ne func dockerNetworkInspect(name string) (*net.IPNet, net.IP, error) { rr, err := runCmd(exec.Command(Docker, "network", "inspect", name, "--format", "{{(index .IPAM.Config 0).Subnet}},{{(index .IPAM.Config 0).Gateway}}")) if err != nil { - if strings.Contains(rr.Output(), "No such network:") { + if strings.Contains(rr.Output(), "No such network") { return nil, nil, ErrNetworkNotFound } return nil, nil, err @@ -136,7 +136,7 @@ func RemoveNetwork(name string) error { } rr, err := runCmd(exec.Command(Docker, "network", "remove", name)) if err != nil { - if strings.Contains(rr.Output(), "No such network:") { + if strings.Contains(rr.Output(), "No such network") { return ErrNetworkNotFound } // Error response from daemon: error while removing network: network mynet123 id f9e1c50b89feb0b8f4b687f3501a81b618252c9907bc20666e386d0928322387 has active endpoints From 31e175c6f84e9cfe8bd68b6e8e72ac8e7c4e325c Mon Sep 17 00:00:00 2001 From: Medya Gh Date: Tue, 29 Sep 2020 14:27:37 -0700 Subject: [PATCH 08/13] move index machine to driver package --- pkg/drivers/kic/kic.go | 15 ++------- pkg/drivers/kic/kic_test.go | 50 ------------------------------ pkg/minikube/driver/driver.go | 13 ++++++++ pkg/minikube/driver/driver_test.go | 46 +++++++++++++++++++++++++++ 4 files changed, 61 insertions(+), 63 deletions(-) diff --git a/pkg/drivers/kic/kic.go b/pkg/drivers/kic/kic.go index 08eef23fd45d..0235f2d9e7fb 100644 --- a/pkg/drivers/kic/kic.go +++ b/pkg/drivers/kic/kic.go @@ -37,6 +37,7 @@ import ( "k8s.io/minikube/pkg/minikube/constants" "k8s.io/minikube/pkg/minikube/cruntime" "k8s.io/minikube/pkg/minikube/download" + "k8s.io/minikube/pkg/minikube/driver" "k8s.io/minikube/pkg/minikube/out" "k8s.io/minikube/pkg/minikube/sysinit" "k8s.io/minikube/pkg/util/retry" @@ -66,18 +67,6 @@ func NewDriver(c Config) *Driver { return d } -// machineIndex returns the order of the container based on it is name -func machineIndex(machineName string) int { - // minikube-m02 - sp := strings.Split(machineName, "-") - m := strings.Trim(sp[len(sp)-1], "m") // m02 - i, err := strconv.Atoi(m) - if err != nil { - return 1 - } - return i -} - // Create a host using the driver's config func (d *Driver) Create() error { params := oci.CreateParams{ @@ -100,7 +89,7 @@ func (d *Driver) Create() error { params.Network = d.NodeConfig.ClusterName ip := gateway.To4() // calculate the container IP based on guessing the machine index - ip[3] += byte(machineIndex(d.NodeConfig.MachineName)) + ip[3] += byte(driver.IndexFromMachineName(d.NodeConfig.MachineName)) glog.Infof("calculated static IP %q for the %q container", ip.String(), d.NodeConfig.MachineName) params.IP = ip.String() } diff --git a/pkg/drivers/kic/kic_test.go b/pkg/drivers/kic/kic_test.go index 47af3763feb0..0f77824b29b5 100644 --- a/pkg/drivers/kic/kic_test.go +++ b/pkg/drivers/kic/kic_test.go @@ -15,53 +15,3 @@ limitations under the License. */ package kic - -import ( - "testing" -) - -func TestMachineIndex(t *testing.T) { - testCases := []struct { - Name string - MachineName string - Want int - }{ - { - Name: "default", - MachineName: "minikube", - Want: 1}, - { - Name: "second-node", - MachineName: "minikube-m02", - Want: 2}, - { - Name: "funny", - MachineName: "hahaha", - Want: 1}, - - { - Name: "dash-profile", - MachineName: "my-dashy-minikube", - Want: 1}, - - { - Name: "dash-profile-second-node", - MachineName: "my-dashy-minikube-m02", - Want: 2}, - { - Name: "michivious-user", - MachineName: "michivious-user-m02-m03", - Want: 3}, - } - - for _, tc := range testCases { - t.Run(tc.Name, func(t *testing.T) { - got := machineIndex(tc.MachineName) - if got != tc.Want { - t.Errorf("want order %q but got %q", tc.Want, got) - - } - }) - - } -} diff --git a/pkg/minikube/driver/driver.go b/pkg/minikube/driver/driver.go index 791b3d0bd3b6..2002ea3afdea 100644 --- a/pkg/minikube/driver/driver.go +++ b/pkg/minikube/driver/driver.go @@ -21,6 +21,7 @@ import ( "os" "runtime" "sort" + "strconv" "strings" "github.com/golang/glog" @@ -297,3 +298,15 @@ func MachineName(cc config.ClusterConfig, n config.Node) string { } return fmt.Sprintf("%s-%s", cc.Name, n.Name) } + +// IndexFromMachineName returns the order of the container based on it is name +func IndexFromMachineName(machineName string) int { + // minikube-m02 + sp := strings.Split(machineName, "-") + m := strings.Trim(sp[len(sp)-1], "m") // m02 + i, err := strconv.Atoi(m) + if err != nil { + return 1 + } + return i +} diff --git a/pkg/minikube/driver/driver_test.go b/pkg/minikube/driver/driver_test.go index b6afd6a62c9e..713efbdd269d 100644 --- a/pkg/minikube/driver/driver_test.go +++ b/pkg/minikube/driver/driver_test.go @@ -201,3 +201,49 @@ func TestSuggest(t *testing.T) { }) } } + +func TestIndexFromMachineName(t *testing.T) { + testCases := []struct { + Name string + MachineName string + Want int + }{ + { + Name: "default", + MachineName: "minikube", + Want: 1}, + { + Name: "second-node", + MachineName: "minikube-m02", + Want: 2}, + { + Name: "funny", + MachineName: "hahaha", + Want: 1}, + + { + Name: "dash-profile", + MachineName: "my-dashy-minikube", + Want: 1}, + + { + Name: "dash-profile-second-node", + MachineName: "my-dashy-minikube-m02", + Want: 2}, + { + Name: "michivious-user", + MachineName: "michivious-user-m02-m03", + Want: 3}, + } + + for _, tc := range testCases { + t.Run(tc.Name, func(t *testing.T) { + got := IndexFromMachineName(tc.MachineName) + if got != tc.Want { + t.Errorf("want order %q but got %q", tc.Want, got) + + } + }) + + } +} From 82f39d1387ec5d81abe9bbfdb6bf27ba6e470b7e Mon Sep 17 00:00:00 2001 From: Medya Gh Date: Tue, 29 Sep 2020 14:29:34 -0700 Subject: [PATCH 09/13] clarify comment --- pkg/drivers/kic/oci/types.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/drivers/kic/oci/types.go b/pkg/drivers/kic/oci/types.go index 0e6de567f54c..b21da438ec5a 100644 --- a/pkg/drivers/kic/oci/types.go +++ b/pkg/drivers/kic/oci/types.go @@ -57,7 +57,7 @@ type CreateParams struct { Envs map[string]string // environment variables to pass to the container ExtraArgs []string // a list of any extra option to pass to oci binary during creation time, for example --expose 8080... OCIBinary string // docker or podman - Network string // docker/podman network that the container will attach to + Network string // network name that the container will attach to IP string // static IP to assign for th container in the cluster network } From c48e8e5ec960689fa89c88fd1ca3f1a4b188a112 Mon Sep 17 00:00:00 2001 From: Medya Gh Date: Tue, 29 Sep 2020 15:00:44 -0700 Subject: [PATCH 10/13] add more unit tests --- pkg/minikube/driver/driver_test.go | 112 +++++++++++++++++++++++++++++ 1 file changed, 112 insertions(+) diff --git a/pkg/minikube/driver/driver_test.go b/pkg/minikube/driver/driver_test.go index 713efbdd269d..ba47bf7c59bc 100644 --- a/pkg/minikube/driver/driver_test.go +++ b/pkg/minikube/driver/driver_test.go @@ -23,6 +23,7 @@ import ( "testing" "github.com/google/go-cmp/cmp" + "k8s.io/minikube/pkg/minikube/config" "k8s.io/minikube/pkg/minikube/registry" ) @@ -202,6 +203,60 @@ func TestSuggest(t *testing.T) { } } +func TestMachineName(t *testing.T) { + testsCases := []struct { + ClusterConfig config.ClusterConfig + Want string + }{ + { + ClusterConfig: config.ClusterConfig{Name: "minikube", + Nodes: []config.Node{ + config.Node{ + Name: "", + IP: "172.17.0.3", + Port: 8443, + KubernetesVersion: "v1.19.2", + ControlPlane: true, + Worker: true, + }, + }, + }, + Want: "minikube", + }, + + { + ClusterConfig: config.ClusterConfig{Name: "p2", + Nodes: []config.Node{ + config.Node{ + Name: "", + IP: "172.17.0.3", + Port: 8443, + KubernetesVersion: "v1.19.2", + ControlPlane: true, + Worker: true, + }, + config.Node{ + Name: "m2", + IP: "172.17.0.4", + Port: 0, + KubernetesVersion: "v1.19.2", + ControlPlane: false, + Worker: true, + }, + }, + }, + Want: "p2-m2", + }, + } + + for _, tc := range testsCases { + got := MachineName(tc.ClusterConfig, tc.ClusterConfig.Nodes[len(tc.ClusterConfig.Nodes)-1]) + if got != tc.Want { + t.Errorf("Expected MachineName to be %q but got %q", tc.Want, got) + } + } +} + func TestIndexFromMachineName(t *testing.T) { testCases := []struct { Name string @@ -247,3 +302,60 @@ func TestIndexFromMachineName(t *testing.T) { } } + +// test against cluster config +func TestIndexFromMachineName2(t *testing.T) { + + testsCases := []struct { + ClusterConfig config.ClusterConfig + Want int + }{ + { + ClusterConfig: config.ClusterConfig{Name: "minikube", + Nodes: []config.Node{ + config.Node{ + Name: "", + IP: "172.17.0.3", + Port: 8443, + KubernetesVersion: "v1.19.2", + ControlPlane: true, + Worker: true, + }, + }, + }, + Want: 1, + }, + + { + ClusterConfig: config.ClusterConfig{Name: "p2", + Nodes: []config.Node{ + config.Node{ + Name: "", + IP: "172.17.0.3", + Port: 8443, + KubernetesVersion: "v1.19.2", + ControlPlane: true, + Worker: true, + }, + config.Node{ + Name: "m2", + IP: "172.17.0.4", + Port: 0, + KubernetesVersion: "v1.19.2", + ControlPlane: false, + Worker: true, + }, + }, + }, + Want: 2, + }, + } + + for _, tc := range testsCases { + got := IndexFromMachineName(MachineName(tc.ClusterConfig, tc.ClusterConfig.Nodes[len(tc.ClusterConfig.Nodes)-1])) + if got != tc.Want { + t.Errorf("expected IndexFromMachineName to be %d but got %d", tc.Want, got) + } + + } +} From b84dedc652a67ef4a461ab8f228827846c7f5b3c Mon Sep 17 00:00:00 2001 From: Medya Gh Date: Tue, 29 Sep 2020 15:07:06 -0700 Subject: [PATCH 11/13] address review comments --- cmd/minikube/cmd/delete.go | 2 +- pkg/drivers/kic/oci/errors.go | 2 +- pkg/drivers/kic/oci/network.go | 5 +++-- pkg/drivers/kic/oci/network_create.go | 26 +++++++++++++------------- 4 files changed, 18 insertions(+), 17 deletions(-) diff --git a/cmd/minikube/cmd/delete.go b/cmd/minikube/cmd/delete.go index 93ca41fd347e..e819fec0f364 100644 --- a/cmd/minikube/cmd/delete.go +++ b/cmd/minikube/cmd/delete.go @@ -260,7 +260,7 @@ func deletePossibleKicLeftOver(cname string, driverName string) { glog.Warningf("error deleting volumes (might be okay).\nTo see the list of volumes run: 'docker volume ls'\n:%v", errs) } - errs = oci.DeleteAllNetworksByKIC() + errs = oci.DeleteKICNetworks() if errs != nil { glog.Warningf("error deleting leftover networks (might be okay).\nTo see the list of networks: 'docker network ls'\n:%v", errs) } diff --git a/pkg/drivers/kic/oci/errors.go b/pkg/drivers/kic/oci/errors.go index 0889358bc0e9..f8e65b7b1bb6 100644 --- a/pkg/drivers/kic/oci/errors.go +++ b/pkg/drivers/kic/oci/errors.go @@ -58,7 +58,7 @@ var ErrNetworkNotFound = errors.New("kic network not found") var ErrNetworkGatewayTaken = errors.New("network gateway is taken") // ErrNetworkInUse is when trying to delete a network which is attached to another container -var ErrNetworkInUse = errors.New("can't delete network attached to a running container") +var ErrNetworkInUse = errors.New("unable to delete a network that is attached to a running container") // LogContainerDebug will print relevant docker/podman infos after a container fails func LogContainerDebug(ociBin string, name string) string { diff --git a/pkg/drivers/kic/oci/network.go b/pkg/drivers/kic/oci/network.go index 2dcf22cc56cc..137063996ae9 100644 --- a/pkg/drivers/kic/oci/network.go +++ b/pkg/drivers/kic/oci/network.go @@ -37,7 +37,8 @@ func RoutableHostIPFromInside(ociBin string, clusterName string, containerName s _, gateway, err := dockerNetworkInspect(clusterName) if err != nil { if errors.Is(err, ErrNetworkNotFound) { - glog.Infof("The container %s is not attached to a network, this could be because the cluster was created by an older than v.1.14 minikube, will try to get the IP using container gatway", containerName) + glog.Infof("The container %s is not attached to a network, this could be because the cluster was created by minikube Date: Wed, 30 Sep 2020 14:58:20 -0700 Subject: [PATCH 12/13] remove kic test --- pkg/drivers/kic/kic_test.go | 17 --------- pkg/drivers/kic/oci/network_create.go | 53 ++++++++++++--------------- 2 files changed, 24 insertions(+), 46 deletions(-) delete mode 100644 pkg/drivers/kic/kic_test.go diff --git a/pkg/drivers/kic/kic_test.go b/pkg/drivers/kic/kic_test.go deleted file mode 100644 index 0f77824b29b5..000000000000 --- a/pkg/drivers/kic/kic_test.go +++ /dev/null @@ -1,17 +0,0 @@ -/* -Copyright 2019 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 kic diff --git a/pkg/drivers/kic/oci/network_create.go b/pkg/drivers/kic/oci/network_create.go index bac03140b707..af8bb7b337b4 100644 --- a/pkg/drivers/kic/oci/network_create.go +++ b/pkg/drivers/kic/oci/network_create.go @@ -50,36 +50,31 @@ func createDockerNetwork(clusterName string) (net.IP, error) { glog.Infof("Found existing network with subnet %s and gateway %s.", subnet, gateway) return gateway, nil } - // simple way to create networks, subnet is taken, try one bigger - attempt := 0 + + attempts := 0 subnetAddr := firstSubnetAddr - gateway, err = tryCreateDockerNetwork(subnetAddr, defaultSubnetMask, clusterName) - if err != nil { - if err != ErrNetworkSubnetTaken { - return nil, errors.Wrapf(err, "error creating network") + // Rather than iterate through all of the valid subnets, give up at 20 to avoid a lengthy user delay for something that is unlikely to work. + // will be like like 192.168.49.0/24 ,...,192.168.239.0/24 + for attempts < 20 { + gateway, err = tryCreateDockerNetwork(subnetAddr, defaultSubnetMask, clusterName) + if err == nil { + return gateway, nil } - // Rather than iterate through all of the valid subnets, give up at 20 to avoid a lengthy user delay for something that is unlikely to work. - // try up to 20 times, third octet would go like 49,59 ,69,..., 239 - for attempt < 20 { - attempt++ - glog.Infof("Couldn't create network %q at %q subnet will try again with a new subnet ...", clusterName, subnetAddr) - // Find an open subnet by incrementing the 3rd octet by 10 for each try - // 13 times adding 10 firstSubnetAddr "192.168.49.0/24" - // at most it will add up to 169 which is still less than max allowed 255 - // this is large enough to try more and not too small to not try enough - // can be tuned in the next iterations - ip := net.ParseIP(subnetAddr).To4() - ip[2] += byte(9 + attempt) - - gateway, err = tryCreateDockerNetwork(ip.String(), defaultSubnetMask, clusterName) - if err == nil { - return gateway, nil - } - if errors.Is(err, ErrNetworkSubnetTaken) || errors.Is(err, ErrNetworkGatewayTaken) { - continue - } - glog.Errorf("unexpected error while trying to create network, will try again anyways: %v", err) + + // don't retry if error is not adddress is taken + if !(errors.Is(err, ErrNetworkSubnetTaken) || errors.Is(err, ErrNetworkGatewayTaken)) { + glog.Errorf("error while trying to create network %v", err) + return nil, errors.Wrap(err, "un-retryable") } + attempts++ + // Find an open subnet by incrementing the 3rd octet by 10 for each try + // 13 times adding 10 firstSubnetAddr "192.168.49.0/24" + // at most it will add up to 169 which is still less than max allowed 255 + // this is large enough to try more and not too small to not try enough + // can be tuned in the next iterations + newSubnet := net.ParseIP(subnetAddr).To4() + newSubnet[2] += byte(9 + attempts) + subnetAddr = newSubnet.String() } return gateway, nil } @@ -87,7 +82,7 @@ func createDockerNetwork(clusterName string) (net.IP, error) { func tryCreateDockerNetwork(subnetAddr string, subnetMask int, name string) (net.IP, error) { gateway := net.ParseIP(subnetAddr) gateway.To4()[3]++ // first ip for gateway - glog.Infof("attempt to create network %q with subnet: %s and gateway %s...", subnetAddr, name, gateway) + glog.Infof("attempt to create network %s/%d with subnet: %s and gateway %s...", subnetAddr, subnetMask, name, gateway) // options documentation https://docs.docker.com/engine/reference/commandline/network_create/#bridge-driver-options rr, err := runCmd(exec.Command(Docker, "network", "create", "--driver=bridge", fmt.Sprintf("--subnet=%s", fmt.Sprintf("%s/%d", subnetAddr, subnetMask)), fmt.Sprintf("--gateway=%s", gateway), "-o", "--ip-masq", "-o", "--icc", fmt.Sprintf("--label=%s=%s", CreatedByLabelKey, "true"), name)) if err != nil { @@ -98,7 +93,7 @@ func tryCreateDockerNetwork(subnetAddr string, subnetMask int, name string) (net if strings.Contains(rr.Output(), "failed to allocate gateway") && strings.Contains(rr.Output(), "Address already in use") { return nil, ErrNetworkGatewayTaken } - return nil, errors.Wrapf(err, "error creating network") + return nil, errors.Wrapf(err, "create network %s", fmt.Sprintf("%s %s/%d", name, subnetAddr, subnetMask)) } return gateway, nil } From 73d77e2463a2710a0518fbd5ac13ebadb348072a Mon Sep 17 00:00:00 2001 From: Medya Gh Date: Wed, 30 Sep 2020 16:42:42 -0700 Subject: [PATCH 13/13] address review comments --- pkg/drivers/kic/kic.go | 2 +- pkg/drivers/kic/oci/network_create.go | 15 ++++++--------- pkg/minikube/driver/driver_test.go | 4 ++-- site/content/en/docs/handbook/vpn_and_proxy.md | 2 +- 4 files changed, 10 insertions(+), 13 deletions(-) diff --git a/pkg/drivers/kic/kic.go b/pkg/drivers/kic/kic.go index 0235f2d9e7fb..120d64a9b646 100644 --- a/pkg/drivers/kic/kic.go +++ b/pkg/drivers/kic/kic.go @@ -84,7 +84,7 @@ func (d *Driver) Create() error { } if gateway, err := oci.CreateNetwork(d.OCIBinary, d.NodeConfig.ClusterName); err != nil { - out.WarningT("Unable to create dedicated network, This might result in cluster IP change after restart. {{.error}}", out.V{"error": err}) + out.WarningT("Unable to create dedicated network, this might result in cluster IP change after restart: {{.error}}", out.V{"error": err}) } else { params.Network = d.NodeConfig.ClusterName ip := gateway.To4() diff --git a/pkg/drivers/kic/oci/network_create.go b/pkg/drivers/kic/oci/network_create.go index af8bb7b337b4..dbbbe151159c 100644 --- a/pkg/drivers/kic/oci/network_create.go +++ b/pkg/drivers/kic/oci/network_create.go @@ -54,7 +54,7 @@ func createDockerNetwork(clusterName string) (net.IP, error) { attempts := 0 subnetAddr := firstSubnetAddr // Rather than iterate through all of the valid subnets, give up at 20 to avoid a lengthy user delay for something that is unlikely to work. - // will be like like 192.168.49.0/24 ,...,192.168.239.0/24 + // will be like 192.168.49.0/24 ,...,192.168.239.0/24 for attempts < 20 { gateway, err = tryCreateDockerNetwork(subnetAddr, defaultSubnetMask, clusterName) if err == nil { @@ -76,7 +76,7 @@ func createDockerNetwork(clusterName string) (net.IP, error) { newSubnet[2] += byte(9 + attempts) subnetAddr = newSubnet.String() } - return gateway, nil + return gateway, fmt.Errorf("failed to create network after 20 attempts") } func tryCreateDockerNetwork(subnetAddr string, subnetMask int, name string) (net.IP, error) { @@ -144,14 +144,11 @@ func RemoveNetwork(name string) error { } func networkExists(name string) bool { - if _, _, err := dockerNetworkInspect(name); err != nil { - if err == ErrNetworkNotFound { - return false - } - glog.Warningf("error inspecting network %s: %v", name, err) - return false + _, _, err := dockerNetworkInspect(name) + if err != nil && !errors.Is(err, ErrNetworkNotFound) { // log unexpected error + glog.Warningf("Error inspecting docker network %s: %v", name, err) } - return true + return err == nil } // networkNamesByLabel returns all network names created by a label diff --git a/pkg/minikube/driver/driver_test.go b/pkg/minikube/driver/driver_test.go index ba47bf7c59bc..953ff53691dc 100644 --- a/pkg/minikube/driver/driver_test.go +++ b/pkg/minikube/driver/driver_test.go @@ -303,8 +303,8 @@ func TestIndexFromMachineName(t *testing.T) { } } -// test against cluster config -func TestIndexFromMachineName2(t *testing.T) { +// test indexFroMachine against cluster config +func TestIndexFromMachineNameClusterConfig(t *testing.T) { testsCases := []struct { ClusterConfig config.ClusterConfig diff --git a/site/content/en/docs/handbook/vpn_and_proxy.md b/site/content/en/docs/handbook/vpn_and_proxy.md index 41174d9999b8..bcd92ee5c8e5 100644 --- a/site/content/en/docs/handbook/vpn_and_proxy.md +++ b/site/content/en/docs/handbook/vpn_and_proxy.md @@ -22,7 +22,7 @@ The NO_PROXY variable here is important: Without setting it, minikube may not be * **192.168.99.0/24**: Used by the minikube VM. Configurable for some hypervisors via `--host-only-cidr` * **192.168.39.0/24**: Used by the minikube kvm2 driver. -* **192.168.49.0/24**: Used by the minikube docker driver. +* **192.168.49.0/24**: Used by the minikube docker driver's first cluster. * **10.96.0.0/12**: Used by service cluster IP's. Configurable via `--service-cluster-ip-range` One important note: If NO_PROXY is required by non-Kubernetes applications, such as Firefox or Chrome, you may want to specifically add the minikube IP to the comma-separated list, as they may not understand IP ranges ([#3827](https://github.com/kubernetes/minikube/issues/3827)).