Merge pull request #1508 from BenTheElder/use-the-network-luke

Use the network luke

images/base/files/usr/local/bin/entrypoint

@@ -179,6 +179,15 @@ enable_network_magic(){
   # now we can ensure that DNS is configured to use our IP
   cp /etc/resolv.conf /etc/resolv.conf.original
   sed -e "s/${docker_embedded_dns_ip}/${docker_host_ip}/g" /etc/resolv.conf.original >/etc/resolv.conf
+
+  # fixup IPs in manifests ...
+  # TODO: ensure this supports rebooting the host w/ IPv6 clusters
+  curr_ip=$(hostname --ip-address)
+  if [ -f /kind/old-ip ]; then
+    old_ip=$(cat /kind/old-ip)
+    sed -i "s#${old_ip}#${curr_ip}#" /etc/kubernetes/manifests/*.yaml /var/lib/kubelet/kubeadm-flags.env
+  fi
+  echo -n "${curr_ip}" >/kind/old-ip
 }
 
 # run pre-init fixups

pkg/apis/config/defaults/image.go

@@ -18,4 +18,4 @@ limitations under the License.
 package defaults
 
 // Image is the default for the Config.Image field, aka the default node image.
-const Image = "kindest/node:v1.17.2@sha256:59df31fc61d1da5f46e8a61ef612fa53d3f9140f82419d1ef1a6b9656c6b737c"
+const Image = "kindest/node:v1.18.2@sha256:be1c355162689e7ca48b4a3fe490695edccae001ff71cfd27686e865786278d8"

pkg/build/nodeimage/const_storage.go

@@ -21,7 +21,7 @@ The default PV driver manifest and images are provisionally rancher.io/local-pat
 NOTE: we have customized it in the following ways:
 - storage is under /var instead of /opt
 - debian-base is used as the helper image (k8s already ships this upstream as the base for many images) instead of busybox
-- schedule to "master" kubeadm nodes (control-plane host)
+- schedule to linux nodes
 - install as the default storage class
 */
 
@@ -87,7 +87,7 @@ spec:
         app: local-path-provisioner
     spec:
       nodeSelector:
-        node-role.kubernetes.io/master: ''
+        kubernetes.io/os: linux
       tolerations:
       - key: node-role.kubernetes.io/master
         operator: Equal

pkg/build/nodeimage/defaults.go

@@ -20,7 +20,7 @@ package nodeimage
 const DefaultImage = "kindest/node:latest"
 
 // DefaultBaseImage is the default base image used
-const DefaultBaseImage = "kindest/base:v20200423-588de789"
+const DefaultBaseImage = "kindest/base:v20200423-30be2258"
 
 // DefaultMode is the default kubernetes build mode for the built image
 // see pkg/build/kube.Bits

pkg/cluster/internal/context/context.go

@@ -74,6 +74,11 @@ func (c *Context) GetAPIServerEndpoint() (string, error) {
 	return c.provider.GetAPIServerEndpoint(c.Name())
 }
 
+// GetAPIServerInternalEndpoint returns the cluster's internal API Server endpoint
+func (c *Context) GetAPIServerInternalEndpoint() (string, error) {
+	return c.provider.GetAPIServerInternalEndpoint(c.Name())
+}
+
 // ListNodes returns the list of container IDs for the "nodes" in the cluster
 func (c *Context) ListNodes() ([]nodes.Node, error) {
 	return c.provider.ListNodes(c.name)

pkg/cluster/internal/create/actions/config/config.go

@@ -51,19 +51,11 @@ func (a *Action) Execute(ctx *actions.ActionContext) error {
 		return err
 	}
 
-	// get the control plane endpoint, in case the cluster has an external load balancer in
-	// front of the control-plane nodes
-	controlPlaneEndpoint, controlPlaneEndpointIPv6, err := nodeutils.GetControlPlaneEndpoint(allNodes)
+	controlPlaneEndpoint, err := ctx.ClusterContext.GetAPIServerInternalEndpoint()
 	if err != nil {
-		// TODO(bentheelder): logging here
 		return err
 	}
 
-	// configure the right protocol addresses
-	if ctx.Config.Networking.IPFamily == "ipv6" {
-		controlPlaneEndpoint = controlPlaneEndpointIPv6
-	}
-
 	// create kubeadm init config
 	fns := []func() error{}
 

pkg/cluster/internal/providers/docker/network.go

@@ -0,0 +1,60 @@
+/*
+Copyright 2020 The Kubernetes Authors.
+
+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 docker
+
+import (
+	"regexp"
+
+	"sigs.k8s.io/kind/pkg/exec"
+
+	"sigs.k8s.io/kind/pkg/internal/apis/config"
+)
+
+// TODO: we'll probably allow configuring this
+//
+// however currently picking a single network is equivalent to the previous
+// behavior *except* that we moved from the default bridge to a user defined
+// network because the default bridge is actually special versus any other
+// docker network and lacks the emebdded DNS
+//
+// for now this also makes it easier for apps to join the same network
+const fixedNetworkName = "kind"
+
+// ensureNetwork checks if docker network by name exists, if not it creates it
+func ensureNetwork(name string, ipFamily config.ClusterIPFamily) error {
+	// TODO: the network might already exist and not have ipv6 ... :|
+	// discussion: https://github.com/kubernetes-sigs/kind/pull/1508#discussion_r414594198
+	out, err := exec.Output(exec.Command(
+		"docker", "network", "ls",
+		"--filter=name=^"+regexp.QuoteMeta(name)+"$",
+		"--format={{.Name}}",
+	))
+	if err != nil {
+		return err
+	}
+	// network already exists
+	if string(out) == name+"\n" {
+		return nil
+	}
+	// TODO: ipv6 subnet should probably not be fixed
+	// Though maybe just require the user to handle this by creating the network
+	// as they desire before running kind ...
+	if ipFamily == config.IPv6Family {
+		return exec.Command("docker", "network", "create", "-d=bridge", "--ipv6", "--subnet=fc00:db8:2::/64", name).Run()
+	}
+	return exec.Command("docker", "network", "create", "-d=bridge", name).Run()
+}

pkg/cluster/internal/providers/docker/provider.go

@@ -58,6 +58,10 @@ func (p *Provider) Provision(status *cli.Status, cluster string, cfg *config.Clu
 		return err
 	}
 
+	if err := ensureNetwork(fixedNetworkName, cfg.Networking.IPFamily); err != nil {
+		return errors.Wrap(err, "failed to ensure docker network")
+	}
+
 	// actually provision the cluster
 	icons := strings.Repeat("📦 ", len(cfg.Nodes))
 	status.Start(fmt.Sprintf("Preparing nodes %s", icons))
@@ -169,6 +173,21 @@ func (p *Provider) GetAPIServerEndpoint(cluster string) (string, error) {
 	return net.JoinHostPort(parts[0], parts[1]), nil
 }
 
+// GetAPIServerInternalEndpoint is part of the providers.Provider interface
+func (p *Provider) GetAPIServerInternalEndpoint(cluster string) (string, error) {
+	// locate the node that hosts this
+	allNodes, err := p.ListNodes(cluster)
+	if err != nil {
+		return "", errors.Wrap(err, "failed to list nodes")
+	}
+	n, err := nodeutils.APIServerEndpointNode(allNodes)
+	if err != nil {
+		return "", errors.Wrap(err, "failed to get api server endpoint")
+	}
+	// NOTE: we're using the nodes's hostnames which are their names
+	return net.JoinHostPort(n.String(), fmt.Sprintf("%d", common.APIServerInternalPort)), nil
+}
+
 // node returns a new node handle for this provider
 func (p *Provider) node(name string) nodes.Node {
 	return &node{

pkg/cluster/internal/providers/docker/provision.go

@@ -144,6 +144,33 @@ func commonArgs(cluster string, cfg *config.Cluster) ([]string, error) {
 		"--tty",    // allocate a tty for entrypoint logs
 		// label the node with the cluster ID
 		"--label", fmt.Sprintf("%s=%s", clusterLabelKey, cluster),
+		// user a user defined docker network so we get embedded DNS
+		"--net", fixedNetworkName,
+		// Docker supports the following restart modes:
+		// - no
+		// - on-failure[:max-retries]
+		// - unless-stopped
+		// - always
+		// https://docs.docker.com/engine/reference/commandline/run/#restart-policies---restart
+		//
+		// What we desire is:
+		// - restart on host / dockerd reboot
+		// - don't restart for any other reason
+		//
+		// This means:
+		// - no is out of the question ... it never restarts
+		// - always is a poor choice, we'll keep trying to restart nodes that were
+		// never going to work
+		// - unless-stopped will also retry failures indefinitely, similar to always
+		// except that it won't restart when the container is `docker stop`ed
+		// - on-failure is not great, we're only interested in restarting on
+		// reboots, not failures. *however* we can limit the number of retries
+		// *and* it forgets all state on dockerd restart and retries anyhow.
+		// - on-failure:0 is what we want .. restart on failures, except max
+		// retries is 0, so only restart on reboots.
+		// however this _actually_ means the same thing as always
+		// so the closest thing is on-failure:1, which will retry *once*
+		"--restart=on-failure:1",
 	}
 
 	// enable IPv6 if necessary

pkg/cluster/internal/providers/podman/provider.go

@@ -195,6 +195,26 @@ func (p *Provider) GetAPIServerEndpoint(cluster string) (string, error) {
 	return "", errors.Errorf("unable to find apiserver endpoint information")
 }
 
+// GetAPIServerInternalEndpoint is part of the providers.Provider interface
+func (p *Provider) GetAPIServerInternalEndpoint(cluster string) (string, error) {
+	// locate the node that hosts this
+	allNodes, err := p.ListNodes(cluster)
+	if err != nil {
+		return "", errors.Wrap(err, "failed to list nodes")
+	}
+	n, err := nodeutils.APIServerEndpointNode(allNodes)
+	if err != nil {
+		return "", errors.Wrap(err, "failed to get apiserver endpoint")
+	}
+	// TODO: check cluster IP family and return the correct IP
+	// This means IPv6 singlestack is broken on podman
+	ipv4, _, err := n.IP()
+	if err != nil {
+		return "", errors.Wrap(err, "failed to get apiserver IP")
+	}
+	return ipv4, nil
+}
+
 // node returns a new node handle for this provider
 func (p *Provider) node(name string) nodes.Node {
 	return &node{

pkg/cluster/internal/providers/provider/provider.go

@@ -41,6 +41,8 @@ type Provider interface {
 	DeleteNodes([]nodes.Node) error
 	// GetAPIServerEndpoint returns the host endpoint for the cluster's API server
 	GetAPIServerEndpoint(cluster string) (string, error)
+	// GetAPIServerEndpoint returns the internal network endpoint for the cluster's API server
+	GetAPIServerInternalEndpoint(cluster string) (string, error)
 	// CollectLogs will populate dir with cluster logs and other debug files
 	CollectLogs(dir string, nodes []nodes.Node) error
 }

pkg/cluster/nodeutils/util.go

@@ -19,38 +19,15 @@ package nodeutils
 import (
 	"bytes"
 	"encoding/json"
-	"fmt"
 	"io"
 	"path/filepath"
 	"strings"
 
 	"sigs.k8s.io/kind/pkg/cluster/nodes"
 	"sigs.k8s.io/kind/pkg/errors"
 	"sigs.k8s.io/kind/pkg/exec"
-
-	"sigs.k8s.io/kind/pkg/cluster/internal/providers/provider/common"
 )
 
-// GetControlPlaneEndpoint returns the control plane endpoints for IPv4 and IPv6
-// in case the cluster has an external load balancer in front of the control-plane nodes,
-// otherwise return the bootstrap node IPs
-func GetControlPlaneEndpoint(allNodes []nodes.Node) (string, string, error) {
-	node, err := APIServerEndpointNode(allNodes)
-	if err != nil {
-		return "", "", err
-	}
-
-	// gets the control plane IP addresses
-	controlPlaneIPv4, controlPlaneIPv6, err := node.IP()
-	if err != nil {
-		return "", "", errors.Wrapf(err, "failed to get IPs for node: %s", node.String())
-	}
-
-	// TODO: place this in a central constant
-	// TODO: should probably use net.JoinHostPort
-	return fmt.Sprintf("%s:%d", controlPlaneIPv4, common.APIServerInternalPort), fmt.Sprintf("[%s]:%d", controlPlaneIPv6, common.APIServerInternalPort), nil
-}
-
 // KubeVersion returns the Kubernetes version installed on the node
 func KubeVersion(n nodes.Node) (version string, err error) {
 	// grab kubernetes version from the node image

pkg/exec/helpers.go

@@ -86,6 +86,14 @@ func OutputLines(cmd Cmd) (lines []string, err error) {
 	return lines, err
 }
 
+// Output is like os/exec's cmd.Output, but over our Cmd interface
+func Output(cmd Cmd) ([]byte, error) {
+	var buff bytes.Buffer
+	cmd.SetStdout(&buff)
+	err := cmd.Run()
+	return buff.Bytes(), err
+}
+
 // InheritOutput sets cmd's output to write to the current process's stdout and stderr
 func InheritOutput(cmd Cmd) Cmd {
 	cmd.SetStderr(os.Stderr)