From e8f5d7502c6276801586417e5f537902962eff96 Mon Sep 17 00:00:00 2001 From: Jack Francis Date: Wed, 20 Apr 2022 13:49:23 -0700 Subject: [PATCH] clusterctl: retry github i/o operations --- .../client/repository/repository_github.go | 106 ++++++++++++------ 1 file changed, 70 insertions(+), 36 deletions(-) diff --git a/cmd/clusterctl/client/repository/repository_github.go b/cmd/clusterctl/client/repository/repository_github.go index ee0ddbe6b069..03d6051e85cb 100644 --- a/cmd/clusterctl/client/repository/repository_github.go +++ b/cmd/clusterctl/client/repository/repository_github.go @@ -24,21 +24,25 @@ import ( "net/url" "path/filepath" "strings" + "time" "github.com/google/go-github/v33/github" "github.com/pkg/errors" "golang.org/x/oauth2" "k8s.io/apimachinery/pkg/util/version" + "k8s.io/apimachinery/pkg/util/wait" clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" "sigs.k8s.io/cluster-api/cmd/clusterctl/client/config" ) const ( - httpsScheme = "https" - githubDomain = "github.com" - githubReleaseRepository = "releases" - githubLatestReleaseLabel = "latest" + httpsScheme = "https" + githubDomain = "github.com" + githubReleaseRepository = "releases" + githubLatestReleaseLabel = "latest" + retryableOperationInterval = 3 * time.Second + retryableOperationTimeout = 5 * time.Minute ) var ( @@ -102,18 +106,23 @@ func (g *gitHubRepository) ComponentsPath() string { // GetFile returns a file for a given provider version. func (g *gitHubRepository) GetFile(version, path string) ([]byte, error) { - release, err := g.getReleaseByTag(version) - if err != nil { - return nil, errors.Wrapf(err, "failed to get GitHub release %s", version) - } + var files []byte + err := wait.PollImmediate(retryableOperationInterval, retryableOperationTimeout, func() (bool, error) { + release, err := g.getReleaseByTag(version) + if err != nil { + return false, errors.Wrapf(err, "failed to get GitHub release %s", version) + } - // download files from the release - files, err := g.downloadFilesFromRelease(release, path) - if err != nil { - return nil, errors.Wrapf(err, "failed to download files from GitHub release %s", version) - } + // download files from the release + var downloadError error + files, downloadError = g.downloadFilesFromRelease(release, path) + if downloadError != nil { + return false, errors.Wrapf(downloadError, "failed to download files from GitHub release %s", version) + } - return files, nil + return true, nil + }) + return files, err } // NewGitHubRepository returns a gitHubRepository implementation. @@ -216,9 +225,17 @@ func (g *gitHubRepository) getVersions() ([]string, error) { // get all the releases // NB. currently Github API does not support result ordering, so it not possible to limit results - releases, _, err := client.Repositories.ListReleases(context.TODO(), g.owner, g.repository, nil) + var releases []*github.RepositoryRelease + err := wait.PollImmediate(retryableOperationInterval, retryableOperationTimeout, func() (bool, error) { + var listReleases error + releases, _, listReleases = client.Repositories.ListReleases(context.TODO(), g.owner, g.repository, nil) + if listReleases != nil { + return false, g.handleGithubErr(listReleases, "failed to get the list of releases") + } + return true, nil + }) if err != nil { - return nil, g.handleGithubErr(err, "failed to get the list of releases") + return nil, err } versions := []string{} for _, r := range releases { @@ -247,13 +264,21 @@ func (g *gitHubRepository) getReleaseByTag(tag string) (*github.RepositoryReleas client := g.getClient() - release, _, err := client.Repositories.GetReleaseByTag(context.TODO(), g.owner, g.repository, tag) - if err != nil { - return nil, g.handleGithubErr(err, "failed to read release %q", tag) - } + var release *github.RepositoryRelease + err := wait.PollImmediate(retryableOperationInterval, retryableOperationTimeout, func() (bool, error) { + var getReleasesErr error + release, _, getReleasesErr = client.Repositories.GetReleaseByTag(context.TODO(), g.owner, g.repository, tag) + if getReleasesErr != nil { + return false, g.handleGithubErr(getReleasesErr, "failed to read release %q", tag) + } - if release == nil { - return nil, errors.Errorf("failed to get release %q", tag) + if release == nil { + return false, errors.Errorf("failed to get release %q", tag) + } + return true, nil + }) + if err != nil { + return nil, err } cacheReleases[cacheID] = release @@ -284,23 +309,32 @@ func (g *gitHubRepository) downloadFilesFromRelease(release *github.RepositoryRe return nil, errors.Errorf("failed to get file %q from %q release", fileName, *release.TagName) } - reader, redirect, err := client.Repositories.DownloadReleaseAsset(context.TODO(), g.owner, g.repository, *assetID, http.DefaultClient) - if err != nil { - return nil, g.handleGithubErr(err, "failed to download file %q from %q release", *release.TagName, fileName) - } - if redirect != "" { - req, err := http.NewRequestWithContext(ctx, http.MethodGet, redirect, http.NoBody) - if err != nil { - return nil, errors.Wrapf(err, "failed to download file %q from %q release via redirect location %q: failed to create request", *release.TagName, fileName, redirect) + var reader io.ReadCloser + err := wait.PollImmediate(retryableOperationInterval, retryableOperationTimeout, func() (bool, error) { + var redirect string + var downloadReleaseError error + reader, redirect, downloadReleaseError = client.Repositories.DownloadReleaseAsset(context.TODO(), g.owner, g.repository, *assetID, http.DefaultClient) + if downloadReleaseError != nil { + return false, g.handleGithubErr(downloadReleaseError, "failed to download file %q from %q release", *release.TagName, fileName) } - - response, err := http.DefaultClient.Do(req) //nolint:bodyclose // (NB: The reader is actually closed in a defer) - if err != nil { - return nil, errors.Wrapf(err, "failed to download file %q from %q release via redirect location %q", *release.TagName, fileName, redirect) + if redirect != "" { + req, err := http.NewRequestWithContext(ctx, http.MethodGet, redirect, http.NoBody) + if err != nil { + return false, errors.Wrapf(err, "failed to download file %q from %q release via redirect location %q: failed to create request", *release.TagName, fileName, redirect) + } + + response, err := http.DefaultClient.Do(req) //nolint:bodyclose // (NB: The reader is actually closed in a defer) + if err != nil { + return false, errors.Wrapf(err, "failed to download file %q from %q release via redirect location %q", *release.TagName, fileName, redirect) + } + reader = response.Body } - reader = response.Body - } + return true, nil + }) defer reader.Close() + if err != nil { + return nil, err + } // Read contents from the reader (redirect or not), and return. content, err := io.ReadAll(reader)