Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Retry deploy from URL in case of errors #188

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions clients/cfrestclient/rest_cloud_foundry_client_extended.go
Original file line number Diff line number Diff line change
@@ -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/"
Expand Down Expand Up @@ -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
Expand Down
5 changes: 3 additions & 2 deletions clients/csrf/csrf_token_manager_test.go
Original file line number Diff line number Diff line change
@@ -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 = ""
Expand Down
14 changes: 12 additions & 2 deletions clients/mtaclient/mta_rest_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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 {
Expand Down Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion clients/mtaclient/retryable_mta_rest_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why resp object must be returned now?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So we can re-use the retry logic:
if resp.StatusCode == 400 { return AsyncUploadJobResult{ ClientActions: []string{"RETRY_UPLOAD"}, }, fmt.Errorf("%s %s: %s, body: %s", couldNotGetAsyncJobError, jobId, resp.Status, string(bodyBytes)) }
In this code the retry is explicitly set and it will be wiped out if we just return an empty object in the retryable client

}
return resp.(AsyncUploadJobResult), nil
}
Expand Down
2 changes: 1 addition & 1 deletion commands/base_command.go
Original file line number Diff line number Diff line change
Expand Up @@ -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}
Expand Down
99 changes: 74 additions & 25 deletions commands/deploy_command.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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 (
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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
Expand All @@ -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 {
Expand All @@ -379,19 +405,42 @@ 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:
}
}
}
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) {
Expand Down
27 changes: 27 additions & 0 deletions commands/retrier/retry_util.go
Original file line number Diff line number Diff line change
@@ -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) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is really cool

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)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

intentionally lowercase

}
log.Tracef("result of the callback %v", result)
}
8 changes: 8 additions & 0 deletions commands/upload_from_url_status.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
package commands

type UploadFromUrlStatus struct {
FileId string
MtaId string
ClientActions []string
ExecutionStatus ExecutionStatus
}