Handle throttling #169

ghost · 2022-10-31T13:48:08Z

What type of PR is this?
/kind feature

What this PR does / why we need it:
To handle throttling

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #3
Special notes for your reviewer:
It handle code error and code type.
No Mock.
First version that will be changes when throttling will have more restrict rules.

TODOs:

squashed commits
includes documentation
adds unit tests

ghost · 2022-11-04T15:50:10Z

https://github.com/outscale-dev/cluster-api-provider-outscale/actions/runs/3388862809/jobs/5638128850
Freeza failed

outscale-mdr · 2022-11-04T16:14:03Z

Frieza should not have to destroy resources, the tests should

outscale-mdr · 2022-11-08T08:42:03Z

util/reconciler/defaults.go

+		buffer := new(strings.Builder)
+		_, err := io.Copy(buffer, httpRes.Body)
+		httpResBody := buffer.String()
+		httpResBodyData := []byte(httpResBody)
+		httpResBodyParsed, err := gabs.ParseJSON(httpResBodyData)
+		if err != nil {
+			return true
+		}
+		httpResCode := strings.Replace(strings.Replace(fmt.Sprintf("%v", httpResBodyParsed.Path("Errors.Code").Data()), "[", "", 1), "]", "", 1)
+		httpResType := strings.Replace(strings.Replace(fmt.Sprintf("%v", httpResBodyParsed.Path("Errors.Type").Data()), "[", "", 1), "]", "", 1)


Use this instead of reading the body manually

outscale-mdr · 2022-11-08T08:43:58Z

controllers/osccluster_securitygroup_controller.go

 			clusterScope.Info("Find body", "httpResBody", httpResBody)
+			clusterScope.Info("Find httpResCode", "httpResCode", httpRes.StatusCode)


Keep logging as concise as possible. One line for logging, not multiple

You should have these info when you have the debug activated

outscale-mdr · 2022-11-08T08:45:33Z

cloud/services/storage/volume.go

+			if httpRes != nil {
+				fmt.Printf("Error with http result %s", httpRes.Status)
+			}
+			allowedErrors := map[string]int{"AccessDenied": 503}


Throttling error should be a constant so that we can modify later

ghost · 2022-11-09T13:56:56Z

Closes #3

ghost · 2022-11-15T14:00:48Z

@outscale-mdr @outscale-hmi are you ok with latest changes ?

outscale-mdr · 2022-11-15T14:30:18Z

cloud/services/compute/image.go

+				allowedErrors) {
+				return false, nil
+			}
+			return false, fmt.Errorf("coulld not read image in Osc: %v", err)


outscale-mdr · 2022-11-15T14:31:24Z

cloud/services/compute/image.go

+				fmt.Printf("Error with http result %s", httpRes.Status)
+			}
+			requestStr := fmt.Sprintf("%v", readImageRequest)
+			allowedErrors := map[string]int{"AccessDenied": 503}


"AccessDenied": 503 should be refactorize

Add the "429" for throttling (in case of changes)

outscale-mdr · 2022-11-15T14:31:52Z

cloud/services/compute/image.go

+				fmt.Printf("Error with http result %s", httpRes.Status)
+			}
+			requestStr := fmt.Sprintf("%v", readImageRequest)
+			allowedErrors := map[string]int{"AccessDenied": 503}


Same refactorisation

outscale-mdr · 2022-11-15T14:32:06Z

cloud/services/compute/image.go

+				fmt.Printf("Error with http result %s", httpRes.Status)
+			}
+			requestStr := fmt.Sprintf("%v", readImageRequest)
+			allowedErrors := map[string]int{"AccessDenied": 503}


Same refactorisation

outscale-mdr · 2022-11-15T14:32:46Z

cloud/services/compute/vm.go

+				fmt.Printf("Error with http result %s", httpRes.Status)
+			}
+			requestStr := fmt.Sprintf("%v", vmOpt)
+			allowedErrors := map[string]int{"AccessDenied": 503}


Same refactorisation

outscale-mdr · 2022-11-15T14:33:52Z

cloud/services/compute/vm.go

+			if httpRes != nil {
+				fmt.Printf("Error with http result %s", httpRes.Status)
+			}
+			allowedErrors := map[string]int{"AccessDenied": 503}


Same refactorisation

