From a5cd414c2d4d8d155a9f11e7af6d944481778feb Mon Sep 17 00:00:00 2001 From: Rahul Sharma Date: Tue, 24 Sep 2024 13:03:31 +0000 Subject: [PATCH] add unittests for ratelimits.go file --- clients/clients.go | 3 +- controller/linodemachine_controller.go | 9 +- mock/client.go | 12 -- util/ratelimits.go | 4 +- util/ratelimits_test.go | 226 +++++++++++++++++++++++++ 5 files changed, 236 insertions(+), 18 deletions(-) create mode 100644 util/ratelimits_test.go diff --git a/clients/clients.go b/clients/clients.go index 81dee542..723d2bea 100644 --- a/clients/clients.go +++ b/clients/clients.go @@ -21,6 +21,8 @@ type LinodeClient interface { LinodePlacementGroupClient LinodeFirewallClient LinodeTokenClient + + OnAfterResponse(m func(response *resty.Response) error) } type AkamClient interface { @@ -52,7 +54,6 @@ type LinodeInstanceClient interface { CreateStackscript(ctx context.Context, opts linodego.StackscriptCreateOptions) (*linodego.Stackscript, error) ListStackscripts(ctx context.Context, opts *linodego.ListOptions) ([]linodego.Stackscript, error) GetType(ctx context.Context, typeID string) (*linodego.LinodeType, error) - OnAfterResponse(m func(response *resty.Response) error) } // LinodeVPCClient defines the methods that interact with Linode's VPC service. diff --git a/controller/linodemachine_controller.go b/controller/linodemachine_controller.go index 4ea97229..0e237863 100644 --- a/controller/linodemachine_controller.go +++ b/controller/linodemachine_controller.go @@ -285,8 +285,8 @@ func (r *LinodeMachineReconciler) createInstance(ctx context.Context, logger log ctr.Mu.Lock() defer ctr.Mu.Unlock() - if ctr.IsPOSTLimitReached(logger) { - logger.Info(fmt.Sprintf("reached linode API rate limit, retry after %v seconds", reconciler.SecondaryLinodeTooManyPOSTRequestsErrorRetryDelay)) + if ctr.IsPOSTLimitReached() { + logger.Info(fmt.Sprintf("Cannot make more requests as max requests have been made. Waiting and retrying after %v seconds", reconciler.SecondaryLinodeTooManyPOSTRequestsErrorRetryDelay)) return nil, nil, true } @@ -317,6 +317,11 @@ func (r *LinodeMachineReconciler) reconcilePreflightCreate(ctx context.Context, } return retryIfTransient(err) } + + if linodeInstance == nil { + return ctrl.Result{RequeueAfter: reconciler.DefaultMachineControllerWaitForPreflightTimeout}, nil + } + conditions.MarkTrue(machineScope.LinodeMachine, ConditionPreflightCreated) // Set the provider ID since the instance is successfully created machineScope.LinodeMachine.Spec.ProviderID = util.Pointer(fmt.Sprintf("linode://%d", linodeInstance.ID)) diff --git a/mock/client.go b/mock/client.go index 7ed4a883..c8d7091f 100644 --- a/mock/client.go +++ b/mock/client.go @@ -1306,18 +1306,6 @@ func (mr *MockLinodeInstanceClientMockRecorder) ListStackscripts(ctx, opts any) return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ListStackscripts", reflect.TypeOf((*MockLinodeInstanceClient)(nil).ListStackscripts), ctx, opts) } -// OnAfterResponse mocks base method. -func (m_2 *MockLinodeInstanceClient) OnAfterResponse(m func(*resty.Response) error) { - m_2.ctrl.T.Helper() - m_2.ctrl.Call(m_2, "OnAfterResponse", m) -} - -// OnAfterResponse indicates an expected call of OnAfterResponse. -func (mr *MockLinodeInstanceClientMockRecorder) OnAfterResponse(m any) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "OnAfterResponse", reflect.TypeOf((*MockLinodeInstanceClient)(nil).OnAfterResponse), m) -} - // ResizeInstanceDisk mocks base method. func (m *MockLinodeInstanceClient) ResizeInstanceDisk(ctx context.Context, linodeID, diskID, size int) error { m.ctrl.T.Helper() diff --git a/util/ratelimits.go b/util/ratelimits.go index 38d6eff2..6a9a860f 100644 --- a/util/ratelimits.go +++ b/util/ratelimits.go @@ -23,7 +23,6 @@ import ( "sync" "time" - "github.com/go-logr/logr" "github.com/go-resty/resty/v2" "github.com/linode/cluster-api-provider-linode/util/reconciler" @@ -63,7 +62,7 @@ func (c *PostRequestCounter) ApiResponseRatelimitCounter(resp *resty.Response) e } // IsPOSTLimitReached checks whether POST limits have been reached. -func (c *PostRequestCounter) IsPOSTLimitReached(logger logr.Logger) bool { +func (c *PostRequestCounter) IsPOSTLimitReached() bool { // TODO: Once linode API adjusts rate-limits, remove secondary rate limit and simplify accordingly if c.ReqRemaining == reconciler.SecondaryPOSTRequestLimit || c.ReqRemaining == 0 { actualRefreshTime := c.RefreshTime @@ -73,7 +72,6 @@ func (c *PostRequestCounter) IsPOSTLimitReached(logger logr.Logger) bool { } // check if refresh time has passed if time.Now().Unix() <= int64(actualRefreshTime) { - logger.Info("Cannot make more requests as max requests have been made. Waiting and retrying ...") return true } else if c.ReqRemaining == 0 { // reset limits, set max allowed POST requests to default max diff --git a/util/ratelimits_test.go b/util/ratelimits_test.go new file mode 100644 index 00000000..66eefdbc --- /dev/null +++ b/util/ratelimits_test.go @@ -0,0 +1,226 @@ +/* +Copyright 2023 Akamai Technologies, Inc. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package util + +import ( + "net/http" + "reflect" + "testing" + "time" + + "github.com/go-resty/resty/v2" + + "github.com/linode/cluster-api-provider-linode/util/reconciler" +) + +func TestGetPostReqCounter(t *testing.T) { + t.Parallel() + tests := []struct { + name string + tokenHash string + want *PostRequestCounter + }{ + { + name: "provide hash which exists in map", + tokenHash: "abcdef", + want: &PostRequestCounter{ + ReqRemaining: 5, + RefreshTime: 3, + }, + }, + { + name: "provide hash which doesn't exist", + tokenHash: "uvwxyz", + want: &PostRequestCounter{ + ReqRemaining: reconciler.DefaultPOSTRequestLimit, + RefreshTime: 0, + }, + }, + } + for _, tt := range tests { + postRequestCounters["abcdef"] = &PostRequestCounter{ + ReqRemaining: reconciler.SecondaryPOSTRequestLimit, + RefreshTime: 3, + } + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + if got := GetPostReqCounter(tt.tokenHash); !reflect.DeepEqual(got, tt.want) { + t.Errorf("GetPostReqCounter() = %v, want %v", got, tt.want) + } + }) + } +} + +func TestPostRequestCounter_IsPOSTLimitReached(t *testing.T) { + t.Parallel() + tests := []struct { + name string + fields *PostRequestCounter + want bool + }{ + { + name: "not reached rate limits", + fields: &PostRequestCounter{ + ReqRemaining: 7, + RefreshTime: int(time.Now().Unix()), + }, + want: false, + }, + { + name: "reached token rate limit", + fields: &PostRequestCounter{ + ReqRemaining: reconciler.SecondaryPOSTRequestLimit, + RefreshTime: int(time.Now().Unix() + 100), + }, + want: true, + }, + { + name: "reached account rate limits", + fields: &PostRequestCounter{ + ReqRemaining: 0, + RefreshTime: int(time.Now().Unix() + 100), + }, + want: true, + }, + { + name: "refresh time smaller than current time", + fields: &PostRequestCounter{ + ReqRemaining: reconciler.SecondaryPOSTRequestLimit, + RefreshTime: int(time.Now().Unix() - 100), + }, + want: false, + }, + { + name: "refresh time smaller than current time", + fields: &PostRequestCounter{ + ReqRemaining: 0, + RefreshTime: int(time.Now().Unix() - 100), + }, + want: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + c := &PostRequestCounter{ + ReqRemaining: tt.fields.ReqRemaining, + RefreshTime: tt.fields.RefreshTime, + } + if got := c.IsPOSTLimitReached(); got != tt.want { + t.Errorf("PostRequestCounter.IsPOSTLimitReached() = %v, want %v", got, tt.want) + } + }) + } +} + +func TestPostRequestCounter_ApiResponseRatelimitCounter(t *testing.T) { + t.Parallel() + tests := []struct { + name string + fields *PostRequestCounter + args *resty.Response + wantErr bool + }{ + { + name: "not a POST call", + fields: &PostRequestCounter{ + ReqRemaining: 6, + RefreshTime: int(time.Now().Unix()), + }, + args: &resty.Response{ + Request: &resty.Request{ + Method: http.MethodGet, + }, + }, + wantErr: false, + }, + { + name: "endpoint different than /linode/instances", + fields: &PostRequestCounter{ + ReqRemaining: 6, + RefreshTime: int(time.Now().Unix()), + }, + args: &resty.Response{ + Request: &resty.Request{ + Method: http.MethodPost, + URL: "/v4/vpc/ips", + }, + }, + wantErr: false, + }, + { + name: "no headers in response", + fields: &PostRequestCounter{ + ReqRemaining: 6, + RefreshTime: int(time.Now().Unix()), + }, + args: &resty.Response{ + Request: &resty.Request{ + Method: http.MethodPost, + URL: "/v4/linode/instances", + }, + }, + wantErr: true, + }, + { + name: "missing one value in response header", + fields: &PostRequestCounter{ + ReqRemaining: 6, + RefreshTime: int(time.Now().Unix()), + }, + args: &resty.Response{ + Request: &resty.Request{ + Method: http.MethodPost, + URL: "/v4/linode/instances", + }, + RawResponse: &http.Response{ + Header: http.Header{"X-Ratelimit-Remaining": []string{"10"}}, + }, + }, + wantErr: true, + }, + { + name: "correct headers in response", + fields: &PostRequestCounter{ + ReqRemaining: 6, + RefreshTime: int(time.Now().Unix()), + }, + args: &resty.Response{ + Request: &resty.Request{ + Method: http.MethodPost, + URL: "/v4/linode/instances", + }, + RawResponse: &http.Response{ + Header: http.Header{"X-Ratelimit-Remaining": []string{"10"}, "X-Ratelimit-Reset": []string{"10"}}, + }, + }, + wantErr: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + c := &PostRequestCounter{ + ReqRemaining: tt.fields.ReqRemaining, + RefreshTime: tt.fields.RefreshTime, + } + if err := c.ApiResponseRatelimitCounter(tt.args); (err != nil) != tt.wantErr { + t.Errorf("PostRequestCounter.ApiResponseRatelimitCounter() error = %v, wantErr %v", err, tt.wantErr) + } + }) + } +}