Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

docker: When creating networks, use MTU of built-in bridge network #9530

Merged
merged 10 commits into from
Oct 23, 2020
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions pkg/drivers/kic/oci/network.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,16 +35,16 @@ import (
func RoutableHostIPFromInside(ociBin string, clusterName string, containerName string) (net.IP, error) {
if ociBin == Docker {
if runtime.GOOS == "linux" {
_, gateway, err := dockerNetworkInspect(clusterName)
info, err := dockerNetworkInspect(clusterName)
if err != nil {
if errors.Is(err, ErrNetworkNotFound) {
klog.Infof("The container %s is not attached to a network, this could be because the cluster was created by minikube <v1.14, will try to get the IP using container gatway", containerName)

return containerGatewayIP(Docker, containerName)
}
return gateway, errors.Wrap(err, "network inspect")
return info.gateway, errors.Wrap(err, "network inspect")
}
return gateway, nil
return info.gateway, nil
}
// for windows and mac, the gateway ip is not routable so we use dns trick.
return digDNS(ociBin, containerName, "host.docker.internal")
Expand Down
100 changes: 73 additions & 27 deletions pkg/drivers/kic/oci/network_create.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"fmt"
"net"
"os/exec"
"strconv"
"strings"

"github.com/pkg/errors"
Expand All @@ -36,6 +37,9 @@ const firstSubnetAddr = "192.168.49.0"
// big enough for a cluster of 254 nodes
const defaultSubnetMask = 24

// name of the bridge network that docker creates by default to be used to get the MTU. ( related issue #9528)
medyagh marked this conversation as resolved.
Show resolved Hide resolved
const dockerDefaultBridgeName = "bridge"
medyagh marked this conversation as resolved.
Show resolved Hide resolved

// 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 {
Expand All @@ -46,20 +50,26 @@ func CreateNetwork(ociBin string, name string) (net.IP, error) {

func createDockerNetwork(clusterName string) (net.IP, error) {
// check if the network already exists
subnet, gateway, err := dockerNetworkInspect(clusterName)
info, err := dockerNetworkInspect(clusterName)
if err == nil {
klog.Infof("Found existing network with subnet %s and gateway %s.", subnet, gateway)
return gateway, nil
klog.Infof("Found existing network %+v", info)
return info.gateway, nil
}

// will try to get MTU from the docker network to avoid issue with systems with exotic MTU settings.
// related issue #9528
info, err = dockerNetworkInspect(dockerDefaultBridgeName)
if err != nil {
klog.Warningf("failed to get mtu information from the docker's default network %q: %v", dockerDefaultBridgeName, err)
}
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 192.168.49.0/24 ,...,192.168.239.0/24
for attempts < 20 {
gateway, err = tryCreateDockerNetwork(subnetAddr, defaultSubnetMask, clusterName)
info.gateway, err = tryCreateDockerNetwork(subnetAddr, defaultSubnetMask, info.mtu, clusterName)
if err == nil {
return gateway, nil
return info.gateway, nil
}

// don't retry if error is not adddress is taken
Expand All @@ -77,15 +87,33 @@ func createDockerNetwork(clusterName string) (net.IP, error) {
newSubnet[2] += byte(9 + attempts)
subnetAddr = newSubnet.String()
}
return gateway, fmt.Errorf("failed to create network after 20 attempts")
return info.gateway, fmt.Errorf("failed to create network after 20 attempts")
}

func tryCreateDockerNetwork(subnetAddr string, subnetMask int, name string) (net.IP, error) {
func tryCreateDockerNetwork(subnetAddr string, subnetMask int, mtu int, name string) (net.IP, error) {
gateway := net.ParseIP(subnetAddr)
gateway.To4()[3]++ // first ip for gateway
klog.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))
klog.Infof("attempt to create network %s/%d with subnet: %s and gateway %s and MTU of %d ...", subnetAddr, subnetMask, name, gateway, mtu)
args := []string{
"network",
"create",
"--driver=bridge",
fmt.Sprintf("--subnet=%s", fmt.Sprintf("%s/%d", subnetAddr, subnetMask)),
fmt.Sprintf("--gateway=%s", gateway),
// options documentation https://docs.docker.com/engine/reference/commandline/network_create/#bridge-driver-options
"-o", "--ip-masq",
"-o", "--icc",
fmt.Sprintf("--label=%s=%s", CreatedByLabelKey, "true"),
name,
}

// adding MTU option because #9528
if mtu != 0 {
medyagh marked this conversation as resolved.
Show resolved Hide resolved
args = append(args, "-o")
args = append(args, fmt.Sprintf("com.docker.network.driver.mtu=%d", mtu))
}

rr, err := runCmd(exec.Command(Docker, args...))
if err != nil {
// Pool overlaps with other one on this address space
if strings.Contains(rr.Output(), "Pool overlaps") {
Expand All @@ -99,32 +127,50 @@ func tryCreateDockerNetwork(subnetAddr string, subnetMask int, name string) (net
return gateway, nil
}

// returns subnet and gate if exists
func dockerNetworkInspect(name string) (*net.IPNet, net.IP, error) {
cmd := exec.Command(Docker, "network", "inspect", name, "--format", "{{(index .IPAM.Config 0).Subnet}},{{(index .IPAM.Config 0).Gateway}}")
// netInfo holds part of a docker or podman network information relevant to kic drivers
type netInfo struct {
name string
subnet *net.IPNet
gateway net.IP
mtu int
}

// if exists returns subnet, gateway and mtu
func dockerNetworkInspect(name string) (netInfo, error) {
var info = netInfo{name: name}
cmd := exec.Command(Docker, "network", "inspect", name, "--format", `{{(index .IPAM.Config 0).Subnet}},{{(index .IPAM.Config 0).Gateway}},{{(index .Options "com.docker.network.driver.mtu")}}`)
rr, err := runCmd(cmd)
if err != nil {
logDockerNetworkInspect(name)
if strings.Contains(rr.Output(), "No such network") {
return nil, nil, ErrNetworkNotFound

return info, ErrNetworkNotFound
}
return nil, nil, err
return info, 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("empty IP list parsed from: %q", rr.Output())

// results looks like 172.17.0.0/16,172.17.0.1,1500
vals := strings.Split(strings.TrimSpace(rr.Stdout.String()), ",")
if len(vals) == 0 {
return info, fmt.Errorf("empty list network inspect: %q", rr.Output())
}

_, subnet, err := net.ParseCIDR(ips[0])
if err != nil {
return nil, nil, errors.Wrapf(err, "parse subnet for %s", name)
if len(vals) > 0 {
info.gateway = net.ParseIP(vals[1])
mtu, err := strconv.Atoi(vals[2])
if err != nil {
klog.Warningf("couldn't parse mtu for docker network %q: %v", name, err)
} else {
info.mtu = mtu
}
}
var gateway net.IP
if len(ips) > 0 {
gateway = net.ParseIP(ips[1])

_, info.subnet, err = net.ParseCIDR(vals[0])
if err != nil {
return info, errors.Wrapf(err, "parse subnet for %s", name)
}
return subnet, gateway, nil

return info, nil
}

func logDockerNetworkInspect(name string) {
Expand Down Expand Up @@ -157,7 +203,7 @@ func RemoveNetwork(name string) error {
}

func networkExists(name string) bool {
_, _, err := dockerNetworkInspect(name)
_, err := dockerNetworkInspect(name)
if err != nil && !errors.Is(err, ErrNetworkNotFound) { // log unexpected error
klog.Warningf("Error inspecting docker network %s: %v", name, err)
}
Expand Down