From 22b5ad99651d434c84a550d249d4b5e960de41c0 Mon Sep 17 00:00:00 2001 From: IvanBorislavovDimitrov Date: Wed, 6 Dec 2023 15:20:48 +0200 Subject: [PATCH] Retry deploy from URL in case of errors --- .../rest_cloud_foundry_client_extended.go | 8 +- clients/csrf/csrf_token_manager_test.go | 5 +- clients/mtaclient/mta_rest_client.go | 14 ++- .../mtaclient/retryable_mta_rest_client.go | 2 +- commands/base_command.go | 2 +- commands/deploy_command.go | 99 ++++++++++++++----- commands/retrier/retry_util.go | 27 +++++ commands/upload_from_url_status.go | 8 ++ 8 files changed, 130 insertions(+), 35 deletions(-) create mode 100644 commands/retrier/retry_util.go create mode 100644 commands/upload_from_url_status.go diff --git a/clients/cfrestclient/rest_cloud_foundry_client_extended.go b/clients/cfrestclient/rest_cloud_foundry_client_extended.go index 7ed2f8d..a13f3bf 100644 --- a/clients/cfrestclient/rest_cloud_foundry_client_extended.go +++ b/clients/cfrestclient/rest_cloud_foundry_client_extended.go @@ -1,15 +1,16 @@ package cfrestclient import ( - "code.cloudfoundry.org/cli/plugin" - "code.cloudfoundry.org/jsonry" "crypto/md5" "encoding/hex" "encoding/json" "fmt" - "github.com/cloudfoundry-incubator/multiapps-cli-plugin/clients/models" "io" "net/http" + + "code.cloudfoundry.org/cli/plugin" + "code.cloudfoundry.org/jsonry" + "github.com/cloudfoundry-incubator/multiapps-cli-plugin/clients/models" ) const cfBaseUrl = "v3/" @@ -147,7 +148,6 @@ func getPaginatedResourcesWithIncluded[T any, Auxiliary any](url, token string, func executeRequest(url, token string) ([]byte, error) { req, _ := http.NewRequest(http.MethodGet, url, nil) req.Header.Add("Authorization", token) - resp, err := http.DefaultClient.Do(req) if err != nil { return nil, err diff --git a/clients/csrf/csrf_token_manager_test.go b/clients/csrf/csrf_token_manager_test.go index 2c7af64..087c539 100644 --- a/clients/csrf/csrf_token_manager_test.go +++ b/clients/csrf/csrf_token_manager_test.go @@ -1,11 +1,12 @@ package csrf import ( + "net/http" + "time" + "github.com/cloudfoundry-incubator/multiapps-cli-plugin/clients/csrf/fakes" . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" - "net/http" - "time" ) const csrfTokenNotSet = "" diff --git a/clients/mtaclient/mta_rest_client.go b/clients/mtaclient/mta_rest_client.go index daa5941..35a6e45 100644 --- a/clients/mtaclient/mta_rest_client.go +++ b/clients/mtaclient/mta_rest_client.go @@ -26,6 +26,8 @@ import ( const spacesURL string = "spaces/" const restBaseURL string = "api/v1/" +const couldNotGetAsyncJobError = "could not get async file upload job" + type MtaRestClient struct { baseclient.BaseClient client *MtaClient @@ -40,6 +42,7 @@ type AsyncUploadJobResult struct { File *models.FileMetadata `json:"file,omitempty"` MtaId string `json:"mta_id,omitempty"` BytesProcessed int64 `json:"bytes_processed,omitempty"` + ClientActions []string `json:"client_actions,omitempty"` } func NewMtaClient(host, spaceID string, rt http.RoundTripper, tokenFactory baseclient.TokenFactory) MtaClientOperations { @@ -383,12 +386,19 @@ func (c MtaRestClient) GetAsyncUploadJob(jobId string, namespace *string, appIns resp, err := httpClient.Do(req) if err != nil { - return AsyncUploadJobResult{}, fmt.Errorf("could not get async file upload job %s: %v", jobId, err) + return AsyncUploadJobResult{}, fmt.Errorf("%s %s: %v", couldNotGetAsyncJobError, jobId, err) } defer resp.Body.Close() if resp.StatusCode/100 != 2 { - return AsyncUploadJobResult{}, fmt.Errorf("could not get async file upload job %s: %s", jobId, resp.Status) + bodyBytes, _ := io.ReadAll(resp.Body) + if resp.StatusCode == 400 { + return AsyncUploadJobResult{ + ClientActions: []string{"RETRY_UPLOAD"}, + }, fmt.Errorf("%s %s: %s, body: %s", couldNotGetAsyncJobError, jobId, resp.Status, string(bodyBytes)) + } + return AsyncUploadJobResult{}, fmt.Errorf("%s %s: %s, body: %s", couldNotGetAsyncJobError, jobId, resp.Status, string(bodyBytes)) + } bodyBytes, err := io.ReadAll(resp.Body) diff --git a/clients/mtaclient/retryable_mta_rest_client.go b/clients/mtaclient/retryable_mta_rest_client.go index b750cc3..7f7b7ae 100644 --- a/clients/mtaclient/retryable_mta_rest_client.go +++ b/clients/mtaclient/retryable_mta_rest_client.go @@ -153,7 +153,7 @@ func (c RetryableMtaRestClient) GetAsyncUploadJob(jobId string, namespace *strin } resp, err := baseclient.CallWithRetry(getAsyncUploadJobCb, c.MaxRetriesCount, c.RetryInterval) if err != nil { - return AsyncUploadJobResult{}, err + return resp.(AsyncUploadJobResult), err } return resp.(AsyncUploadJobResult), nil } diff --git a/commands/base_command.go b/commands/base_command.go index cb9ba60..872cbba 100644 --- a/commands/base_command.go +++ b/commands/base_command.go @@ -266,7 +266,7 @@ func (c *BaseCommand) shouldAbortConflictingOperation(mtaID string, force bool) func newTransport() http.RoundTripper { csrfx := csrf.CsrfTokenHelper{NonProtectedMethods: getNonProtectedMethods()} - httpTransport := http.DefaultTransport.(*http.Transport) + httpTransport := http.DefaultTransport.(*http.Transport).Clone() // Increase tls handshake timeout to cope with slow internet connections. 3 x default value =30s. httpTransport.TLSHandshakeTimeout = 30 * time.Second return &csrf.Transport{Delegate: httpTransport, Csrf: &csrfx} diff --git a/commands/deploy_command.go b/commands/deploy_command.go index ca8a94e..8c947e8 100644 --- a/commands/deploy_command.go +++ b/commands/deploy_command.go @@ -2,20 +2,10 @@ package commands import ( "bufio" - "code.cloudfoundry.org/cli/cf/terminal" - "code.cloudfoundry.org/cli/plugin" "encoding/base64" "errors" "flag" "fmt" - "github.com/cloudfoundry-incubator/multiapps-cli-plugin/clients/baseclient" - "github.com/cloudfoundry-incubator/multiapps-cli-plugin/clients/models" - "github.com/cloudfoundry-incubator/multiapps-cli-plugin/clients/mtaclient" - "github.com/cloudfoundry-incubator/multiapps-cli-plugin/configuration" - "github.com/cloudfoundry-incubator/multiapps-cli-plugin/log" - "github.com/cloudfoundry-incubator/multiapps-cli-plugin/ui" - "github.com/cloudfoundry-incubator/multiapps-cli-plugin/util" - "gopkg.in/cheggaaa/pb.v1" "io/fs" "net/http" "net/url" @@ -24,6 +14,18 @@ import ( "strconv" "strings" "time" + + "code.cloudfoundry.org/cli/cf/terminal" + "code.cloudfoundry.org/cli/plugin" + "github.com/cloudfoundry-incubator/multiapps-cli-plugin/clients/baseclient" + "github.com/cloudfoundry-incubator/multiapps-cli-plugin/clients/models" + "github.com/cloudfoundry-incubator/multiapps-cli-plugin/clients/mtaclient" + "github.com/cloudfoundry-incubator/multiapps-cli-plugin/commands/retrier" + "github.com/cloudfoundry-incubator/multiapps-cli-plugin/configuration" + "github.com/cloudfoundry-incubator/multiapps-cli-plugin/log" + "github.com/cloudfoundry-incubator/multiapps-cli-plugin/ui" + "github.com/cloudfoundry-incubator/multiapps-cli-plugin/util" + "gopkg.in/cheggaaa/pb.v1" ) const ( @@ -224,12 +226,12 @@ func (c *DeployCommand) executeInternal(positionalArgs []string, dsHost string, if isUrl { var fileId string - var isFailure bool - fileId, mtaId, isFailure = c.uploadFromUrl(mtaArchive, mtaClient, namespace, disableProgressBar) - if isFailure { + + asyncUploadJobResult := c.uploadFromUrl(mtaArchive, mtaClient, namespace, disableProgressBar) + if asyncUploadJobResult.ExecutionStatus == Failure { return Failure } - + mtaId, fileId = asyncUploadJobResult.MtaId, asyncUploadJobResult.FileId // Check for an ongoing operation for this MTA ID and abort it wasAborted, err := c.CheckOngoingOperation(mtaId, namespace, dsHost, force, cfTarget) if err != nil { @@ -330,14 +332,28 @@ func parseMtaArchiveArgument(rawMtaArchive interface{}) (bool, string) { } func (c *DeployCommand) uploadFromUrl(url string, mtaClient mtaclient.MtaClientOperations, namespace string, - disableProgressBar bool) (fileId, mtaId string, failure bool) { - progressBar := c.tryFetchMtarSize(url, disableProgressBar) - + disableProgressBar bool) UploadFromUrlStatus { encodedFileUrl := base64.URLEncoding.EncodeToString([]byte(url)) + uploadStatus, _ := retrier.Execute[UploadFromUrlStatus](3, func() (UploadFromUrlStatus, error) { + progressBar := c.tryFetchMtarSize(url, disableProgressBar) + uploadFromUrlStatus := c.doUploadFromUrl(encodedFileUrl, mtaClient, namespace, progressBar) + return uploadFromUrlStatus, nil + }, func(result UploadFromUrlStatus, err error) bool { + return shouldRetryUpload(result) + }) + return uploadStatus +} + +func (c *DeployCommand) doUploadFromUrl(encodedFileUrl string, mtaClient mtaclient.MtaClientOperations, namespace string, progressBar *pb.ProgressBar) UploadFromUrlStatus { responseHeaders, err := mtaClient.StartUploadMtaArchiveFromUrl(encodedFileUrl, &namespace) if err != nil { ui.Failed("Could not upload from url: %s", err) - return "", "", true + return UploadFromUrlStatus{ + FileId: "", + MtaId: "", + ClientActions: make([]string, 0), + ExecutionStatus: Failure, + } } var totalBytesProcessed int64 = 0 @@ -356,17 +372,27 @@ func (c *DeployCommand) uploadFromUrl(url string, mtaClient mtaclient.MtaClientO defer ticker.Stop() var file *models.FileMetadata + var jobResult mtaclient.AsyncUploadJobResult for file == nil { jobResult, err := mtaClient.GetAsyncUploadJob(jobId, &namespace, responseHeaders.Get("x-cf-app-instance")) if err != nil { ui.Failed("Could not upload from url: %s", err) - return "", "", true + return UploadFromUrlStatus{ + FileId: "", + MtaId: "", + ClientActions: jobResult.ClientActions, + ExecutionStatus: Failure, + } } - file, mtaId = jobResult.File, jobResult.MtaId - + file = jobResult.File if len(jobResult.Error) != 0 { ui.Failed("Async upload job failed: %s", jobResult.Error) - return "", "", true + return UploadFromUrlStatus{ + FileId: "", + MtaId: "", + ClientActions: jobResult.ClientActions, + ExecutionStatus: Failure, + } } if progressBar != nil && jobResult.BytesProcessed != -1 { @@ -379,11 +405,16 @@ func (c *DeployCommand) uploadFromUrl(url string, mtaClient mtaclient.MtaClientO totalBytesProcessed = jobResult.BytesProcessed } - if len(mtaId) == 0 { + if len(jobResult.MtaId) == 0 { select { case <-timeout.C: ui.Failed("Upload from URL timed out after 1 hour") - return "", "", true + return UploadFromUrlStatus{ + FileId: "", + MtaId: "", + ClientActions: make([]string, 0), + ExecutionStatus: Failure, + } case <-ticker.C: } } @@ -391,7 +422,25 @@ func (c *DeployCommand) uploadFromUrl(url string, mtaClient mtaclient.MtaClientO if progressBar != nil && totalBytesProcessed < progressBar.Total { progressBar.Add64(progressBar.Total - totalBytesProcessed) } - return file.ID, mtaId, false + return UploadFromUrlStatus{ + FileId: file.ID, + MtaId: jobResult.MtaId, + ClientActions: jobResult.ClientActions, + ExecutionStatus: Success, + } +} + +func shouldRetryUpload(uploadFromUrlStatus UploadFromUrlStatus) bool { + if uploadFromUrlStatus.ExecutionStatus == Success { + return false + } + for _, clientAction := range uploadFromUrlStatus.ClientActions { + if clientAction == "RETRY_UPLOAD" { + ui.Warn("Upload request must be retried") + return true + } + } + return false } func (c *DeployCommand) uploadFiles(files []string, fileUploader *FileUploader) ([]string, ExecutionStatus) { diff --git a/commands/retrier/retry_util.go b/commands/retrier/retry_util.go new file mode 100644 index 0000000..73126d7 --- /dev/null +++ b/commands/retrier/retry_util.go @@ -0,0 +1,27 @@ +package retrier + +import ( + "time" + + "github.com/cloudfoundry-incubator/multiapps-cli-plugin/log" +) + +func Execute[T any](attempts int, callback func() (T, error), shouldRetry func(result T, err error) bool) (T, error) { + for i := 0; i < attempts; i++ { + result, err := callback() + if shouldRetry(result, err) { + logError[T](result, err) + time.Sleep(3 * time.Second) + continue + } + return result, err + } + return callback() +} + +func logError[T any](result T, err error) { + if err != nil { + log.Tracef("retrying an operation that failed with: %v", err) + } + log.Tracef("result of the callback %v", result) +} diff --git a/commands/upload_from_url_status.go b/commands/upload_from_url_status.go new file mode 100644 index 0000000..b5b9319 --- /dev/null +++ b/commands/upload_from_url_status.go @@ -0,0 +1,8 @@ +package commands + +type UploadFromUrlStatus struct { + FileId string + MtaId string + ClientActions []string + ExecutionStatus ExecutionStatus +}