From 268009541411ef83ff0d33bbe4a2c2143a593ef0 Mon Sep 17 00:00:00 2001 From: Dan Winship Date: Sat, 3 Oct 2020 09:10:01 -0400 Subject: [PATCH] kubelet: Remove unnecessary sorting in NodeAddress tests Several of the tests in TestNodeAddress() were no-ops because the test code was only testing that NodeAddresses() returned all of the expected addresses, but not testing that it was returning them in the correct order. The order that NodeAddresses() returns addresses in is very important, so fix the tests to actually test it. One existing test ("NodeIP is external") had its expectedAddresses in the wrong order, but it seems clear from the name of the test that this isn't actually what it expected. Also, previously testKubeletHostname was "127.0.0.1" which ended up interacting weirdly with the IPv4-vs-IPv6 sorting code in a way that made some of the test results confusing if you didn't realize that testKubeletHostname was an IPv4 address. Fix that by making it an actual hostname instead, which then preserves the expected sorting. --- pkg/kubelet/nodestatus/setters_test.go | 21 ++------------------- 1 file changed, 2 insertions(+), 19 deletions(-) diff --git a/pkg/kubelet/nodestatus/setters_test.go b/pkg/kubelet/nodestatus/setters_test.go index d3ac8a3dc95..66105ce8df1 100644 --- a/pkg/kubelet/nodestatus/setters_test.go +++ b/pkg/kubelet/nodestatus/setters_test.go @@ -50,7 +50,7 @@ import ( ) const ( - testKubeletHostname = "127.0.0.1" + testKubeletHostname = "hostname" ) // TODO(mtaufen): below is ported from the old kubelet_node_status_test.go code, potentially add more test coverage for NodeAddress setter in future @@ -86,8 +86,8 @@ func TestNodeAddress(t *testing.T) { {Type: v1.NodeHostName, Address: testKubeletHostname}, }, expectedAddresses: []v1.NodeAddress{ - {Type: v1.NodeInternalIP, Address: "10.1.1.1"}, {Type: v1.NodeExternalIP, Address: "55.55.55.55"}, + {Type: v1.NodeInternalIP, Address: "10.1.1.1"}, {Type: v1.NodeHostName, Address: testKubeletHostname}, }, shouldError: false, @@ -433,10 +433,6 @@ func TestNodeAddress(t *testing.T) { return } - // Sort both sets for consistent equality - sortNodeAddresses(testCase.expectedAddresses) - sortNodeAddresses(existingNode.Status.Addresses) - assert.True(t, apiequality.Semantic.DeepEqual(testCase.expectedAddresses, existingNode.Status.Addresses), "Diff: %s", diff.ObjectDiff(testCase.expectedAddresses, existingNode.Status.Addresses)) }) @@ -1678,19 +1674,6 @@ func TestVolumeLimits(t *testing.T) { // Test Helpers: -// sortableNodeAddress is a type for sorting []v1.NodeAddress -type sortableNodeAddress []v1.NodeAddress - -func (s sortableNodeAddress) Len() int { return len(s) } -func (s sortableNodeAddress) Less(i, j int) bool { - return (string(s[i].Type) + s[i].Address) < (string(s[j].Type) + s[j].Address) -} -func (s sortableNodeAddress) Swap(i, j int) { s[j], s[i] = s[i], s[j] } - -func sortNodeAddresses(addrs sortableNodeAddress) { - sort.Sort(addrs) -} - // testEvent is used to record events for tests type testEvent struct { eventType string