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

clusterctl only considers 30 github releases #8233

Closed
killianmuldoon opened this issue Mar 6, 2023 · 7 comments · Fixed by #8240
Closed

clusterctl only considers 30 github releases #8233

killianmuldoon opened this issue Mar 6, 2023 · 7 comments · Fixed by #8240
Assignees
Labels
area/clusterctl Issues or PRs related to clusterctl kind/bug Categorizes issue or PR as related to a bug. triage/accepted Indicates an issue or PR is ready to be actively worked on.

Comments

@killianmuldoon
Copy link
Contributor

killianmuldoon commented Mar 6, 2023

When clusterctl looks for releases for providers it looks up the https://api.github.com/repos/kubernetes-sigs/cluster-api/releases endpoint. This endpoint by default only lists the most recent 30 releases. Even if an earlier release does exist clusterctl will not find it if it's not in the most recent 30 releases.

The current impact of this is that clusterctl v0.4.X and clusterctl v0.3.X no longer work to initialize providers. A workaround is to pull the provider yamls manually and put them in a local clusterctl repository.

v0.4.X and v0.3.X of Cluster API are out of date so there is no need to update those older versions to fix this bug. This should be fixed in supported versions of Cluster API as we don't control the number of releases done by providers and different approaches e.g. many published pre-releases, could result in an older supported version not being installable.

The API call should be able to look through subsequent pages of results in order to find older valid tags. Note: This should also be checked for the gitlab API to see if it suffers from a similar limitation.

/kind bug
/area clusterctl

@k8s-ci-robot k8s-ci-robot added kind/bug Categorizes issue or PR as related to a bug. area/clusterctl Issues or PRs related to clusterctl needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Mar 6, 2023
@killianmuldoon killianmuldoon changed the title clusterctl only considers 60 releases clusterctl only considers 60 github releases Mar 6, 2023
@abhay-krishna
Copy link
Contributor

abhay-krishna commented Mar 6, 2023

I faced this issue when emulating the ListReleases call here and not the GetReleaseByTag call here which does indeed fetch an older tagged release, as far back as v0.2.6. While testing using curl, adding a per_page search query to the /releases API endpoint ensured that upto 100 versions are listed, the default being 30. Go-github supports the per_page input through the ListOptions parameter to the ListReleases call, is adding that a valid solution?

@killianmuldoon killianmuldoon changed the title clusterctl only considers 60 github releases clusterctl only considers 30 github releases Mar 6, 2023
@killianmuldoon
Copy link
Contributor Author

killianmuldoon commented Mar 6, 2023

IMO instead of picking an arbitrary limit of per_page for the call it would be best if the client could go through each page of releases so it finds the latest in the series even if it's not in the first X results. You're right that the api by default returns 30 items - not sure how I counted to 60 earlier 😅

@abhay-krishna
Copy link
Contributor

abhay-krishna commented Mar 6, 2023

Yeah that would indeed be best. In most paginated API calls, one of the fields in the response is a token which, when passed into a subsequent call to the same API, fetches the next set of items. And the calls are made to the API until the token field is either empty or same as that returned from a previous call. This is prevalent in several production APIs such as those provided by AWS. I don't think GitHub does that, so what we can do is retrieve results page by page, appending the results to a list, until the results array is an empty list.

@abhay-krishna
Copy link
Contributor

abhay-krishna commented Mar 6, 2023

Was able to test with this code snippet that the approach of combing through the pages one by one and pooling releases together works, fetches all 119 releases it can find. Would you like me to open a PR for this?

Code:

// capi-releases.go
package main

import (
	"fmt"
	"context"
	"os"

	"github.com/google/go-github/v48/github"
	"golang.org/x/oauth2"
)

func main() {
	httpTokenSource := oauth2.StaticTokenSource(
		&oauth2.Token{AccessToken: os.Getenv("GITHUB_TOKEN")},
	)
	httpClient := oauth2.NewClient(context.TODO(), httpTokenSource)
	client := github.NewClient(httpClient)
	allReleases := []*github.RepositoryRelease{}

	// Get the first page of releases
	releases, response, err := client.Repositories.ListReleases(context.TODO(), "kubernetes-sigs", "cluster-api", nil)
	if err != nil {
		fmt.Printf("Error listing github releases: %v\n", err)
		os.Exit(1)
	}
	allReleases = append(allReleases, releases...)

	// Github API Response includes a LastPage field for paginated calls
	// https://github.com/google/go-github/blob/master/github/github.go#L536-L546
	lastPage := response.LastPage

	// From the second page to the last page, fetch all releases, passing in the page number to the ListOptions argument
	for pageNum := 2; pageNum <= lastPage; pageNum++ {
		releases, response, err = client.Repositories.ListReleases(context.TODO(), "kubernetes-sigs", "cluster-api", &github.ListOptions{Page: pageNum})
		if err != nil {
			fmt.Printf("Error listing github releases: %v\n", err)
			os.Exit(1)
		}
		allReleases = append(allReleases, releases...)
	}

	fmt.Println(len(allReleases))
}

Output:

go run capi-releases.go
119

@abhay-krishna
Copy link
Contributor

@killianmuldoon Opened a PR, let me know what you think!

@killianmuldoon
Copy link
Contributor Author

Just to add some clarity and context: This is only impactful when the latest version of a release series is not in the first 30 results in the github releases. Currently the returned releases are all from the v1beta1 release series. This would only impact a v1beta1 release after 30 v1beta2 releases. This means versions like 1.0.0 are available today even though it was released before v0.4.8

@fabriziopandini
Copy link
Member

/triage accepted
/assign @abhay-krishna

@k8s-ci-robot k8s-ci-robot added triage/accepted Indicates an issue or PR is ready to be actively worked on. and removed needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Mar 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/clusterctl Issues or PRs related to clusterctl kind/bug Categorizes issue or PR as related to a bug. triage/accepted Indicates an issue or PR is ready to be actively worked on.
Projects
None yet
4 participants