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

Add private network implementation for podman #9716

Merged
merged 1 commit into from
Dec 2, 2020
Merged
Show file tree
Hide file tree
Changes from all 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
2 changes: 1 addition & 1 deletion cmd/minikube/cmd/delete.go
Original file line number Diff line number Diff line change
Expand Up @@ -260,7 +260,7 @@ func deletePossibleKicLeftOver(cname string, driverName string) {
klog.Warningf("error deleting volumes (might be okay).\nTo see the list of volumes run: 'docker volume ls'\n:%v", errs)
}

errs = oci.DeleteKICNetworks()
errs = oci.DeleteKICNetworks(bin)
if errs != nil {
klog.Warningf("error deleting leftover networks (might be okay).\nTo see the list of networks: 'docker network ls'\n:%v", errs)
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/drivers/kic/kic.go
Original file line number Diff line number Diff line change
Expand Up @@ -320,7 +320,7 @@ func (d *Driver) Remove() error {
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 {
if err := oci.RemoveNetwork(d.OCIBinary, d.NodeConfig.ClusterName); err != nil {
klog.Warningf("failed to remove network (which might be okay) %s: %v", d.NodeConfig.ClusterName, err)
}
return nil
Expand Down
5 changes: 3 additions & 2 deletions pkg/drivers/kic/oci/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,14 +85,15 @@ func LogContainerDebug(ociBin string, name string) string {
} else {
klog.Infof("postmortem docker info: %+v", di)
}
logDockerNetworkInspect(name)
logDockerNetworkInspect(ociBin, name)
} else {
pi, err := podmanSystemInfo()
if err != nil {
klog.Warningf("couldn't get postmortem info, failed to to run podman info: %v", err)
klog.Warningf("couldn't get postmortem podman info: %v", err)
} else {
klog.Infof("postmortem podman info: %+v", pi)
}
logDockerNetworkInspect(ociBin, name)
}

if rr.Stdout.Len() == 0 {
Expand Down
21 changes: 13 additions & 8 deletions pkg/drivers/kic/oci/network.go
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" {
info, err := dockerNetworkInspect(clusterName)
info, err := containerNetworkInspect(ociBin, 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 All @@ -51,7 +51,7 @@ func RoutableHostIPFromInside(ociBin string, clusterName string, containerName s
}
// podman
if runtime.GOOS == "linux" {
return containerGatewayIP(Podman, containerName)
return containerGatewayIP(ociBin, containerName)
}

return nil, fmt.Errorf("RoutableHostIPFromInside is currently only implemented for linux")
Expand Down Expand Up @@ -128,30 +128,35 @@ func ForwardedPort(ociBin string, ociID string, contPort int) (int, error) {
// ContainerIPs returns ipv4,ipv6, error of a container by their name
func ContainerIPs(ociBin string, name string) (string, string, error) {
if ociBin == Podman {
return podmanContainerIP(name)
return podmanContainerIP(ociBin, name)
}
return dockerContainerIP(name)
return dockerContainerIP(ociBin, name)
}

// podmanContainerIP returns ipv4, ipv6 of container or error
func podmanContainerIP(name string) (string, string, error) {
rr, err := runCmd(exec.Command(Podman, "container", "inspect",
func podmanContainerIP(ociBin string, name string) (string, string, error) {
rr, err := runCmd(exec.Command(ociBin, "container", "inspect",
"-f", "{{.NetworkSettings.IPAddress}}",
name))
if err != nil {
return "", "", errors.Wrapf(err, "podman inspect ip %s", name)
}
output := strings.TrimSpace(rr.Stdout.String())
if err == nil && output == "" { // podman returns empty for 127.0.0.1
// check network, if the ip address is missing
ipv4, ipv6, err := dockerContainerIP(ociBin, name)
if err == nil {
return ipv4, ipv6, nil
}
return DefaultBindIPV4, "", nil
}
return output, "", nil
}

// dockerContainerIP returns ipv4, ipv6 of container or error
func dockerContainerIP(name string) (string, string, error) {
func dockerContainerIP(ociBin string, name string) (string, string, error) {
// retrieve the IP address of the node using docker inspect
lines, err := inspect(Docker, name, "{{range .NetworkSettings.Networks}}{{.IPAddress}},{{.GlobalIPv6Address}}{{end}}")
lines, err := inspect(ociBin, name, "{{range .NetworkSettings.Networks}}{{.IPAddress}},{{.GlobalIPv6Address}}{{end}}")
if err != nil {
return "", "", errors.Wrap(err, "inspecting NetworkSettings.Networks")
}
Expand Down
115 changes: 77 additions & 38 deletions pkg/drivers/kic/oci/network_create.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,37 +37,34 @@ const firstSubnetAddr = "192.168.49.0"
// big enough for a cluster of 254 nodes
const defaultSubnetMask = 24

// name of the default Docker bridge network, used to lookup the MTU (see #9528)
const dockerDefaultBridge = "bridge"
// name of the default bridge network, used to lookup the MTU (see #9528)
const defaultBridgeName = "bridge"

// 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)
return createDockerNetwork(ociBin, name)
}

func createDockerNetwork(clusterName string) (net.IP, error) {
func createDockerNetwork(ociBin string, clusterName string) (net.IP, error) {
// check if the network already exists
info, err := dockerNetworkInspect(clusterName)
info, err := containerNetworkInspect(ociBin, clusterName)
if err == 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(dockerDefaultBridge)
info, err = containerNetworkInspect(ociBin, defaultBridgeName)
if err != nil {
klog.Warningf("failed to get mtu information from the docker's default network %q: %v", dockerDefaultBridge, err)
klog.Warningf("failed to get mtu information from the %s's default network %q: %v", ociBin, defaultBridgeName, 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 {
info.gateway, err = tryCreateDockerNetwork(subnetAddr, defaultSubnetMask, info.mtu, clusterName)
info.gateway, err = tryCreateDockerNetwork(ociBin, subnetAddr, defaultSubnetMask, info.mtu, clusterName)
if err == nil {
return info.gateway, nil
}
Expand All @@ -90,7 +87,7 @@ func createDockerNetwork(clusterName string) (net.IP, error) {
return info.gateway, fmt.Errorf("failed to create network after 20 attempts")
}

func tryCreateDockerNetwork(subnetAddr string, subnetMask int, mtu int, name string) (net.IP, error) {
func tryCreateDockerNetwork(ociBin string, 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 and MTU of %d ...", subnetAddr, subnetMask, name, gateway, mtu)
Expand All @@ -100,20 +97,25 @@ func tryCreateDockerNetwork(subnetAddr string, subnetMask int, mtu int, name str
"--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 {
if ociBin == Docker {
// options documentation https://docs.docker.com/engine/reference/commandline/network_create/#bridge-driver-options
args = append(args, "-o")
args = append(args, "--ip-masq")
args = append(args, "-o")
args = append(args, fmt.Sprintf("com.docker.network.driver.mtu=%d", mtu))
args = append(args, "--icc")

// adding MTU option because #9528
if mtu > 0 {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could podman network benefit from specifying the MTU ? (this fixed the issue of minikube in cloud shell not being able to pull large images like golang:1.15 due to different cloud shell MTUs

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might, but it will not benefit from "com.docker.network.driver.mtu" at any rate.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It will be available in the future (like podman 3.0 or so), with a similar option

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe add a comment, to remember TODO add mtu for podman 3 (with an issue liunk)

args = append(args, "-o")
args = append(args, fmt.Sprintf("com.docker.network.driver.mtu=%d", mtu))
}

args = append(args, fmt.Sprintf("--label=%s=%s", CreatedByLabelKey, "true"))
}
args = append(args, name)

rr, err := runCmd(exec.Command(Docker, args...))
rr, err := runCmd(exec.Command(ociBin, args...))
if err != nil {
// Pool overlaps with other one on this address space
if strings.Contains(rr.Output(), "Pool overlaps") {
Expand All @@ -135,6 +137,16 @@ type netInfo struct {
mtu int
}

func containerNetworkInspect(ociBin string, name string) (netInfo, error) {
if ociBin == Docker {
return dockerNetworkInspect(name)
}
if ociBin == Podman {
return podmanNetworkInspect(name)
}
return netInfo{}, fmt.Errorf("%s unknown", ociBin)
}

// networkInspect is only used to unmarshal the docker network inspect output and translate it to netInfo
type networkInspect struct {
Name string
Expand All @@ -153,7 +165,7 @@ func dockerNetworkInspect(name string) (netInfo, error) {
cmd := exec.Command(Docker, "network", "inspect", name, "--format", `{"Name": "{{.Name}}","Driver": "{{.Driver}}","Subnet": "{{range .IPAM.Config}}{{.Subnet}}{{end}}","Gateway": "{{range .IPAM.Config}}{{.Gateway}}{{end}}","MTU": {{(index .Options "com.docker.network.driver.mtu")}},{{$first := true}} "ContainerIPs": [{{range $k,$v := .Containers }}{{if $first}}{{$first = false}}{{else}}, {{end}}"{{$v.IPv4Address}}"{{end}}]}`)
rr, err := runCmd(cmd)
if err != nil {
logDockerNetworkInspect(name)
logDockerNetworkInspect(Docker, name)
if strings.Contains(rr.Output(), "No such network") {

return info, ErrNetworkNotFound
Expand All @@ -177,8 +189,39 @@ func dockerNetworkInspect(name string) (netInfo, error) {
return info, nil
}

func logDockerNetworkInspect(name string) {
cmd := exec.Command(Docker, "network", "inspect", name)
func podmanNetworkInspect(name string) (netInfo, error) {
var info = netInfo{name: name}
cmd := exec.Command(Podman, "network", "inspect", name, "--format", `{{(index .IPAM.Config 0).Subnet}},{{(index .IPAM.Config 0).Gateway}}`)
rr, err := runCmd(cmd)
if err != nil {
logDockerNetworkInspect(Podman, name)
if strings.Contains(rr.Output(), "No such network") {

return info, ErrNetworkNotFound
}
return info, err
}

// 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())
}

if len(vals) > 0 {
info.gateway = net.ParseIP(vals[1])
}

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

return info, nil
}

func logDockerNetworkInspect(ociBin string, name string) {
cmd := exec.Command(ociBin, "network", "inspect", name)
klog.Infof("running %v to gather additional debugging logs...", cmd.Args)
rr, err := runCmd(cmd)
if err != nil {
Expand All @@ -188,11 +231,11 @@ func logDockerNetworkInspect(name string) {
}

// RemoveNetwork removes a network
func RemoveNetwork(name string) error {
if !networkExists(name) {
func RemoveNetwork(ociBin string, name string) error {
if !networkExists(ociBin, name) {
return nil
}
rr, err := runCmd(exec.Command(Docker, "network", "remove", name))
rr, err := runCmd(exec.Command(ociBin, "network", "rm", name))
if err != nil {
if strings.Contains(rr.Output(), "No such network") {
return ErrNetworkNotFound
Expand All @@ -206,8 +249,8 @@ func RemoveNetwork(name string) error {
return err
}

func networkExists(name string) bool {
_, err := dockerNetworkInspect(name)
func networkExists(ociBin string, name string) bool {
_, err := containerNetworkInspect(ociBin, name)
if err != nil && !errors.Is(err, ErrNetworkNotFound) { // log unexpected error
klog.Warningf("Error inspecting docker network %s: %v", name, err)
}
Expand All @@ -216,12 +259,8 @@ func networkExists(name string) bool {

// networkNamesByLabel returns all network names created by a label
func networkNamesByLabel(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}}"))
rr, err := runCmd(exec.Command(ociBin, "network", "ls", fmt.Sprintf("--filter=label=%s", label), "--format", "{{.Name}}"))
if err != nil {
return nil, err
}
Expand All @@ -235,14 +274,14 @@ func networkNamesByLabel(ociBin string, label string) ([]string, error) {
}

// DeleteKICNetworks deletes all networks created by kic
func DeleteKICNetworks() []error {
func DeleteKICNetworks(ociBin string) []error {
var errs []error
ns, err := networkNamesByLabel(Docker, CreatedByLabelKey+"=true")
ns, err := networkNamesByLabel(ociBin, CreatedByLabelKey)
if err != nil {
return []error{errors.Wrap(err, "list all volume")}
}
for _, n := range ns {
err := RemoveNetwork(n)
err := RemoveNetwork(ociBin, n)
if err != nil {
errs = append(errs, err)
}
Expand Down
11 changes: 6 additions & 5 deletions pkg/drivers/kic/oci/oci.go
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,12 @@ func CreateContainerNode(p CreateParams) error {
// label th enode wuth the node ID
"--label", p.NodeLabel,
}
// to provide a static IP
if p.Network != "" && p.IP != "" {
runArgs = append(runArgs, "--network", p.Network)
runArgs = append(runArgs, "--ip", p.IP)
}

memcgSwap := true
if runtime.GOOS == "linux" {
if _, err := os.Stat("/sys/fs/cgroup/memory/memsw.limit_in_bytes"); os.IsNotExist(err) {
Expand All @@ -170,11 +176,6 @@ 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")
Expand Down