From 25349455abdda2e809e2a361a12c3b86fa9e89ff Mon Sep 17 00:00:00 2001 From: Erik Nobel Date: Mon, 7 Dec 2020 20:16:54 +0100 Subject: [PATCH] Use ListEnabledRepos & check within sync-period time --- controllers/autoscaling.go | 35 ++++++++++--------- .../horizontalrunnerautoscaler_controller.go | 1 + github/github.go | 21 +++++++---- go.mod | 2 -- go.sum | 4 --- main.go | 1 + 6 files changed, 35 insertions(+), 29 deletions(-) diff --git a/controllers/autoscaling.go b/controllers/autoscaling.go index eb8a63cb93..ea4222cc13 100644 --- a/controllers/autoscaling.go +++ b/controllers/autoscaling.go @@ -5,8 +5,8 @@ import ( "fmt" "log" "strings" + "time" - "github.com/google/go-github/v32/github" "github.com/summerwind/actions-runner-controller/api/v1alpha1" ) @@ -31,36 +31,39 @@ func (r *HorizontalRunnerAutoscalerReconciler) determineDesiredReplicas(rd v1alp return nil, fmt.Errorf("validting autoscaling metrics: unsupported metric type %q: only supported value is %s", tpe, v1alpha1.AutoscalingMetricTypeTotalNumberOfQueuedAndInProgressWorkflowRuns) } + // Github Enterprise API doesn't offer an endpoint for checking the Actions queue on org level, neither does it offer an endpoint for checking what repos Actions is enabled. This means that, in case of GH Enterprise, you must pass a slice of repositories. + if len(metrics[0].RepositoryNames) < 1 && r.GitHubClient.GithubEnterprise { + return nil, fmt.Errorf("[ERROR] user didn't pass any repository! Please pass a list of repositories the controller has to monitor") + } + + // If the above conditionally didn't return an error, we automatically assume, in case on an empty repository slice, that an organization is used. if len(metrics[0].RepositoryNames) == 0 { - options := &github.RepositoryListByOrgOptions{ - Type: "private", - Sort: "pushed", - } - orgRepos, _, err := r.GitHubClient.Repositories.ListByOrg(context.Background(), orgName, options) + enabledRepos, _, err := r.GitHubClient.Actions.ListEnabledReposInOrg(context.Background(), orgName, nil) if err != nil { - return nil, fmt.Errorf("[ERROR] error fetching a list of repositories for the %s organization with error message: %s", orgName, err) + return nil, fmt.Errorf("[ERROR] 'ListEnabledReposInOrg' failed with error message: %s", err) } - if len(orgRepos) < 1 { - return nil, fmt.Errorf("[ERROR] ListByOrg returned empty slice! Does your PAT have enough access and is it authorized to list the organizational repositories?") + if len(enabledRepos.Repositories) < 1 { + return nil, fmt.Errorf("[ERROR] 'ListEnabledReposInOrg' returned an empty slice of repositories, check your permissions. Error message: %s", err) } - for _, v := range orgRepos { + for _, v := range enabledRepos.Repositories { repoName := fmt.Sprint(*v.Name) - // We kind of already make sure that we don't use these repo's by using the `ListByOrgOptions` field, this is just an extra safeguard. if *v.Archived || *v.Disabled { continue } - // Some organizations have hundreds to thousands of repositories; we only need the X most recent ones. - if len(repos) >= 10 { - log.Printf("[INFO] Reached the limit of repos, performing check on these repositories: %s", repos) - break + lastChange := (int(time.Now().UTC().Sub(v.PushedAt.Time).Minutes())) + // We need a conditional here, since the `ListEnabledReposInOrg(ListOptions)` doesn't allow us to filter on `pushedAt`: https://docs.github.com/en/free-pro-team@latest/rest/reference/actions#list-selected-repositories-enabled-for-github-actions-in-an-organization + if lastChange > int(r.SyncPeriod.Minutes()) { + continue + } else if len(repos) < 20 { + repos = append(repos, []string{orgName, repoName}) } - repos = append(repos, []string{orgName, repoName}) } log.Printf("[INFO] watching the following organizational repositories: %s", repos) + } else { repoID := rd.Spec.Template.Spec.Repository if repoID == "" { diff --git a/controllers/horizontalrunnerautoscaler_controller.go b/controllers/horizontalrunnerautoscaler_controller.go index 5dcbfdc298..5bc564ece0 100644 --- a/controllers/horizontalrunnerautoscaler_controller.go +++ b/controllers/horizontalrunnerautoscaler_controller.go @@ -46,6 +46,7 @@ type HorizontalRunnerAutoscalerReconciler struct { Log logr.Logger Recorder record.EventRecorder Scheme *runtime.Scheme + SyncPeriod *time.Duration } // +kubebuilder:rbac:groups=actions.summerwind.dev,resources=runnerdeployments,verbs=get;list;watch;update;patch diff --git a/github/github.go b/github/github.go index 5ab41d177d..cc36da68bc 100644 --- a/github/github.go +++ b/github/github.go @@ -29,16 +29,19 @@ type Client struct { regTokens map[string]*github.RegistrationToken mu sync.Mutex // GithubBaseURL to Github without API suffix. - GithubBaseURL string + GithubBaseURL string + GithubEnterprise bool } // NewClient creates a Github Client func (c *Config) NewClient() (*Client, error) { var ( - httpClient *http.Client - client *github.Client + httpClient *http.Client + client *github.Client + githubEnterprise bool ) githubBaseURL := "https://github.com/" + if len(c.Token) > 0 { httpClient = oauth2.NewClient(context.Background(), oauth2.StaticTokenSource( &oauth2.Token{AccessToken: c.Token}, @@ -60,6 +63,8 @@ func (c *Config) NewClient() (*Client, error) { if len(c.EnterpriseURL) > 0 { var err error + githubEnterprise = true + client, err = github.NewEnterpriseClient(c.EnterpriseURL, c.EnterpriseURL, httpClient) if err != nil { return nil, fmt.Errorf("enterprise client creation failed: %v", err) @@ -67,13 +72,15 @@ func (c *Config) NewClient() (*Client, error) { githubBaseURL = fmt.Sprintf("%s://%s%s", client.BaseURL.Scheme, client.BaseURL.Host, strings.TrimSuffix(client.BaseURL.Path, "api/v3/")) } else { client = github.NewClient(httpClient) + githubEnterprise = false } return &Client{ - Client: client, - regTokens: map[string]*github.RegistrationToken{}, - mu: sync.Mutex{}, - GithubBaseURL: githubBaseURL, + Client: client, + regTokens: map[string]*github.RegistrationToken{}, + mu: sync.Mutex{}, + GithubBaseURL: githubBaseURL, + GithubEnterprise: githubEnterprise, }, nil } diff --git a/go.mod b/go.mod index 29c388a4a6..4c0731a68e 100644 --- a/go.mod +++ b/go.mod @@ -6,8 +6,6 @@ require ( github.com/bradleyfalzon/ghinstallation v1.1.1 github.com/davecgh/go-spew v1.1.1 github.com/go-logr/logr v0.1.0 - github.com/google/go-github v17.0.0+incompatible // indirect - github.com/google/go-github/v32 v32.1.1-0.20200822031813-d57a3a84ba04 github.com/google/go-github/v33 v33.0.0 github.com/google/go-querystring v1.0.0 github.com/gorilla/mux v1.8.0 diff --git a/go.sum b/go.sum index d4c55973bc..f044df7f6f 100644 --- a/go.sum +++ b/go.sum @@ -116,12 +116,8 @@ github.com/google/go-cmp v0.3.0 h1:crn/baboCvb5fXaQ0IJ1SGTsTVrWpDsCWC8EGETZijY= github.com/google/go-cmp v0.3.0/go.mod h1:8QqcDgzrUqlUb/G2PQTWiueGozuR1884gddMywk6iLU= github.com/google/go-cmp v0.3.1 h1:Xye71clBPdm5HgqGwUkwhbynsUJZhDbS20FvLhQ2izg= github.com/google/go-cmp v0.3.1/go.mod h1:8QqcDgzrUqlUb/G2PQTWiueGozuR1884gddMywk6iLU= -github.com/google/go-github v17.0.0+incompatible h1:N0LgJ1j65A7kfXrZnUDaYCs/Sf4rEjNlfyDHW9dolSY= -github.com/google/go-github v17.0.0+incompatible/go.mod h1:zLgOLi98H3fifZn+44m+umXrS52loVEgC2AApnigrVQ= github.com/google/go-github/v29 v29.0.2 h1:opYN6Wc7DOz7Ku3Oh4l7prmkOMwEcQxpFtxdU8N8Pts= github.com/google/go-github/v29 v29.0.2/go.mod h1:CHKiKKPHJ0REzfwc14QMklvtHwCveD0PxlMjLlzAM5E= -github.com/google/go-github/v32 v32.1.1-0.20200822031813-d57a3a84ba04 h1:wEYk2h/GwOhImcVjiTIceP88WxVbXw2F+ARYUQMEsfg= -github.com/google/go-github/v32 v32.1.1-0.20200822031813-d57a3a84ba04/go.mod h1:rIEpZD9CTDQwDK9GDrtMTycQNA4JU3qBsCizh3q2WCI= github.com/google/go-github/v33 v33.0.0 h1:qAf9yP0qc54ufQxzwv+u9H0tiVOnPJxo0lI/JXqw3ZM= github.com/google/go-github/v33 v33.0.0/go.mod h1:GMdDnVZY/2TsWgp/lkYnpSAh6TrzhANBBwm6k6TTEXg= github.com/google/go-querystring v1.0.0 h1:Xkwi/a1rcvNg1PPYe5vI8GbeBY/jrVuDX5ASuANWTrk= diff --git a/main.go b/main.go index 63fcff1ffb..1133ade42c 100644 --- a/main.go +++ b/main.go @@ -148,6 +148,7 @@ func main() { Log: ctrl.Log.WithName("controllers").WithName("HorizontalRunnerAutoscaler"), Scheme: mgr.GetScheme(), GitHubClient: ghClient, + SyncPeriod: &syncPeriod, } if err = horizontalRunnerAutoscaler.SetupWithManager(mgr); err != nil {