Skip to content

[improvement] Refactor code blocks ignored by errorlint #370

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Apr 15, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
97 changes: 44 additions & 53 deletions cloud/linode/loadbalancers.go
Original file line number Diff line number Diff line change
Expand Up @@ -126,15 +126,13 @@
}

previousNB, err := l.getNodeBalancerByStatus(ctx, service)
//nolint: errorlint //conversion to errors.Is() may break chainsaw tests
switch err.(type) {
case nil:
// continue execution
break
case lbNotFoundError:
return nil
default:
return err
if err != nil {
var targetError lbNotFoundError
if errors.As(err, &targetError) {
return nil
} else {
return err
}

Check warning on line 135 in cloud/linode/loadbalancers.go

View check run for this annotation

Codecov / codecov/patch

cloud/linode/loadbalancers.go#L134-L135

Added lines #L134 - L135 were not covered by tests
}

nb, err := l.getNodeBalancerForService(ctx, service)
Expand Down Expand Up @@ -178,17 +176,14 @@
}

nb, err := l.getNodeBalancerForService(ctx, service)
//nolint: errorlint //conversion to errors.Is() may break chainsaw tests
switch err.(type) {
case nil:
break

case lbNotFoundError:
return nil, false, nil

default:
sentry.CaptureError(ctx, err)
return nil, false, err
if err != nil {
var targetError lbNotFoundError
if errors.As(err, &targetError) {
return nil, false, nil
} else {
sentry.CaptureError(ctx, err)
return nil, false, err
}

Check warning on line 186 in cloud/linode/loadbalancers.go

View check run for this annotation

Codecov / codecov/patch

cloud/linode/loadbalancers.go#L184-L186

Added lines #L184 - L186 were not covered by tests
}

return makeLoadBalancerStatus(service, nb), true, nil
Expand Down Expand Up @@ -257,31 +252,30 @@
var nb *linodego.NodeBalancer

nb, err = l.getNodeBalancerForService(ctx, service)
//nolint: errorlint //conversion to errors.Is() may break chainsaw tests
switch err.(type) {
case lbNotFoundError:
if service.GetAnnotations()[annotations.AnnLinodeNodeBalancerID] != "" {
// a load balancer annotation has been created so a NodeBalancer is coming, error out and retry later
klog.Infof("NodeBalancer created but not available yet, waiting...")
sentry.CaptureError(ctx, err)
return nil, err
}

if nb, err = l.buildLoadBalancerRequest(ctx, clusterName, service, nodes); err != nil {
if err == nil {
if err = l.updateNodeBalancer(ctx, clusterName, service, nodes, nb); err != nil {
sentry.CaptureError(ctx, err)
return nil, err
}
klog.Infof("created new NodeBalancer (%d) for service (%s)", nb.ID, serviceNn)
} else {
var targetError lbNotFoundError
if errors.As(err, &targetError) {
if service.GetAnnotations()[annotations.AnnLinodeNodeBalancerID] != "" {
// a load balancer annotation has been created so a NodeBalancer is coming, error out and retry later
klog.Infof("NodeBalancer created but not available yet, waiting...")
sentry.CaptureError(ctx, err)
return nil, err
}

Check warning on line 268 in cloud/linode/loadbalancers.go

View check run for this annotation

Codecov / codecov/patch

cloud/linode/loadbalancers.go#L264-L268

Added lines #L264 - L268 were not covered by tests

case nil:
if err = l.updateNodeBalancer(ctx, clusterName, service, nodes, nb); err != nil {
if nb, err = l.buildLoadBalancerRequest(ctx, clusterName, service, nodes); err != nil {
sentry.CaptureError(ctx, err)
return nil, err
}

Check warning on line 273 in cloud/linode/loadbalancers.go

View check run for this annotation

Codecov / codecov/patch

cloud/linode/loadbalancers.go#L271-L273

Added lines #L271 - L273 were not covered by tests
klog.Infof("created new NodeBalancer (%d) for service (%s)", nb.ID, serviceNn)
} else {

Check warning on line 275 in cloud/linode/loadbalancers.go

View check run for this annotation

Codecov / codecov/patch

cloud/linode/loadbalancers.go#L275

Added line #L275 was not covered by tests
sentry.CaptureError(ctx, err)
return nil, err
}

default:
sentry.CaptureError(ctx, err)
return nil, err
}

klog.Infof("NodeBalancer (%d) has been ensured for service (%s)", nb.ID, serviceNn)
Expand Down Expand Up @@ -558,19 +552,16 @@
}

nb, err := l.getNodeBalancerForService(ctx, service)
//nolint: errorlint //conversion to errors.Is() may break chainsaw tests
switch getErr := err.(type) {
case nil:
break

case lbNotFoundError:
klog.Infof("short-circuiting deletion for NodeBalancer for service (%s) as one does not exist: %s", serviceNn, err)
return nil

default:
klog.Errorf("failed to get NodeBalancer for service (%s): %s", serviceNn, err)
sentry.CaptureError(ctx, getErr)
return err
if err != nil {
var targetError lbNotFoundError
if errors.As(err, &targetError) {
klog.Infof("short-circuiting deletion for NodeBalancer for service (%s) as one does not exist: %s", serviceNn, err)
return nil
} else {
klog.Errorf("failed to get NodeBalancer for service (%s): %s", serviceNn, err)
sentry.CaptureError(ctx, err)
return err
}

Check warning on line 564 in cloud/linode/loadbalancers.go

View check run for this annotation

Codecov / codecov/patch

cloud/linode/loadbalancers.go#L556-L564

Added lines #L556 - L564 were not covered by tests
}

if l.shouldPreserveNodeBalancer(service) {
Expand Down Expand Up @@ -628,8 +619,8 @@
func (l *loadbalancers) getNodeBalancerByID(ctx context.Context, service *v1.Service, id int) (*linodego.NodeBalancer, error) {
nb, err := l.client.GetNodeBalancer(ctx, id)
if err != nil {
//nolint: errorlint //need type assertion for code field to work
if apiErr, ok := err.(*linodego.Error); ok && apiErr.Code == http.StatusNotFound {
var targetError *linodego.Error
if errors.As(err, &targetError) && targetError.Code == http.StatusNotFound {
return nil, lbNotFoundError{serviceNn: getServiceNn(service), nodeBalancerID: id}
}
return nil, err
Expand Down
17 changes: 7 additions & 10 deletions cloud/linode/node_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package linode

import (
"context"
"errors"
"net/http"
"os"
"strconv"
Expand Down Expand Up @@ -235,19 +236,15 @@ func (s *nodeController) processNext() bool {
return true
}
err := s.handleNode(context.TODO(), request.node)
//nolint: errorlint //switching to errors.Is()/errors.As() causes errors with Code field
switch deleteErr := err.(type) {
case nil:
break

case *linodego.Error:
if deleteErr.Code >= http.StatusInternalServerError || deleteErr.Code == http.StatusTooManyRequests {
if err != nil {
var targetError *linodego.Error
if errors.As(err, &targetError) &&
(targetError.Code >= http.StatusInternalServerError || targetError.Code == http.StatusTooManyRequests) {
klog.Errorf("failed to add metadata for node (%s); retrying in 1 minute: %s", request.node.Name, err)
s.queue.AddAfter(request, retryInterval)
} else {
klog.Errorf("failed to add metadata for node (%s); will not retry: %s", request.node.Name, err)
}

default:
klog.Errorf("failed to add metadata for node (%s); will not retry: %s", request.node.Name, err)
}

registeredK8sNodeCache.updateCache(s.kubeclient)
Expand Down
17 changes: 7 additions & 10 deletions cloud/linode/service_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import (
"context"
"errors"
"net/http"
"strings"
"time"
Expand Down Expand Up @@ -91,19 +92,15 @@
}

err := s.handleServiceDeleted(service)
//nolint: errorlint //switching to errors.Is()/errors.As() causes errors with Code field
switch deleteErr := err.(type) {
case nil:
break

case *linodego.Error:
if deleteErr.Code >= http.StatusInternalServerError || deleteErr.Code == http.StatusTooManyRequests {
var targetError *linodego.Error
if err != nil {
if errors.As(err, &targetError) &&
(targetError.Code >= http.StatusInternalServerError || targetError.Code == http.StatusTooManyRequests) {

Check warning on line 98 in cloud/linode/service_controller.go

View check run for this annotation

Codecov / codecov/patch

cloud/linode/service_controller.go#L97-L98

Added lines #L97 - L98 were not covered by tests
klog.Errorf("failed to delete NodeBalancer for service (%s); retrying in 1 minute: %s", getServiceNn(service), err)
s.queue.AddAfter(service, retryInterval)
} else {
klog.Errorf("failed to delete NodeBalancer for service (%s); will not retry: %s", getServiceNn(service), err)

Check warning on line 102 in cloud/linode/service_controller.go

View check run for this annotation

Codecov / codecov/patch

cloud/linode/service_controller.go#L101-L102

Added lines #L101 - L102 were not covered by tests
}

default:
klog.Errorf("failed to delete NodeBalancer for service (%s); will not retry: %s", getServiceNn(service), err)
}

return true
Expand Down
Loading