[CI] Golangci-lint update part 3 (linode#345)

* Resolved thelper issues and switched tenv to usetesting
* Added annotation for paralleltest and resolved govet errors
* Resolved forcetypeassert errors

Author: Andrews2024

.golangci.yml

@@ -92,13 +92,13 @@ linters:
     - errcheck
     - errchkjson
     - errorlint
-    - govet
+    #- govet
     - testifylint
   enable:
     # these are enabled by default
     #- errcheck
     - gosimple
-    #- govet
+    - govet
     - ineffassign
     - staticcheck
     - typecheck
@@ -123,7 +123,7 @@ linters:
     #- errorlint
     - exhaustive
     #- forbidigo
-    #- forcetypeassert
+    - forcetypeassert
     - gci
     - gocheckcompilerdirectives
     - gofmt
@@ -147,15 +147,15 @@ linters:
     - noctx
     - nolintlint
     - nosprintfhostport
-    #- paralleltest
+    #- paralleltest # adding t.Parallel() to tests broke so many and made others flaky
     - prealloc
     - predeclared
     - reassign
-    #- tenv
-    #- thelper
+    - thelper
     - unconvert
     - unparam
     - usestdlibvars
+    - usetesting
     - varnamelen
     - wastedassign
     - whitespace

cloud/linode/cilium_loadbalancers.go

@@ -294,10 +294,12 @@ func (l *loadbalancers) deleteSharedIP(ctx context.Context, service *v1.Service)
 	}
 	svcIngress := service.Status.LoadBalancer.Ingress
 	if len(svcIngress) > 0 && ipHolder != nil {
+		var nodeLinodeID int
+
 		for _, ingress := range svcIngress {
 			// delete the shared IP on the Linodes it's shared on
 			for _, node := range bgpNodes {
-				nodeLinodeID, err := parseProviderID(node.Spec.ProviderID)
+				nodeLinodeID, err = parseProviderID(node.Spec.ProviderID)
 				if err != nil {
 					return err
 				}

cloud/linode/cilium_loadbalancers_test.go

@@ -141,6 +141,7 @@ func TestCiliumCCMLoadBalancers(t *testing.T) {
 			f:    testCiliumUpdateLoadBalancerAddNodeWithNewIpHolderNamingConvention,
 		},
 	}
+	//nolint: paralleltest // two tests use t.Setenv, which fails after t.Parallel() call
 	for _, tc := range testCases {
 		ctrl := gomock.NewController(t)
 		mc := mocks.NewMockClient(ctrl)
@@ -180,13 +181,17 @@ func createTestService() *v1.Service {
 }
 
 func addService(t *testing.T, kubeClient kubernetes.Interface, svc *v1.Service) {
+	t.Helper()
+
 	_, err := kubeClient.CoreV1().Services(svc.Namespace).Create(context.TODO(), svc, metav1.CreateOptions{})
 	if err != nil {
 		t.Fatalf("failed to add Service: %v", err)
 	}
 }
 
 func addNodes(t *testing.T, kubeClient kubernetes.Interface, nodes []*v1.Node) {
+	t.Helper()
+
 	for _, node := range nodes {
 		_, err := kubeClient.CoreV1().Nodes().Create(context.TODO(), node, metav1.CreateOptions{})
 		if err != nil {
@@ -206,6 +211,8 @@ func createNewIpHolderInstance() linodego.Instance {
 }
 
 func testNoBGPNodeLabel(t *testing.T, mc *mocks.MockClient) {
+	t.Helper()
+
 	Options.BGPNodeSelector = ""
 	Options.IpHolderSuffix = clusterName
 	t.Setenv("BGP_PEER_PREFIX", "2600:3cef")
@@ -255,6 +262,8 @@ func testNoBGPNodeLabel(t *testing.T, mc *mocks.MockClient) {
 }
 
 func testUnsupportedRegion(t *testing.T, mc *mocks.MockClient) {
+	t.Helper()
+
 	Options.BGPNodeSelector = nodeSelector
 	svc := createTestService()
 
@@ -284,6 +293,8 @@ func testUnsupportedRegion(t *testing.T, mc *mocks.MockClient) {
 }
 
 func testCreateWithExistingIPHolderWithOldIpHolderNamingConvention(t *testing.T, mc *mocks.MockClient) {
+	t.Helper()
+
 	Options.BGPNodeSelector = nodeSelector
 	svc := createTestService()
 	newIpHolderInstance = createNewIpHolderInstance()
@@ -323,6 +334,8 @@ func testCreateWithExistingIPHolderWithOldIpHolderNamingConvention(t *testing.T,
 }
 
 func testCreateWithExistingIPHolderWithNewIpHolderNamingConvention(t *testing.T, mc *mocks.MockClient) {
+	t.Helper()
+
 	Options.BGPNodeSelector = nodeSelector
 	Options.IpHolderSuffix = clusterName
 	svc := createTestService()
@@ -363,6 +376,8 @@ func testCreateWithExistingIPHolderWithNewIpHolderNamingConvention(t *testing.T,
 }
 
 func testCreateWithExistingIPHolderWithNewIpHolderNamingConventionUsingLongSuffix(t *testing.T, mc *mocks.MockClient) {
+	t.Helper()
+
 	Options.BGPNodeSelector = nodeSelector
 	Options.IpHolderSuffix = "OaTJrRuufacHVougjwkpBpmstiqvswvBNEMWXsRYfMBTCkKIUTXpbGIcIbDWSQp"
 	svc := createTestService()
@@ -403,6 +418,8 @@ func testCreateWithExistingIPHolderWithNewIpHolderNamingConventionUsingLongSuffi
 }
 
 func testCreateWithNoExistingIPHolderUsingNoSuffix(t *testing.T, mc *mocks.MockClient) {
+	t.Helper()
+
 	Options.BGPNodeSelector = nodeSelector
 	Options.IpHolderSuffix = ""
 	svc := createTestService()
@@ -447,6 +464,8 @@ func testCreateWithNoExistingIPHolderUsingNoSuffix(t *testing.T, mc *mocks.MockC
 }
 
 func testCreateWithNoExistingIPHolderUsingShortSuffix(t *testing.T, mc *mocks.MockClient) {
+	t.Helper()
+
 	Options.BGPNodeSelector = nodeSelector
 	Options.IpHolderSuffix = clusterName
 	svc := createTestService()
@@ -491,6 +510,8 @@ func testCreateWithNoExistingIPHolderUsingShortSuffix(t *testing.T, mc *mocks.Mo
 }
 
 func testCreateWithNoExistingIPHolderUsingLongSuffix(t *testing.T, mc *mocks.MockClient) {
+	t.Helper()
+
 	Options.BGPNodeSelector = nodeSelector
 	Options.IpHolderSuffix = "OaTJrRuufacHVougjwkpBpmstiqvswvBNEMWXsRYfMBTCkKIUTXpbGIcIbDWSQp"
 	svc := createTestService()
@@ -535,6 +556,8 @@ func testCreateWithNoExistingIPHolderUsingLongSuffix(t *testing.T, mc *mocks.Moc
 }
 
 func testEnsureCiliumLoadBalancerDeletedWithOldIpHolderNamingConvention(t *testing.T, mc *mocks.MockClient) {
+	t.Helper()
+
 	Options.BGPNodeSelector = nodeSelector
 	svc := createTestService()
 
@@ -561,6 +584,8 @@ func testEnsureCiliumLoadBalancerDeletedWithOldIpHolderNamingConvention(t *testi
 }
 
 func testEnsureCiliumLoadBalancerDeletedWithNewIpHolderNamingConvention(t *testing.T, mc *mocks.MockClient) {
+	t.Helper()
+
 	Options.BGPNodeSelector = nodeSelector
 	Options.IpHolderSuffix = clusterName
 	svc := createTestService()
@@ -592,6 +617,8 @@ func testEnsureCiliumLoadBalancerDeletedWithNewIpHolderNamingConvention(t *testi
 }
 
 func testCiliumUpdateLoadBalancerAddNodeWithOldIpHolderNamingConvention(t *testing.T, mc *mocks.MockClient) {
+	t.Helper()
+
 	Options.BGPNodeSelector = nodeSelector
 	svc := createTestService()
 
@@ -648,6 +675,8 @@ func testCiliumUpdateLoadBalancerAddNodeWithOldIpHolderNamingConvention(t *testi
 }
 
 func testCiliumUpdateLoadBalancerAddNodeWithNewIpHolderNamingConvention(t *testing.T, mc *mocks.MockClient) {
+	t.Helper()
+
 	Options.BGPNodeSelector = nodeSelector
 	Options.IpHolderSuffix = clusterName
 	svc := createTestService()

cloud/linode/cloud.go

@@ -115,7 +115,8 @@ func newCloud() (cloudprovider.Interface, error) {
 	var healthChecker *healthChecker
 
 	if Options.EnableTokenHealthChecker {
-		authenticated, err := client.CheckClientAuthenticated(context.TODO(), linodeClient)
+		var authenticated bool
+		authenticated, err = client.CheckClientAuthenticated(context.TODO(), linodeClient)
 		if err != nil {
 			return nil, fmt.Errorf("linode client authenticated connection error: %w", err)
 		}
@@ -187,7 +188,12 @@ func (c *linodeCloud) Initialize(clientBuilder cloudprovider.ControllerClientBui
 		go c.linodeTokenHealthChecker.Run(stopCh)
 	}
 
-	serviceController := newServiceController(c.loadbalancers.(*loadbalancers), serviceInformer)
+	lb, assertion := c.loadbalancers.(*loadbalancers)
+	if !assertion {
+		klog.Error("type assertion during Initialize() failed")
+		return
+	}
+	serviceController := newServiceController(lb, serviceInformer)
 	go serviceController.Run(stopCh)
 
 	nodeController := newNodeController(kubeclient, c.client, nodeInformer, instanceCache)

cloud/linode/fake_linode_test.go

@@ -40,6 +40,8 @@ type fakeRequest struct {
 }
 
 func newFake(t *testing.T) *fakeAPI {
+	t.Helper()
+
 	fake := &fakeAPI{
 		t:        t,
 		nb:       make(map[string]*linodego.NodeBalancer),

cloud/linode/health_check_test.go

@@ -41,6 +41,8 @@ func TestHealthCheck(t *testing.T) {
 }
 
 func testSucceedingCallsToLinodeAPIHappenStopSignalNotFired(t *testing.T, client *mocks.MockClient) {
+	t.Helper()
+
 	writableStopCh := make(chan struct{})
 	readableStopCh := make(chan struct{})
 
@@ -62,6 +64,8 @@ func testSucceedingCallsToLinodeAPIHappenStopSignalNotFired(t *testing.T, client
 }
 
 func testFailingCallsToLinodeAPIHappenStopSignalFired(t *testing.T, client *mocks.MockClient) {
+	t.Helper()
+
 	writableStopCh := make(chan struct{})
 	readableStopCh := make(chan struct{})
 
@@ -95,6 +99,8 @@ func testFailingCallsToLinodeAPIHappenStopSignalFired(t *testing.T, client *mock
 }
 
 func testErrorCallsToLinodeAPIHappenStopSignalNotFired(t *testing.T, client *mocks.MockClient) {
+	t.Helper()
+
 	writableStopCh := make(chan struct{})
 	readableStopCh := make(chan struct{})