outscale-mdr · 2022-11-15T14:36:55Z

cloud/services/compute/vm.go

+				fmt.Printf("Error with http result %s", httpRes.Status)
+			}
+			allowedErrors := map[string]int{"AccessDenied": 503}
+			requestStr := fmt.Sprintf("Name")


What is it ? Your message will be

Retry even if got (%v) error type with code error (%v) on request Name

You should show the request.

outscale-mdr · 2022-11-15T14:40:46Z

cloud/services/compute/vm.go

+				allowedErrors) {
+				return false, nil
+			}
+			return false, fmt.Errorf("could not read vm in Osc:: %v", err)


outscale-mdr · 2022-11-15T14:41:09Z

cloud/services/compute/vm.go

+				fmt.Printf("Error with http result %s", httpRes.Status)
+			}
+			allowedErrors := map[string]int{"AccessDennied": 503}
+			requestStr := fmt.Sprintf("OscK8sNodeName")


Improve the requestStr

outscale-mdr · 2022-11-15T14:47:37Z

cloud/services/net/internetservice.go

+		_, httpRes, err = oscApiClient.InternetServiceApi.UnlinkInternetService(oscAuthClient).UnlinkInternetServiceRequest(unlinkInternetServiceRequest).Execute()
+		if err != nil {
+			if httpRes != nil {
+				fmt.Printf("Error with http resul %s", httpRes.Status)


ghost · 2022-11-07T19:32:34Z

cloud/services/net/internetservice.go

+		_, httpRes, err = oscApiClient.InternetServiceApi.LinkInternetService(oscAuthClient).LinkInternetServiceRequest(linkInternetServiceRequest).Execute()
+		if err != nil {
+			if httpRes != nil {
+				fmt.Printf("Error ith http result %s", httpRes.Status)


ghost · 2022-11-07T19:32:46Z

cloud/services/net/internetservice.go

+		_, httpRes, err = oscApiClient.InternetServiceApi.UnlinkInternetService(oscAuthClient).UnlinkInternetServiceRequest(unlinkInternetServiceRequest).Execute()
+		if err != nil {
+			if httpRes != nil {
+				fmt.Printf("Error with http resul %s", httpRes.Status)


ghost · 2022-11-07T19:33:11Z

cloud/services/net/net.go

+				allowedErrors) {
+				return false, nil
+			}
+			return false, fmt.Errorf("could noot create net in Osc: %v", err)


ghost · 2022-11-07T19:34:32Z

cloud/services/storage/volume.go

+				allowedErrors) {
+				return false, nil
+			}
+			return false, fmt.Errorf("couldd not create volume in OSC: %v", err)


ghost · 2022-11-10T09:04:26Z

controllers/osccluster_securitygroup_controller.go

 			clusterScope.Info("Find body", "httpResBody", httpResBody)
+			clusterScope.Info("Find httpResCode", "httpResCode", httpRes.StatusCode)


ghost · 2022-11-18T10:37:12Z

cloud/services/compute/vm.go

+			if httpRes != nil {
+				fmt.Printf("Error with http result %s", httpRes.Status)
+			}
+			allowedErrors := map[string]int{"AccessDenied": 503}


ghost · 2022-11-18T10:37:16Z

cloud/services/compute/vm.go

+				fmt.Printf("Error with http result %s", httpRes.Status)
+			}
+			allowedErrors := map[string]int{"AccessDenied": 503}
+			requestStr := fmt.Sprintf("Name")


ghost · 2022-11-18T10:37:20Z

cloud/services/compute/vm.go

+				allowedErrors) {
+				return false, nil
+			}
+			return false, fmt.Errorf("could not read vm in Osc:: %v", err)


ghost · 2022-11-18T10:37:25Z

cloud/services/compute/vm.go

+				fmt.Printf("Error with http result %s", httpRes.Status)
+			}
+			allowedErrors := map[string]int{"AccessDennied": 503}
+			requestStr := fmt.Sprintf("OscK8sNodeName")


ghost · 2022-11-18T10:37:29Z

cloud/services/net/internetservice.go

+		_, httpRes, err = oscApiClient.InternetServiceApi.UnlinkInternetService(oscAuthClient).UnlinkInternetServiceRequest(unlinkInternetServiceRequest).Execute()
+		if err != nil {
+			if httpRes != nil {
+				fmt.Printf("Error with http resul %s", httpRes.Status)


outscale-mdr · 2022-11-18T10:56:09Z

cloud/services/net/net.go

-	err = tag.AddTag("Name", netName, resourceIds, oscApiClient, oscAuthClient)
-	if err != nil {
-		fmt.Printf("Error with http result %s", httpRes.Status)
-		return nil, err
+	netTag := osc.ResourceTag{
+		Key:   "Name",
+		Value: netName,
 	}
-	err = tag.AddTag("OscK8sClusterID/"+clusterName, "owned", resourceIds, oscApiClient, oscAuthClient)
-	if err != nil {
-		fmt.Printf("Error with http result %s", httpRes.Status)
+	netTagRequest := osc.CreateTagsRequest{
+		ResourceIds: resourceIds,
+		Tags:        []osc.ResourceTag{netTag},
+	}
+
+	addTagNameCallBack := func() (bool, error) {
+		err, httpRes := tag.AddTag(netTagRequest, resourceIds, oscApiClient, oscAuthClient)
+		if err != nil {
+			if httpRes != nil {
+				fmt.Printf("Error with http result %s", httpRes.Status)
+			}
+			requestStr := fmt.Sprintf("%v", netTagRequest)
+			if reconciler.KeepRetryWithError(
+				requestStr,
+				httpRes.StatusCode,
+				reconciler.AllowedErrors) {
+				return false, nil
+			}
+			return false, fmt.Errorf("%w failed to add Name tag", err)
+		}
+		return true, err
+	}
+	waitErr = wait.ExponentialBackoff(backoff, addTagNameCallBack)
+	if waitErr != nil {
 		return nil, err
 	}


I did see during the first review but the retry should be done when you make the call API.

Here the retry should be done inside the tag.AddTag.

With you current changes, we have a lot of duplicate codes !

outscale-mdr · 2022-11-18T10:58:02Z

util/reconciler/defaults.go

@@ -25,10 +29,60 @@ const (
 	DefaultMappingTimeout = 60 * time.Second
 )

+var AllowedErrors = []int{429, 503}


Change to ThrottlingErrors or add a comment

outscale-mdr · 2022-12-15T10:34:15Z

main.go

-	leaseDuration := 40 * time.Second
+	leaseDuration := 60 * time.Second


Not for the PR right ?

No it is good thanks.

ghost requested review from outscale-hmi and outscale-mdr November 3, 2022 09:20

outscale-mdr requested changes Nov 8, 2022

View reviewed changes

ghost requested a review from outscale-mdr November 15, 2022 14:00

outscale-mdr requested changes Nov 15, 2022

View reviewed changes

ghost requested a review from outscale-mdr November 17, 2022 17:47

ghost commented Nov 18, 2022

View reviewed changes

outscale-mdr requested changes Nov 18, 2022

View reviewed changes

ghost requested a review from outscale-mdr November 21, 2022 18:06

ghost assigned outscale-hmi Nov 24, 2022

ghost removed the request for review from outscale-mdr December 2, 2022 13:19

ghost added the kind/feature Feature resolution label Dec 7, 2022

vincentBaer added 2 commits December 14, 2022 14:36

Handle throttling

cb2ab25

Add unittest and increase loadbalancer timeout

ea8eeee

ghost requested review from outscale-mdr and removed request for outscale-mdr December 14, 2022 17:28

outscale-mdr approved these changes Dec 15, 2022

View reviewed changes

ghost merged commit 7875256 into outscale:main Dec 15, 2022

This pull request was closed.

		clusterScope.Info("Find body", "httpResBody", httpResBody)
		clusterScope.Info("Find httpResCode", "httpResCode", httpRes.StatusCode)

		leaseDuration := 40 * time.Second
		leaseDuration := 60 * time.Second

Handle throttling #169

Handle throttling #169

Conversation

ghost commented Oct 31, 2022 • edited by ghost Loading

ghost commented Nov 4, 2022

outscale-mdr commented Nov 4, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ghost commented Nov 9, 2022 • edited by ghost Loading

ghost commented Nov 15, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ghost commented Oct 31, 2022 •

edited by ghost

Loading

outscale-mdr commented Nov 4, 2022 •

edited

Loading

ghost commented Nov 9, 2022 •

edited by ghost

Loading