Skip to content

Commit 4e9cb5f

Browse files
committed
use std errors package
Signed-off-by: sivchari <shibuuuu5@gmail.com>
1 parent 8f348b6 commit 4e9cb5f

File tree

13 files changed

+153
-27
lines changed

13 files changed

+153
-27
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: 4 additions & 2 deletions
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
}
311-
return errors.Wrapf(err, message, args...)
313+
return fmt.Errorf("%s: %w", fmt.Sprintf(message, args...), err)
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

@@ -517,6 +518,43 @@ func Test_templateClient_GetFromURL(t *testing.T) {
517518
}
518519
}
519520

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

controlplane/kubeadm/internal/controllers/controller_test.go

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2831,6 +2831,45 @@ func TestKubeadmControlPlaneReconciler_reconcileControlPlaneAndMachinesCondition
28312831
}
28322832
}
28332833

2834+
func TestKubeadmControlPlaneReconciler_deferPatch(t *testing.T) {
2835+
g := NewWithT(t)
2836+
2837+
ns, err := env.CreateNamespace(ctx, "test-reconcile-update-status")
2838+
g.Expect(err).ToNot(HaveOccurred())
2839+
2840+
cluster, kcp, _ := createClusterWithControlPlane(ns.Name)
2841+
g.Expect(env.Create(ctx, cluster)).To(Succeed())
2842+
g.Expect(env.Create(ctx, kcp)).To(Succeed())
2843+
defer func(do ...client.Object) {
2844+
g.Expect(env.Cleanup(ctx, do...)).To(Succeed())
2845+
}(kcp, ns)
2846+
2847+
r := &KubeadmControlPlaneReconciler{
2848+
Client: env,
2849+
SecretCachingClient: secretCachingClient,
2850+
managementCluster: &fakeManagementCluster{
2851+
WorkloadErr: &internal.RemoteClusterConnectionError{
2852+
Name: util.ObjectKey(cluster).String(),
2853+
Err: errors.New("connection error"),
2854+
},
2855+
},
2856+
}
2857+
2858+
controlPlane, _, err := r.initControlPlaneScope(ctx, cluster, kcp)
2859+
g.Expect(err).ToNot(HaveOccurred())
2860+
2861+
patchHelper, err := patch.NewHelper(kcp, env)
2862+
g.Expect(err).ToNot(HaveOccurred())
2863+
2864+
result, err := r.deferPatch(ctx, kcp, controlPlane, patchHelper)
2865+
// Should bump RemoteClusterConnectionError
2866+
g.Expect(err).NotTo(HaveOccurred())
2867+
g.Expect(result).To(Equal(ctrl.Result{RequeueAfter: 20 * time.Second}))
2868+
2869+
// calling reconcile should return error
2870+
g.Expect(env.CleanupAndWait(ctx, cluster)).To(Succeed())
2871+
}
2872+
28342873
type fakeClusterCache struct {
28352874
clustercache.ClusterCache
28362875
lastProbeSuccessTime time.Time

controlplane/kubeadm/internal/controllers/status.go

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@ import (
2323
"strings"
2424
"time"
2525

26-
"github.com/pkg/errors"
2726
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2827
"k8s.io/apimachinery/pkg/util/sets"
2928
"k8s.io/utils/ptr"
@@ -100,7 +99,7 @@ func (r *KubeadmControlPlaneReconciler) updateStatus(ctx context.Context, contro
10099

101100
workloadCluster, err := controlPlane.GetWorkloadCluster(ctx)
102101
if err != nil {
103-
return errors.Wrap(err, "failed to create remote cluster client")
102+
return fmt.Errorf("failed to create remote cluster client: %w", err)
104103
}
105104
status, err := workloadCluster.ClusterStatus(ctx)
106105
if err != nil {

exp/internal/controllers/machinepool_controller_noderef.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,7 @@ func (r *MachinePoolReconciler) reconcileNodeRefs(ctx context.Context, s *scope)
9797
// Get the Node references.
9898
nodeRefsResult, err := r.getNodeReferences(ctx, mp.Spec.ProviderIDList, mp.Spec.MinReadySeconds, s.nodeRefMap)
9999
if err != nil {
100-
if err == errNoAvailableNodes {
100+
if errors.Is(err, errNoAvailableNodes) {
101101
log.Info("Cannot assign NodeRefs to MachinePool, no matching Nodes")
102102
// No need to requeue here. Nodes emit an event that triggers reconciliation.
103103
return ctrl.Result{}, nil

exp/internal/controllers/machinepool_controller_phases.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -346,7 +346,7 @@ func (r *MachinePoolReconciler) reconcileInfrastructure(ctx context.Context, s *
346346
// Get and set Status.Replicas from the infrastructure provider.
347347
err = util.UnstructuredUnmarshalField(infraConfig, &mp.Status.Replicas, "status", "replicas")
348348
if err != nil {
349-
if err != util.ErrUnstructuredFieldNotFound {
349+
if !errors.Is(err, util.ErrUnstructuredFieldNotFound) {
350350
return ctrl.Result{}, errors.Wrapf(err, "failed to retrieve replicas from infrastructure provider for MachinePool %q in namespace %q", mp.Name, mp.Namespace)
351351
}
352352
}

internal/controllers/cluster/cluster_controller_phases.go

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

0 commit comments

Comments
 (0)