From 83868ae921735a48d704f1e25fc5814673f76ec4 Mon Sep 17 00:00:00 2001 From: okokes-akamai <126059888+okokes-akamai@users.noreply.github.com> Date: Wed, 3 Jan 2024 17:05:00 +0100 Subject: [PATCH] Stricter linting (#159) --- .golangci.yml | 50 +++++++---- Makefile | 2 + cloud/linode/common_test.go | 4 - cloud/linode/fake_linode_test.go | 5 +- cloud/linode/instances_test.go | 49 +++++++--- cloud/linode/loadbalancers.go | 6 +- cloud/linode/loadbalancers_test.go | 138 +++++++++++++++-------------- e2e/test/ccm_e2e_test.go | 44 +++++---- e2e/test/ccm_suite_test.go | 6 +- e2e/test/framework/service.go | 17 +--- e2e/test/framework/util.go | 3 +- 11 files changed, 172 insertions(+), 152 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 9ec87ab1..19df20d2 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -1,24 +1,36 @@ +run: + timeout: 5m + linters: + disable-all: true enable: + # these are enabled by default + - errcheck + - gosimple + - govet + - ineffassign + - staticcheck + - typecheck + - unused + # cherry picked from https://golangci-lint.run/usage/linters/ + # - ginkgolinter # to be enabled once #158 is merged + - bodyclose + - exportloopref + - gocheckcompilerdirectives - gofmt - goimports - - structcheck - - varcheck - - staticcheck + - importas + - loggercheck + - makezero + - nilerr + - prealloc + - reassign + - tenv - unconvert - - revive - - ineffassign - - vet - - unused - - misspell - disable: - - errcheck - -run: - tests: false - timeout: 8m - skip-dirs: - - api - - design - - docs - - docs/man + - wastedassign + - unparam + - gofumpt + - nosprintfhostport + - musttag + - exhaustive + - nilnil diff --git a/Makefile b/Makefile index 027f4386..b37d96fb 100644 --- a/Makefile +++ b/Makefile @@ -18,6 +18,8 @@ codegen: lint: docker run --rm -v "$(shell pwd):/var/work:ro" -w /var/work \ golangci/golangci-lint:v1.55.2 golangci-lint run -v --timeout=5m + docker run --rm -v "$(shell pwd):/var/work:ro" -w /var/work/e2e \ + golangci/golangci-lint:v1.55.2 golangci-lint run -v --timeout=5m .PHONY: fmt fmt: diff --git a/cloud/linode/common_test.go b/cloud/linode/common_test.go index 98b4499a..94dcb099 100644 --- a/cloud/linode/common_test.go +++ b/cloud/linode/common_test.go @@ -46,7 +46,3 @@ func TestParseProviderID(t *testing.T) { }) } } - -func stringPtr(s string) *string { - return &s -} diff --git a/cloud/linode/fake_linode_test.go b/cloud/linode/fake_linode_test.go index 283b959f..fa0138a5 100644 --- a/cloud/linode/fake_linode_test.go +++ b/cloud/linode/fake_linode_test.go @@ -240,7 +240,6 @@ func (f *fakeAPI) ServeHTTP(w http.ResponseWriter, r *http.Request) { _, _ = w.Write(rr) return } - } case "POST": @@ -596,9 +595,9 @@ func (f *fakeAPI) ServeHTTP(w http.ResponseWriter, r *http.Request) { } } -func randString(n int) string { +func randString() string { const letterBytes = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ" - b := make([]byte, n) + b := make([]byte, 10) for i := range b { b[i] = letterBytes[rand.Intn(len(letterBytes))] } diff --git a/cloud/linode/instances_test.go b/cloud/linode/instances_test.go index acfba0ab..878b1298 100644 --- a/cloud/linode/instances_test.go +++ b/cloud/linode/instances_test.go @@ -57,10 +57,12 @@ func TestInstanceExists(t *testing.T) { instances := newInstances(client) node := nodeWithProviderID(providerIDPrefix + "123") client.EXPECT().ListInstances(gomock.Any(), nil).Times(1).Return([]linodego.Instance{ - {ID: 123, + { + ID: 123, Label: "mock", Region: "us-east", - Type: "g6-standard-2"}, + Type: "g6-standard-2", + }, }, nil) exists, err := instances.InstanceExists(ctx, node) @@ -154,16 +156,36 @@ func TestMetadataRetrieval(t *testing.T) { expectedErr error }{ {"no IPs", nil, nil, instanceNoIPAddressesError{192910}}, - {"one public, one private", []string{"32.74.121.25", "192.168.121.42"}, - []v1.NodeAddress{{Type: v1.NodeExternalIP, Address: "32.74.121.25"}, {Type: v1.NodeInternalIP, Address: "192.168.121.42"}}, nil}, - {"one public, no private", []string{"32.74.121.25"}, - []v1.NodeAddress{{Type: v1.NodeExternalIP, Address: "32.74.121.25"}}, nil}, - {"one private, no public", []string{"192.168.121.42"}, - []v1.NodeAddress{{Type: v1.NodeInternalIP, Address: "192.168.121.42"}}, nil}, - {"two public addresses", []string{"32.74.121.25", "32.74.121.22"}, - []v1.NodeAddress{{Type: v1.NodeExternalIP, Address: "32.74.121.25"}, {Type: v1.NodeExternalIP, Address: "32.74.121.22"}}, nil}, - {"two private addresses", []string{"192.168.121.42", "10.0.2.15"}, - []v1.NodeAddress{{Type: v1.NodeInternalIP, Address: "192.168.121.42"}, {Type: v1.NodeInternalIP, Address: "10.0.2.15"}}, nil}, + { + "one public, one private", + []string{"32.74.121.25", "192.168.121.42"}, + []v1.NodeAddress{{Type: v1.NodeExternalIP, Address: "32.74.121.25"}, {Type: v1.NodeInternalIP, Address: "192.168.121.42"}}, + nil, + }, + { + "one public, no private", + []string{"32.74.121.25"}, + []v1.NodeAddress{{Type: v1.NodeExternalIP, Address: "32.74.121.25"}}, + nil, + }, + { + "one private, no public", + []string{"192.168.121.42"}, + []v1.NodeAddress{{Type: v1.NodeInternalIP, Address: "192.168.121.42"}}, + nil, + }, + { + "two public addresses", + []string{"32.74.121.25", "32.74.121.22"}, + []v1.NodeAddress{{Type: v1.NodeExternalIP, Address: "32.74.121.25"}, {Type: v1.NodeExternalIP, Address: "32.74.121.22"}}, + nil, + }, + { + "two private addresses", + []string{"192.168.121.42", "10.0.2.15"}, + []v1.NodeAddress{{Type: v1.NodeInternalIP, Address: "192.168.121.42"}, {Type: v1.NodeInternalIP, Address: "10.0.2.15"}}, + nil, + }, } for _, test := range ipTests { @@ -281,7 +303,8 @@ func TestInstanceShutdown(t *testing.T) { id := 12345 node := nodeWithProviderID(providerIDPrefix + strconv.Itoa(id)) client.EXPECT().ListInstances(gomock.Any(), nil).Times(1).Return([]linodego.Instance{ - {ID: id, Label: "shutting-down-linode", Status: linodego.InstanceShuttingDown}}, nil) + {ID: id, Label: "shutting-down-linode", Status: linodego.InstanceShuttingDown}, + }, nil) shutdown, err := instances.InstanceShutdown(ctx, node) assert.NoError(t, err) diff --git a/cloud/linode/loadbalancers.go b/cloud/linode/loadbalancers.go index 8de8bbc8..1565f1a5 100644 --- a/cloud/linode/loadbalancers.go +++ b/cloud/linode/loadbalancers.go @@ -55,9 +55,7 @@ const ( annLinodeNodePrivateIP = "node.k8s.linode.com/private-ip" ) -var ( - errNoNodesAvailable = errors.New("no nodes available for nodebalancer") -) +var errNoNodesAvailable = errors.New("no nodes available for nodebalancer") type lbNotFoundError struct { serviceNn string @@ -547,7 +545,7 @@ func (l *loadbalancers) buildNodeBalancerConfig(ctx context.Context, service *v1 health, err := getHealthCheckType(service) if err != nil { - return linodego.NodeBalancerConfig{}, nil + return linodego.NodeBalancerConfig{}, err } config := linodego.NodeBalancerConfig{ diff --git a/cloud/linode/loadbalancers_test.go b/cloud/linode/loadbalancers_test.go index 684f0353..fd20129b 100644 --- a/cloud/linode/loadbalancers_test.go +++ b/cloud/linode/loadbalancers_test.go @@ -52,6 +52,7 @@ pCLzKBQTXQmeIWJue3/GcA8RLzcGtaTtQTJcAwNZp4V6exA869uDwFzbZA/z9jHJ mmccdLY3hP1Ozwikm5Pecysk+bdx9rbzHbA6xLz8fp5oJYUbyyaqnWLdTZvubpur 2/6vm/KHkJHqFcF/LtIxgaZFnGYR -----END CERTIFICATE-----` + const testKey string = `-----BEGIN RSA PRIVATE KEY----- MIIJKAIBAAKCAgEA1QLQpK2vzg8uczV1Ni4S2Tgc5Ny59vqkwfK20m/mhjEIAlo3 kAj1Bc+omlQUjoaVLWgOmNF71FCCFeyj8iKEP16gQ/XOQdwcnJvpNdOGh9q3FfmM @@ -210,13 +211,13 @@ func TestCCMLoadBalancers(t *testing.T) { } func stubService(fake *fake.Clientset, service *v1.Service) { - fake.CoreV1().Services("").Create(context.TODO(), service, metav1.CreateOptions{}) + _, _ = fake.CoreV1().Services("").Create(context.TODO(), service, metav1.CreateOptions{}) } func testCreateNodeBalancer(t *testing.T, client *linodego.Client, _ *fakeAPI, firewallID *string) error { svc := &v1.Service{ ObjectMeta: metav1.ObjectMeta{ - Name: randString(10), + Name: randString(), UID: "foobar123", Annotations: map[string]string{ annLinodeThrottle: "15", @@ -226,13 +227,13 @@ func testCreateNodeBalancer(t *testing.T, client *linodego.Client, _ *fakeAPI, f Spec: v1.ServiceSpec{ Ports: []v1.ServicePort{ { - Name: randString(10), + Name: randString(), Protocol: "TCP", Port: int32(80), NodePort: int32(30000), }, { - Name: randString(10), + Name: randString(), Protocol: "TCP", Port: int32(8080), NodePort: int32(30001), @@ -320,7 +321,7 @@ func testCreateNodeBalancerWithInvalidFirewall(t *testing.T, client *linodego.Cl func testUpdateLoadBalancerAddAnnotation(t *testing.T, client *linodego.Client, _ *fakeAPI) { svc := &v1.Service{ ObjectMeta: metav1.ObjectMeta{ - Name: randString(10), + Name: randString(), UID: "foobar123", Annotations: map[string]string{ annLinodeThrottle: "15", @@ -329,7 +330,7 @@ func testUpdateLoadBalancerAddAnnotation(t *testing.T, client *linodego.Client, Spec: v1.ServiceSpec{ Ports: []v1.ServicePort{ { - Name: randString(10), + Name: randString(), Protocol: "TCP", Port: int32(80), NodePort: int32(30000), @@ -355,7 +356,9 @@ func testUpdateLoadBalancerAddAnnotation(t *testing.T, client *linodego.Client, fakeClientset := fake.NewSimpleClientset() lb.kubeClient = fakeClientset - defer lb.EnsureLoadBalancerDeleted(context.TODO(), "linodelb", svc) + defer func() { + _ = lb.EnsureLoadBalancerDeleted(context.TODO(), "linodelb", svc) + }() lbStatus, err := lb.EnsureLoadBalancer(context.TODO(), "linodelb", svc, nodes) if err != nil { @@ -390,14 +393,14 @@ func testUpdateLoadBalancerAddPortAnnotation(t *testing.T, client *linodego.Clie portConfigAnnotation := fmt.Sprintf("%s%d", annLinodePortConfigPrefix, targetTestPort) svc := &v1.Service{ ObjectMeta: metav1.ObjectMeta{ - Name: randString(10), + Name: randString(), UID: "foobar123", Annotations: map[string]string{}, }, Spec: v1.ServiceSpec{ Ports: []v1.ServicePort{ { - Name: randString(10), + Name: randString(), Protocol: "TCP", Port: int32(80), NodePort: int32(30000), @@ -407,7 +410,7 @@ func testUpdateLoadBalancerAddPortAnnotation(t *testing.T, client *linodego.Clie } nodes := []*v1.Node{ - &v1.Node{ + { Status: v1.NodeStatus{ Addresses: []v1.NodeAddress{ { @@ -423,7 +426,9 @@ func testUpdateLoadBalancerAddPortAnnotation(t *testing.T, client *linodego.Clie fakeClientset := fake.NewSimpleClientset() lb.kubeClient = fakeClientset - defer lb.EnsureLoadBalancerDeleted(context.TODO(), "linodelb", svc) + defer func() { + _ = lb.EnsureLoadBalancerDeleted(context.TODO(), "linodelb", svc) + }() lbStatus, err := lb.EnsureLoadBalancer(context.TODO(), "linodelb", svc, nodes) if err != nil { @@ -457,7 +462,7 @@ func testUpdateLoadBalancerAddPortAnnotation(t *testing.T, client *linodego.Clie observedPortConfigs := make(map[int]string) for _, cfg := range cfgs { - observedPortConfigs[int(cfg.Port)] = string(cfg.Protocol) + observedPortConfigs[cfg.Port] = string(cfg.Protocol) } if !reflect.DeepEqual(expectedPortConfigs, observedPortConfigs) { @@ -468,14 +473,14 @@ func testUpdateLoadBalancerAddPortAnnotation(t *testing.T, client *linodego.Clie func testUpdateLoadBalancerAddTags(t *testing.T, client *linodego.Client, _ *fakeAPI) { svc := &v1.Service{ ObjectMeta: metav1.ObjectMeta{ - Name: randString(10), + Name: randString(), UID: "foobar123", Annotations: map[string]string{}, }, Spec: v1.ServiceSpec{ Ports: []v1.ServicePort{ { - Name: randString(10), + Name: randString(), Protocol: "TCP", Port: int32(80), NodePort: int32(30000), @@ -485,7 +490,7 @@ func testUpdateLoadBalancerAddTags(t *testing.T, client *linodego.Client, _ *fak } nodes := []*v1.Node{ - &v1.Node{ + { Status: v1.NodeStatus{ Addresses: []v1.NodeAddress{ { @@ -502,7 +507,9 @@ func testUpdateLoadBalancerAddTags(t *testing.T, client *linodego.Client, _ *fak lb.kubeClient = fakeClientset clusterName := "linodelb" - defer lb.EnsureLoadBalancerDeleted(context.TODO(), clusterName, svc) + defer func() { + _ = lb.EnsureLoadBalancerDeleted(context.TODO(), clusterName, svc) + }() lbStatus, err := lb.EnsureLoadBalancer(context.TODO(), clusterName, svc, nodes) if err != nil { @@ -537,7 +544,7 @@ func testUpdateLoadBalancerAddTags(t *testing.T, client *linodego.Client, _ *fak func testUpdateLoadBalancerAddTLSPort(t *testing.T, client *linodego.Client, _ *fakeAPI) { svc := &v1.Service{ ObjectMeta: metav1.ObjectMeta{ - Name: randString(10), + Name: randString(), UID: "foobar123", Annotations: map[string]string{ annLinodeThrottle: "15", @@ -546,7 +553,7 @@ func testUpdateLoadBalancerAddTLSPort(t *testing.T, client *linodego.Client, _ * Spec: v1.ServiceSpec{ Ports: []v1.ServicePort{ { - Name: randString(10), + Name: randString(), Protocol: "TCP", Port: int32(80), NodePort: int32(30000), @@ -556,7 +563,7 @@ func testUpdateLoadBalancerAddTLSPort(t *testing.T, client *linodego.Client, _ * } nodes := []*v1.Node{ - &v1.Node{ + { Status: v1.NodeStatus{ Addresses: []v1.NodeAddress{ { @@ -569,7 +576,7 @@ func testUpdateLoadBalancerAddTLSPort(t *testing.T, client *linodego.Client, _ * } extraPort := v1.ServicePort{ - Name: randString(10), + Name: randString(), Protocol: "TCP", Port: int32(443), NodePort: int32(30001), @@ -577,7 +584,9 @@ func testUpdateLoadBalancerAddTLSPort(t *testing.T, client *linodego.Client, _ * lb := &loadbalancers{client, "us-west", nil} - defer lb.EnsureLoadBalancerDeleted(context.TODO(), "linodelb", svc) + defer func() { + _ = lb.EnsureLoadBalancerDeleted(context.TODO(), "linodelb", svc) + }() fakeClientset := fake.NewSimpleClientset() lb.kubeClient = fakeClientset @@ -610,8 +619,8 @@ func testUpdateLoadBalancerAddTLSPort(t *testing.T, client *linodego.Client, _ * } expectedPorts := map[int]struct{}{ - 80: struct{}{}, - 443: struct{}{}, + 80: {}, + 443: {}, } observedPorts := make(map[int]struct{}) @@ -626,7 +635,7 @@ func testUpdateLoadBalancerAddTLSPort(t *testing.T, client *linodego.Client, _ * t.Errorf("no nodes found for port %d", cfg.Port) } - observedPorts[int(cfg.Port)] = struct{}{} + observedPorts[cfg.Port] = struct{}{} } if !reflect.DeepEqual(expectedPorts, observedPorts) { @@ -678,14 +687,14 @@ func testUpdateLoadBalancerAddProxyProtocol(t *testing.T, client *linodego.Clien t.Run(tc.name, func(t *testing.T) { svc := &v1.Service{ ObjectMeta: metav1.ObjectMeta{ - Name: randString(10), + Name: randString(), UID: "foobar123", Annotations: map[string]string{}, }, Spec: v1.ServiceSpec{ Ports: []v1.ServicePort{ { - Name: randString(10), + Name: randString(), Protocol: "tcp", Port: int32(80), NodePort: int32(8080), @@ -694,11 +703,12 @@ func testUpdateLoadBalancerAddProxyProtocol(t *testing.T, client *linodego.Clien }, } - defer lb.EnsureLoadBalancerDeleted(context.TODO(), "linodelb", svc) + defer func() { + _ = lb.EnsureLoadBalancerDeleted(context.TODO(), "linodelb", svc) + }() nodeBalancer, err := client.CreateNodeBalancer(context.TODO(), linodego.NodeBalancerCreateOptions{ Region: lb.zone, }) - if err != nil { t.Fatalf("failed to create NodeBalancer: %s", err) } @@ -739,14 +749,14 @@ func testUpdateLoadBalancerAddProxyProtocol(t *testing.T, client *linodego.Clien func testUpdateLoadBalancerAddNodeBalancerID(t *testing.T, client *linodego.Client, fakeAPI *fakeAPI) { svc := &v1.Service{ ObjectMeta: metav1.ObjectMeta{ - Name: randString(10), + Name: randString(), UID: "foobar123", Annotations: map[string]string{}, }, Spec: v1.ServiceSpec{ Ports: []v1.ServicePort{ { - Name: randString(10), + Name: randString(), Protocol: "http", Port: int32(80), NodePort: int32(8080), @@ -769,7 +779,9 @@ func testUpdateLoadBalancerAddNodeBalancerID(t *testing.T, client *linodego.Clie } lb := &loadbalancers{client, "us-west", nil} - defer lb.EnsureLoadBalancerDeleted(context.TODO(), "linodelb", svc) + defer func() { + _ = lb.EnsureLoadBalancerDeleted(context.TODO(), "linodelb", svc) + }() fakeClientset := fake.NewSimpleClientset() lb.kubeClient = fakeClientset @@ -824,7 +836,7 @@ func Test_getConnectionThrottle(t *testing.T) { "throttle not specified", &v1.Service{ ObjectMeta: metav1.ObjectMeta{ - Name: randString(10), + Name: randString(), UID: "abc123", Annotations: map[string]string{}, }, @@ -835,7 +847,7 @@ func Test_getConnectionThrottle(t *testing.T) { "throttle value is a string", &v1.Service{ ObjectMeta: metav1.ObjectMeta{ - Name: randString(10), + Name: randString(), UID: "abc123", Annotations: map[string]string{ annLinodeThrottle: "foo", @@ -848,7 +860,7 @@ func Test_getConnectionThrottle(t *testing.T) { "throttle value is less than 0", &v1.Service{ ObjectMeta: metav1.ObjectMeta{ - Name: randString(10), + Name: randString(), UID: "abc123", Annotations: map[string]string{ annLinodeThrottle: "-123", @@ -861,7 +873,7 @@ func Test_getConnectionThrottle(t *testing.T) { "throttle value is valid", &v1.Service{ ObjectMeta: metav1.ObjectMeta{ - Name: randString(10), + Name: randString(), UID: "abc123", Annotations: map[string]string{ annLinodeThrottle: "1", @@ -874,7 +886,7 @@ func Test_getConnectionThrottle(t *testing.T) { "throttle value is too high", &v1.Service{ ObjectMeta: metav1.ObjectMeta{ - Name: randString(10), + Name: randString(), UID: "abc123", Annotations: map[string]string{ annLinodeThrottle: "21", @@ -907,7 +919,7 @@ func Test_getPortConfig(t *testing.T) { "default no proxy protocol specified", &v1.Service{ ObjectMeta: metav1.ObjectMeta{ - Name: randString(10), + Name: randString(), UID: "abc123", }, }, @@ -918,7 +930,7 @@ func Test_getPortConfig(t *testing.T) { "default proxy protocol specified", &v1.Service{ ObjectMeta: metav1.ObjectMeta{ - Name: randString(10), + Name: randString(), UID: "abc123", Annotations: map[string]string{ annLinodeDefaultProxyProtocol: string(linodego.ProxyProtocolV2), @@ -932,7 +944,7 @@ func Test_getPortConfig(t *testing.T) { "port specific proxy protocol specified", &v1.Service{ ObjectMeta: metav1.ObjectMeta{ - Name: randString(10), + Name: randString(), UID: "abc123", Annotations: map[string]string{ annLinodeDefaultProxyProtocol: string(linodego.ProxyProtocolV2), @@ -947,7 +959,7 @@ func Test_getPortConfig(t *testing.T) { "default invalid proxy protocol", &v1.Service{ ObjectMeta: metav1.ObjectMeta{ - Name: randString(10), + Name: randString(), UID: "abc123", Annotations: map[string]string{ annLinodeDefaultProxyProtocol: "invalid", @@ -961,7 +973,7 @@ func Test_getPortConfig(t *testing.T) { "default no protocol specified", &v1.Service{ ObjectMeta: metav1.ObjectMeta{ - Name: randString(10), + Name: randString(), UID: "abc123", }, }, @@ -973,7 +985,7 @@ func Test_getPortConfig(t *testing.T) { "default tcp protocol specified", &v1.Service{ ObjectMeta: metav1.ObjectMeta{ - Name: randString(10), + Name: randString(), UID: "abc123", Annotations: map[string]string{ annLinodeDefaultProtocol: "tcp", @@ -987,7 +999,7 @@ func Test_getPortConfig(t *testing.T) { "default capitalized protocol specified", &v1.Service{ ObjectMeta: metav1.ObjectMeta{ - Name: randString(10), + Name: randString(), UID: "abc123", Annotations: map[string]string{ annLinodeDefaultProtocol: "HTTP", @@ -1001,7 +1013,7 @@ func Test_getPortConfig(t *testing.T) { "default invalid protocol", &v1.Service{ ObjectMeta: metav1.ObjectMeta{ - Name: randString(10), + Name: randString(), UID: "abc123", Annotations: map[string]string{ annLinodeDefaultProtocol: "invalid", @@ -1015,7 +1027,7 @@ func Test_getPortConfig(t *testing.T) { "port config falls back to default", &v1.Service{ ObjectMeta: metav1.ObjectMeta{ - Name: randString(10), + Name: randString(), UID: "abc123", Annotations: map[string]string{ annLinodeDefaultProtocol: "http", @@ -1030,7 +1042,7 @@ func Test_getPortConfig(t *testing.T) { "port config capitalized protocol", &v1.Service{ ObjectMeta: metav1.ObjectMeta{ - Name: randString(10), + Name: randString(), UID: "abc123", Annotations: map[string]string{ annLinodePortConfigPrefix + "443": `{ "protocol": "HTTp" }`, @@ -1044,7 +1056,7 @@ func Test_getPortConfig(t *testing.T) { "port config invalid protocol", &v1.Service{ ObjectMeta: metav1.ObjectMeta{ - Name: randString(10), + Name: randString(), UID: "abc123", Annotations: map[string]string{ annLinodePortConfigPrefix + "443": `{ "protocol": "invalid" }`, @@ -1087,7 +1099,7 @@ func Test_getHealthCheckType(t *testing.T) { "no type specified", &v1.Service{ ObjectMeta: metav1.ObjectMeta{ - Name: randString(10), + Name: randString(), UID: "abc123", Annotations: map[string]string{}, }, @@ -1208,7 +1220,6 @@ func Test_getNodePrivateIP(t *testing.T) { } }) } - } func testBuildLoadBalancerRequest(t *testing.T, client *linodego.Client, _ *fakeAPI) { @@ -1255,11 +1266,6 @@ func testBuildLoadBalancerRequest(t *testing.T, client *linodego.Client, _ *fake t.Fatal(err) } - if nb == nil { - t.Error("unexpected nodeID") - t.Logf("expected: != \"\"") - t.Logf("actual: %v", lb) - } if !reflect.DeepEqual(err, err) { t.Error("unexpected error") t.Logf("expected: %v", nil) @@ -1277,14 +1283,16 @@ func testBuildLoadBalancerRequest(t *testing.T, client *linodego.Client, _ *fake t.Logf("actual: %v", len(configs)) } - nbNodes, _ := client.ListNodeBalancerNodes(context.TODO(), nb.ID, configs[0].ID, nil) + nbNodes, err := client.ListNodeBalancerNodes(context.TODO(), nb.ID, configs[0].ID, nil) + if err != nil { + t.Fatal(err) + } if len(nbNodes) != len(nodes) { t.Error("unexpected nodebalancer nodes count") t.Logf("expected: %v", len(nodes)) t.Logf("actual: %v", len(nbNodes)) } - } func testEnsureLoadBalancerPreserveAnnotation(t *testing.T, client *linodego.Client, fake *fakeAPI) { @@ -1325,7 +1333,7 @@ func testEnsureLoadBalancerPreserveAnnotation(t *testing.T, client *linodego.Cli svc := &v1.Service{ ObjectMeta: metav1.ObjectMeta{ Name: "test", - UID: types.UID("foobar" + randString(10)), + UID: types.UID("foobar" + randString()), Annotations: test.annotations, }, Spec: testServiceSpec, @@ -1617,21 +1625,21 @@ func testMakeLoadBalancerStatusEnvVar(t *testing.T, client *linodego.Client, _ * t.Errorf("expected status for basic service to be %#v; got %#v", expectedStatus, status) } - os.Setenv("LINODE_HOSTNAME_ONLY_INGRESS", "true") + t.Setenv("LINODE_HOSTNAME_ONLY_INGRESS", "true") expectedStatus.Ingress[0] = v1.LoadBalancerIngress{Hostname: hostname} status = makeLoadBalancerStatus(svc, nb) if !reflect.DeepEqual(status, expectedStatus) { t.Errorf("expected status for %q annotated service to be %#v; got %#v", annLinodeHostnameOnlyIngress, expectedStatus, status) } - os.Setenv("LINODE_HOSTNAME_ONLY_INGRESS", "false") + t.Setenv("LINODE_HOSTNAME_ONLY_INGRESS", "false") expectedStatus.Ingress[0] = v1.LoadBalancerIngress{Hostname: hostname} status = makeLoadBalancerStatus(svc, nb) if reflect.DeepEqual(status, expectedStatus) { t.Errorf("expected status for %q annotated service to be %#v; got %#v", annLinodeHostnameOnlyIngress, expectedStatus, status) } - os.Setenv("LINODE_HOSTNAME_ONLY_INGRESS", "banana") + t.Setenv("LINODE_HOSTNAME_ONLY_INGRESS", "banana") expectedStatus.Ingress[0] = v1.LoadBalancerIngress{Hostname: hostname} status = makeLoadBalancerStatus(svc, nb) if reflect.DeepEqual(status, expectedStatus) { @@ -1691,14 +1699,14 @@ func testCleanupDoesntCall(t *testing.T, client *linodego.Client, fakeAPI *fakeA func testUpdateLoadBalancerNoNodes(t *testing.T, client *linodego.Client, _ *fakeAPI) { svc := &v1.Service{ ObjectMeta: metav1.ObjectMeta{ - Name: randString(10), + Name: randString(), UID: "foobar123", Annotations: map[string]string{}, }, Spec: v1.ServiceSpec{ Ports: []v1.ServicePort{ { - Name: randString(10), + Name: randString(), Protocol: "http", Port: int32(80), NodePort: int32(8080), @@ -1708,7 +1716,9 @@ func testUpdateLoadBalancerNoNodes(t *testing.T, client *linodego.Client, _ *fak } lb := &loadbalancers{client, "us-west", nil} - defer lb.EnsureLoadBalancerDeleted(context.TODO(), "linodelb", svc) + defer func() { + _ = lb.EnsureLoadBalancerDeleted(context.TODO(), "linodelb", svc) + }() fakeClientset := fake.NewSimpleClientset() lb.kubeClient = fakeClientset @@ -1957,7 +1967,6 @@ func testGetLoadBalancer(t *testing.T, client *linodego.Client, _ *fakeAPI) { for _, test := range testcases { t.Run(test.name, func(t *testing.T) { - _, found, err := lb.GetLoadBalancer(context.TODO(), test.clusterName, test.service) if found != test.found { t.Error("unexpected error") @@ -2185,5 +2194,4 @@ func Test_LoadbalNodeNameCoercion(t *testing.T) { t.Fatalf("Expected loadbal backend name to be %s (got: %s)", tc.expectedOutput, out) } } - } diff --git a/e2e/test/ccm_e2e_test.go b/e2e/test/ccm_e2e_test.go index c906ff36..b181d127 100644 --- a/e2e/test/ccm_e2e_test.go +++ b/e2e/test/ccm_e2e_test.go @@ -59,7 +59,7 @@ var _ = Describe("e2e tests", func() { Expect(len(workers)).Should(BeNumerically(">=", 2)) }) - var createPodWithLabel = func(pods []string, ports []core.ContainerPort, image string, labels map[string]string, selectNode bool) { + createPodWithLabel := func(pods []string, ports []core.ContainerPort, image string, labels map[string]string, selectNode bool) { for i, pod := range pods { p := f.LoadBalancer.GetPodObject(pod, image, ports, labels) if selectNode { @@ -70,73 +70,73 @@ var _ = Describe("e2e tests", func() { } } - var deletePods = func(pods []string) { + deletePods := func(pods []string) { for _, pod := range pods { Expect(f.LoadBalancer.DeletePod(pod)).NotTo(HaveOccurred()) } } - var deleteService = func() { + deleteService := func() { Expect(f.LoadBalancer.DeleteService()).NotTo(HaveOccurred()) } - var deleteSecret = func(name string) { + deleteSecret := func(name string) { Expect(f.LoadBalancer.DeleteSecret(name)).NotTo(HaveOccurred()) } - var ensureServiceLoadBalancer = func() { + ensureServiceLoadBalancer := func() { watcher, err := f.LoadBalancer.GetServiceWatcher() Expect(err).NotTo(HaveOccurred()) Eventually(watcher.ResultChan()).Should(Receive(EnsuredService())) } - var createServiceWithSelector = func(selector map[string]string, ports []core.ServicePort, isSessionAffinityClientIP bool) { + createServiceWithSelector := func(selector map[string]string, ports []core.ServicePort, isSessionAffinityClientIP bool) { Expect(f.LoadBalancer.CreateService(selector, nil, ports, isSessionAffinityClientIP)).NotTo(HaveOccurred()) Eventually(f.LoadBalancer.GetServiceEndpoints).Should(Not(BeEmpty())) ensureServiceLoadBalancer() } - var createServiceWithAnnotations = func(labels map[string]string, annotations map[string]string, ports []core.ServicePort, isSessionAffinityClientIP bool) { + createServiceWithAnnotations := func(labels, annotations map[string]string, ports []core.ServicePort, isSessionAffinityClientIP bool) { Expect(f.LoadBalancer.CreateService(labels, annotations, ports, isSessionAffinityClientIP)).NotTo(HaveOccurred()) Eventually(f.LoadBalancer.GetServiceEndpoints).Should(Not(BeEmpty())) ensureServiceLoadBalancer() } - var updateServiceWithAnnotations = func(labels map[string]string, annotations map[string]string, ports []core.ServicePort, isSessionAffinityClientIP bool) { + updateServiceWithAnnotations := func(labels, annotations map[string]string, ports []core.ServicePort, isSessionAffinityClientIP bool) { Expect(f.LoadBalancer.UpdateService(labels, annotations, ports, isSessionAffinityClientIP)).NotTo(HaveOccurred()) Eventually(f.LoadBalancer.GetServiceEndpoints).Should(Not(BeEmpty())) ensureServiceLoadBalancer() } - var deleteNodeBalancer = func(id int) { + deleteNodeBalancer := func(id int) { Expect(getLinodeClient().DeleteNodeBalancer(context.Background(), id)).NotTo(HaveOccurred()) } - var createNodeBalancer = func() int { + createNodeBalancer := func() int { var nb *linodego.NodeBalancer nb, err = getLinodeClient().CreateNodeBalancer(context.TODO(), linodego.NodeBalancerCreateOptions{ - Region: fmt.Sprintf("%s", region), + Region: region, }) Expect(err).NotTo(HaveOccurred()) Expect(nb).NotTo(BeNil()) return nb.ID } - var checkNumberOfWorkerNodes = func(numNodes int) { + checkNumberOfWorkerNodes := func(numNodes int) { Eventually(f.GetNodeList).Should(HaveLen(numNodes)) } - var checkNumberOfUpNodes = func(numNodes int) { + checkNumberOfUpNodes := func(numNodes int) { By("Checking the Number of Up Nodes") Eventually(f.LoadBalancer.GetNodeBalancerUpNodes).WithArguments(framework.TestServerResourceName).Should(BeNumerically(">=", numNodes)) } - var checkNodeBalancerExists = func(id int) { + checkNodeBalancerExists := func(id int) { By("Checking if the NodeBalancer exists") Eventually(getLinodeClient().GetNodeBalancer).WithArguments(context.Background(), id).Should(HaveField("ID", Equal(id))) } - var checkNodeBalancerNotExists = func(id int) { + checkNodeBalancerNotExists := func(id int) { Eventually(func() int { _, err := getLinodeClient().GetNodeBalancer(context.Background(), id) if err == nil { @@ -152,15 +152,15 @@ var _ = Describe("e2e tests", func() { checkNodes bool } - var checkNodeBalancerID = func(service string, expectedID int) { + checkNodeBalancerID := func(service string, expectedID int) { Eventually(f.LoadBalancer.GetNodeBalancerID).WithArguments(service).Should(Equal(expectedID)) } - var checkLBStatus = func(service string, hasIP bool) { + checkLBStatus := func(service string, hasIP bool) { Eventually(f.LoadBalancer.GetNodeBalancerFromService).WithArguments(service, hasIP).Should(Not(BeNil())) } - var checkNodeBalancerConfigForPort = func(port int, args checkArgs) { + checkNodeBalancerConfigForPort := func(port int, args checkArgs) { By("Getting NodeBalancer Configuration for port " + strconv.Itoa(port)) var nbConfig *linodego.NodeBalancerConfig Eventually(func() error { @@ -230,17 +230,17 @@ var _ = Describe("e2e tests", func() { } } - var addNewNode = func() { + addNewNode := func() { err := exec.Command("terraform", "apply", "-var", "nodes=3", "-auto-approve").Run() Expect(err).NotTo(HaveOccurred()) } - var deleteNewNode = func() { + deleteNewNode := func() { err := exec.Command("terraform", "apply", "-var", "nodes=2", "-auto-approve").Run() Expect(err).NotTo(HaveOccurred()) } - var waitForNodeAddition = func() { + waitForNodeAddition := func() { checkNumberOfUpNodes(3) } @@ -371,7 +371,6 @@ var _ = Describe("e2e tests", func() { By("Waiting for Response from the LoadBalancer url: " + eps[0]) Eventually(framework.WaitForHTTPSResponse).WithArguments(eps[0]).Should(ContainSubstring(pods[0])) - }) }) @@ -1047,7 +1046,6 @@ var _ = Describe("e2e tests", func() { By("Checking the old NodeBalancer exists") checkNodeBalancerExists(nodeBalancerID) }) - }) Context("With Node Addition", func() { diff --git a/e2e/test/ccm_suite_test.go b/e2e/test/ccm_suite_test.go index 53975a04..ffc8cb74 100644 --- a/e2e/test/ccm_suite_test.go +++ b/e2e/test/ccm_suite_test.go @@ -34,22 +34,18 @@ func init() { flag.StringVar(&framework.KubeConfigFile, "kubeconfig", os.Getenv("TEST_KUBECONFIG"), "To use existing cluster provide kubeconfig file") flag.StringVar(®ion, "region", region, "Region to create load balancers") flag.StringVar(&k8s_version, "k8s_version", k8s_version, "k8s_version for child cluster") - } const ( TIMEOUT = 5 * time.Minute ) -var ( - root *framework.Framework -) +var root *framework.Framework func TestE2e(t *testing.T) { RegisterFailHandler(Fail) SetDefaultEventuallyTimeout(TIMEOUT) RunSpecs(t, "e2e Suite") - } var getLinodeClient = func() *linodego.Client { diff --git a/e2e/test/framework/service.go b/e2e/test/framework/service.go index 01feee7c..2f0b8aac 100644 --- a/e2e/test/framework/service.go +++ b/e2e/test/framework/service.go @@ -6,12 +6,11 @@ import ( core "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/util/intstr" "k8s.io/apimachinery/pkg/watch" "k8s.io/client-go/util/retry" ) -func (i *lbInvocation) createOrUpdateService(selector, annotations map[string]string, ports []core.ServicePort, isSessionAffinityClientIP bool, isCreate bool) error { +func (i *lbInvocation) createOrUpdateService(selector, annotations map[string]string, ports []core.ServicePort, isSessionAffinityClientIP, isCreate bool) error { var sessionAffinity core.ServiceAffinity = "None" if isSessionAffinityClientIP { sessionAffinity = "ClientIP" @@ -62,7 +61,8 @@ func (i *lbInvocation) GetServiceWatcher() (watch.Interface, error) { watcher, err := i.kubeClient.CoreV1().Events(i.Namespace()).Watch(context.TODO(), metav1.ListOptions{ FieldSelector: "involvedObject.kind=Service", Watch: true, - TimeoutSeconds: &timeoutSeconds}) + TimeoutSeconds: &timeoutSeconds, + }) if err != nil { return nil, err } @@ -131,14 +131,3 @@ func (i *lbInvocation) getServiceIngress(name, namespace string) ([]core.LoadBal } return svc.Status.LoadBalancer.Ingress, nil } - -func (i *lbInvocation) testServerServicePorts() []core.ServicePort { - return []core.ServicePort{ - { - Name: "http-1", - Port: 80, - TargetPort: intstr.FromInt(8080), - Protocol: "TCP", - }, - } -} diff --git a/e2e/test/framework/util.go b/e2e/test/framework/util.go index 08faaa03..8ceb8c5d 100644 --- a/e2e/test/framework/util.go +++ b/e2e/test/framework/util.go @@ -71,7 +71,6 @@ func runCommand(cmd string, args ...string) error { c := exec.Command(cmd, args...) c.Stdout = os.Stdout c.Stderr = os.Stderr - c.Env = append(c.Env, append(os.Environ())...) glog.Infof("Running command %q\n", cmd) return c.Run() } @@ -146,7 +145,7 @@ func WaitForHTTPSResponse(link string) (string, error) { if err != nil { return "", err } - return string(resp), nil + return resp, nil } func getHTTPResponse(link string) (bool, string, error) {