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

[release-1.4] 🐛 clusterctl: return early if release for latest tag does not exist yet #8965

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
18 changes: 17 additions & 1 deletion cmd/clusterctl/client/repository/repository_github.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,8 @@ const (
)

var (
errNotFound = errors.New("404 Not Found")

// Caches used to limit the number of GitHub API calls.

cacheVersions = map[string][]string{}
Expand Down Expand Up @@ -156,6 +158,11 @@ func (g *gitHubRepository) ComponentsPath() string {
func (g *gitHubRepository) GetFile(version, path string) ([]byte, error) {
release, err := g.getReleaseByTag(version)
if err != nil {
if errors.Is(err, errNotFound) {
// If it was ErrNotFound, then there is no release yet for the resolved tag.
// Ref: https://github.com/kubernetes-sigs/cluster-api/issues/7889
return nil, errors.Wrapf(err, "release not found for version %s, please retry later or set \"GOPROXY=off\" to get the current stable release", version)
}
return nil, errors.Wrapf(err, "failed to get GitHub release %s", version)
}

Expand Down Expand Up @@ -227,7 +234,7 @@ func NewGitHubRepository(providerConfig config.Provider, configVariablesClient c
if defaultVersion == githubLatestReleaseLabel {
repo.defaultVersion, err = latestContractRelease(repo, clusterv1.GroupVersion.Version)
if err != nil {
return nil, errors.Wrap(err, "failed to get GitHub latest version")
return nil, errors.Wrap(err, "failed to get latest release")
}
}

Expand Down Expand Up @@ -351,6 +358,10 @@ func (g *gitHubRepository) getReleaseByTag(tag string) (*github.RepositoryReleas
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 not found
if errors.Is(retryError, errNotFound) {
return false, retryError
}
// Return immediately if we are rate limited.
if _, ok := getReleasesErr.(*github.RateLimitError); ok {
return false, retryError
Expand Down Expand Up @@ -437,5 +448,10 @@ func (g *gitHubRepository) handleGithubErr(err error, message string, args ...in
if _, ok := err.(*github.RateLimitError); ok {
return errors.New("rate limit for github api has been reached. Please wait one hour or get a personal API token and assign it to the GITHUB_TOKEN environment variable")
}
if ghErr, ok := err.(*github.ErrorResponse); ok {
if ghErr.Response.StatusCode == http.StatusNotFound {
return errNotFound
}
}
return errors.Wrapf(err, message, args...)
}
33 changes: 28 additions & 5 deletions cmd/clusterctl/client/repository/repository_github_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -441,6 +441,15 @@ func Test_gitHubRepository_getLatestContractRelease(t *testing.T) {
fmt.Fprint(w, "v0.3.1\n")
})

// setup an handler for returning 4 fake releases but no actual tagged release
muxGoproxy.HandleFunc("/github.com/o/r2/@v/list", func(w http.ResponseWriter, r *http.Request) {
testMethod(t, r, "GET")
fmt.Fprint(w, "v0.5.0\n")
fmt.Fprint(w, "v0.4.0\n")
fmt.Fprint(w, "v0.3.2\n")
fmt.Fprint(w, "v0.3.1\n")
})

configVariablesClient := test.NewFakeVariableClient()

type field struct {
Expand Down Expand Up @@ -480,6 +489,15 @@ func Test_gitHubRepository_getLatestContractRelease(t *testing.T) {
contract: "foo",
wantErr: false,
},
{
name: "Return 404 if there is no release for the tag",
field: field{
providerConfig: config.NewProvider("test", "https://github.com/o/r2/releases/v0.99.0/path", clusterctlv1.CoreProviderType),
},
want: "0.99.0",
contract: "v1alpha4",
wantErr: true,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
Expand Down Expand Up @@ -514,6 +532,11 @@ func Test_gitHubRepository_getLatestRelease(t *testing.T) {
fmt.Fprint(w, "v0.4.3-alpha\n") // prerelease
fmt.Fprint(w, "foo\n") // no semantic version tag
})
// And also expose a release for them
muxGoproxy.HandleFunc("/api.github.com/repos/o/r1/releases/tags/v0.4.2", func(w http.ResponseWriter, r *http.Request) {
testMethod(t, r, "GET")
fmt.Fprint(w, "{}\n")
})

// Setup a handler for returning no releases.
muxGoproxy.HandleFunc("/github.com/o/r2/@v/list", func(w http.ResponseWriter, r *http.Request) {
Expand Down Expand Up @@ -543,7 +566,7 @@ func Test_gitHubRepository_getLatestRelease(t *testing.T) {
{
name: "Get latest release, ignores pre-release version",
field: field{
providerConfig: config.NewProvider("test", "https://github.com/o/r1/releases/latest/path", clusterctlv1.CoreProviderType),
providerConfig: config.NewProvider("test", "https://github.com/o/r1/releases/v0.4.2/path", clusterctlv1.CoreProviderType),
},
want: "v0.4.2",
wantErr: false,
Expand All @@ -559,7 +582,7 @@ func Test_gitHubRepository_getLatestRelease(t *testing.T) {
{
name: "Falls back to latest prerelease when no official release present",
field: field{
providerConfig: config.NewProvider("test", "https://github.com/o/r3/releases/latest/path", clusterctlv1.CoreProviderType),
providerConfig: config.NewProvider("test", "https://github.com/o/r3/releases/v0.1.0-alpha.2/path", clusterctlv1.CoreProviderType),
},
want: "v0.1.0-alpha.2",
wantErr: false,
Expand Down Expand Up @@ -619,7 +642,7 @@ func Test_gitHubRepository_getLatestPatchRelease(t *testing.T) {
{
name: "Get latest patch release, no Major/Minor specified",
field: field{
providerConfig: config.NewProvider("test", "https://github.com/o/r1/releases/latest/path", clusterctlv1.CoreProviderType),
providerConfig: config.NewProvider("test", "https://github.com/o/r1/releases/v1.3.2/path", clusterctlv1.CoreProviderType),
},
minor: nil,
major: nil,
Expand All @@ -629,7 +652,7 @@ func Test_gitHubRepository_getLatestPatchRelease(t *testing.T) {
{
name: "Get latest patch release, for Major 0 and Minor 3",
field: field{
providerConfig: config.NewProvider("test", "https://github.com/o/r1/releases/latest/path", clusterctlv1.CoreProviderType),
providerConfig: config.NewProvider("test", "https://github.com/o/r1/releases/v0.3.2/path", clusterctlv1.CoreProviderType),
},
major: &major0,
minor: &minor3,
Expand All @@ -639,7 +662,7 @@ func Test_gitHubRepository_getLatestPatchRelease(t *testing.T) {
{
name: "Get latest patch release, for Major 0 and Minor 4",
field: field{
providerConfig: config.NewProvider("test", "https://github.com/o/r1/releases/latest/path", clusterctlv1.CoreProviderType),
providerConfig: config.NewProvider("test", "https://github.com/o/r1/releases/v0.4.0/path", clusterctlv1.CoreProviderType),
},
major: &major0,
minor: &minor4,
Expand Down
9 changes: 7 additions & 2 deletions cmd/clusterctl/client/repository/repository_versions.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,11 +39,16 @@ func latestContractRelease(repo Repository, contract string) (string, error) {
}
// Attempt to check if the latest release satisfies the API Contract
// This is a best-effort attempt to find the latest release for an older API contract if it's not the latest release.
// If an error occurs, we just return the latest release.
file, err := repo.GetFile(latest, metadataFile)
// If an error occurs, we just return the latest release.
if err != nil {
if errors.Is(err, errNotFound) {
// If it was ErrNotFound, then there is no release yet for the resolved tag.
// Ref: https://github.com/kubernetes-sigs/cluster-api/issues/7889
return "", err
}
// if we can't get the metadata file from the release, we return latest.
return latest, nil //nolint:nilerr
return latest, nil
}
latestMetadata := &clusterctlv1.Metadata{}
codecFactory := serializer.NewCodecFactory(scheme.Scheme)
Expand Down