Skip to content

Commit a9fbe11

Browse files
sivcharisivchari
authored
🌱 Use errors package of Go (#10875)
* use std errors package Signed-off-by: sivchari <shibuuuu5@gmail.com> * fix review findings Signed-off-by: sivchari <shibuuuu5@gmail.com> * revert fmt.Errorf Signed-off-by: sivchari <shibuuuu5@gmail.com> --------- Signed-off-by: sivchari <shibuuuu5@gmail.com>
1 parent 2b66e1c commit a9fbe11

File tree

11 files changed

+112
-24
lines changed

11 files changed

+112
-24
lines changed

‎bootstrap/kubeadm/internal/controllers/kubeadmconfig_controller.go‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -194,11 +194,6 @@ func (r *KubeadmConfigReconciler) Reconcile(ctx context.Context, req ctrl.Reques
194194
// Lookup the cluster the config owner is associated with
195195
cluster, err := util.GetClusterByName(ctx, r.Client, configOwner.GetNamespace(), configOwner.ClusterName())
196196
if err != nil {
197-
if errors.Cause(err) == util.ErrNoCluster {
198-
log.Info(fmt.Sprintf("%s does not belong to a cluster yet, waiting until it's part of a cluster", configOwner.GetKind()))
199-
return ctrl.Result{}, nil
200-
}
201-
202197
if apierrors.IsNotFound(err) {
203198
log.Info("Cluster does not exist yet, waiting until it is created")
204199
return ctrl.Result{}, nil

‎cmd/clusterctl/client/cluster/template.go‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -303,10 +303,12 @@ func getGitHubClient(ctx context.Context, configVariablesClient config.Variables
303303
return github.NewClient(authenticatingHTTPClient), nil
304304
}
305305

306+
var errRateLimit = errors.New("rate limit for github api has been reached. Please wait one hour or get a personal API token and assign it to the GITHUB_TOKEN environment variable")
307+
306308
// handleGithubErr wraps error messages.
307309
func handleGithubErr(err error, message string, args ...interface{}) error {
308310
if _, ok := err.(*github.RateLimitError); ok {
309-
return errors.New("rate limit for github api has been reached. Please wait one hour or get a personal API token and assign it to the GITHUB_TOKEN environment variable")
311+
return errRateLimit
310312
}
311313
return errors.Wrapf(err, message, args...)
312314
}

‎cmd/clusterctl/client/cluster/template_test.go‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ import (
2929

3030
"github.com/google/go-github/v53/github"
3131
. "github.com/onsi/gomega"
32+
"github.com/pkg/errors"
3233
corev1 "k8s.io/api/core/v1"
3334
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
3435

@@ -513,6 +514,43 @@ func Test_templateClient_GetFromURL(t *testing.T) {
513514
}
514515
}
515516

517+
func Test_handleGithubErr(t *testing.T) {
518+
tests := []struct {
519+
name string
520+
err error
521+
message string
522+
args []any
523+
want error
524+
}{
525+
{
526+
name: "Return error",
527+
err: errors.New("error"),
528+
message: "message %s and %s",
529+
args: []any{"arg1", "arg2"},
530+
want: fmt.Errorf("message arg1 and arg2: %w", errors.New("error")),
531+
},
532+
{
533+
name: "Return RateLimitError",
534+
err: &github.RateLimitError{
535+
Response: &http.Response{
536+
StatusCode: http.StatusForbidden,
537+
},
538+
},
539+
message: "",
540+
args: nil,
541+
want: errRateLimit,
542+
},
543+
}
544+
for _, tt := range tests {
545+
t.Run(tt.name, func(t *testing.T) {
546+
g := NewWithT(t)
547+
548+
got := handleGithubErr(tt.err, tt.message, tt.args...)
549+
g.Expect(got.Error()).To(Equal(tt.want.Error()))
550+
})
551+
}
552+
}
553+
516554
func mustParseURL(rawURL string) *url.URL {
517555
rURL, err := url.Parse(rawURL)
518556
if err != nil {

‎cmd/clusterctl/client/repository/repository_github.go‎

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,8 @@ const (
5050
)
5151

5252
var (
53-
errNotFound = errors.New("404 Not Found")
53+
errNotFound = errors.New("404 Not Found")
54+
errRateLimit = errors.New("rate limit for github api has been reached. Please wait one hour or get a personal API token and assign it to the GITHUB_TOKEN environment variable")
5455

5556
// Caches used to limit the number of GitHub API calls.
5657

@@ -319,7 +320,7 @@ func (g *gitHubRepository) getVersions(ctx context.Context) ([]string, error) {
319320
if listReleasesErr != nil {
320321
retryError = g.handleGithubErr(listReleasesErr, "failed to get the list of releases")
321322
// Return immediately if we are rate limited.
322-
if _, ok := listReleasesErr.(*github.RateLimitError); ok {
323+
if errors.Is(retryError, errRateLimit) {
323324
return false, retryError
324325
}
325326
return false, nil
@@ -334,7 +335,7 @@ func (g *gitHubRepository) getVersions(ctx context.Context) ([]string, error) {
334335
if listReleasesErr != nil {
335336
retryError = g.handleGithubErr(listReleasesErr, "failed to get the list of releases")
336337
// Return immediately if we are rate limited.
337-
if _, ok := listReleasesErr.(*github.RateLimitError); ok {
338+
if errors.Is(retryError, errRateLimit) {
338339
return false, retryError
339340
}
340341
return false, nil
@@ -384,7 +385,7 @@ func (g *gitHubRepository) getReleaseByTag(ctx context.Context, tag string) (*gi
384385
return false, retryError
385386
}
386387
// Return immediately if we are rate limited.
387-
if _, ok := getReleasesErr.(*github.RateLimitError); ok {
388+
if errors.Is(retryError, errRateLimit) {
388389
return false, retryError
389390
}
390391
return false, nil
@@ -466,7 +467,7 @@ func (g *gitHubRepository) downloadFilesFromRelease(ctx context.Context, release
466467
if downloadReleaseError != nil {
467468
retryError = g.handleGithubErr(downloadReleaseError, "failed to download file %q from %q release", *release.TagName, fileName)
468469
// Return immediately if we are rate limited.
469-
if _, ok := downloadReleaseError.(*github.RateLimitError); ok {
470+
if errors.Is(retryError, errRateLimit) {
470471
return false, retryError
471472
}
472473
return false, nil
@@ -500,12 +501,13 @@ func (g *gitHubRepository) downloadFilesFromRelease(ctx context.Context, release
500501
// handleGithubErr wraps error messages.
501502
func (g *gitHubRepository) handleGithubErr(err error, message string, args ...interface{}) error {
502503
if _, ok := err.(*github.RateLimitError); ok {
503-
return errors.New("rate limit for github api has been reached. Please wait one hour or get a personal API token and assign it to the GITHUB_TOKEN environment variable")
504+
return errRateLimit
504505
}
505-
if ghErr, ok := err.(*github.ErrorResponse); ok {
506-
if ghErr.Response.StatusCode == http.StatusNotFound {
507-
return errNotFound
508-
}
506+
507+
var ghErr *github.ErrorResponse
508+
if errors.As(err, &ghErr) && ghErr.Response.StatusCode == http.StatusNotFound {
509+
return errNotFound
509510
}
510-
return errors.Wrapf(err, message, args...)
511+
512+
return fmt.Errorf("%s: %w", fmt.Sprintf(message, args...), err)
511513
}

‎cmd/clusterctl/client/repository/repository_github_test.go‎

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import (
2626

2727
"github.com/google/go-github/v53/github"
2828
. "github.com/onsi/gomega"
29+
"github.com/pkg/errors"
2930
"k8s.io/utils/ptr"
3031

3132
clusterctlv1 "sigs.k8s.io/cluster-api/cmd/clusterctl/api/v1alpha3"
@@ -1108,3 +1109,54 @@ func Test_gitHubRepository_releaseNotFound(t *testing.T) {
11081109
})
11091110
}
11101111
}
1112+
1113+
func Test_handleGithubErr(t *testing.T) {
1114+
tests := []struct {
1115+
name string
1116+
err error
1117+
message string
1118+
args []any
1119+
want error
1120+
}{
1121+
{
1122+
name: "Return error",
1123+
err: errors.New("error"),
1124+
message: "message %s and %s",
1125+
args: []any{"arg1", "arg2"},
1126+
want: fmt.Errorf("message arg1 and arg2: %w", errors.New("error")),
1127+
},
1128+
{
1129+
name: "Return RateLimitError",
1130+
err: &github.RateLimitError{
1131+
Response: &http.Response{
1132+
StatusCode: http.StatusForbidden,
1133+
},
1134+
},
1135+
message: "",
1136+
args: nil,
1137+
want: errRateLimit,
1138+
},
1139+
{
1140+
name: "Return ErrorResponse",
1141+
err: &github.ErrorResponse{
1142+
Response: &http.Response{
1143+
StatusCode: http.StatusNotFound,
1144+
},
1145+
},
1146+
message: "",
1147+
args: nil,
1148+
want: errNotFound,
1149+
},
1150+
}
1151+
1152+
gRepo := &gitHubRepository{}
1153+
1154+
for _, tt := range tests {
1155+
t.Run(tt.name, func(t *testing.T) {
1156+
g := NewWithT(t)
1157+
1158+
got := gRepo.handleGithubErr(tt.err, tt.message, tt.args...)
1159+
g.Expect(got.Error()).To(Equal(tt.want.Error()))
1160+
})
1161+
}
1162+
}

‎internal/controllers/cluster/cluster_controller_phases.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -361,7 +361,7 @@ func (r *Reconciler) reconcileKubeconfig(ctx context.Context, s *scope) (ctrl.Re
361361
switch {
362362
case apierrors.IsNotFound(err):
363363
if err := kubeconfig.CreateSecret(ctx, r.Client, cluster); err != nil {
364-
if err == kubeconfig.ErrDependentCertificateNotFound {
364+
if errors.Is(err, kubeconfig.ErrDependentCertificateNotFound) {
365365
log.Info("Could not find secret for cluster, requeuing", "Secret", secret.ClusterCA)
366366
return ctrl.Result{RequeueAfter: 30 * time.Second}, nil
367367
}

‎internal/controllers/clusterresourceset/clusterresourceset_controller.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -314,7 +314,7 @@ func (r *Reconciler) ApplyClusterResourceSet(ctx context.Context, cluster *clust
314314
for i, resource := range clusterResourceSet.Spec.Resources {
315315
unstructuredObj, err := r.getResource(ctx, resource, cluster.GetNamespace())
316316
if err != nil {
317-
if err == ErrSecretTypeNotSupported {
317+
if errors.Is(err, ErrSecretTypeNotSupported) {
318318
v1beta1conditions.MarkFalse(clusterResourceSet, addonsv1.ResourcesAppliedV1Beta1Condition, addonsv1.WrongSecretTypeV1Beta1Reason, clusterv1.ConditionSeverityWarning, "%s", err.Error())
319319
conditions.Set(clusterResourceSet, metav1.Condition{
320320
Type: addonsv1.ClusterResourceSetResourcesAppliedCondition,

‎internal/controllers/machine/drain/drain.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -375,7 +375,7 @@ evictionLoop:
375375
// Ensure the causes are also included in the error message.
376376
// Before: "Cannot evict pod as it would violate the pod's disruption budget."
377377
// After: "Cannot evict pod as it would violate the pod's disruption budget. The disruption budget nginx needs 20 healthy pods and has 20 currently"
378-
if ok := errors.As(err, &statusError); ok {
378+
if errors.As(err, &statusError) {
379379
errorMessage := statusError.Status().Message
380380
if statusError.Status().Details != nil {
381381
var causes []string

‎internal/controllers/machine/machine_controller_noderef.go‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -83,12 +83,11 @@ func (r *Reconciler) reconcileNode(ctx context.Context, s *scope) (ctrl.Result,
8383
// Even if Status.NodeRef exists, continue to do the following checks to make sure Node is healthy
8484
node, err := r.getNode(ctx, remoteClient, machine.Spec.ProviderID)
8585
if err != nil {
86-
if err == ErrNodeNotFound {
86+
if errors.Is(err, ErrNodeNotFound) {
8787
if !s.machine.DeletionTimestamp.IsZero() {
8888
// Tolerate node not found when the machine is being deleted.
8989
return ctrl.Result{}, nil
9090
}
91-
9291
// While a NodeRef is set in the status, failing to get that node means the node is deleted.
9392
// If Status.NodeRef is not set before, node still can be in the provisioning state.
9493
if machine.Status.NodeRef.IsDefined() {

‎internal/controllers/machinepool/machinepool_controller_noderef.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,7 @@ func (r *Reconciler) reconcileNodeRefs(ctx context.Context, s *scope) (ctrl.Resu
9696

9797
nodeRefsResult, err := r.getNodeReferences(ctx, mp.Spec.ProviderIDList, ptr.Deref(mp.Spec.Template.Spec.MinReadySeconds, 0), s.nodeRefMap)
9898
if err != nil {
99-
if err == errNoAvailableNodes {
99+
if errors.Is(err, errNoAvailableNodes) {
100100
log.Info("Cannot assign NodeRefs to MachinePool, no matching Nodes")
101101
// No need to requeue here. Nodes emit an event that triggers reconciliation.
102102
return ctrl.Result{}, nil

0 commit comments

Comments
 (0)