Merge pull request #1831 from BenTheElder/no-dupe-networks

fix ambiguous networks

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

@@ -17,13 +17,17 @@ limitations under the License.
 package docker
 
 import (
+	"bytes"
 	"crypto/sha1"
 	"encoding/binary"
-	"errors"
+	"encoding/json"
+	"io"
 	"net"
 	"regexp"
+	"sort"
 	"strings"
 
+	"sigs.k8s.io/kind/pkg/errors"
 	"sigs.k8s.io/kind/pkg/exec"
 )
 
@@ -42,13 +46,15 @@ const fixedNetworkName = "kind"
 
 // ensureNetwork checks if docker network by name exists, if not it creates it
 func ensureNetwork(name string) error {
-	// TODO: the network might already exist and not have ipv6 ... :|
-	// discussion: https://github.com/kubernetes-sigs/kind/pull/1508#discussion_r414594198
-	exists, err := checkIfNetworkExists(name)
+	// check if network exists already and remove any duplicate networks
+	exists, err := removeDuplicateNetworks(name)
 	if err != nil {
 		return err
 	}
+
 	// network already exists, we're good
+	// TODO: the network might already exist and not have ipv6 ... :|
+	// discussion: https://github.com/kubernetes-sigs/kind/pull/1508#discussion_r414594198
 	if exists {
 		return nil
 	}
@@ -57,7 +63,7 @@ func ensureNetwork(name string) error {
 	// obtained from the ULA fc00::/8 range
 	// Make N attempts with "probing" in case we happen to collide
 	subnet := generateULASubnetFromName(name, 0)
-	err = createNetwork(name, subnet)
+	err = createNetworkNoDuplicates(name, subnet)
 	if err == nil {
 		// Success!
 		return nil
@@ -69,8 +75,20 @@ func ensureNetwork(name string) error {
 	// If it is, make more attempts below
 	if isIPv6UnavailableError(err) {
 		// only one attempt, IPAM is automatic in ipv4 only
-		return createNetwork(name, "")
-	} else if !isPoolOverlapError(err) {
+		return createNetworkNoDuplicates(name, "")
+	}
+	if isPoolOverlapError(err) {
+		// pool overlap suggests perhaps another process created the network
+		// check if network exists already and remove any duplicate networks
+		exists, err := checkIfNetworkExists(name)
+		if err != nil {
+			return err
+		}
+		if exists {
+			return nil
+		}
+		// otherwise we'll start trying with different subnets
+	} else {
 		// unknown error ...
 		return err
 	}
@@ -79,18 +97,51 @@ func ensureNetwork(name string) error {
 	const maxAttempts = 5
 	for attempt := int32(1); attempt < maxAttempts; attempt++ {
 		subnet := generateULASubnetFromName(name, attempt)
-		err = createNetwork(name, subnet)
+		err = createNetworkNoDuplicates(name, subnet)
 		if err == nil {
 			// success!
 			return nil
-		} else if !isPoolOverlapError(err) {
-			// unknown error ...
-			return err
 		}
+		if isPoolOverlapError(err) {
+			// pool overlap suggests perhaps another process created the network
+			// check if network exists already and remove any duplicate networks
+			exists, err := checkIfNetworkExists(name)
+			if err != nil {
+				return err
+			}
+			if exists {
+				return nil
+			}
+			// otherwise we'll try again
+			continue
+		}
+		// unknown error ...
+		return err
 	}
 	return errors.New("exhausted attempts trying to find a non-overlapping subnet")
 }
 
+func createNetworkNoDuplicates(name, ipv6Subnet string) error {
+	if err := createNetwork(name, ipv6Subnet); err != nil && !isNetworkAlreadyExistsError(err) {
+		return err
+	}
+	_, err := removeDuplicateNetworks(name)
+	return err
+}
+
+func removeDuplicateNetworks(name string) (bool, error) {
+	networks, err := sortedNetworksWithName(name)
+	if err != nil {
+		return false, err
+	}
+	if len(networks) > 1 {
+		if err := deleteNetworks(networks[1:]...); err != nil && !isOnlyErrorNoSuchNetwork(err) {
+			return false, err
+		}
+	}
+	return len(networks) > 0, nil
+}
+
 func createNetwork(name, ipv6Subnet string) error {
 	if ipv6Subnet == "" {
 		return exec.Command("docker", "network", "create", "-d=bridge",
@@ -102,6 +153,81 @@ func createNetwork(name, ipv6Subnet string) error {
 		"--ipv6", "--subnet", ipv6Subnet, name).Run()
 }
 
+func sortedNetworksWithName(name string) ([]string, error) {
+	// query which networks exist with the name
+	ids, err := networksWithName(name)
+	if err != nil {
+		return nil, err
+	}
+	// we can skip sorting if there are less than 2
+	if len(ids) < 2 {
+		return ids, nil
+	}
+	// inspect them to get more detail for sorting
+	networks, err := inspectNetworks(ids)
+	if err != nil {
+		return nil, err
+	}
+	// deterministically sort networks
+	// NOTE: THIS PART IS IMPORTANT!
+	sortNetworkInspectEntries(networks)
+	// return network IDs
+	sortedIDs := make([]string, 0, len(networks))
+	for i := range networks {
+		sortedIDs = append(sortedIDs, networks[i].ID)
+	}
+	return sortedIDs, nil
+}
+
+func sortNetworkInspectEntries(networks []networkInspectEntry) {
+	sort.Slice(networks, func(i, j int) bool {
+		// we want networks with active containers first
+		if len(networks[i].Containers) > len(networks[j].Containers) {
+			return true
+		}
+		return networks[i].ID < networks[j].ID
+	})
+}
+
+func inspectNetworks(networkIDs []string) ([]networkInspectEntry, error) {
+	inspectOut, err := exec.Output(exec.Command("docker", append([]string{"network", "inspect"}, networkIDs...)...))
+	// NOTE: the caller can detect if the network isn't present in the output anyhow
+	// we don't want to fail on this here.
+	if err != nil && !isOnlyErrorNoSuchNetwork(err) {
+		return nil, err
+	}
+	// parse
+	networks := []networkInspectEntry{}
+	if err := json.Unmarshal(inspectOut, &networks); err != nil {
+		return nil, errors.Wrap(err, "failed to decode networks list")
+	}
+	return networks, nil
+}
+
+type networkInspectEntry struct {
+	ID string `json:"Id"`
+	// NOTE: we don't care about the contents here but we need to parse
+	// how many entries exist in the containers map
+	Containers map[string]map[string]string `json:"Containers"`
+}
+
+// networksWithName returns a list of network IDs for networks with this name
+func networksWithName(name string) ([]string, error) {
+	lsOut, err := exec.Output(exec.Command(
+		"docker", "network", "ls",
+		"--filter=name=^"+regexp.QuoteMeta(name)+"$",
+		"--format={{.ID}}", // output as unambiguous IDs
+	))
+	if err != nil {
+		return nil, err
+	}
+	cleaned := strings.TrimSuffix(string(lsOut), "\n")
+	if cleaned == "" { // avoid returning []string{""}
+		return nil, nil
+	}
+	return strings.Split(cleaned, "\n"), nil
+}
+
 func checkIfNetworkExists(name string) (bool, error) {
 	out, err := exec.Output(exec.Command(
 		"docker", "network", "ls",
@@ -121,6 +247,46 @@ func isPoolOverlapError(err error) bool {
 	return rerr != nil && strings.HasPrefix(string(rerr.Output), "Error response from daemon: Pool overlaps with other one on this address space")
 }
 
+func isNetworkAlreadyExistsError(err error) bool {
+	rerr := exec.RunErrorForError(err)
+	return rerr != nil && strings.HasPrefix(string(rerr.Output), "Error response from daemon: network with name") && strings.Contains(string(rerr.Output), "already exists")
+}
+
+// returns true if:
+// - err is nil
+// - err only contains no such network errors
+func isOnlyErrorNoSuchNetwork(err error) bool {
+	rerr := exec.RunErrorForError(err)
+	if rerr == nil {
+		return false
+	}
+	// check all lines of output from errored command
+	b := bytes.NewBuffer(rerr.Output)
+	for {
+		l, err := b.ReadBytes('\n')
+		if err == io.EOF {
+			break
+		} else if err != nil {
+			return false
+		}
+		// if the line begins with Eror: No such network: it's fine
+		s := string(l)
+		if strings.HasPrefix(s, "Error: No such network:") {
+			continue
+		}
+		// other errors are not fine
+		if strings.HasPrefix(s, "Error: ") {
+			return false
+		}
+		// other line contents should just be network references
+	}
+	return true
+}
+
+func deleteNetworks(networks ...string) error {
+	return exec.Command("docker", append([]string{"network", "rm"}, networks...)...).Run()
+}
+
 // generateULASubnetFromName generate an IPv6 subnet based on the
 // name and Nth probing attempt
 func generateULASubnetFromName(name string, attempt int32) string {