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.

Sign up for GitHub

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

🐛 clusterctl: retry github i/o operations #6430

Merged
merged 1 commit into from
Apr 28, 2022
Merged

🐛 clusterctl: retry github i/o operations #6430

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
104 changes: 77 additions & 27 deletions cmd/clusterctl/client/repository/repository_github.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,13 @@ 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"
Expand All @@ -44,9 +46,11 @@ const (
var (
// Caches used to limit the number of GitHub API calls.

cacheVersions = map[string][]string{}
cacheReleases = map[string]*github.RepositoryRelease{}
cacheFiles = map[string][]byte{}
cacheVersions = map[string][]string{}
cacheReleases = map[string]*github.RepositoryRelease{}
cacheFiles = map[string][]byte{}
retryableOperationInterval = 10 * time.Second
retryableOperationTimeout = 1 * time.Minute
)

// gitHubRepository provides support for providers hosted on GitHub.
Expand Down Expand Up @@ -216,9 +220,24 @@ 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)
if err != nil {
return nil, g.handleGithubErr(err, "failed to get the list of releases")
var releases []*github.RepositoryRelease
var retryError error
_ = wait.PollImmediate(retryableOperationInterval, retryableOperationTimeout, func() (bool, error) {
var listReleasesErr error
jackfrancis marked this conversation as resolved.
Show resolved Hide resolved
releases, _, listReleasesErr = client.Repositories.ListReleases(context.TODO(), g.owner, g.repository, nil)
if listReleasesErr != nil {
retryError = g.handleGithubErr(listReleasesErr, "failed to get the list of releases")
// return immediately if we are rate limited
if _, ok := listReleasesErr.(*github.RateLimitError); ok {
return false, retryError
}
return false, nil
}
retryError = nil
return true, nil
})
if retryError != nil {
return nil, retryError
}
versions := []string{}
for _, r := range releases {
Expand Down Expand Up @@ -247,13 +266,24 @@ 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)
}

if release == nil {
return nil, errors.Errorf("failed to get release %q", tag)
var release *github.RepositoryRelease
var retryError error
_ = 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 {
retryError = g.handleGithubErr(getReleasesErr, "failed to read release %q", tag)
// return immediately if we are rate limited
if _, ok := getReleasesErr.(*github.RateLimitError); ok {
return false, retryError
}
return false, nil
}
retryError = nil
return true, nil
})
if retryError != nil {
return nil, retryError
}

cacheReleases[cacheID] = release
Expand Down Expand Up @@ -284,23 +314,43 @@ 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
var retryError error
_ = 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 {
retryError = g.handleGithubErr(downloadReleaseError, "failed to download file %q from %q release", *release.TagName, fileName)
// return immediately if we are rate limited
if _, ok := downloadReleaseError.(*github.RateLimitError); ok {
return false, retryError
}
return false, nil
}

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 {
retryError = errors.Wrapf(err, "failed to download file %q from %q release via redirect location %q: failed to create request", *release.TagName, fileName, redirect)
return false, nil
}

response, err := http.DefaultClient.Do(req) //nolint:bodyclose // (NB: The reader is actually closed in a defer)
if err != nil {
retryError = errors.Wrapf(err, "failed to download file %q from %q release via redirect location %q", *release.TagName, fileName, redirect)
return false, nil
}
reader = response.Body
}
reader = response.Body
retryError = nil
return true, nil
})
if reader != nil {
defer reader.Close()
}
if retryError != nil {
return nil, retryError
}
defer reader.Close()

// Read contents from the reader (redirect or not), and return.
content, err := io.ReadAll(reader)
Expand Down
13 changes: 13 additions & 0 deletions cmd/clusterctl/client/repository/repository_github_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"fmt"
"net/http"
"testing"
"time"

"github.com/google/go-github/v33/github"
. "github.com/onsi/gomega"
Expand All @@ -31,6 +32,8 @@ import (
)

func Test_githubRepository_newGitHubRepository(t *testing.T) {
retryableOperationInterval = 200 * time.Millisecond
retryableOperationTimeout = 1 * time.Second
type field struct {
providerConfig config.Provider
variableClient config.VariablesClient
Expand Down Expand Up @@ -156,6 +159,8 @@ func Test_githubRepository_getComponentsPath(t *testing.T) {
}

func Test_githubRepository_getFile(t *testing.T) {
retryableOperationInterval = 200 * time.Millisecond
retryableOperationTimeout = 1 * time.Second
client, mux, teardown := test.NewFakeGitHub()
defer teardown()

Expand Down Expand Up @@ -228,6 +233,8 @@ func Test_githubRepository_getFile(t *testing.T) {
}

func Test_gitHubRepository_getVersions(t *testing.T) {
retryableOperationInterval = 200 * time.Millisecond
retryableOperationTimeout = 1 * time.Second
client, mux, teardown := test.NewFakeGitHub()
defer teardown()

Expand Down Expand Up @@ -284,6 +291,8 @@ func Test_gitHubRepository_getVersions(t *testing.T) {
}

func Test_gitHubRepository_getLatestContractRelease(t *testing.T) {
retryableOperationInterval = 200 * time.Millisecond
retryableOperationTimeout = 1 * time.Second
client, mux, teardown := test.NewFakeGitHub()
defer teardown()

Expand Down Expand Up @@ -540,6 +549,8 @@ func Test_gitHubRepository_getLatestPatchRelease(t *testing.T) {
}

func Test_gitHubRepository_getReleaseByTag(t *testing.T) {
retryableOperationInterval = 200 * time.Millisecond
retryableOperationTimeout = 1 * time.Second
client, mux, teardown := test.NewFakeGitHub()
defer teardown()

Expand Down Expand Up @@ -605,6 +616,8 @@ func Test_gitHubRepository_getReleaseByTag(t *testing.T) {
}

func Test_gitHubRepository_downloadFilesFromRelease(t *testing.T) {
retryableOperationInterval = 200 * time.Millisecond
retryableOperationTimeout = 1 * time.Second
client, mux, teardown := test.NewFakeGitHub()
defer teardown()

Expand Down