Skip to content

Commit fbe2c98

Browse files
hrakhrak
committed
chore(lint): Lint fixes
1 parent a39b387 commit fbe2c98

6 files changed

Lines changed: 58 additions & 41 deletions

File tree

‎cloudstack/cloudstack_instances_test.go‎

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
package cloudstack
22

33
import (
4-
"context"
54
"errors"
65
"testing"
76

@@ -94,8 +93,10 @@ func TestInstanceExists(t *testing.T) {
9493
mockCtrl := gomock.NewController(t)
9594
defer mockCtrl.Finish()
9695

96+
ctx := t.Context()
97+
9798
cs := cloudstack.NewMockClient(mockCtrl)
98-
ms := cs.VirtualMachine.(*cloudstack.MockVirtualMachineServiceIface) //nolint: forcetypeassert
99+
ms := cs.VirtualMachine.(*cloudstack.MockVirtualMachineServiceIface) //nolint:forcetypeassert
99100

100101
fakeInstances := &CSCloud{
101102
client: cs,
@@ -142,13 +143,13 @@ func TestInstanceExists(t *testing.T) {
142143
if test.node.Name == "testDummyVM" {
143144
ms.EXPECT().GetVirtualMachineByName("testDummyVM", gomock.Any()).Return(test.mockedCSOutput, 1, nil)
144145
} else {
145-
ms.EXPECT().GetVirtualMachineByName("nonExistingVM", gomock.Any()).Return(test.mockedCSOutput, 0, errors.New("No match found for ...")) //nolint: revive
146+
ms.EXPECT().GetVirtualMachineByName("nonExistingVM", gomock.Any()).Return(test.mockedCSOutput, 0, errors.New("No match found for ...")) //nolint:revive
146147
}
147148
} else {
148149
ms.EXPECT().GetVirtualMachineByID("915653c4-298b-4d74-bdee-4ced282114f1", gomock.Any()).Return(test.mockedCSOutput, 1, nil)
149150
}
150151

151-
exists, err := fakeInstances.InstanceExists(context.TODO(), test.node)
152+
exists, err := fakeInstances.InstanceExists(ctx, test.node)
152153
if err != nil {
153154
t.Errorf("InstanceExists failed with node %v: %v", nodeName, err)
154155
}
@@ -164,8 +165,10 @@ func TestInstanceShutdown(t *testing.T) {
164165
mockCtrl := gomock.NewController(t)
165166
defer mockCtrl.Finish()
166167

168+
ctx := t.Context()
169+
167170
cs := cloudstack.NewMockClient(mockCtrl)
168-
ms := cs.VirtualMachine.(*cloudstack.MockVirtualMachineServiceIface) //nolint: forcetypeassert
171+
ms := cs.VirtualMachine.(*cloudstack.MockVirtualMachineServiceIface) //nolint:forcetypeassert
169172

170173
fakeInstances := &CSCloud{
171174
client: cs,
@@ -225,7 +228,7 @@ func TestInstanceShutdown(t *testing.T) {
225228
ms.EXPECT().GetVirtualMachineByID("915653c4-298b-4d74-bdee-4ced282114f1", gomock.Any()).Return(test.mockedCSOutput, 1, nil)
226229
}
227230

228-
shutdown, err := fakeInstances.InstanceShutdown(context.TODO(), test.node)
231+
shutdown, err := fakeInstances.InstanceShutdown(ctx, test.node)
229232
if err != nil {
230233
t.Logf("InstanceShutdown failed with node %v: %v", nodeName, err)
231234
}
@@ -241,8 +244,10 @@ func TestInstanceMetadata(t *testing.T) {
241244
mockCtrl := gomock.NewController(t)
242245
defer mockCtrl.Finish()
243246

247+
ctx := t.Context()
248+
244249
cs := cloudstack.NewMockClient(mockCtrl)
245-
ms := cs.VirtualMachine.(*cloudstack.MockVirtualMachineServiceIface) //nolint: forcetypeassert
250+
ms := cs.VirtualMachine.(*cloudstack.MockVirtualMachineServiceIface) //nolint:forcetypeassert
246251

247252
fakeInstances := &CSCloud{
248253
client: cs,
@@ -374,7 +379,7 @@ func TestInstanceMetadata(t *testing.T) {
374379
ms.EXPECT().GetVirtualMachineByID("915653c4-298b-4d74-bdee-4ced282114f1", gomock.Any()).Return(test.mockedCSOutput, 1, nil)
375380
}
376381

377-
metadata, err := fakeInstances.InstanceMetadata(context.TODO(), test.node)
382+
metadata, err := fakeInstances.InstanceMetadata(ctx, test.node)
378383
if err != nil {
379384
t.Logf("InstanceMetadata failed with node %v: %v", nodeName, err)
380385
}

‎cloudstack/cloudstack_loadbalancer.go‎

Lines changed: 18 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,7 @@ func (cs *CSCloud) GetLoadBalancer(ctx context.Context, clusterName string, serv
9696
}
9797

9898
// EnsureLoadBalancer creates a new load balancer, or updates the existing one. Returns the status of the balancer.
99-
func (cs *CSCloud) EnsureLoadBalancer(ctx context.Context, clusterName string, service *corev1.Service, nodes []*corev1.Node) (status *corev1.LoadBalancerStatus, err error) { //nolint:gocognit,gocyclo,nestif
99+
func (cs *CSCloud) EnsureLoadBalancer(ctx context.Context, clusterName string, service *corev1.Service, nodes []*corev1.Node) (status *corev1.LoadBalancerStatus, err error) { //nolint:gocognit,gocyclo,nestif,maintidx
100100
klog.V(4).InfoS("EnsureLoadBalancer", "cluster", clusterName, "service", klog.KObj(service))
101101
serviceName := fmt.Sprintf("%s/%s", service.Namespace, service.Name)
102102

@@ -141,7 +141,6 @@ func (cs *CSCloud) EnsureLoadBalancer(ctx context.Context, clusterName string, s
141141
msg := fmt.Sprintf("Created new load balancer for service %s with algorithm '%s' and IP address %s", serviceName, lb.algorithm, lb.ipAddr)
142142
cs.eventRecorder.Event(service, corev1.EventTypeNormal, "CreatedLoadBalancer", msg)
143143
klog.Info(msg)
144-
145144
} else if service.Spec.LoadBalancerIP != "" && service.Spec.LoadBalancerIP != lb.ipAddr {
146145
// LoadBalancerIP was specified and it's different from the current IP
147146
// Release the old IP first
@@ -161,8 +160,10 @@ func (cs *CSCloud) EnsureLoadBalancer(ctx context.Context, clusterName string, s
161160
// CloudStack sometimes reports deletes as "invalid value" when the entity is already gone.
162161
if strings.Contains(delErr.Error(), "does not exist") || strings.Contains(delErr.Error(), "Invalid parameter id value") {
163162
klog.V(4).Infof("Load balancer rule %s already removed, continuing: %v", oldRule.Name, delErr)
163+
164164
continue
165165
}
166+
166167
return nil, delErr
167168
}
168169
}
@@ -172,11 +173,13 @@ func (cs *CSCloud) EnsureLoadBalancer(ctx context.Context, clusterName string, s
172173

173174
if err := lb.releaseLoadBalancerIP(); err != nil {
174175
klog.Errorf("attempt to release old load balancer IP failed: %s", err.Error())
176+
175177
return nil, fmt.Errorf("failed to release old load balancer IP: %w", err)
176178
}
177179

178180
if err := lb.getLoadBalancerIP(service.Spec.LoadBalancerIP); err != nil {
179181
klog.Errorf("failed to allocate specified IP %v: %v", service.Spec.LoadBalancerIP, err)
182+
180183
return nil, fmt.Errorf("failed to allocate specified load balancer IP: %w", err)
181184
}
182185

@@ -339,6 +342,7 @@ func (cs *CSCloud) EnsureLoadBalancerDeleted(ctx context.Context, clusterName st
339342
// If no rules exist, the load balancer doesn't exist - nothing to delete
340343
if len(lb.rules) == 0 {
341344
klog.V(4).Infof("No load balancer rules found for service, nothing to delete")
345+
342346
return nil
343347
}
344348

@@ -390,16 +394,17 @@ func (cs *CSCloud) EnsureLoadBalancerDeleted(ctx context.Context, clusterName st
390394
}
391395

392396
// Delete the public IP address if appropriate
393-
if lb.ipAddr != "" {
397+
if lb.ipAddr != "" { //nolint:nestif
394398
klog.V(4).Infof("Processing public IP deletion for load balancer: IP=%v, ID=%v", lb.ipAddr, lb.ipAddrID)
395399

396400
// Check if we should release the IP
397401
shouldReleaseIP, err := cs.shouldReleaseLoadBalancerIP(lb, service)
398-
if err != nil {
402+
switch {
403+
case err != nil:
399404
err := fmt.Errorf("error determining if IP should be released: %w", err)
400405
klog.Errorf("%v", err)
401406
deletionErrors = append(deletionErrors, err)
402-
} else if shouldReleaseIP {
407+
case shouldReleaseIP:
403408
klog.V(4).Infof("Releasing load balancer IP: %v", lb.ipAddr)
404409
if err := lb.releaseLoadBalancerIP(); err != nil {
405410
err := fmt.Errorf("error releasing load balancer IP %v: %w", lb.ipAddr, err)
@@ -410,7 +415,7 @@ func (cs *CSCloud) EnsureLoadBalancerDeleted(ctx context.Context, clusterName st
410415
cs.eventRecorder.Event(service, corev1.EventTypeNormal, "ReleasedLoadBalancerIP", msg)
411416
klog.Info(msg)
412417
}
413-
} else {
418+
default:
414419
klog.V(4).Infof("Load balancer IP %v is in use by other services, keeping it allocated", lb.ipAddr)
415420
}
416421
}
@@ -420,23 +425,25 @@ func (cs *CSCloud) EnsureLoadBalancerDeleted(ctx context.Context, clusterName st
420425
msg := fmt.Sprintf("Encountered %d error(s) while deleting load balancer for service %s", len(deletionErrors), serviceName)
421426
klog.Warningf("%s: %v", msg, deletionErrors)
422427
cs.eventRecorder.Event(service, corev1.EventTypeWarning, "DeletingLoadBalancerFailed", msg)
428+
423429
// Return the first error or a combined error message
424-
return fmt.Errorf("load balancer deletion completed with errors: %v", deletionErrors[0])
430+
return fmt.Errorf("load balancer deletion completed with errors: %w", deletionErrors[0])
425431
}
426432

427-
msg := fmt.Sprintf("Successfully deleted load balancer for service %s", serviceName)
433+
msg := "Successfully deleted load balancer for service " + serviceName
428434
cs.eventRecorder.Event(service, corev1.EventTypeNormal, "DeletedLoadBalancer", msg)
429435
klog.Info(msg)
430436

431437
return nil
432438
}
433439

434-
// shouldReleaseLoadBalancerIP determines whether the public IP should be released
440+
// shouldReleaseLoadBalancerIP determines whether the public IP should be released.
435441
func (cs *CSCloud) shouldReleaseLoadBalancerIP(lb *loadBalancer, service *corev1.Service) (bool, error) {
436442
// If the IP was explicitly specified in the service spec, don't release it
437443
// The user is responsible for managing the lifecycle of user-provided IPs
438444
if service.Spec.LoadBalancerIP != "" && service.Spec.LoadBalancerIP == lb.ipAddr {
439445
klog.V(4).Infof("IP %v was explicitly specified in service spec, not releasing", lb.ipAddr)
446+
440447
return false, nil
441448
}
442449

@@ -456,11 +463,13 @@ func (cs *CSCloud) shouldReleaseLoadBalancerIP(lb *loadBalancer, service *corev1
456463
// If other rules exist, this IP is in use by other services
457464
if otherRules.Count > 0 {
458465
klog.V(4).Infof("IP %v has %d other load balancer rule(s) in use, not releasing", lb.ipAddr, otherRules.Count)
466+
459467
return false, nil
460468
}
461469

462470
// IP is safe to release - it's either controller-allocated or no longer in use
463471
klog.V(4).Infof("IP %v is no longer in use and safe to release", lb.ipAddr)
472+
464473
return true, nil
465474
}
466475

‎cloudstack/cloudstack_loadbalancer_test.go‎

Lines changed: 21 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@
2020
package cloudstack
2121

2222
import (
23-
"fmt"
23+
"errors"
2424
"sort"
2525
"strings"
2626
"testing"
@@ -495,7 +495,6 @@ func TestCheckLoadBalancerRule(t *testing.T) {
495495
t.Fatalf("expected rule entry to be removed from map")
496496
}
497497
})
498-
499498
}
500499

501500
func TestRuleToString(t *testing.T) {
@@ -727,20 +726,23 @@ func TestRulesMapToString(t *testing.T) {
727726
parts := strings.Split(got, ", ")
728727
if len(parts) != len(expectedRules) {
729728
t.Errorf("rulesMapToString() returned %d rules, want %d", len(parts), len(expectedRules))
729+
730730
return
731731
}
732732
for _, expectedRule := range expectedRules {
733733
found := false
734734
for _, part := range parts {
735735
if part == expectedRule {
736736
found = true
737+
737738
break
738739
}
739740
}
740741
if !found {
741742
t.Errorf("rulesMapToString() missing rule %q in output %q", expectedRule, got)
742743
}
743744
}
745+
744746
return
745747
}
746748

@@ -922,7 +924,7 @@ func TestGetPublicIPAddress(t *testing.T) {
922924

923925
mockAddress := cloudstack.NewMockAddressServiceIface(ctrl)
924926
listParams := &cloudstack.ListPublicIpAddressesParams{}
925-
apiErr := fmt.Errorf("API error")
927+
apiErr := errors.New("API error")
926928

927929
gomock.InOrder(
928930
mockAddress.EXPECT().NewListPublicIpAddressesParams().Return(listParams),
@@ -1080,7 +1082,7 @@ func TestAssociatePublicIPAddress(t *testing.T) {
10801082
t.Cleanup(ctrl.Finish)
10811083

10821084
mockNetwork := cloudstack.NewMockNetworkServiceIface(ctrl)
1083-
apiErr := fmt.Errorf("network API error")
1085+
apiErr := errors.New("network API error")
10841086

10851087
mockNetwork.EXPECT().GetNetworkByID("net-123", gomock.Any()).Return(nil, 1, apiErr)
10861088

@@ -1106,7 +1108,7 @@ func TestAssociatePublicIPAddress(t *testing.T) {
11061108

11071109
mockNetwork := cloudstack.NewMockNetworkServiceIface(ctrl)
11081110

1109-
mockNetwork.EXPECT().GetNetworkByID("net-123", gomock.Any()).Return(nil, 0, fmt.Errorf("not found"))
1111+
mockNetwork.EXPECT().GetNetworkByID("net-123", gomock.Any()).Return(nil, 0, errors.New("not found"))
11101112

11111113
lb := &loadBalancer{
11121114
CloudStackClient: &cloudstack.CloudStackClient{
@@ -1137,7 +1139,7 @@ func TestAssociatePublicIPAddress(t *testing.T) {
11371139
}
11381140

11391141
associateParams := &cloudstack.AssociateIpAddressParams{}
1140-
apiErr := fmt.Errorf("associate API error")
1142+
apiErr := errors.New("associate API error")
11411143

11421144
gomock.InOrder(
11431145
mockNetwork.EXPECT().GetNetworkByID("net-123", gomock.Any()).Return(networkResp, 1, nil),
@@ -1235,7 +1237,7 @@ func TestReleaseLoadBalancerIP(t *testing.T) {
12351237

12361238
mockAddress := cloudstack.NewMockAddressServiceIface(ctrl)
12371239
disassociateParams := &cloudstack.DisassociateIpAddressParams{}
1238-
apiErr := fmt.Errorf("disassociate API error")
1240+
apiErr := errors.New("disassociate API error")
12391241

12401242
gomock.InOrder(
12411243
mockAddress.EXPECT().NewDisassociateIpAddressParams("ip-123").Return(disassociateParams),
@@ -1510,7 +1512,7 @@ func TestCreateLoadBalancerRule(t *testing.T) {
15101512

15111513
mockLB := cloudstack.NewMockLoadBalancerServiceIface(ctrl)
15121514
createParams := &cloudstack.CreateLoadBalancerRuleParams{}
1513-
apiErr := fmt.Errorf("create rule API error")
1515+
apiErr := errors.New("create rule API error")
15141516

15151517
gomock.InOrder(
15161518
mockLB.EXPECT().NewCreateLoadBalancerRuleParams("roundrobin", "test-rule-tcp-80", 30000, 80).Return(createParams),
@@ -1653,7 +1655,7 @@ func TestDeleteLoadBalancerRule(t *testing.T) {
16531655

16541656
mockLB := cloudstack.NewMockLoadBalancerServiceIface(ctrl)
16551657
deleteParams := &cloudstack.DeleteLoadBalancerRuleParams{}
1656-
apiErr := fmt.Errorf("delete rule API error")
1658+
apiErr := errors.New("delete rule API error")
16571659

16581660
gomock.InOrder(
16591661
mockLB.EXPECT().NewDeleteLoadBalancerRuleParams("rule-123").Return(deleteParams),
@@ -1723,7 +1725,7 @@ func TestAssignHostsToRule(t *testing.T) {
17231725

17241726
mockLB := cloudstack.NewMockLoadBalancerServiceIface(ctrl)
17251727
assignParams := &cloudstack.AssignToLoadBalancerRuleParams{}
1726-
apiErr := fmt.Errorf("assign API error")
1728+
apiErr := errors.New("assign API error")
17271729

17281730
gomock.InOrder(
17291731
mockLB.EXPECT().NewAssignToLoadBalancerRuleParams("rule-123").Return(assignParams),
@@ -1816,7 +1818,7 @@ func TestRemoveHostsFromRule(t *testing.T) {
18161818

18171819
mockLB := cloudstack.NewMockLoadBalancerServiceIface(ctrl)
18181820
removeParams := &cloudstack.RemoveFromLoadBalancerRuleParams{}
1819-
apiErr := fmt.Errorf("remove API error")
1821+
apiErr := errors.New("remove API error")
18201822

18211823
gomock.InOrder(
18221824
mockLB.EXPECT().NewRemoveFromLoadBalancerRuleParams("rule-123").Return(removeParams),
@@ -2052,7 +2054,7 @@ func TestUpdateFirewallRule(t *testing.T) {
20522054

20532055
mockFirewall := cloudstack.NewMockFirewallServiceIface(ctrl)
20542056
listParams := &cloudstack.ListFirewallRulesParams{}
2055-
apiErr := fmt.Errorf("list API error")
2057+
apiErr := errors.New("list API error")
20562058

20572059
gomock.InOrder(
20582060
mockFirewall.EXPECT().NewListFirewallRulesParams().Return(listParams),
@@ -2087,7 +2089,7 @@ func TestUpdateFirewallRule(t *testing.T) {
20872089
}
20882090

20892091
createParams := &cloudstack.CreateFirewallRuleParams{}
2090-
apiErr := fmt.Errorf("create API error")
2092+
apiErr := errors.New("create API error")
20912093

20922094
gomock.InOrder(
20932095
mockFirewall.EXPECT().NewListFirewallRulesParams().Return(listParams),
@@ -2134,7 +2136,7 @@ func TestUpdateFirewallRule(t *testing.T) {
21342136
}
21352137

21362138
deleteParams := &cloudstack.DeleteFirewallRuleParams{}
2137-
deleteErr := fmt.Errorf("delete API error")
2139+
deleteErr := errors.New("delete API error")
21382140
createParams := &cloudstack.CreateFirewallRuleParams{}
21392141
createResp := &cloudstack.CreateFirewallRuleResponse{
21402142
Id: "fw-124",
@@ -2248,7 +2250,7 @@ func TestDeleteFirewallRule(t *testing.T) {
22482250

22492251
mockFirewall := cloudstack.NewMockFirewallServiceIface(ctrl)
22502252
listParams := &cloudstack.ListFirewallRulesParams{}
2251-
apiErr := fmt.Errorf("list API error")
2253+
apiErr := errors.New("list API error")
22522254

22532255
gomock.InOrder(
22542256
mockFirewall.EXPECT().NewListFirewallRulesParams().Return(listParams),
@@ -2290,7 +2292,7 @@ func TestDeleteFirewallRule(t *testing.T) {
22902292
}
22912293

22922294
deleteParams := &cloudstack.DeleteFirewallRuleParams{}
2293-
deleteErr := fmt.Errorf("delete API error")
2295+
deleteErr := errors.New("delete API error")
22942296

22952297
gomock.InOrder(
22962298
mockFirewall.EXPECT().NewListFirewallRulesParams().Return(listParams),
@@ -2310,7 +2312,7 @@ func TestDeleteFirewallRule(t *testing.T) {
23102312
if deleted {
23112313
t.Errorf("deleted = true, want false")
23122314
}
2313-
if err != deleteErr {
2315+
if !errors.Is(err, deleteErr) {
23142316
t.Errorf("error = %v, want %v", err, deleteErr)
23152317
}
23162318
})
@@ -2858,7 +2860,7 @@ func TestReconcileHostsForRule(t *testing.T) {
28582860
},
28592861
}, nil),
28602862
mockLB.EXPECT().NewAssignToLoadBalancerRuleParams("rule-1").Return(assignParams),
2861-
mockLB.EXPECT().AssignToLoadBalancerRule(gomock.Any()).Return(nil, fmt.Errorf("assign API error")),
2863+
mockLB.EXPECT().AssignToLoadBalancerRule(gomock.Any()).Return(nil, errors.New("assign API error")),
28622864
// removeHostsFromRule should NOT be called because assign failed
28632865
)
28642866

@@ -2886,7 +2888,7 @@ func TestReconcileHostsForRule(t *testing.T) {
28862888
listParams := &cloudstack.ListLoadBalancerRuleInstancesParams{}
28872889

28882890
mockLB.EXPECT().NewListLoadBalancerRuleInstancesParams("rule-1").Return(listParams)
2889-
mockLB.EXPECT().ListLoadBalancerRuleInstances(gomock.Any()).Return(nil, fmt.Errorf("API error"))
2891+
mockLB.EXPECT().ListLoadBalancerRuleInstances(gomock.Any()).Return(nil, errors.New("API error"))
28902892

28912893
lb := &loadBalancer{
28922894
CloudStackClient: &cloudstack.CloudStackClient{

0 commit comments

Comments
 (0)