From 89940eb48dff5e37e42fcc18c219e440dd6c2003 Mon Sep 17 00:00:00 2001 From: Patrick Bogen Date: Thu, 19 May 2016 10:57:23 -0700 Subject: [PATCH] Write tests to include testing determinancy of various slice orders; ensure that container order is deterministic --- retrieval/discovery/kubernetes/discovery.go | 10 +- .../discovery/kubernetes/discovery_test.go | 244 ++++++++++-------- 2 files changed, 141 insertions(+), 113 deletions(-) diff --git a/retrieval/discovery/kubernetes/discovery.go b/retrieval/discovery/kubernetes/discovery.go index feba251566..781bb314af 100644 --- a/retrieval/discovery/kubernetes/discovery.go +++ b/retrieval/discovery/kubernetes/discovery.go @@ -904,6 +904,12 @@ func (a ByContainerPort) Len() int { return len(a) } func (a ByContainerPort) Less(i, j int) bool { return a[i].ContainerPort < a[j].ContainerPort } func (a ByContainerPort) Swap(i, j int) { a[i], a[j] = a[j], a[i] } +type ByContainerName []Container + +func (a ByContainerName) Len() int { return len(a) } +func (a ByContainerName) Less(i, j int) bool { return a[i].Name < a[j].Name } +func (a ByContainerName) Swap(i, j int) { a[i], a[j] = a[j], a[i] } + func updatePodTargets(pod *Pod, allContainers bool) []model.LabelSet { var targets []model.LabelSet = make([]model.LabelSet, 0, len(pod.PodSpec.Containers)) if pod.PodStatus.PodIP == "" { @@ -923,6 +929,8 @@ func updatePodTargets(pod *Pod, allContainers bool) []model.LabelSet { } } + sort.Sort(ByContainerName(pod.PodSpec.Containers)) + for _, container := range pod.PodSpec.Containers { // Collect a list of TCP ports // Sort by port number, ascending @@ -959,7 +967,7 @@ func updatePodTargets(pod *Pod, allContainers bool) []model.LabelSet { portLabel.WriteString("=") portLabel.WriteString(strconv.FormatInt(int64(port.ContainerPort), 10)) portLabel.WriteString(",") - t[model.LabelName(podContainerPortMapPrefix + port.Name)] = model.LabelValue(strconv.FormatInt(int64(port.ContainerPort), 10)) + t[model.LabelName(podContainerPortMapPrefix+port.Name)] = model.LabelValue(strconv.FormatInt(int64(port.ContainerPort), 10)) } t[model.LabelName(podContainerPortListLabel)] = model.LabelValue(portLabel.String()) diff --git a/retrieval/discovery/kubernetes/discovery_test.go b/retrieval/discovery/kubernetes/discovery_test.go index bcce56f944..78144791c3 100644 --- a/retrieval/discovery/kubernetes/discovery_test.go +++ b/retrieval/discovery/kubernetes/discovery_test.go @@ -15,6 +15,7 @@ package kubernetes import ( "flag" + "math/rand" "os" "testing" @@ -27,72 +28,82 @@ func TestMain(m *testing.M) { os.Exit(m.Run()) } -var containerA = Container{ - Name: "a", - Ports: []ContainerPort{ - ContainerPort{ - Name: "http", - ContainerPort: 80, - Protocol: "TCP", - }, +var portsA = []ContainerPort{ + ContainerPort{ + Name: "http", + ContainerPort: 80, + Protocol: "TCP", }, } -var containerB = Container{ - Name: "b", - Ports: []ContainerPort{ - ContainerPort{ - Name: "https", - ContainerPort: 443, - Protocol: "TCP", - }, +var portsB = []ContainerPort{ + ContainerPort{ + Name: "https", + ContainerPort: 443, + Protocol: "TCP", }, } -var containerNoTcp = Container{ - Name: "no-tcp", - Ports: []ContainerPort{ - ContainerPort{ - Name: "dns", - ContainerPort: 53, - Protocol: "UDP", - }, +var portsNoTcp = []ContainerPort{ + ContainerPort{ + Name: "dns", + ContainerPort: 53, + Protocol: "UDP", }, } -var containerMultiA = Container{ - Name: "a", - Ports: []ContainerPort{ - ContainerPort{ - Name: "http", - ContainerPort: 80, - Protocol: "TCP", - }, - ContainerPort{ - Name: "ssh", - ContainerPort: 22, - Protocol: "TCP", - }, +var portsMultiA = []ContainerPort{ + ContainerPort{ + Name: "http", + ContainerPort: 80, + Protocol: "TCP", + }, + ContainerPort{ + Name: "ssh", + ContainerPort: 22, + Protocol: "TCP", }, } -var containerMultiB = Container{ - Name: "b", - Ports: []ContainerPort{ - ContainerPort{ - Name: "http", - ContainerPort: 80, - Protocol: "TCP", - }, - ContainerPort{ - Name: "https", - ContainerPort: 443, - Protocol: "TCP", - }, +var portsMultiB = []ContainerPort{ + ContainerPort{ + Name: "http", + ContainerPort: 80, + Protocol: "TCP", }, + ContainerPort{ + Name: "https", + ContainerPort: 443, + Protocol: "TCP", + }, +} + +func container(name string, ports []ContainerPort) Container { + p := make([]ContainerPort, len(ports)) + copy(p, ports) + + // Shuffle order of ports to ensure code enforces determinism + for i := range p { + j := rand.Intn(i + 1) + p[i], p[j] = p[j], p[i] + } + + return Container{ + Name: name, + Ports: p, + } } func pod(name string, containers []Container) *Pod { + c := make([]Container, len(containers)) + copy(c, containers) + + // Shuffle order of containers to ensure code enforces determinism + for i := range c { + j := rand.Intn(i + 1) + c[i], c[j] = c[j], c[i] + } + return &Pod{ ObjectMeta: ObjectMeta{ Name: name, @@ -108,7 +119,7 @@ func pod(name string, containers []Container) *Pod { }, }, PodSpec: PodSpec{ - Containers: containers, + Containers: c, }, } } @@ -116,72 +127,81 @@ func pod(name string, containers []Container) *Pod { func TestUpdatePodTargets(t *testing.T) { var result []model.LabelSet - // Return no targets for a pod that isn't "Running" - result = updatePodTargets(&Pod{PodStatus: PodStatus{PodIP: "1.1.1.1"}}, true) - if len(result) > 0 { - t.Fatalf("expected 0 targets, received %d", len(result)) - } + // Multiple iterations help ensure that we'll see different permutations via the various randomizations that occur + for i := 0; i < 100; i++ { + // Return no targets for a pod that isn't "Running" + result = updatePodTargets(&Pod{PodStatus: PodStatus{PodIP: "1.1.1.1"}}, true) + if len(result) > 0 { + t.Fatalf("expected 0 targets, received %d", len(result)) + } - // Return no targets for a pod with no IP - result = updatePodTargets(&Pod{PodStatus: PodStatus{Phase: "Running"}}, true) - if len(result) > 0 { - t.Fatalf("expected 0 targets, received %d", len(result)) - } + // Return no targets for a pod with no IP + result = updatePodTargets(&Pod{PodStatus: PodStatus{Phase: "Running"}}, true) + if len(result) > 0 { + t.Fatalf("expected 0 targets, received %d", len(result)) + } - // A pod with no containers (?!) should not produce any targets - result = updatePodTargets(pod("empty", []Container{}), true) - if len(result) > 0 { - t.Fatalf("expected 0 targets, received %d", len(result)) - } + // A pod with no containers (?!) should not produce any targets + result = updatePodTargets(pod("empty", []Container{}), true) + if len(result) > 0 { + t.Fatalf("expected 0 targets, received %d", len(result)) + } - // A pod with all valid containers should return one target per container with allContainers=true - result = updatePodTargets(pod("easy", []Container{containerA, containerB}), true) - if len(result) != 2 { - t.Fatalf("expected 2 targets, received %d", len(result)) - } - if result[0][podReadyLabel] != "true" { - t.Fatalf("expected result[0] podReadyLabel 'true', received '%s'", result[0][podReadyLabel]) - } - if _, ok := result[0][podContainerPortMapPrefix + "http"]; !ok { - t.Fatalf("expected result[0][podContainerPortMapPrefix + 'http'] to be '80', but was missing") - } - if result[0][podContainerPortMapPrefix + "http"] != "80" { - t.Fatalf("expected result[0][podContainerPortMapPrefix + 'http'] to be '80', but was %s", result[0][podContainerPortMapPrefix + "http"]) - } - if _, ok := result[1][podContainerPortMapPrefix + "https"]; !ok { - t.Fatalf("expected result[1][podContainerPortMapPrefix + 'https'] to be '443', but was missing") - } - if result[1][podContainerPortMapPrefix + "https"] != "443" { - t.Fatalf("expected result[1][podContainerPortMapPrefix + 'https'] to be '443', but was %s", result[1][podContainerPortMapPrefix + "https"]) - } + // A pod with all valid containers should return one target per container with allContainers=true + result = updatePodTargets(pod("easy", []Container{container("a", portsA), container("b", portsB)}), true) + if len(result) != 2 { + t.Fatalf("expected 2 targets, received %d", len(result)) + } + if result[0][podReadyLabel] != "true" { + t.Fatalf("expected result[0] podReadyLabel 'true', received '%s'", result[0][podReadyLabel]) + } + if _, ok := result[0][podContainerPortMapPrefix+"http"]; !ok { + t.Fatalf("expected result[0][podContainerPortMapPrefix + 'http'] to be '80', but was missing") + } + if result[0][podContainerPortMapPrefix+"http"] != "80" { + t.Fatalf("expected result[0][podContainerPortMapPrefix + 'http'] to be '80', but was %s", result[0][podContainerPortMapPrefix+"http"]) + } + if _, ok := result[1][podContainerPortMapPrefix+"https"]; !ok { + t.Fatalf("expected result[1][podContainerPortMapPrefix + 'https'] to be '443', but was missing") + } + if result[1][podContainerPortMapPrefix+"https"] != "443" { + t.Fatalf("expected result[1][podContainerPortMapPrefix + 'https'] to be '443', but was %s", result[1][podContainerPortMapPrefix+"https"]) + } - // A pod with all valid containers should return one target with allContainers=false - result = updatePodTargets(pod("easy", []Container{containerA, containerB}), false) - if len(result) != 1 { - t.Fatalf("expected 1 targets, received %d", len(result)) - } + // A pod with all valid containers should return one target with allContainers=false, and it should be the alphabetically first container + result = updatePodTargets(pod("easy", []Container{container("a", portsA), container("b", portsB)}), false) + if len(result) != 1 { + t.Fatalf("expected 1 targets, received %d", len(result)) + } + if _, ok := result[0][podContainerNameLabel]; !ok { + t.Fatalf("expected result[0][podContainerNameLabel] to be 'a', but was missing") + } + if result[0][podContainerNameLabel] != "a" { + t.Fatalf("expected result[0][podContainerNameLabel] to be 'a', but was '%s'", result[0][podContainerNameLabel]) + } - // A pod with some non-targetable containers should return one target per targetable container with allContainers=true - result = updatePodTargets(pod("mixed", []Container{containerA, containerNoTcp, containerB}), true) - if len(result) != 2 { - t.Fatalf("expected 2 targets, received %d", len(result)) - } + // A pod with some non-targetable containers should return one target per targetable container with allContainers=true + result = updatePodTargets(pod("mixed", []Container{container("a", portsA), container("no-tcp", portsNoTcp), container("b", portsB)}), true) + if len(result) != 2 { + t.Fatalf("expected 2 targets, received %d", len(result)) + } - // A pod with a container with multiple ports should return the numerically smallest port - result = updatePodTargets(pod("hard", []Container{containerMultiA, containerMultiB}), true) - if len(result) != 2 { - t.Fatalf("expected 2 targets, received %d", len(result)) - } - if result[0][model.AddressLabel] != "1.1.1.1:22" { - t.Fatalf("expected result[0] address to be 1.1.1.1:22, received %s", result[0][model.AddressLabel]) - } - if result[0][podContainerPortListLabel] != ",ssh=22,http=80," { - t.Fatalf("expected result[0] podContainerPortListLabel to be ',ssh=22,http=80,', received '%s'", result[0][podContainerPortListLabel]) - } - if result[1][model.AddressLabel] != "1.1.1.1:80" { - t.Fatalf("expected result[1] address to be 1.1.1.1:80, received %s", result[1][model.AddressLabel]) - } - if result[1][podContainerPortListLabel] != ",http=80,https=443," { - t.Fatalf("expected result[1] podContainerPortListLabel to be ',http=80,https=443,', received '%s'", result[1][podContainerPortListLabel]) + // A pod with a container with multiple ports should return the numerically smallest port + result = updatePodTargets(pod("hard", []Container{container("multiA", portsMultiA), container("multiB", portsMultiB)}), true) + if len(result) != 2 { + t.Fatalf("expected 2 targets, received %d", len(result)) + } + if result[0][model.AddressLabel] != "1.1.1.1:22" { + t.Fatalf("expected result[0] address to be 1.1.1.1:22, received %s", result[0][model.AddressLabel]) + } + if result[0][podContainerPortListLabel] != ",ssh=22,http=80," { + t.Fatalf("expected result[0] podContainerPortListLabel to be ',ssh=22,http=80,', received '%s'", result[0][podContainerPortListLabel]) + } + if result[1][model.AddressLabel] != "1.1.1.1:80" { + t.Fatalf("expected result[1] address to be 1.1.1.1:80, received %s", result[1][model.AddressLabel]) + } + if result[1][podContainerPortListLabel] != ",http=80,https=443," { + t.Fatalf("expected result[1] podContainerPortListLabel to be ',http=80,https=443,', received '%s'", result[1][podContainerPortListLabel]) + } } }