diff --git a/pkg/cluster/internal/providers/docker/network.go b/pkg/cluster/internal/providers/docker/network.go index d8306f9494..d73347294c 100644 --- a/pkg/cluster/internal/providers/docker/network.go +++ b/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 { diff --git a/pkg/cluster/internal/providers/docker/network_test.go b/pkg/cluster/internal/providers/docker/network_test.go index 789a0d93fa..73e0e326de 100644 --- a/pkg/cluster/internal/providers/docker/network_test.go +++ b/pkg/cluster/internal/providers/docker/network_test.go @@ -18,9 +18,67 @@ package docker import ( "fmt" + "regexp" "testing" + + "sigs.k8s.io/kind/pkg/errors" + "sigs.k8s.io/kind/pkg/exec" + + "sigs.k8s.io/kind/pkg/internal/assert" + "sigs.k8s.io/kind/pkg/internal/integration" ) +func TestIntegrationEnsureNetworkConcurrent(t *testing.T) { + integration.MaybeSkip(t) + + testNetworkName := "integration-test-ensure-kind-network" + + // cleanup + cleanup := func() { + ids, _ := networksWithName(testNetworkName) + if len(ids) > 0 { + _ = deleteNetworks(ids...) + } + } + cleanup() + defer cleanup() + + // this is more than enough to trigger race conditions + networkConcurrency := 10 + + // Create multiple networks concurrently + errCh := make(chan error, networkConcurrency) + for i := 0; i < networkConcurrency; i++ { + go func() { + errCh <- ensureNetwork(testNetworkName) + }() + } + for i := 0; i < networkConcurrency; i++ { + if err := <-errCh; err != nil { + t.Errorf("error creating network: %v", err) + rerr := exec.RunErrorForError(err) + if rerr != nil { + t.Errorf("%q", rerr.Output) + } + t.Errorf("%+v", errors.StackTrace(err)) + } + } + + cmd := exec.Command( + "docker", "network", "ls", + fmt.Sprintf("--filter=name=^%s$", regexp.QuoteMeta(testNetworkName)), + "--format={{.Name}}", + ) + + lines, err := exec.OutputLines(cmd) + if err != nil { + t.Errorf("obtaining the docker networks") + } + if len(lines) != 1 { + t.Errorf("wrong number of networks created: %d", len(lines)) + } +} + func Test_generateULASubnetFromName(t *testing.T) { t.Parallel() cases := []struct { @@ -66,3 +124,97 @@ func Test_generateULASubnetFromName(t *testing.T) { }) } } + +func Test_sortNetworkInspectEntries(t *testing.T) { + cases := []struct { + Name string + Networks []networkInspectEntry + Sorted []networkInspectEntry + }{ + { + Name: "simple ID sort", + Networks: []networkInspectEntry{ + { + ID: "dc7f897c237215c3b73d2c9ba1d4e116d872793a6c1c0e5bf083762998de8b4e", + }, + { + ID: "1ed9912325a0d08594ee786de91ebd961e631643877b5ee58ec906b640813eae", + }, + }, + Sorted: []networkInspectEntry{ + { + ID: "1ed9912325a0d08594ee786de91ebd961e631643877b5ee58ec906b640813eae", + }, + { + ID: "dc7f897c237215c3b73d2c9ba1d4e116d872793a6c1c0e5bf083762998de8b4e", + }, + }, + }, + { + Name: "containers attached sort", + Networks: []networkInspectEntry{ + { + ID: "1ed9912325a0d08594ee786de91ebd961e631643877b5ee58ec906b640813eae", + }, + { + ID: "dc7f897c237215c3b73d2c9ba1d4e116d872793a6c1c0e5bf083762998de8b4e", + Containers: map[string]map[string]string{ + "a37779e06f3b694eba491dd450aad18bbbaa0a0fce2952e7c9195ea45ae79d41": { + "Name": "buildx_buildkit_kind-builder0", + "EndpointID": "8f6411fb4360059b2f91028f91ef03130abc96d6381afc265ce53c9df89d5a3d", + }, + }, + }, + { + ID: "f0445f08b9989921da00250d778975202267fbab364e5fbad0ceb6db24f3f91e", + }, + { + ID: "128154205c7d88c7bb9c255d389bc9e222b58a48cf83619976e7665a48e79918", + Containers: map[string]map[string]string{ + "aad18bbbaa0a0fce2952e7c9195ea45ae79d41a37779e06f3b694eba491dd450": { + "Name": "fakey-fake", + "EndpointID": "f03130abc96d6381afc265ce53c9df89d5a3d8f6411fb4360059b2f91028f91e", + }, + }, + }, + }, + Sorted: []networkInspectEntry{ + { + ID: "128154205c7d88c7bb9c255d389bc9e222b58a48cf83619976e7665a48e79918", + Containers: map[string]map[string]string{ + "aad18bbbaa0a0fce2952e7c9195ea45ae79d41a37779e06f3b694eba491dd450": { + "Name": "fakey-fake", + "EndpointID": "f03130abc96d6381afc265ce53c9df89d5a3d8f6411fb4360059b2f91028f91e", + }, + }, + }, + { + ID: "dc7f897c237215c3b73d2c9ba1d4e116d872793a6c1c0e5bf083762998de8b4e", + Containers: map[string]map[string]string{ + "a37779e06f3b694eba491dd450aad18bbbaa0a0fce2952e7c9195ea45ae79d41": { + "Name": "buildx_buildkit_kind-builder0", + "EndpointID": "8f6411fb4360059b2f91028f91ef03130abc96d6381afc265ce53c9df89d5a3d", + }, + }, + }, + { + ID: "1ed9912325a0d08594ee786de91ebd961e631643877b5ee58ec906b640813eae", + }, + { + ID: "f0445f08b9989921da00250d778975202267fbab364e5fbad0ceb6db24f3f91e", + }, + }, + }, + } + for _, tc := range cases { + tc := tc + t.Run(tc.Name, func(t *testing.T) { + toSort := make([]networkInspectEntry, len(tc.Networks)) + for i := range tc.Networks { + toSort[i] = tc.Networks[i] + } + sortNetworkInspectEntries(toSort) + assert.DeepEqual(t, tc.Sorted, toSort) + }) + } +} diff --git a/pkg/internal/integration/integration.go b/pkg/internal/integration/integration.go new file mode 100644 index 0000000000..9eb5480df9 --- /dev/null +++ b/pkg/internal/integration/integration.go @@ -0,0 +1,33 @@ +/* +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 integration + +import "testing" + +// *testing.T methods used by assert +type testingDotT interface { + Skip(args ...interface{}) +} + +// MaybeSkip skips if integration tests should be skipped +// currently this is when testing.Short() is true +// This should be called at the beginning of an integration test +func MaybeSkip(t testingDotT) { + if testing.Short() { + t.Skip("Skipping integration test due to -short") + } +}