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.

Sign up for GitHub

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

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

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
a6e5fb8
lower mtu for cloud shell
medyagh Oct 22, 2020
bfa83ff
fix option
medyagh Oct 22, 2020
50a04a4
try
medyagh Oct 23, 2020
cde546e
detect MTU from docker brdige
medyagh Oct 23, 2020
94485d5
lint
medyagh Oct 23, 2020
aa97aca
make struct for netinfo
medyagh Oct 23, 2020
1dd5583
template
medyagh Oct 23, 2020
4993dc6
log
medyagh Oct 23, 2020
43c2a5c
dont pass mtu if 0
medyagh Oct 23, 2020
9f729ac
readability
medyagh Oct 23, 2020
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
2 changes: 1 addition & 1 deletion pkg/drivers/kic/oci/network.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ import (
func RoutableHostIPFromInside(ociBin string, clusterName string, containerName string) (net.IP, error) {
if ociBin == Docker {
if runtime.GOOS == "linux" {
_, gateway, err := dockerNetworkInspect(clusterName)
_, gateway, _, 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)
Expand Down
55 changes: 34 additions & 21 deletions pkg/drivers/kic/oci/network_create.go
View file
Open in desktop
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

// will be used if docker bridge config doesn't exist related issue #9528
const defaultNetworkMTU = 1500
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,7 +50,7 @@ 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)
subnet, gateway, mtu, err := dockerNetworkInspect(clusterName)
if err == nil {
klog.Infof("Found existing network with subnet %s and gateway %s.", subnet, gateway)
return gateway, nil
Expand All @@ -57,7 +61,7 @@ func createDockerNetwork(clusterName string) (net.IP, error) {
// 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)
gateway, err = tryCreateDockerNetwork(subnetAddr, defaultSubnetMask, mtu, clusterName)
if err == nil {
return gateway, nil
}
Expand All @@ -80,12 +84,13 @@ func createDockerNetwork(clusterName string) (net.IP, error) {
return 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)
klog.Infof("attempt to create network %s/%d with subnet: %s and gateway %s and MTU of %d ...", subnetAddr, subnetMask, name, gateway, mtu)
// 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))
// adding MTU option because #9528
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", fmt.Sprintf("com.docker.network.driver.mtu=%d", mtu), "-o", "--icc", fmt.Sprintf("--label=%s=%s", CreatedByLabelKey, "true"), name))
if err != nil {
// Pool overlaps with other one on this address space
if strings.Contains(rr.Output(), "Pool overlaps") {
Expand All @@ -99,32 +104,40 @@ 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}}")
// returns subnet and gate if exists returns subnet, gateway and mtu
func dockerNetworkInspect(name string) (*net.IPNet, net.IP, int, error) {
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 nil, nil, defaultNetworkMTU, ErrNetworkNotFound
}
return nil, nil, err
return nil, nil, defaultNetworkMTU, 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())
vals := strings.Split(strings.TrimSpace(rr.Stdout.String()), ",")
if len(vals) == 0 {
return nil, nil, defaultNetworkMTU, fmt.Errorf("empty IP list parsed from: %q", rr.Output())
}

_, 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])
mtu := defaultNetworkMTU
if len(vals) > 0 {
gateway = net.ParseIP(vals[1])
mtu, err = strconv.Atoi(vals[2])
if err != nil {
klog.Warningf("failed to parse docker network %s mtu, will use the default %d : %v", name, defaultNetworkMTU, err)
mtu = defaultNetworkMTU
}
}
return subnet, gateway, nil

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

return subnet, gateway, mtu, nil
}

func logDockerNetworkInspect(name string) {
Expand Down Expand Up @@ -157,7 +170,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