From ce3c917ae5010b7db36534b0168a42d626a0ff0a Mon Sep 17 00:00:00 2001 From: Gabriel Adrian Samfira Date: Thu, 14 Mar 2024 20:04:34 +0000 Subject: [PATCH 1/6] Add pool balancing strategy This change adds the ability to specify the pool balancing strategy to use when processing queued jobs. Before this change, GARM would round-robin through all pools that matched the set of tags requested by queued jobs. When round-robin (default) is used for an entity (repo, org or enterprise) and you have 2 pools defined for that entity with a common set of tags that match 10 jobs (for example), then those jobs would trigger the creation of a new runner in each of the two pools in turn. Job 1 would go to pool 1, job 2 would go to pool 2, job 3 to pool 1, job 4 to pool 2 and so on. When "stack" is used, those same 10 jobs would trigger the creation of a new runner in the pool with the highest priority, every time. In both cases, if a pool is full, the next one would be tried automatically. For the stack case, this would mean that if pool 2 had a priority of 10 and pool 1 would have a priority of 5, pool 2 would be saturated first, then pool 1. Signed-off-by: Gabriel Adrian Samfira --- cmd/garm-cli/cmd/enterprise.go | 19 +++++++---- cmd/garm-cli/cmd/organization.go | 19 +++++++---- cmd/garm-cli/cmd/pool.go | 12 +++++-- cmd/garm-cli/cmd/repository.go | 21 +++++++----- cmd/garm-cli/cmd/root.go | 1 + database/common/common.go | 6 ++-- database/common/mocks/Store.go | 54 +++++++++++++++--------------- database/sql/enterprise.go | 14 +++++--- database/sql/enterprise_test.go | 10 ++++-- database/sql/instances_test.go | 2 +- database/sql/models.go | 36 +++++++++++--------- database/sql/organizations.go | 14 +++++--- database/sql/organizations_test.go | 10 ++++-- database/sql/pools.go | 4 ++- database/sql/pools_test.go | 4 +-- database/sql/repositories.go | 16 ++++++--- database/sql/repositories_test.go | 4 +++ database/sql/util.go | 52 +++++++++++++++++++--------- params/params.go | 43 ++++++++++++++++++++++++ params/requests.go | 49 ++++++++++++++++++++------- runner/enterprises.go | 12 ++++++- runner/enterprises_test.go | 1 + runner/organizations.go | 12 ++++++- runner/organizations_test.go | 1 + runner/pool/enterprise.go | 7 ++++ runner/pool/interfaces.go | 1 + runner/pool/organization.go | 7 ++++ runner/pool/pool.go | 6 +++- runner/pool/repository.go | 7 ++++ runner/pool/util.go | 25 +++++++++++--- runner/pools_test.go | 2 +- runner/repositories.go | 12 ++++++- runner/repositories_test.go | 1 + runner/runner.go | 15 +++++---- 34 files changed, 362 insertions(+), 137 deletions(-) diff --git a/cmd/garm-cli/cmd/enterprise.go b/cmd/garm-cli/cmd/enterprise.go index b01b0413..27ca662c 100644 --- a/cmd/garm-cli/cmd/enterprise.go +++ b/cmd/garm-cli/cmd/enterprise.go @@ -57,9 +57,10 @@ var enterpriseAddCmd = &cobra.Command{ newEnterpriseReq := apiClientEnterprises.NewCreateEnterpriseParams() newEnterpriseReq.Body = params.CreateEnterpriseParams{ - Name: enterpriseName, - WebhookSecret: enterpriseWebhookSecret, - CredentialsName: enterpriseCreds, + Name: enterpriseName, + WebhookSecret: enterpriseWebhookSecret, + CredentialsName: enterpriseCreds, + PoolBalancerType: params.PoolBalancerType(poolBalancerType), } response, err := apiCli.Enterprises.CreateEnterprise(newEnterpriseReq, authToken) if err != nil { @@ -161,8 +162,9 @@ var enterpriseUpdateCmd = &cobra.Command{ } updateEnterpriseReq := apiClientEnterprises.NewUpdateEnterpriseParams() updateEnterpriseReq.Body = params.UpdateEntityParams{ - WebhookSecret: repoWebhookSecret, - CredentialsName: repoCreds, + WebhookSecret: repoWebhookSecret, + CredentialsName: repoCreds, + PoolBalancerType: params.PoolBalancerType(poolBalancerType), } updateEnterpriseReq.EnterpriseID = args[0] response, err := apiCli.Enterprises.UpdateEnterprise(updateEnterpriseReq, authToken) @@ -178,11 +180,13 @@ func init() { enterpriseAddCmd.Flags().StringVar(&enterpriseName, "name", "", "The name of the enterprise") enterpriseAddCmd.Flags().StringVar(&enterpriseWebhookSecret, "webhook-secret", "", "The webhook secret for this enterprise") enterpriseAddCmd.Flags().StringVar(&enterpriseCreds, "credentials", "", "Credentials name. See credentials list.") + enterpriseAddCmd.Flags().StringVar(&poolBalancerType, "pool-balancer-type", string(params.PoolBalancerTypeRoundRobin), "The balancing strategy to use when creating runners in pools matching requested labels.") enterpriseAddCmd.MarkFlagRequired("credentials") //nolint enterpriseAddCmd.MarkFlagRequired("name") //nolint enterpriseUpdateCmd.Flags().StringVar(&enterpriseWebhookSecret, "webhook-secret", "", "The webhook secret for this enterprise") enterpriseUpdateCmd.Flags().StringVar(&enterpriseCreds, "credentials", "", "Credentials name. See credentials list.") + enterpriseUpdateCmd.Flags().StringVar(&poolBalancerType, "pool-balancer-type", "", "The balancing strategy to use when creating runners in pools matching requested labels.") enterpriseCmd.AddCommand( enterpriseListCmd, @@ -197,10 +201,10 @@ func init() { func formatEnterprises(enterprises []params.Enterprise) { t := table.NewWriter() - header := table.Row{"ID", "Name", "Credentials name", "Pool mgr running"} + header := table.Row{"ID", "Name", "Credentials name", "Pool Balancer Type", "Pool mgr running"} t.AppendHeader(header) for _, val := range enterprises { - t.AppendRow(table.Row{val.ID, val.Name, val.CredentialsName, val.PoolManagerStatus.IsRunning}) + t.AppendRow(table.Row{val.ID, val.Name, val.CredentialsName, val.GetBalancerType(), val.PoolManagerStatus.IsRunning}) t.AppendSeparator() } fmt.Println(t.Render()) @@ -213,6 +217,7 @@ func formatOneEnterprise(enterprise params.Enterprise) { t.AppendHeader(header) t.AppendRow(table.Row{"ID", enterprise.ID}) t.AppendRow(table.Row{"Name", enterprise.Name}) + t.AppendRow(table.Row{"Pool balancer type", enterprise.GetBalancerType()}) t.AppendRow(table.Row{"Credentials", enterprise.CredentialsName}) t.AppendRow(table.Row{"Pool manager running", enterprise.PoolManagerStatus.IsRunning}) if !enterprise.PoolManagerStatus.IsRunning { diff --git a/cmd/garm-cli/cmd/organization.go b/cmd/garm-cli/cmd/organization.go index 2b5def34..020452a1 100644 --- a/cmd/garm-cli/cmd/organization.go +++ b/cmd/garm-cli/cmd/organization.go @@ -163,9 +163,10 @@ var orgAddCmd = &cobra.Command{ newOrgReq := apiClientOrgs.NewCreateOrgParams() newOrgReq.Body = params.CreateOrgParams{ - Name: orgName, - WebhookSecret: orgWebhookSecret, - CredentialsName: orgCreds, + Name: orgName, + WebhookSecret: orgWebhookSecret, + CredentialsName: orgCreds, + PoolBalancerType: params.PoolBalancerType(poolBalancerType), } response, err := apiCli.Organizations.CreateOrg(newOrgReq, authToken) if err != nil { @@ -213,8 +214,9 @@ var orgUpdateCmd = &cobra.Command{ } updateOrgReq := apiClientOrgs.NewUpdateOrgParams() updateOrgReq.Body = params.UpdateEntityParams{ - WebhookSecret: orgWebhookSecret, - CredentialsName: orgCreds, + WebhookSecret: orgWebhookSecret, + CredentialsName: orgCreds, + PoolBalancerType: params.PoolBalancerType(poolBalancerType), } updateOrgReq.OrgID = args[0] response, err := apiCli.Organizations.UpdateOrg(updateOrgReq, authToken) @@ -301,6 +303,7 @@ var orgDeleteCmd = &cobra.Command{ func init() { orgAddCmd.Flags().StringVar(&orgName, "name", "", "The name of the organization") + orgAddCmd.Flags().StringVar(&poolBalancerType, "pool-balancer-type", string(params.PoolBalancerTypeRoundRobin), "The balancing strategy to use when creating runners in pools matching requested labels.") orgAddCmd.Flags().StringVar(&orgWebhookSecret, "webhook-secret", "", "The webhook secret for this organization") orgAddCmd.Flags().StringVar(&orgCreds, "credentials", "", "Credentials name. See credentials list.") orgAddCmd.Flags().BoolVar(&orgRandomWebhookSecret, "random-webhook-secret", false, "Generate a random webhook secret for this organization.") @@ -315,6 +318,7 @@ func init() { orgUpdateCmd.Flags().StringVar(&orgWebhookSecret, "webhook-secret", "", "The webhook secret for this organization") orgUpdateCmd.Flags().StringVar(&orgCreds, "credentials", "", "Credentials name. See credentials list.") + orgUpdateCmd.Flags().StringVar(&poolBalancerType, "pool-balancer-type", "", "The balancing strategy to use when creating runners in pools matching requested labels.") orgWebhookInstallCmd.Flags().BoolVar(&insecureOrgWebhook, "insecure", false, "Ignore self signed certificate errors.") orgWebhookCmd.AddCommand( @@ -337,10 +341,10 @@ func init() { func formatOrganizations(orgs []params.Organization) { t := table.NewWriter() - header := table.Row{"ID", "Name", "Credentials name", "Pool mgr running"} + header := table.Row{"ID", "Name", "Credentials name", "Pool Balancer Type", "Pool mgr running"} t.AppendHeader(header) for _, val := range orgs { - t.AppendRow(table.Row{val.ID, val.Name, val.CredentialsName, val.PoolManagerStatus.IsRunning}) + t.AppendRow(table.Row{val.ID, val.Name, val.CredentialsName, val.GetBalancerType(), val.PoolManagerStatus.IsRunning}) t.AppendSeparator() } fmt.Println(t.Render()) @@ -353,6 +357,7 @@ func formatOneOrganization(org params.Organization) { t.AppendHeader(header) t.AppendRow(table.Row{"ID", org.ID}) t.AppendRow(table.Row{"Name", org.Name}) + t.AppendRow(table.Row{"Pool balancer type", org.GetBalancerType()}) t.AppendRow(table.Row{"Credentials", org.CredentialsName}) t.AppendRow(table.Row{"Pool manager running", org.PoolManagerStatus.IsRunning}) if !org.PoolManagerStatus.IsRunning { diff --git a/cmd/garm-cli/cmd/pool.go b/cmd/garm-cli/cmd/pool.go index 31fe326d..f08192fc 100644 --- a/cmd/garm-cli/cmd/pool.go +++ b/cmd/garm-cli/cmd/pool.go @@ -51,6 +51,7 @@ var ( poolExtraSpecs string poolAll bool poolGitHubRunnerGroup string + priority uint ) type poolPayloadGetter interface { @@ -218,6 +219,7 @@ var poolAddCmd = &cobra.Command{ Enabled: poolEnabled, RunnerBootstrapTimeout: poolRunnerBootstrapTimeout, GitHubRunnerGroup: poolGitHubRunnerGroup, + Priority: priority, } if cmd.Flags().Changed("extra-specs") { @@ -326,6 +328,9 @@ explicitly remove them using the runner delete command. if cmd.Flags().Changed("max-runners") { poolUpdateParams.MaxRunners = &poolMaxRunners } + if cmd.Flags().Changed("priority") { + poolUpdateParams.Priority = &priority + } if cmd.Flags().Changed("min-idle-runners") { poolUpdateParams.MinIdleRunners = &poolMinIdleRunners @@ -385,6 +390,7 @@ func init() { poolListCmd.MarkFlagsMutuallyExclusive("repo", "org", "all", "enterprise") poolUpdateCmd.Flags().StringVar(&poolImage, "image", "", "The provider-specific image name to use for runners in this pool.") + poolUpdateCmd.Flags().UintVar(&priority, "priority", 0, "When multiple pools match the same labels, priority dictates the order by which they are returned, in descending order.") poolUpdateCmd.Flags().StringVar(&poolFlavor, "flavor", "", "The flavor to use for this runner.") poolUpdateCmd.Flags().StringVar(&poolTags, "tags", "", "A comma separated list of tags to assign to this runner.") poolUpdateCmd.Flags().StringVar(&poolOSType, "os-type", "linux", "Operating system type (windows, linux, etc).") @@ -400,6 +406,7 @@ func init() { poolUpdateCmd.MarkFlagsMutuallyExclusive("extra-specs-file", "extra-specs") poolAddCmd.Flags().StringVar(&poolProvider, "provider-name", "", "The name of the provider where runners will be created.") + poolAddCmd.Flags().UintVar(&priority, "priority", 0, "When multiple pools match the same labels, priority dictates the order by which they are returned, in descending order.") poolAddCmd.Flags().StringVar(&poolImage, "image", "", "The provider-specific image name to use for runners in this pool.") poolAddCmd.Flags().StringVar(&poolFlavor, "flavor", "", "The flavor to use for this runner.") poolAddCmd.Flags().StringVar(&poolRunnerPrefix, "runner-prefix", "", "The name prefix to use for runners in this pool.") @@ -462,7 +469,7 @@ func asRawMessage(data []byte) (json.RawMessage, error) { func formatPools(pools []params.Pool) { t := table.NewWriter() - header := table.Row{"ID", "Image", "Flavor", "Tags", "Belongs to", "Level", "Enabled", "Runner Prefix"} + header := table.Row{"ID", "Image", "Flavor", "Tags", "Belongs to", "Level", "Enabled", "Runner Prefix", "Priority"} t.AppendHeader(header) for _, pool := range pools { @@ -484,7 +491,7 @@ func formatPools(pools []params.Pool) { belongsTo = pool.EnterpriseName level = "enterprise" } - t.AppendRow(table.Row{pool.ID, pool.Image, pool.Flavor, strings.Join(tags, " "), belongsTo, level, pool.Enabled, pool.GetRunnerPrefix()}) + t.AppendRow(table.Row{pool.ID, pool.Image, pool.Flavor, strings.Join(tags, " "), belongsTo, level, pool.Enabled, pool.GetRunnerPrefix(), pool.Priority}) t.AppendSeparator() } fmt.Println(t.Render()) @@ -519,6 +526,7 @@ func formatOnePool(pool params.Pool) { t.AppendHeader(header) t.AppendRow(table.Row{"ID", pool.ID}) t.AppendRow(table.Row{"Provider Name", pool.ProviderName}) + t.AppendRow(table.Row{"Priority", pool.Priority}) t.AppendRow(table.Row{"Image", pool.Image}) t.AppendRow(table.Row{"Flavor", pool.Flavor}) t.AppendRow(table.Row{"OS Type", pool.OSType}) diff --git a/cmd/garm-cli/cmd/repository.go b/cmd/garm-cli/cmd/repository.go index a603988b..845252a5 100644 --- a/cmd/garm-cli/cmd/repository.go +++ b/cmd/garm-cli/cmd/repository.go @@ -164,10 +164,11 @@ var repoAddCmd = &cobra.Command{ newRepoReq := apiClientRepos.NewCreateRepoParams() newRepoReq.Body = params.CreateRepoParams{ - Owner: repoOwner, - Name: repoName, - WebhookSecret: repoWebhookSecret, - CredentialsName: repoCreds, + Owner: repoOwner, + Name: repoName, + WebhookSecret: repoWebhookSecret, + CredentialsName: repoCreds, + PoolBalancerType: params.PoolBalancerType(poolBalancerType), } response, err := apiCli.Repositories.CreateRepo(newRepoReq, authToken) if err != nil { @@ -236,8 +237,9 @@ var repoUpdateCmd = &cobra.Command{ } updateReposReq := apiClientRepos.NewUpdateRepoParams() updateReposReq.Body = params.UpdateEntityParams{ - WebhookSecret: repoWebhookSecret, - CredentialsName: repoCreds, + WebhookSecret: repoWebhookSecret, + CredentialsName: repoCreds, + PoolBalancerType: params.PoolBalancerType(poolBalancerType), } updateReposReq.RepoID = args[0] @@ -304,6 +306,7 @@ var repoDeleteCmd = &cobra.Command{ func init() { repoAddCmd.Flags().StringVar(&repoOwner, "owner", "", "The owner of this repository") + repoAddCmd.Flags().StringVar(&poolBalancerType, "pool-balancer-type", string(params.PoolBalancerTypeRoundRobin), "The balancing strategy to use when creating runners in pools matching requested labels.") repoAddCmd.Flags().StringVar(&repoName, "name", "", "The name of the repository") repoAddCmd.Flags().StringVar(&repoWebhookSecret, "webhook-secret", "", "The webhook secret for this repository") repoAddCmd.Flags().StringVar(&repoCreds, "credentials", "", "Credentials name. See credentials list.") @@ -320,6 +323,7 @@ func init() { repoUpdateCmd.Flags().StringVar(&repoWebhookSecret, "webhook-secret", "", "The webhook secret for this repository. If you update this secret, you will have to manually update the secret in GitHub as well.") repoUpdateCmd.Flags().StringVar(&repoCreds, "credentials", "", "Credentials name. See credentials list.") + repoUpdateCmd.Flags().StringVar(&poolBalancerType, "pool-balancer-type", "", "The balancing strategy to use when creating runners in pools matching requested labels.") repoWebhookInstallCmd.Flags().BoolVar(&insecureRepoWebhook, "insecure", false, "Ignore self signed certificate errors.") @@ -343,10 +347,10 @@ func init() { func formatRepositories(repos []params.Repository) { t := table.NewWriter() - header := table.Row{"ID", "Owner", "Name", "Credentials name", "Pool mgr running"} + header := table.Row{"ID", "Owner", "Name", "Credentials name", "Pool Balancer Type", "Pool mgr running"} t.AppendHeader(header) for _, val := range repos { - t.AppendRow(table.Row{val.ID, val.Owner, val.Name, val.CredentialsName, val.PoolManagerStatus.IsRunning}) + t.AppendRow(table.Row{val.ID, val.Owner, val.Name, val.CredentialsName, val.GetBalancerType(), val.PoolManagerStatus.IsRunning}) t.AppendSeparator() } fmt.Println(t.Render()) @@ -360,6 +364,7 @@ func formatOneRepository(repo params.Repository) { t.AppendRow(table.Row{"ID", repo.ID}) t.AppendRow(table.Row{"Owner", repo.Owner}) t.AppendRow(table.Row{"Name", repo.Name}) + t.AppendRow(table.Row{"Pool balancer type", repo.GetBalancerType()}) t.AppendRow(table.Row{"Credentials", repo.CredentialsName}) t.AppendRow(table.Row{"Pool manager running", repo.PoolManagerStatus.IsRunning}) if !repo.PoolManagerStatus.IsRunning { diff --git a/cmd/garm-cli/cmd/root.go b/cmd/garm-cli/cmd/root.go index 86bd47ac..6e053def 100644 --- a/cmd/garm-cli/cmd/root.go +++ b/cmd/garm-cli/cmd/root.go @@ -38,6 +38,7 @@ var ( authToken runtime.ClientAuthInfoWriter needsInit bool debug bool + poolBalancerType string errNeedsInitError = fmt.Errorf("please log into a garm installation first") ) diff --git a/database/common/common.go b/database/common/common.go index 6f8dc820..f41a3559 100644 --- a/database/common/common.go +++ b/database/common/common.go @@ -21,7 +21,7 @@ import ( ) type RepoStore interface { - CreateRepository(ctx context.Context, owner, name, credentialsName, webhookSecret string) (params.Repository, error) + CreateRepository(ctx context.Context, owner, name, credentialsName, webhookSecret string, poolBalancerType params.PoolBalancerType) (params.Repository, error) GetRepository(ctx context.Context, owner, name string) (params.Repository, error) GetRepositoryByID(ctx context.Context, repoID string) (params.Repository, error) ListRepositories(ctx context.Context) ([]params.Repository, error) @@ -40,7 +40,7 @@ type RepoStore interface { } type OrgStore interface { - CreateOrganization(ctx context.Context, name, credentialsName, webhookSecret string) (params.Organization, error) + CreateOrganization(ctx context.Context, name, credentialsName, webhookSecret string, poolBalancerType params.PoolBalancerType) (params.Organization, error) GetOrganization(ctx context.Context, name string) (params.Organization, error) GetOrganizationByID(ctx context.Context, orgID string) (params.Organization, error) ListOrganizations(ctx context.Context) ([]params.Organization, error) @@ -58,7 +58,7 @@ type OrgStore interface { } type EnterpriseStore interface { - CreateEnterprise(ctx context.Context, name, credentialsName, webhookSecret string) (params.Enterprise, error) + CreateEnterprise(ctx context.Context, name, credentialsName, webhookSecret string, poolBalancerType params.PoolBalancerType) (params.Enterprise, error) GetEnterprise(ctx context.Context, name string) (params.Enterprise, error) GetEnterpriseByID(ctx context.Context, enterpriseID string) (params.Enterprise, error) ListEnterprises(ctx context.Context) ([]params.Enterprise, error) diff --git a/database/common/mocks/Store.go b/database/common/mocks/Store.go index e709d770..f20b3830 100644 --- a/database/common/mocks/Store.go +++ b/database/common/mocks/Store.go @@ -78,9 +78,9 @@ func (_m *Store) ControllerInfo() (params.ControllerInfo, error) { return r0, r1 } -// CreateEnterprise provides a mock function with given fields: ctx, name, credentialsName, webhookSecret -func (_m *Store) CreateEnterprise(ctx context.Context, name string, credentialsName string, webhookSecret string) (params.Enterprise, error) { - ret := _m.Called(ctx, name, credentialsName, webhookSecret) +// CreateEnterprise provides a mock function with given fields: ctx, name, credentialsName, webhookSecret, poolBalancerType +func (_m *Store) CreateEnterprise(ctx context.Context, name string, credentialsName string, webhookSecret string, poolBalancerType params.PoolBalancerType) (params.Enterprise, error) { + ret := _m.Called(ctx, name, credentialsName, webhookSecret, poolBalancerType) if len(ret) == 0 { panic("no return value specified for CreateEnterprise") @@ -88,17 +88,17 @@ func (_m *Store) CreateEnterprise(ctx context.Context, name string, credentialsN var r0 params.Enterprise var r1 error - if rf, ok := ret.Get(0).(func(context.Context, string, string, string) (params.Enterprise, error)); ok { - return rf(ctx, name, credentialsName, webhookSecret) + if rf, ok := ret.Get(0).(func(context.Context, string, string, string, params.PoolBalancerType) (params.Enterprise, error)); ok { + return rf(ctx, name, credentialsName, webhookSecret, poolBalancerType) } - if rf, ok := ret.Get(0).(func(context.Context, string, string, string) params.Enterprise); ok { - r0 = rf(ctx, name, credentialsName, webhookSecret) + if rf, ok := ret.Get(0).(func(context.Context, string, string, string, params.PoolBalancerType) params.Enterprise); ok { + r0 = rf(ctx, name, credentialsName, webhookSecret, poolBalancerType) } else { r0 = ret.Get(0).(params.Enterprise) } - if rf, ok := ret.Get(1).(func(context.Context, string, string, string) error); ok { - r1 = rf(ctx, name, credentialsName, webhookSecret) + if rf, ok := ret.Get(1).(func(context.Context, string, string, string, params.PoolBalancerType) error); ok { + r1 = rf(ctx, name, credentialsName, webhookSecret, poolBalancerType) } else { r1 = ret.Error(1) } @@ -190,9 +190,9 @@ func (_m *Store) CreateOrUpdateJob(ctx context.Context, job params.Job) (params. return r0, r1 } -// CreateOrganization provides a mock function with given fields: ctx, name, credentialsName, webhookSecret -func (_m *Store) CreateOrganization(ctx context.Context, name string, credentialsName string, webhookSecret string) (params.Organization, error) { - ret := _m.Called(ctx, name, credentialsName, webhookSecret) +// CreateOrganization provides a mock function with given fields: ctx, name, credentialsName, webhookSecret, poolBalancerType +func (_m *Store) CreateOrganization(ctx context.Context, name string, credentialsName string, webhookSecret string, poolBalancerType params.PoolBalancerType) (params.Organization, error) { + ret := _m.Called(ctx, name, credentialsName, webhookSecret, poolBalancerType) if len(ret) == 0 { panic("no return value specified for CreateOrganization") @@ -200,17 +200,17 @@ func (_m *Store) CreateOrganization(ctx context.Context, name string, credential var r0 params.Organization var r1 error - if rf, ok := ret.Get(0).(func(context.Context, string, string, string) (params.Organization, error)); ok { - return rf(ctx, name, credentialsName, webhookSecret) + if rf, ok := ret.Get(0).(func(context.Context, string, string, string, params.PoolBalancerType) (params.Organization, error)); ok { + return rf(ctx, name, credentialsName, webhookSecret, poolBalancerType) } - if rf, ok := ret.Get(0).(func(context.Context, string, string, string) params.Organization); ok { - r0 = rf(ctx, name, credentialsName, webhookSecret) + if rf, ok := ret.Get(0).(func(context.Context, string, string, string, params.PoolBalancerType) params.Organization); ok { + r0 = rf(ctx, name, credentialsName, webhookSecret, poolBalancerType) } else { r0 = ret.Get(0).(params.Organization) } - if rf, ok := ret.Get(1).(func(context.Context, string, string, string) error); ok { - r1 = rf(ctx, name, credentialsName, webhookSecret) + if rf, ok := ret.Get(1).(func(context.Context, string, string, string, params.PoolBalancerType) error); ok { + r1 = rf(ctx, name, credentialsName, webhookSecret, poolBalancerType) } else { r1 = ret.Error(1) } @@ -246,9 +246,9 @@ func (_m *Store) CreateOrganizationPool(ctx context.Context, orgID string, param return r0, r1 } -// CreateRepository provides a mock function with given fields: ctx, owner, name, credentialsName, webhookSecret -func (_m *Store) CreateRepository(ctx context.Context, owner string, name string, credentialsName string, webhookSecret string) (params.Repository, error) { - ret := _m.Called(ctx, owner, name, credentialsName, webhookSecret) +// CreateRepository provides a mock function with given fields: ctx, owner, name, credentialsName, webhookSecret, poolBalancerType +func (_m *Store) CreateRepository(ctx context.Context, owner string, name string, credentialsName string, webhookSecret string, poolBalancerType params.PoolBalancerType) (params.Repository, error) { + ret := _m.Called(ctx, owner, name, credentialsName, webhookSecret, poolBalancerType) if len(ret) == 0 { panic("no return value specified for CreateRepository") @@ -256,17 +256,17 @@ func (_m *Store) CreateRepository(ctx context.Context, owner string, name string var r0 params.Repository var r1 error - if rf, ok := ret.Get(0).(func(context.Context, string, string, string, string) (params.Repository, error)); ok { - return rf(ctx, owner, name, credentialsName, webhookSecret) + if rf, ok := ret.Get(0).(func(context.Context, string, string, string, string, params.PoolBalancerType) (params.Repository, error)); ok { + return rf(ctx, owner, name, credentialsName, webhookSecret, poolBalancerType) } - if rf, ok := ret.Get(0).(func(context.Context, string, string, string, string) params.Repository); ok { - r0 = rf(ctx, owner, name, credentialsName, webhookSecret) + if rf, ok := ret.Get(0).(func(context.Context, string, string, string, string, params.PoolBalancerType) params.Repository); ok { + r0 = rf(ctx, owner, name, credentialsName, webhookSecret, poolBalancerType) } else { r0 = ret.Get(0).(params.Repository) } - if rf, ok := ret.Get(1).(func(context.Context, string, string, string, string) error); ok { - r1 = rf(ctx, owner, name, credentialsName, webhookSecret) + if rf, ok := ret.Get(1).(func(context.Context, string, string, string, string, params.PoolBalancerType) error); ok { + r1 = rf(ctx, owner, name, credentialsName, webhookSecret, poolBalancerType) } else { r1 = ret.Error(1) } diff --git a/database/sql/enterprise.go b/database/sql/enterprise.go index 2537557a..dc2629de 100644 --- a/database/sql/enterprise.go +++ b/database/sql/enterprise.go @@ -13,7 +13,7 @@ import ( "github.com/cloudbase/garm/params" ) -func (s *sqlDatabase) CreateEnterprise(_ context.Context, name, credentialsName, webhookSecret string) (params.Enterprise, error) { +func (s *sqlDatabase) CreateEnterprise(_ context.Context, name, credentialsName, webhookSecret string, poolBalancerType params.PoolBalancerType) (params.Enterprise, error) { if webhookSecret == "" { return params.Enterprise{}, errors.New("creating enterprise: missing secret") } @@ -22,9 +22,10 @@ func (s *sqlDatabase) CreateEnterprise(_ context.Context, name, credentialsName, return params.Enterprise{}, errors.Wrap(err, "encoding secret") } newEnterprise := Enterprise{ - Name: name, - WebhookSecret: secret, - CredentialsName: credentialsName, + Name: name, + WebhookSecret: secret, + CredentialsName: credentialsName, + PoolBalancerType: poolBalancerType, } q := s.conn.Create(&newEnterprise) @@ -117,6 +118,10 @@ func (s *sqlDatabase) UpdateEnterprise(ctx context.Context, enterpriseID string, enterprise.WebhookSecret = secret } + if param.PoolBalancerType != "" { + enterprise.PoolBalancerType = param.PoolBalancerType + } + q := s.conn.Save(&enterprise) if q.Error != nil { return params.Enterprise{}, errors.Wrap(q.Error, "saving enterprise") @@ -152,6 +157,7 @@ func (s *sqlDatabase) CreateEnterprisePool(ctx context.Context, enterpriseID str Enabled: param.Enabled, RunnerBootstrapTimeout: param.RunnerBootstrapTimeout, GitHubRunnerGroup: param.GitHubRunnerGroup, + Priority: param.Priority, } if len(param.ExtraSpecs) > 0 { diff --git a/database/sql/enterprise_test.go b/database/sql/enterprise_test.go index e00af874..fa709b89 100644 --- a/database/sql/enterprise_test.go +++ b/database/sql/enterprise_test.go @@ -85,6 +85,7 @@ func (s *EnterpriseTestSuite) SetupTest() { fmt.Sprintf("test-enterprise-%d", i), fmt.Sprintf("test-creds-%d", i), fmt.Sprintf("test-webhook-secret-%d", i), + params.PoolBalancerTypeRoundRobin, ) if err != nil { s.FailNow(fmt.Sprintf("failed to create database object (test-enterprise-%d)", i)) @@ -162,7 +163,8 @@ func (s *EnterpriseTestSuite) TestCreateEnterprise() { context.Background(), s.Fixtures.CreateEnterpriseParams.Name, s.Fixtures.CreateEnterpriseParams.CredentialsName, - s.Fixtures.CreateEnterpriseParams.WebhookSecret) + s.Fixtures.CreateEnterpriseParams.WebhookSecret, + params.PoolBalancerTypeRoundRobin) // assertions s.Require().Nil(err) @@ -192,7 +194,8 @@ func (s *EnterpriseTestSuite) TestCreateEnterpriseInvalidDBPassphrase() { context.Background(), s.Fixtures.CreateEnterpriseParams.Name, s.Fixtures.CreateEnterpriseParams.CredentialsName, - s.Fixtures.CreateEnterpriseParams.WebhookSecret) + s.Fixtures.CreateEnterpriseParams.WebhookSecret, + params.PoolBalancerTypeRoundRobin) s.Require().NotNil(err) s.Require().Equal("encoding secret: invalid passphrase length (expected length 32 characters)", err.Error()) @@ -209,7 +212,8 @@ func (s *EnterpriseTestSuite) TestCreateEnterpriseDBCreateErr() { context.Background(), s.Fixtures.CreateEnterpriseParams.Name, s.Fixtures.CreateEnterpriseParams.CredentialsName, - s.Fixtures.CreateEnterpriseParams.WebhookSecret) + s.Fixtures.CreateEnterpriseParams.WebhookSecret, + params.PoolBalancerTypeRoundRobin) s.assertSQLMockExpectations() s.Require().NotNil(err) diff --git a/database/sql/instances_test.go b/database/sql/instances_test.go index 44fd95c3..0c0eadcf 100644 --- a/database/sql/instances_test.go +++ b/database/sql/instances_test.go @@ -77,7 +77,7 @@ func (s *InstancesTestSuite) SetupTest() { s.Store = db // create an organization for testing purposes - org, err := s.Store.CreateOrganization(context.Background(), "test-org", "test-creds", "test-webhookSecret") + org, err := s.Store.CreateOrganization(context.Background(), "test-org", "test-creds", "test-webhookSecret", params.PoolBalancerTypeRoundRobin) if err != nil { s.FailNow(fmt.Sprintf("failed to create org: %s", err)) } diff --git a/database/sql/models.go b/database/sql/models.go index 047d4195..874a375d 100644 --- a/database/sql/models.go +++ b/database/sql/models.go @@ -83,37 +83,41 @@ type Pool struct { Enterprise Enterprise `gorm:"foreignKey:EnterpriseID"` Instances []Instance `gorm:"foreignKey:PoolID"` + Priority uint `gorm:"index:idx_pool_priority"` } type Repository struct { Base - CredentialsName string - Owner string `gorm:"index:idx_owner_nocase,unique,collate:nocase"` - Name string `gorm:"index:idx_owner_nocase,unique,collate:nocase"` - WebhookSecret []byte - Pools []Pool `gorm:"foreignKey:RepoID"` - Jobs []WorkflowJob `gorm:"foreignKey:RepoID;constraint:OnDelete:SET NULL"` + CredentialsName string + Owner string `gorm:"index:idx_owner_nocase,unique,collate:nocase"` + Name string `gorm:"index:idx_owner_nocase,unique,collate:nocase"` + WebhookSecret []byte + Pools []Pool `gorm:"foreignKey:RepoID"` + Jobs []WorkflowJob `gorm:"foreignKey:RepoID;constraint:OnDelete:SET NULL"` + PoolBalancerType params.PoolBalancerType `gorm:"type:varchar(64)"` } type Organization struct { Base - CredentialsName string - Name string `gorm:"index:idx_org_name_nocase,collate:nocase"` - WebhookSecret []byte - Pools []Pool `gorm:"foreignKey:OrgID"` - Jobs []WorkflowJob `gorm:"foreignKey:OrgID;constraint:OnDelete:SET NULL"` + CredentialsName string + Name string `gorm:"index:idx_org_name_nocase,collate:nocase"` + WebhookSecret []byte + Pools []Pool `gorm:"foreignKey:OrgID"` + Jobs []WorkflowJob `gorm:"foreignKey:OrgID;constraint:OnDelete:SET NULL"` + PoolBalancerType params.PoolBalancerType `gorm:"type:varchar(64)"` } type Enterprise struct { Base - CredentialsName string - Name string `gorm:"index:idx_ent_name_nocase,collate:nocase"` - WebhookSecret []byte - Pools []Pool `gorm:"foreignKey:EnterpriseID"` - Jobs []WorkflowJob `gorm:"foreignKey:EnterpriseID;constraint:OnDelete:SET NULL"` + CredentialsName string + Name string `gorm:"index:idx_ent_name_nocase,collate:nocase"` + WebhookSecret []byte + Pools []Pool `gorm:"foreignKey:EnterpriseID"` + Jobs []WorkflowJob `gorm:"foreignKey:EnterpriseID;constraint:OnDelete:SET NULL"` + PoolBalancerType params.PoolBalancerType `gorm:"type:varchar(64)"` } type Address struct { diff --git a/database/sql/organizations.go b/database/sql/organizations.go index 74075ffe..2fd2b5cf 100644 --- a/database/sql/organizations.go +++ b/database/sql/organizations.go @@ -28,7 +28,7 @@ import ( "github.com/cloudbase/garm/params" ) -func (s *sqlDatabase) CreateOrganization(_ context.Context, name, credentialsName, webhookSecret string) (params.Organization, error) { +func (s *sqlDatabase) CreateOrganization(_ context.Context, name, credentialsName, webhookSecret string, poolBalancerType params.PoolBalancerType) (params.Organization, error) { if webhookSecret == "" { return params.Organization{}, errors.New("creating org: missing secret") } @@ -37,9 +37,10 @@ func (s *sqlDatabase) CreateOrganization(_ context.Context, name, credentialsNam return params.Organization{}, fmt.Errorf("failed to encrypt string") } newOrg := Organization{ - Name: name, - WebhookSecret: secret, - CredentialsName: credentialsName, + Name: name, + WebhookSecret: secret, + CredentialsName: credentialsName, + PoolBalancerType: poolBalancerType, } q := s.conn.Create(&newOrg) @@ -121,6 +122,10 @@ func (s *sqlDatabase) UpdateOrganization(ctx context.Context, orgID string, para org.WebhookSecret = secret } + if param.PoolBalancerType != "" { + org.PoolBalancerType = param.PoolBalancerType + } + q := s.conn.Save(&org) if q.Error != nil { return params.Organization{}, errors.Wrap(q.Error, "saving org") @@ -169,6 +174,7 @@ func (s *sqlDatabase) CreateOrganizationPool(ctx context.Context, orgID string, Enabled: param.Enabled, RunnerBootstrapTimeout: param.RunnerBootstrapTimeout, GitHubRunnerGroup: param.GitHubRunnerGroup, + Priority: param.Priority, } if len(param.ExtraSpecs) > 0 { diff --git a/database/sql/organizations_test.go b/database/sql/organizations_test.go index 50e46191..db4f8ccd 100644 --- a/database/sql/organizations_test.go +++ b/database/sql/organizations_test.go @@ -85,6 +85,7 @@ func (s *OrgTestSuite) SetupTest() { fmt.Sprintf("test-org-%d", i), fmt.Sprintf("test-creds-%d", i), fmt.Sprintf("test-webhook-secret-%d", i), + params.PoolBalancerTypeRoundRobin, ) if err != nil { s.FailNow(fmt.Sprintf("failed to create database object (test-org-%d)", i)) @@ -162,7 +163,8 @@ func (s *OrgTestSuite) TestCreateOrganization() { context.Background(), s.Fixtures.CreateOrgParams.Name, s.Fixtures.CreateOrgParams.CredentialsName, - s.Fixtures.CreateOrgParams.WebhookSecret) + s.Fixtures.CreateOrgParams.WebhookSecret, + params.PoolBalancerTypeRoundRobin) // assertions s.Require().Nil(err) @@ -192,7 +194,8 @@ func (s *OrgTestSuite) TestCreateOrganizationInvalidDBPassphrase() { context.Background(), s.Fixtures.CreateOrgParams.Name, s.Fixtures.CreateOrgParams.CredentialsName, - s.Fixtures.CreateOrgParams.WebhookSecret) + s.Fixtures.CreateOrgParams.WebhookSecret, + params.PoolBalancerTypeRoundRobin) s.Require().NotNil(err) s.Require().Equal("failed to encrypt string", err.Error()) @@ -209,7 +212,8 @@ func (s *OrgTestSuite) TestCreateOrganizationDBCreateErr() { context.Background(), s.Fixtures.CreateOrgParams.Name, s.Fixtures.CreateOrgParams.CredentialsName, - s.Fixtures.CreateOrgParams.WebhookSecret) + s.Fixtures.CreateOrgParams.WebhookSecret, + params.PoolBalancerTypeRoundRobin) s.assertSQLMockExpectations() s.Require().NotNil(err) diff --git a/database/sql/pools.go b/database/sql/pools.go index 9d6737e6..f38eb7d0 100644 --- a/database/sql/pools.go +++ b/database/sql/pools.go @@ -190,7 +190,9 @@ func (s *sqlDatabase) findPoolByTags(id string, poolType params.PoolType, tags [ Group("pools.id"). Preload("Tags"). Having("count(1) = ?", len(tags)). - Where(where, tags, u).Find(&pools) + Where(where, tags, u). + Order("priority desc"). + Find(&pools) if q.Error != nil { if errors.Is(q.Error, gorm.ErrRecordNotFound) { diff --git a/database/sql/pools_test.go b/database/sql/pools_test.go index c9a26fe0..33fe8725 100644 --- a/database/sql/pools_test.go +++ b/database/sql/pools_test.go @@ -61,7 +61,7 @@ func (s *PoolsTestSuite) SetupTest() { s.Store = db // create an organization for testing purposes - org, err := s.Store.CreateOrganization(context.Background(), "test-org", "test-creds", "test-webhookSecret") + org, err := s.Store.CreateOrganization(context.Background(), "test-org", "test-creds", "test-webhookSecret", params.PoolBalancerTypeRoundRobin) if err != nil { s.FailNow(fmt.Sprintf("failed to create org: %s", err)) } @@ -128,7 +128,7 @@ func (s *PoolsTestSuite) TestListAllPools() { func (s *PoolsTestSuite) TestListAllPoolsDBFetchErr() { s.Fixtures.SQLMock. - ExpectQuery(regexp.QuoteMeta("SELECT `pools`.`id`,`pools`.`created_at`,`pools`.`updated_at`,`pools`.`deleted_at`,`pools`.`provider_name`,`pools`.`runner_prefix`,`pools`.`max_runners`,`pools`.`min_idle_runners`,`pools`.`runner_bootstrap_timeout`,`pools`.`image`,`pools`.`flavor`,`pools`.`os_type`,`pools`.`os_arch`,`pools`.`enabled`,`pools`.`git_hub_runner_group`,`pools`.`repo_id`,`pools`.`org_id`,`pools`.`enterprise_id` FROM `pools` WHERE `pools`.`deleted_at` IS NULL")). + ExpectQuery(regexp.QuoteMeta("SELECT `pools`.`id`,`pools`.`created_at`,`pools`.`updated_at`,`pools`.`deleted_at`,`pools`.`provider_name`,`pools`.`runner_prefix`,`pools`.`max_runners`,`pools`.`min_idle_runners`,`pools`.`runner_bootstrap_timeout`,`pools`.`image`,`pools`.`flavor`,`pools`.`os_type`,`pools`.`os_arch`,`pools`.`enabled`,`pools`.`git_hub_runner_group`,`pools`.`repo_id`,`pools`.`org_id`,`pools`.`enterprise_id`,`pools`.`priority` FROM `pools` WHERE `pools`.`deleted_at` IS NULL")). WillReturnError(fmt.Errorf("mocked fetching all pools error")) _, err := s.StoreSQLMocked.ListAllPools(context.Background()) diff --git a/database/sql/repositories.go b/database/sql/repositories.go index e88c4b9b..cff10f8a 100644 --- a/database/sql/repositories.go +++ b/database/sql/repositories.go @@ -28,7 +28,7 @@ import ( "github.com/cloudbase/garm/params" ) -func (s *sqlDatabase) CreateRepository(_ context.Context, owner, name, credentialsName, webhookSecret string) (params.Repository, error) { +func (s *sqlDatabase) CreateRepository(_ context.Context, owner, name, credentialsName, webhookSecret string, poolBalancerType params.PoolBalancerType) (params.Repository, error) { if webhookSecret == "" { return params.Repository{}, errors.New("creating repo: missing secret") } @@ -37,10 +37,11 @@ func (s *sqlDatabase) CreateRepository(_ context.Context, owner, name, credentia return params.Repository{}, fmt.Errorf("failed to encrypt string") } newRepo := Repository{ - Name: name, - Owner: owner, - WebhookSecret: secret, - CredentialsName: credentialsName, + Name: name, + Owner: owner, + WebhookSecret: secret, + CredentialsName: credentialsName, + PoolBalancerType: poolBalancerType, } q := s.conn.Create(&newRepo) @@ -121,6 +122,10 @@ func (s *sqlDatabase) UpdateRepository(ctx context.Context, repoID string, param repo.WebhookSecret = secret } + if param.PoolBalancerType != "" { + repo.PoolBalancerType = param.PoolBalancerType + } + q := s.conn.Save(&repo) if q.Error != nil { return params.Repository{}, errors.Wrap(q.Error, "saving repo") @@ -169,6 +174,7 @@ func (s *sqlDatabase) CreateRepositoryPool(ctx context.Context, repoID string, p Enabled: param.Enabled, RunnerBootstrapTimeout: param.RunnerBootstrapTimeout, GitHubRunnerGroup: param.GitHubRunnerGroup, + Priority: param.Priority, } if len(param.ExtraSpecs) > 0 { diff --git a/database/sql/repositories_test.go b/database/sql/repositories_test.go index 7f3903b0..796048ea 100644 --- a/database/sql/repositories_test.go +++ b/database/sql/repositories_test.go @@ -96,6 +96,7 @@ func (s *RepoTestSuite) SetupTest() { fmt.Sprintf("test-repo-%d", i), fmt.Sprintf("test-creds-%d", i), fmt.Sprintf("test-webhook-secret-%d", i), + params.PoolBalancerTypeRoundRobin, ) if err != nil { s.FailNow(fmt.Sprintf("failed to create database object (test-repo-%d): %v", i, err)) @@ -176,6 +177,7 @@ func (s *RepoTestSuite) TestCreateRepository() { s.Fixtures.CreateRepoParams.Name, s.Fixtures.CreateRepoParams.CredentialsName, s.Fixtures.CreateRepoParams.WebhookSecret, + params.PoolBalancerTypeRoundRobin, ) // assertions @@ -209,6 +211,7 @@ func (s *RepoTestSuite) TestCreateRepositoryInvalidDBPassphrase() { s.Fixtures.CreateRepoParams.Name, s.Fixtures.CreateRepoParams.CredentialsName, s.Fixtures.CreateRepoParams.WebhookSecret, + params.PoolBalancerTypeRoundRobin, ) s.Require().NotNil(err) @@ -228,6 +231,7 @@ func (s *RepoTestSuite) TestCreateRepositoryInvalidDBCreateErr() { s.Fixtures.CreateRepoParams.Name, s.Fixtures.CreateRepoParams.CredentialsName, s.Fixtures.CreateRepoParams.WebhookSecret, + params.PoolBalancerTypeRoundRobin, ) s.assertSQLMockExpectations() diff --git a/database/sql/util.go b/database/sql/util.go index e9ccc3e5..2dd810f5 100644 --- a/database/sql/util.go +++ b/database/sql/util.go @@ -113,11 +113,16 @@ func (s *sqlDatabase) sqlToCommonOrganization(org Organization) (params.Organiza } ret := params.Organization{ - ID: org.ID.String(), - Name: org.Name, - CredentialsName: org.CredentialsName, - Pools: make([]params.Pool, len(org.Pools)), - WebhookSecret: string(secret), + ID: org.ID.String(), + Name: org.Name, + CredentialsName: org.CredentialsName, + Pools: make([]params.Pool, len(org.Pools)), + WebhookSecret: string(secret), + PoolBalancerType: org.PoolBalancerType, + } + + if ret.PoolBalancerType == "" { + ret.PoolBalancerType = params.PoolBalancerTypeRoundRobin } for idx, pool := range org.Pools { @@ -140,11 +145,16 @@ func (s *sqlDatabase) sqlToCommonEnterprise(enterprise Enterprise) (params.Enter } ret := params.Enterprise{ - ID: enterprise.ID.String(), - Name: enterprise.Name, - CredentialsName: enterprise.CredentialsName, - Pools: make([]params.Pool, len(enterprise.Pools)), - WebhookSecret: string(secret), + ID: enterprise.ID.String(), + Name: enterprise.Name, + CredentialsName: enterprise.CredentialsName, + Pools: make([]params.Pool, len(enterprise.Pools)), + WebhookSecret: string(secret), + PoolBalancerType: enterprise.PoolBalancerType, + } + + if ret.PoolBalancerType == "" { + ret.PoolBalancerType = params.PoolBalancerTypeRoundRobin } for idx, pool := range enterprise.Pools { @@ -176,6 +186,7 @@ func (s *sqlDatabase) sqlToCommonPool(pool Pool) (params.Pool, error) { RunnerBootstrapTimeout: pool.RunnerBootstrapTimeout, ExtraSpecs: json.RawMessage(pool.ExtraSpecs), GitHubRunnerGroup: pool.GitHubRunnerGroup, + Priority: pool.Priority, } if pool.RepoID != nil { @@ -227,12 +238,17 @@ func (s *sqlDatabase) sqlToCommonRepository(repo Repository) (params.Repository, } ret := params.Repository{ - ID: repo.ID.String(), - Name: repo.Name, - Owner: repo.Owner, - CredentialsName: repo.CredentialsName, - Pools: make([]params.Pool, len(repo.Pools)), - WebhookSecret: string(secret), + ID: repo.ID.String(), + Name: repo.Name, + Owner: repo.Owner, + CredentialsName: repo.CredentialsName, + Pools: make([]params.Pool, len(repo.Pools)), + WebhookSecret: string(secret), + PoolBalancerType: repo.PoolBalancerType, + } + + if ret.PoolBalancerType == "" { + ret.PoolBalancerType = params.PoolBalancerTypeRoundRobin } for idx, pool := range repo.Pools { @@ -324,6 +340,10 @@ func (s *sqlDatabase) updatePool(pool Pool, param params.UpdatePoolParams) (para pool.GitHubRunnerGroup = *param.GitHubRunnerGroup } + if param.Priority != nil { + pool.Priority = *param.Priority + } + if q := s.conn.Save(&pool); q.Error != nil { return params.Pool{}, errors.Wrap(q.Error, "saving database entry") } diff --git a/params/params.go b/params/params.go index 133da05b..3bca739e 100644 --- a/params/params.go +++ b/params/params.go @@ -39,6 +39,19 @@ type ( RunnerStatus string WebhookEndpointType string GithubAuthType string + PoolBalancerType string +) + +const ( + // PoolBalancerTypeRoundRobin will try to cycle through the pools of an entity + // in a round robin fashion. For example, if a repository has multiple pools that + // match a certain set of labels, and the entity is configured to use round robin + // balancer, the pool manager will attempt to create instances in each pool in turn + // for each job that needs to be serviced. So job1 in pool1, job2 in pool2 and so on. + PoolBalancerTypeRoundRobin PoolBalancerType = "roundrobin" + // PoolBalancerTypeStack will try to create instances in the first pool that matches + // the required labels. If the pool is full, it will move on to the next pool and so on. + PoolBalancerTypeStack PoolBalancerType = "stack" ) const ( @@ -284,6 +297,11 @@ type Pool struct { // GithubRunnerGroup is the github runner group in which the runners will be added. // The runner group must be created by someone with access to the enterprise. GitHubRunnerGroup string `json:"github-runner-group"` + + // Priority is the priority of the pool. The higher the number, the higher the priority. + // When fetching matching pools for a set of tags, the result will be sorted in descending + // order of priority. + Priority uint `json:"priority"` } func (p Pool) GetID() string { @@ -337,6 +355,7 @@ type Internal struct { // GithubCredentialsDetails contains all info about the credentials, except the // token, which is added above. GithubCredentialsDetails GithubCredentials `json:"gh_creds_details"` + PoolBalancerType PoolBalancerType `json:"pool_balancing_type"` } type Repository struct { @@ -346,6 +365,7 @@ type Repository struct { Pools []Pool `json:"pool,omitempty"` CredentialsName string `json:"credentials_name"` PoolManagerStatus PoolManagerStatus `json:"pool_manager_status,omitempty"` + PoolBalancerType PoolBalancerType `json:"pool_balancing_type"` // Do not serialize sensitive info. WebhookSecret string `json:"-"` } @@ -358,6 +378,13 @@ func (r Repository) GetID() string { return r.ID } +func (r Repository) GetBalancerType() PoolBalancerType { + if r.PoolBalancerType == "" { + return PoolBalancerTypeRoundRobin + } + return r.PoolBalancerType +} + // used by swagger client generated code type Repositories []Repository @@ -367,6 +394,7 @@ type Organization struct { Pools []Pool `json:"pool,omitempty"` CredentialsName string `json:"credentials_name"` PoolManagerStatus PoolManagerStatus `json:"pool_manager_status,omitempty"` + PoolBalancerType PoolBalancerType `json:"pool_balancing_type"` // Do not serialize sensitive info. WebhookSecret string `json:"-"` } @@ -379,6 +407,13 @@ func (o Organization) GetID() string { return o.ID } +func (o Organization) GetBalancerType() PoolBalancerType { + if o.PoolBalancerType == "" { + return PoolBalancerTypeRoundRobin + } + return o.PoolBalancerType +} + // used by swagger client generated code type Organizations []Organization @@ -388,6 +423,7 @@ type Enterprise struct { Pools []Pool `json:"pool,omitempty"` CredentialsName string `json:"credentials_name"` PoolManagerStatus PoolManagerStatus `json:"pool_manager_status,omitempty"` + PoolBalancerType PoolBalancerType `json:"pool_balancing_type"` // Do not serialize sensitive info. WebhookSecret string `json:"-"` } @@ -400,6 +436,13 @@ func (e Enterprise) GetID() string { return e.ID } +func (e Enterprise) GetBalancerType() PoolBalancerType { + if e.PoolBalancerType == "" { + return PoolBalancerTypeRoundRobin + } + return e.PoolBalancerType +} + // used by swagger client generated code type Enterprises []Enterprise diff --git a/params/requests.go b/params/requests.go index 2ad4fc1e..8551d249 100644 --- a/params/requests.go +++ b/params/requests.go @@ -31,10 +31,11 @@ type InstanceRequest struct { } type CreateRepoParams struct { - Owner string `json:"owner"` - Name string `json:"name"` - CredentialsName string `json:"credentials_name"` - WebhookSecret string `json:"webhook_secret"` + Owner string `json:"owner"` + Name string `json:"name"` + CredentialsName string `json:"credentials_name"` + WebhookSecret string `json:"webhook_secret"` + PoolBalancerType PoolBalancerType `json:"pool_balancer_type"` } func (c *CreateRepoParams) Validate() error { @@ -52,13 +53,21 @@ func (c *CreateRepoParams) Validate() error { if c.WebhookSecret == "" { return errors.NewMissingSecretError("missing secret") } + + switch c.PoolBalancerType { + case PoolBalancerTypeRoundRobin, PoolBalancerTypeStack: + default: + return errors.NewBadRequestError("invalid pool balancer type") + } + return nil } type CreateOrgParams struct { - Name string `json:"name"` - CredentialsName string `json:"credentials_name"` - WebhookSecret string `json:"webhook_secret"` + Name string `json:"name"` + CredentialsName string `json:"credentials_name"` + WebhookSecret string `json:"webhook_secret"` + PoolBalancerType PoolBalancerType `json:"pool_balancer_type"` } func (c *CreateOrgParams) Validate() error { @@ -72,13 +81,20 @@ func (c *CreateOrgParams) Validate() error { if c.WebhookSecret == "" { return errors.NewMissingSecretError("missing secret") } + + switch c.PoolBalancerType { + case PoolBalancerTypeRoundRobin, PoolBalancerTypeStack: + default: + return errors.NewBadRequestError("invalid pool balancer type") + } return nil } type CreateEnterpriseParams struct { - Name string `json:"name"` - CredentialsName string `json:"credentials_name"` - WebhookSecret string `json:"webhook_secret"` + Name string `json:"name"` + CredentialsName string `json:"credentials_name"` + WebhookSecret string `json:"webhook_secret"` + PoolBalancerType PoolBalancerType `json:"pool_balancer_type"` } func (c *CreateEnterpriseParams) Validate() error { @@ -91,6 +107,12 @@ func (c *CreateEnterpriseParams) Validate() error { if c.WebhookSecret == "" { return errors.NewMissingSecretError("missing secret") } + + switch c.PoolBalancerType { + case PoolBalancerTypeRoundRobin, PoolBalancerTypeStack: + default: + return errors.NewBadRequestError("invalid pool balancer type") + } return nil } @@ -122,6 +144,7 @@ type UpdatePoolParams struct { // pool will be added to. // The runner group must be created by someone with access to the enterprise. GitHubRunnerGroup *string `json:"github-runner-group,omitempty"` + Priority *uint `json:"priority,omitempty"` } type CreateInstanceParams struct { @@ -159,6 +182,7 @@ type CreatePoolParams struct { // pool will be added to. // The runner group must be created by someone with access to the enterprise. GitHubRunnerGroup string `json:"github-runner-group"` + Priority uint `json:"priority"` } func (p *CreatePoolParams) Validate() error { @@ -231,8 +255,9 @@ func (p PasswordLoginParams) Validate() error { } type UpdateEntityParams struct { - CredentialsName string `json:"credentials_name"` - WebhookSecret string `json:"webhook_secret"` + CredentialsName string `json:"credentials_name"` + WebhookSecret string `json:"webhook_secret"` + PoolBalancerType PoolBalancerType `json:"pool_balancer_type"` } type InstanceUpdateMessage struct { diff --git a/runner/enterprises.go b/runner/enterprises.go index 84278a60..3f7f9f72 100644 --- a/runner/enterprises.go +++ b/runner/enterprises.go @@ -20,6 +20,10 @@ func (r *Runner) CreateEnterprise(ctx context.Context, param params.CreateEnterp return enterprise, runnerErrors.ErrUnauthorized } + if param.PoolBalancerType == "" { + param.PoolBalancerType = params.PoolBalancerTypeRoundRobin + } + err = param.Validate() if err != nil { return params.Enterprise{}, errors.Wrap(err, "validating params") @@ -39,7 +43,7 @@ func (r *Runner) CreateEnterprise(ctx context.Context, param params.CreateEnterp return params.Enterprise{}, runnerErrors.NewConflictError("enterprise %s already exists", param.Name) } - enterprise, err = r.store.CreateEnterprise(ctx, param.Name, creds.Name, param.WebhookSecret) + enterprise, err = r.store.CreateEnterprise(ctx, param.Name, creds.Name, param.WebhookSecret, param.PoolBalancerType) if err != nil { return params.Enterprise{}, errors.Wrap(err, "creating enterprise") } @@ -168,6 +172,12 @@ func (r *Runner) UpdateEnterprise(ctx context.Context, enterpriseID string, para } } + switch param.PoolBalancerType { + case params.PoolBalancerTypeRoundRobin, params.PoolBalancerTypeStack, param.PoolBalancerType: + default: + return params.Enterprise{}, runnerErrors.NewBadRequestError("invalid pool balancer type: %s", param.PoolBalancerType) + } + enterprise, err = r.store.UpdateEnterprise(ctx, enterpriseID, param) if err != nil { return params.Enterprise{}, errors.Wrap(err, "updating enterprise") diff --git a/runner/enterprises_test.go b/runner/enterprises_test.go index 5af95d96..9648e34f 100644 --- a/runner/enterprises_test.go +++ b/runner/enterprises_test.go @@ -78,6 +78,7 @@ func (s *EnterpriseTestSuite) SetupTest() { name, fmt.Sprintf("test-creds-%v", i), fmt.Sprintf("test-webhook-secret-%v", i), + params.PoolBalancerTypeRoundRobin, ) if err != nil { s.FailNow(fmt.Sprintf("failed to create database object (test-enterprise-%v)", i)) diff --git a/runner/organizations.go b/runner/organizations.go index ee863125..abd53f35 100644 --- a/runner/organizations.go +++ b/runner/organizations.go @@ -34,6 +34,10 @@ func (r *Runner) CreateOrganization(ctx context.Context, param params.CreateOrgP return org, runnerErrors.ErrUnauthorized } + if param.PoolBalancerType == "" { + param.PoolBalancerType = params.PoolBalancerTypeRoundRobin + } + if err := param.Validate(); err != nil { return params.Organization{}, errors.Wrap(err, "validating params") } @@ -52,7 +56,7 @@ func (r *Runner) CreateOrganization(ctx context.Context, param params.CreateOrgP return params.Organization{}, runnerErrors.NewConflictError("organization %s already exists", param.Name) } - org, err = r.store.CreateOrganization(ctx, param.Name, creds.Name, param.WebhookSecret) + org, err = r.store.CreateOrganization(ctx, param.Name, creds.Name, param.WebhookSecret, param.PoolBalancerType) if err != nil { return params.Organization{}, errors.Wrap(err, "creating organization") } @@ -197,6 +201,12 @@ func (r *Runner) UpdateOrganization(ctx context.Context, orgID string, param par } } + switch param.PoolBalancerType { + case params.PoolBalancerTypeRoundRobin, params.PoolBalancerTypeStack, param.PoolBalancerType: + default: + return params.Organization{}, runnerErrors.NewBadRequestError("invalid pool balancer type: %s", param.PoolBalancerType) + } + org, err = r.store.UpdateOrganization(ctx, orgID, param) if err != nil { return params.Organization{}, errors.Wrap(err, "updating org") diff --git a/runner/organizations_test.go b/runner/organizations_test.go index e3a43fad..974b2f7c 100644 --- a/runner/organizations_test.go +++ b/runner/organizations_test.go @@ -78,6 +78,7 @@ func (s *OrgTestSuite) SetupTest() { name, fmt.Sprintf("test-creds-%v", i), fmt.Sprintf("test-webhook-secret-%v", i), + params.PoolBalancerTypeRoundRobin, ) if err != nil { s.FailNow(fmt.Sprintf("failed to create database object (test-org-%v)", i)) diff --git a/runner/pool/enterprise.go b/runner/pool/enterprise.go index ef2265a1..a3fcdad6 100644 --- a/runner/pool/enterprise.go +++ b/runner/pool/enterprise.go @@ -77,6 +77,13 @@ type enterprise struct { mux sync.Mutex } +func (e *enterprise) PoolBalancerType() params.PoolBalancerType { + if e.cfgInternal.PoolBalancerType == "" { + return params.PoolBalancerTypeRoundRobin + } + return e.cfgInternal.PoolBalancerType +} + func (e *enterprise) findRunnerGroupByName(name string) (*github.EnterpriseRunnerGroup, error) { // nolint:golangci-lint,godox // TODO(gabriel-samfira): implement caching diff --git a/runner/pool/interfaces.go b/runner/pool/interfaces.go index 058e4cd2..6cdf6fcb 100644 --- a/runner/pool/interfaces.go +++ b/runner/pool/interfaces.go @@ -51,4 +51,5 @@ type poolHelper interface { WebhookSecret() string ID() string PoolType() params.PoolType + PoolBalancerType() params.PoolBalancerType } diff --git a/runner/pool/organization.go b/runner/pool/organization.go index c54cfcb3..a3e465ee 100644 --- a/runner/pool/organization.go +++ b/runner/pool/organization.go @@ -89,6 +89,13 @@ type organization struct { mux sync.Mutex } +func (o *organization) PoolBalancerType() params.PoolBalancerType { + if o.cfgInternal.PoolBalancerType == "" { + return params.PoolBalancerTypeRoundRobin + } + return o.cfgInternal.PoolBalancerType +} + func (o *organization) findRunnerGroupByName(name string) (*github.RunnerGroup, error) { // nolint:golangci-lint,godox // TODO(gabriel-samfira): implement caching diff --git a/runner/pool/pool.go b/runner/pool/pool.go index bd9b3543..c509813d 100644 --- a/runner/pool/pool.go +++ b/runner/pool/pool.go @@ -1764,7 +1764,11 @@ func (r *basePoolManager) consumeQueuedJobs() error { return errors.Wrap(err, "listing queued jobs") } - poolsCache := poolsForTags{} + poolsCache := poolsForTags{ + poolCacheType: r.helper.PoolBalancerType(), + } + + slog.InfoContext(r.ctx, "using pool cache type", "cache_type", poolsCache.poolCacheType) slog.DebugContext( r.ctx, "found queued jobs", diff --git a/runner/pool/repository.go b/runner/pool/repository.go index 4f31cc91..e37ed55b 100644 --- a/runner/pool/repository.go +++ b/runner/pool/repository.go @@ -91,6 +91,13 @@ type repository struct { mux sync.Mutex } +func (r *repository) PoolBalancerType() params.PoolBalancerType { + if r.cfgInternal.PoolBalancerType == "" { + return params.PoolBalancerTypeRoundRobin + } + return r.cfgInternal.PoolBalancerType +} + // nolint:golint,revive // pool is used in enterprise and organzation func (r *repository) GetJITConfig(ctx context.Context, instance string, pool params.Pool, labels []string) (jitConfigMap map[string]string, runner *github.Runner, err error) { diff --git a/runner/pool/util.go b/runner/pool/util.go index b97bf26f..e9fbdf1a 100644 --- a/runner/pool/util.go +++ b/runner/pool/util.go @@ -10,6 +10,12 @@ import ( "github.com/cloudbase/garm/params" ) +type poolCacheStore interface { + Next() (params.Pool, error) + Reset() + Len() int +} + type poolRoundRobin struct { pools []params.Pool next uint32 @@ -33,10 +39,11 @@ func (p *poolRoundRobin) Reset() { } type poolsForTags struct { - pools sync.Map + pools sync.Map + poolCacheType params.PoolBalancerType } -func (p *poolsForTags) Get(tags []string) (*poolRoundRobin, bool) { +func (p *poolsForTags) Get(tags []string) (poolCacheStore, bool) { sort.Strings(tags) key := strings.Join(tags, "^") @@ -44,11 +51,21 @@ func (p *poolsForTags) Get(tags []string) (*poolRoundRobin, bool) { if !ok { return nil, false } - - return v.(*poolRoundRobin), true + poolCache := v.(*poolRoundRobin) + if p.poolCacheType == params.PoolBalancerTypeStack { + // When we service a list of jobs, we want to try each pool in turn + // for each job. Pools are sorted by priority so we always start from the + // highest priority pool and move on to the next if the first one is full. + poolCache.Reset() + } + return poolCache, true } func (p *poolsForTags) Add(tags []string, pools []params.Pool) *poolRoundRobin { + sort.Slice(pools, func(i, j int) bool { + return pools[i].Priority > pools[j].Priority + }) + sort.Strings(tags) key := strings.Join(tags, "^") diff --git a/runner/pools_test.go b/runner/pools_test.go index 8bd33a5e..59d6ff27 100644 --- a/runner/pools_test.go +++ b/runner/pools_test.go @@ -58,7 +58,7 @@ func (s *PoolTestSuite) SetupTest() { } // create an organization for testing purposes - org, err := db.CreateOrganization(context.Background(), "test-org", "test-creds", "test-webhookSecret") + org, err := db.CreateOrganization(context.Background(), "test-org", "test-creds", "test-webhookSecret", params.PoolBalancerTypeRoundRobin) if err != nil { s.FailNow(fmt.Sprintf("failed to create org: %s", err)) } diff --git a/runner/repositories.go b/runner/repositories.go index 297b0f01..669fb323 100644 --- a/runner/repositories.go +++ b/runner/repositories.go @@ -34,6 +34,10 @@ func (r *Runner) CreateRepository(ctx context.Context, param params.CreateRepoPa return repo, runnerErrors.ErrUnauthorized } + if param.PoolBalancerType == "" { + param.PoolBalancerType = params.PoolBalancerTypeRoundRobin + } + if err := param.Validate(); err != nil { return params.Repository{}, errors.Wrap(err, "validating params") } @@ -52,7 +56,7 @@ func (r *Runner) CreateRepository(ctx context.Context, param params.CreateRepoPa return params.Repository{}, runnerErrors.NewConflictError("repository %s/%s already exists", param.Owner, param.Name) } - repo, err = r.store.CreateRepository(ctx, param.Owner, param.Name, creds.Name, param.WebhookSecret) + repo, err = r.store.CreateRepository(ctx, param.Owner, param.Name, creds.Name, param.WebhookSecret, param.PoolBalancerType) if err != nil { return params.Repository{}, errors.Wrap(err, "creating repository") } @@ -196,6 +200,12 @@ func (r *Runner) UpdateRepository(ctx context.Context, repoID string, param para } } + switch param.PoolBalancerType { + case params.PoolBalancerTypeRoundRobin, params.PoolBalancerTypeStack, param.PoolBalancerType: + default: + return params.Repository{}, runnerErrors.NewBadRequestError("invalid pool balancer type: %s", param.PoolBalancerType) + } + repo, err = r.store.UpdateRepository(ctx, repoID, param) if err != nil { return params.Repository{}, errors.Wrap(err, "updating repo") diff --git a/runner/repositories_test.go b/runner/repositories_test.go index 088301d3..12f62478 100644 --- a/runner/repositories_test.go +++ b/runner/repositories_test.go @@ -78,6 +78,7 @@ func (s *RepoTestSuite) SetupTest() { name, fmt.Sprintf("test-creds-%v", i), fmt.Sprintf("test-webhook-secret-%v", i), + params.PoolBalancerTypeRoundRobin, ) if err != nil { s.FailNow(fmt.Sprintf("failed to create database object (test-repo-%v)", i)) diff --git a/runner/runner.go b/runner/runner.go index d24565fd..df31ffa7 100644 --- a/runner/runner.go +++ b/runner/runner.go @@ -105,7 +105,7 @@ func (p *poolManagerCtrl) CreateRepoPoolManager(ctx context.Context, repo params p.mux.Lock() defer p.mux.Unlock() - cfgInternal, err := p.getInternalConfig(ctx, repo.CredentialsName) + cfgInternal, err := p.getInternalConfig(ctx, repo.CredentialsName, repo.GetBalancerType()) if err != nil { return nil, errors.Wrap(err, "fetching internal config") } @@ -126,7 +126,7 @@ func (p *poolManagerCtrl) UpdateRepoPoolManager(ctx context.Context, repo params return nil, errors.Wrapf(runnerErrors.ErrNotFound, "repository %s/%s pool manager not loaded", repo.Owner, repo.Name) } - internalCfg, err := p.getInternalConfig(ctx, repo.CredentialsName) + internalCfg, err := p.getInternalConfig(ctx, repo.CredentialsName, repo.GetBalancerType()) if err != nil { return nil, errors.Wrap(err, "fetching internal config") } @@ -171,7 +171,7 @@ func (p *poolManagerCtrl) CreateOrgPoolManager(ctx context.Context, org params.O p.mux.Lock() defer p.mux.Unlock() - cfgInternal, err := p.getInternalConfig(ctx, org.CredentialsName) + cfgInternal, err := p.getInternalConfig(ctx, org.CredentialsName, org.GetBalancerType()) if err != nil { return nil, errors.Wrap(err, "fetching internal config") } @@ -192,7 +192,7 @@ func (p *poolManagerCtrl) UpdateOrgPoolManager(ctx context.Context, org params.O return nil, errors.Wrapf(runnerErrors.ErrNotFound, "org %s pool manager not loaded", org.Name) } - internalCfg, err := p.getInternalConfig(ctx, org.CredentialsName) + internalCfg, err := p.getInternalConfig(ctx, org.CredentialsName, org.GetBalancerType()) if err != nil { return nil, errors.Wrap(err, "fetching internal config") } @@ -237,7 +237,7 @@ func (p *poolManagerCtrl) CreateEnterprisePoolManager(ctx context.Context, enter p.mux.Lock() defer p.mux.Unlock() - cfgInternal, err := p.getInternalConfig(ctx, enterprise.CredentialsName) + cfgInternal, err := p.getInternalConfig(ctx, enterprise.CredentialsName, enterprise.GetBalancerType()) if err != nil { return nil, errors.Wrap(err, "fetching internal config") } @@ -258,7 +258,7 @@ func (p *poolManagerCtrl) UpdateEnterprisePoolManager(ctx context.Context, enter return nil, errors.Wrapf(runnerErrors.ErrNotFound, "enterprise %s pool manager not loaded", enterprise.Name) } - internalCfg, err := p.getInternalConfig(ctx, enterprise.CredentialsName) + internalCfg, err := p.getInternalConfig(ctx, enterprise.CredentialsName, enterprise.GetBalancerType()) if err != nil { return nil, errors.Wrap(err, "fetching internal config") } @@ -299,7 +299,7 @@ func (p *poolManagerCtrl) GetEnterprisePoolManagers() (map[string]common.PoolMan return p.enterprises, nil } -func (p *poolManagerCtrl) getInternalConfig(ctx context.Context, credsName string) (params.Internal, error) { +func (p *poolManagerCtrl) getInternalConfig(ctx context.Context, credsName string, poolBalancerType params.PoolBalancerType) (params.Internal, error) { creds, ok := p.credentials[credsName] if !ok { return params.Internal{}, runnerErrors.NewBadRequestError("invalid credential name (%s)", credsName) @@ -325,6 +325,7 @@ func (p *poolManagerCtrl) getInternalConfig(ctx context.Context, credsName strin BaseWebhookURL: p.config.Default.WebhookURL, ControllerWebhookURL: controllerWebhookURL, JWTSecret: p.config.JWTAuth.Secret, + PoolBalancerType: poolBalancerType, GithubCredentialsDetails: params.GithubCredentials{ Name: creds.Name, Description: creds.Description, From b58555bc109529b0eee247374c6010bae36cfd47 Mon Sep 17 00:00:00 2001 From: Gabriel Adrian Samfira Date: Thu, 14 Mar 2024 20:19:54 +0000 Subject: [PATCH 2/6] Fix missing info in pool list Without preloading the entity we're listing pools for, we don't get that info when listing pools for a repo/org/enterprise. Signed-off-by: Gabriel Adrian Samfira --- database/sql/enterprise.go | 2 +- database/sql/organizations.go | 2 +- database/sql/repositories.go | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/database/sql/enterprise.go b/database/sql/enterprise.go index dc2629de..274201db 100644 --- a/database/sql/enterprise.go +++ b/database/sql/enterprise.go @@ -239,7 +239,7 @@ func (s *sqlDatabase) FindEnterprisePoolByTags(_ context.Context, enterpriseID s } func (s *sqlDatabase) ListEnterprisePools(ctx context.Context, enterpriseID string) ([]params.Pool, error) { - pools, err := s.listEntityPools(ctx, params.EnterprisePool, enterpriseID, "Tags", "Instances", "Instances.Job") + pools, err := s.listEntityPools(ctx, params.EnterprisePool, enterpriseID, "Tags", "Instances", "Enterprise") if err != nil { return nil, errors.Wrap(err, "fetching pools") } diff --git a/database/sql/organizations.go b/database/sql/organizations.go index 2fd2b5cf..2dee60b4 100644 --- a/database/sql/organizations.go +++ b/database/sql/organizations.go @@ -219,7 +219,7 @@ func (s *sqlDatabase) CreateOrganizationPool(ctx context.Context, orgID string, } func (s *sqlDatabase) ListOrgPools(ctx context.Context, orgID string) ([]params.Pool, error) { - pools, err := s.listEntityPools(ctx, params.OrganizationPool, orgID, "Tags", "Instances") + pools, err := s.listEntityPools(ctx, params.OrganizationPool, orgID, "Tags", "Instances", "Organization") if err != nil { return nil, errors.Wrap(err, "fetching pools") } diff --git a/database/sql/repositories.go b/database/sql/repositories.go index cff10f8a..8131f1f3 100644 --- a/database/sql/repositories.go +++ b/database/sql/repositories.go @@ -219,7 +219,7 @@ func (s *sqlDatabase) CreateRepositoryPool(ctx context.Context, repoID string, p } func (s *sqlDatabase) ListRepoPools(ctx context.Context, repoID string) ([]params.Pool, error) { - pools, err := s.listEntityPools(ctx, params.RepositoryPool, repoID, "Tags", "Instances") + pools, err := s.listEntityPools(ctx, params.RepositoryPool, repoID, "Tags", "Instances", "Repository") if err != nil { return nil, errors.Wrap(err, "fetching pools") } From cdfda0321a3cda7fd4d4d25da24e5b36d20c91e6 Mon Sep 17 00:00:00 2001 From: Gabriel Adrian Samfira Date: Fri, 15 Mar 2024 07:26:04 +0000 Subject: [PATCH 3/6] Fix balancer type validation Signed-off-by: Gabriel Adrian Samfira --- params/params.go | 7 +++++-- params/requests.go | 6 +++--- runner/enterprises.go | 2 +- runner/organizations.go | 2 +- runner/pool/util.go | 2 +- runner/repositories.go | 2 +- 6 files changed, 12 insertions(+), 9 deletions(-) diff --git a/params/params.go b/params/params.go index 3bca739e..b4157206 100644 --- a/params/params.go +++ b/params/params.go @@ -49,9 +49,12 @@ const ( // balancer, the pool manager will attempt to create instances in each pool in turn // for each job that needs to be serviced. So job1 in pool1, job2 in pool2 and so on. PoolBalancerTypeRoundRobin PoolBalancerType = "roundrobin" - // PoolBalancerTypeStack will try to create instances in the first pool that matches + // PoolBalancerTypePack will try to create instances in the first pool that matches // the required labels. If the pool is full, it will move on to the next pool and so on. - PoolBalancerTypeStack PoolBalancerType = "stack" + PoolBalancerTypePack PoolBalancerType = "pack" + // PoolBalancerTypeNone denotes to the default behavior of the pool manager, which is + // to use the round robin balancer. + PoolBalancerTypeNone PoolBalancerType = "" ) const ( diff --git a/params/requests.go b/params/requests.go index 8551d249..81108493 100644 --- a/params/requests.go +++ b/params/requests.go @@ -55,7 +55,7 @@ func (c *CreateRepoParams) Validate() error { } switch c.PoolBalancerType { - case PoolBalancerTypeRoundRobin, PoolBalancerTypeStack: + case PoolBalancerTypeRoundRobin, PoolBalancerTypePack: default: return errors.NewBadRequestError("invalid pool balancer type") } @@ -83,7 +83,7 @@ func (c *CreateOrgParams) Validate() error { } switch c.PoolBalancerType { - case PoolBalancerTypeRoundRobin, PoolBalancerTypeStack: + case PoolBalancerTypeRoundRobin, PoolBalancerTypePack: default: return errors.NewBadRequestError("invalid pool balancer type") } @@ -109,7 +109,7 @@ func (c *CreateEnterpriseParams) Validate() error { } switch c.PoolBalancerType { - case PoolBalancerTypeRoundRobin, PoolBalancerTypeStack: + case PoolBalancerTypeRoundRobin, PoolBalancerTypePack: default: return errors.NewBadRequestError("invalid pool balancer type") } diff --git a/runner/enterprises.go b/runner/enterprises.go index 3f7f9f72..01c5a8d0 100644 --- a/runner/enterprises.go +++ b/runner/enterprises.go @@ -173,7 +173,7 @@ func (r *Runner) UpdateEnterprise(ctx context.Context, enterpriseID string, para } switch param.PoolBalancerType { - case params.PoolBalancerTypeRoundRobin, params.PoolBalancerTypeStack, param.PoolBalancerType: + case params.PoolBalancerTypeRoundRobin, params.PoolBalancerTypePack, params.PoolBalancerTypeNone: default: return params.Enterprise{}, runnerErrors.NewBadRequestError("invalid pool balancer type: %s", param.PoolBalancerType) } diff --git a/runner/organizations.go b/runner/organizations.go index abd53f35..2d837fd0 100644 --- a/runner/organizations.go +++ b/runner/organizations.go @@ -202,7 +202,7 @@ func (r *Runner) UpdateOrganization(ctx context.Context, orgID string, param par } switch param.PoolBalancerType { - case params.PoolBalancerTypeRoundRobin, params.PoolBalancerTypeStack, param.PoolBalancerType: + case params.PoolBalancerTypeRoundRobin, params.PoolBalancerTypePack, params.PoolBalancerTypeNone: default: return params.Organization{}, runnerErrors.NewBadRequestError("invalid pool balancer type: %s", param.PoolBalancerType) } diff --git a/runner/pool/util.go b/runner/pool/util.go index e9fbdf1a..8769fe0b 100644 --- a/runner/pool/util.go +++ b/runner/pool/util.go @@ -52,7 +52,7 @@ func (p *poolsForTags) Get(tags []string) (poolCacheStore, bool) { return nil, false } poolCache := v.(*poolRoundRobin) - if p.poolCacheType == params.PoolBalancerTypeStack { + if p.poolCacheType == params.PoolBalancerTypePack { // When we service a list of jobs, we want to try each pool in turn // for each job. Pools are sorted by priority so we always start from the // highest priority pool and move on to the next if the first one is full. diff --git a/runner/repositories.go b/runner/repositories.go index 669fb323..d5b4db77 100644 --- a/runner/repositories.go +++ b/runner/repositories.go @@ -201,7 +201,7 @@ func (r *Runner) UpdateRepository(ctx context.Context, repoID string, param para } switch param.PoolBalancerType { - case params.PoolBalancerTypeRoundRobin, params.PoolBalancerTypeStack, param.PoolBalancerType: + case params.PoolBalancerTypeRoundRobin, params.PoolBalancerTypePack, params.PoolBalancerTypeNone: default: return params.Repository{}, runnerErrors.NewBadRequestError("invalid pool balancer type: %s", param.PoolBalancerType) } From d7ea80a657da089469f297f22310930a5e3cf3a0 Mon Sep 17 00:00:00 2001 From: Gabriel Adrian Samfira Date: Fri, 15 Mar 2024 08:12:16 +0000 Subject: [PATCH 4/6] Remove log message Signed-off-by: Gabriel Adrian Samfira --- runner/pool/pool.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/runner/pool/pool.go b/runner/pool/pool.go index c509813d..2d52ca02 100644 --- a/runner/pool/pool.go +++ b/runner/pool/pool.go @@ -1768,8 +1768,6 @@ func (r *basePoolManager) consumeQueuedJobs() error { poolCacheType: r.helper.PoolBalancerType(), } - slog.InfoContext(r.ctx, "using pool cache type", "cache_type", poolsCache.poolCacheType) - slog.DebugContext( r.ctx, "found queued jobs", "job_count", len(queued)) From ac29af6eff6721d51d748ab4d8a063802b5a3ff9 Mon Sep 17 00:00:00 2001 From: Gabriel Adrian Samfira Date: Fri, 15 Mar 2024 14:35:05 +0000 Subject: [PATCH 5/6] Add some unit tests Signed-off-by: Gabriel Adrian Samfira --- params/requests.go | 6 +- runner/enterprises.go | 4 - runner/organizations.go | 4 - runner/pool/util.go | 2 +- runner/pool/util_test.go | 228 +++++++++++++++++++++++++++++++++++++++ runner/repositories.go | 4 - 6 files changed, 232 insertions(+), 16 deletions(-) create mode 100644 runner/pool/util_test.go diff --git a/params/requests.go b/params/requests.go index 81108493..885ed678 100644 --- a/params/requests.go +++ b/params/requests.go @@ -55,7 +55,7 @@ func (c *CreateRepoParams) Validate() error { } switch c.PoolBalancerType { - case PoolBalancerTypeRoundRobin, PoolBalancerTypePack: + case PoolBalancerTypeRoundRobin, PoolBalancerTypePack, PoolBalancerTypeNone: default: return errors.NewBadRequestError("invalid pool balancer type") } @@ -83,7 +83,7 @@ func (c *CreateOrgParams) Validate() error { } switch c.PoolBalancerType { - case PoolBalancerTypeRoundRobin, PoolBalancerTypePack: + case PoolBalancerTypeRoundRobin, PoolBalancerTypePack, PoolBalancerTypeNone: default: return errors.NewBadRequestError("invalid pool balancer type") } @@ -109,7 +109,7 @@ func (c *CreateEnterpriseParams) Validate() error { } switch c.PoolBalancerType { - case PoolBalancerTypeRoundRobin, PoolBalancerTypePack: + case PoolBalancerTypeRoundRobin, PoolBalancerTypePack, PoolBalancerTypeNone: default: return errors.NewBadRequestError("invalid pool balancer type") } diff --git a/runner/enterprises.go b/runner/enterprises.go index 01c5a8d0..c76d3973 100644 --- a/runner/enterprises.go +++ b/runner/enterprises.go @@ -20,10 +20,6 @@ func (r *Runner) CreateEnterprise(ctx context.Context, param params.CreateEnterp return enterprise, runnerErrors.ErrUnauthorized } - if param.PoolBalancerType == "" { - param.PoolBalancerType = params.PoolBalancerTypeRoundRobin - } - err = param.Validate() if err != nil { return params.Enterprise{}, errors.Wrap(err, "validating params") diff --git a/runner/organizations.go b/runner/organizations.go index 2d837fd0..3d24dcda 100644 --- a/runner/organizations.go +++ b/runner/organizations.go @@ -34,10 +34,6 @@ func (r *Runner) CreateOrganization(ctx context.Context, param params.CreateOrgP return org, runnerErrors.ErrUnauthorized } - if param.PoolBalancerType == "" { - param.PoolBalancerType = params.PoolBalancerTypeRoundRobin - } - if err := param.Validate(); err != nil { return params.Organization{}, errors.Wrap(err, "validating params") } diff --git a/runner/pool/util.go b/runner/pool/util.go index 8769fe0b..5a3a3c8c 100644 --- a/runner/pool/util.go +++ b/runner/pool/util.go @@ -61,7 +61,7 @@ func (p *poolsForTags) Get(tags []string) (poolCacheStore, bool) { return poolCache, true } -func (p *poolsForTags) Add(tags []string, pools []params.Pool) *poolRoundRobin { +func (p *poolsForTags) Add(tags []string, pools []params.Pool) poolCacheStore { sort.Slice(pools, func(i, j int) bool { return pools[i].Priority > pools[j].Priority }) diff --git a/runner/pool/util_test.go b/runner/pool/util_test.go new file mode 100644 index 00000000..bcfea879 --- /dev/null +++ b/runner/pool/util_test.go @@ -0,0 +1,228 @@ +package pool + +import ( + "sync" + "testing" + + runnerErrors "github.com/cloudbase/garm-provider-common/errors" + "github.com/cloudbase/garm/params" +) + +func TestPoolRoundRobinRollsOver(t *testing.T) { + p := &poolRoundRobin{ + pools: []params.Pool{ + { + ID: "1", + }, + { + ID: "2", + }, + }, + } + + pool, err := p.Next() + if err != nil { + t.Fatalf("unexpected error: %s", err) + } + if pool.ID != "1" { + t.Fatalf("expected pool 1, got %s", pool.ID) + } + + pool, err = p.Next() + if err != nil { + t.Fatalf("unexpected error: %s", err) + } + if pool.ID != "2" { + t.Fatalf("expected pool 2, got %s", pool.ID) + } + + pool, err = p.Next() + if err != nil { + t.Fatalf("unexpected error: %s", err) + } + if pool.ID != "1" { + t.Fatalf("expected pool 1, got %s", pool.ID) + } +} + +func TestPoolRoundRobinEmptyPoolErrorsOut(t *testing.T) { + p := &poolRoundRobin{} + + _, err := p.Next() + if err == nil { + t.Fatalf("expected error, got nil") + } + if err != runnerErrors.ErrNoPoolsAvailable { + t.Fatalf("expected ErrNoPoolsAvailable, got %s", err) + } +} + +func TestPoolRoundRobinLen(t *testing.T) { + p := &poolRoundRobin{ + pools: []params.Pool{ + { + ID: "1", + }, + { + ID: "2", + }, + }, + } + + if p.Len() != 2 { + t.Fatalf("expected 2, got %d", p.Len()) + } +} + +func TestPoolRoundRobinReset(t *testing.T) { + p := &poolRoundRobin{ + pools: []params.Pool{ + { + ID: "1", + }, + { + ID: "2", + }, + }, + } + + p.Next() + p.Reset() + if p.next != 0 { + t.Fatalf("expected 0, got %d", p.next) + } +} + +func TestPoolsForTagsPackGet(t *testing.T) { + p := &poolsForTags{ + poolCacheType: params.PoolBalancerTypePack, + } + + pools := []params.Pool{ + { + ID: "1", + Priority: 0, + }, + { + ID: "2", + Priority: 100, + }, + } + _ = p.Add([]string{"key"}, pools) + cache, ok := p.Get([]string{"key"}) + if !ok { + t.Fatalf("expected true, got false") + } + if cache.Len() != 2 { + t.Fatalf("expected 2, got %d", cache.Len()) + } + + poolRR, ok := cache.(*poolRoundRobin) + if !ok { + t.Fatalf("expected poolRoundRobin, got %v", cache) + } + if poolRR.next != 0 { + t.Fatalf("expected 0, got %d", poolRR.next) + } + pool, err := poolRR.Next() + if err != nil { + t.Fatalf("unexpected error: %s", err) + } + if pool.ID != "2" { + t.Fatalf("expected pool 2, got %s", pool.ID) + } + + if poolRR.next != 1 { + t.Fatalf("expected 1, got %d", poolRR.next) + } + // Getting the pool cache again should reset next + cache, ok = p.Get([]string{"key"}) + if !ok { + t.Fatalf("expected true, got false") + } + poolRR, ok = cache.(*poolRoundRobin) + if !ok { + t.Fatalf("expected poolRoundRobin, got %v", cache) + } + if poolRR.next != 0 { + t.Fatalf("expected 0, got %d", poolRR.next) + } +} + +func TestPoolsForTagsRoundRobinGet(t *testing.T) { + p := &poolsForTags{ + poolCacheType: params.PoolBalancerTypeRoundRobin, + } + + pools := []params.Pool{ + { + ID: "1", + Priority: 0, + }, + { + ID: "2", + Priority: 100, + }, + } + _ = p.Add([]string{"key"}, pools) + cache, ok := p.Get([]string{"key"}) + if !ok { + t.Fatalf("expected true, got false") + } + if cache.Len() != 2 { + t.Fatalf("expected 2, got %d", cache.Len()) + } + + pool, err := cache.Next() + if err != nil { + t.Fatalf("unexpected error: %s", err) + } + if pool.ID != "2" { + t.Fatalf("expected pool 2, got %s", pool.ID) + } + // Getting the pool cache again should not reset next, and + // should return the next pool. + cache, ok = p.Get([]string{"key"}) + if !ok { + t.Fatalf("expected true, got false") + } + pool, err = cache.Next() + if err != nil { + t.Fatalf("unexpected error: %s", err) + } + if pool.ID != "1" { + t.Fatalf("expected pool 1, got %s", pool.ID) + } +} + +func TestPoolsForTagsNoPoolsForTag(t *testing.T) { + p := &poolsForTags{ + pools: sync.Map{}, + } + + _, ok := p.Get([]string{"key"}) + if ok { + t.Fatalf("expected false, got true") + } +} + +func TestPoolsForTagsBalancerTypePack(t *testing.T) { + p := &poolsForTags{ + pools: sync.Map{}, + poolCacheType: params.PoolBalancerTypePack, + } + + poolCache := &poolRoundRobin{} + p.pools.Store("key", poolCache) + + cache, ok := p.Get([]string{"key"}) + if !ok { + t.Fatalf("expected true, got false") + } + if cache != poolCache { + t.Fatalf("expected poolCache, got %v", cache) + } + if poolCache.next != 0 { + t.Fatalf("expected 0, got %d", poolCache.next) + } +} diff --git a/runner/repositories.go b/runner/repositories.go index d5b4db77..c71fab39 100644 --- a/runner/repositories.go +++ b/runner/repositories.go @@ -34,10 +34,6 @@ func (r *Runner) CreateRepository(ctx context.Context, param params.CreateRepoPa return repo, runnerErrors.ErrUnauthorized } - if param.PoolBalancerType == "" { - param.PoolBalancerType = params.PoolBalancerTypeRoundRobin - } - if err := param.Validate(); err != nil { return params.Repository{}, errors.Wrap(err, "validating params") } From 206fe42c734f70edb37629668c61d00280b642e8 Mon Sep 17 00:00:00 2001 From: Gabriel Adrian Samfira Date: Fri, 15 Mar 2024 15:48:53 +0000 Subject: [PATCH 6/6] Remove unused code, update test Signed-off-by: Gabriel Adrian Samfira --- runner/enterprises_test.go | 6 +++++- runner/organizations_test.go | 29 +++++++++++++++++++++++++++ runner/pool/enterprise.go | 12 ----------- runner/pool/interfaces.go | 4 ---- runner/pool/organization.go | 12 ----------- runner/pool/pool.go | 10 ++++++++- runner/pool/repository.go | 12 ----------- runner/repositories_test.go | 39 +++++++++++++++++++++++++++++++++++- 8 files changed, 81 insertions(+), 43 deletions(-) diff --git a/runner/enterprises_test.go b/runner/enterprises_test.go index 9648e34f..311e743a 100644 --- a/runner/enterprises_test.go +++ b/runner/enterprises_test.go @@ -170,6 +170,7 @@ func (s *EnterpriseTestSuite) TestCreateEnterprise() { s.Require().Nil(err) s.Require().Equal(s.Fixtures.CreateEnterpriseParams.Name, enterprise.Name) s.Require().Equal(s.Fixtures.Credentials[s.Fixtures.CreateEnterpriseParams.CredentialsName].Name, enterprise.CredentialsName) + s.Require().Equal(params.PoolBalancerTypeRoundRobin, enterprise.PoolBalancerType) } func (s *EnterpriseTestSuite) TestCreateEnterpriseErrUnauthorized() { @@ -294,13 +295,16 @@ func (s *EnterpriseTestSuite) TestUpdateEnterprise() { s.Fixtures.PoolMgrCtrlMock.On("UpdateEnterprisePoolManager", s.Fixtures.AdminContext, mock.AnythingOfType("params.Enterprise")).Return(s.Fixtures.PoolMgrMock, nil) s.Fixtures.PoolMgrMock.On("Status").Return(params.PoolManagerStatus{IsRunning: true}, nil) - org, err := s.Runner.UpdateEnterprise(s.Fixtures.AdminContext, s.Fixtures.StoreEnterprises["test-enterprise-1"].ID, s.Fixtures.UpdateRepoParams) + param := s.Fixtures.UpdateRepoParams + param.PoolBalancerType = params.PoolBalancerTypePack + org, err := s.Runner.UpdateEnterprise(s.Fixtures.AdminContext, s.Fixtures.StoreEnterprises["test-enterprise-1"].ID, param) s.Fixtures.PoolMgrMock.AssertExpectations(s.T()) s.Fixtures.PoolMgrCtrlMock.AssertExpectations(s.T()) s.Require().Nil(err) s.Require().Equal(s.Fixtures.UpdateRepoParams.CredentialsName, org.CredentialsName) s.Require().Equal(s.Fixtures.UpdateRepoParams.WebhookSecret, org.WebhookSecret) + s.Require().Equal(params.PoolBalancerTypePack, org.PoolBalancerType) } func (s *EnterpriseTestSuite) TestUpdateEnterpriseErrUnauthorized() { diff --git a/runner/organizations_test.go b/runner/organizations_test.go index 974b2f7c..7ebfcff8 100644 --- a/runner/organizations_test.go +++ b/runner/organizations_test.go @@ -170,6 +170,20 @@ func (s *OrgTestSuite) TestCreateOrganization() { s.Require().Nil(err) s.Require().Equal(s.Fixtures.CreateOrgParams.Name, org.Name) s.Require().Equal(s.Fixtures.Credentials[s.Fixtures.CreateOrgParams.CredentialsName].Name, org.CredentialsName) + s.Require().Equal(params.PoolBalancerTypeRoundRobin, org.PoolBalancerType) +} + +func (s *OrgTestSuite) TestCreateOrganizationPoolBalancerTypePack() { + s.Fixtures.CreateOrgParams.PoolBalancerType = params.PoolBalancerTypePack + s.Fixtures.PoolMgrMock.On("Start").Return(nil) + s.Fixtures.PoolMgrCtrlMock.On("CreateOrgPoolManager", s.Fixtures.AdminContext, mock.AnythingOfType("params.Organization"), s.Fixtures.Providers, s.Fixtures.Store).Return(s.Fixtures.PoolMgrMock, nil) + + org, err := s.Runner.CreateOrganization(s.Fixtures.AdminContext, s.Fixtures.CreateOrgParams) + + s.Fixtures.PoolMgrMock.AssertExpectations(s.T()) + s.Fixtures.PoolMgrCtrlMock.AssertExpectations(s.T()) + s.Require().Nil(err) + s.Require().Equal(params.PoolBalancerTypePack, org.PoolBalancerType) } func (s *OrgTestSuite) TestCreateOrganizationErrUnauthorized() { @@ -303,6 +317,21 @@ func (s *OrgTestSuite) TestUpdateOrganization() { s.Require().Equal(s.Fixtures.UpdateRepoParams.WebhookSecret, org.WebhookSecret) } +func (s *OrgTestSuite) TestUpdateRepositoryBalancingType() { + s.Fixtures.UpdateRepoParams.PoolBalancerType = params.PoolBalancerTypePack + s.Fixtures.PoolMgrCtrlMock.On("UpdateOrgPoolManager", s.Fixtures.AdminContext, mock.AnythingOfType("params.Organization")).Return(s.Fixtures.PoolMgrMock, nil) + s.Fixtures.PoolMgrMock.On("Status").Return(params.PoolManagerStatus{IsRunning: true}, nil) + + param := s.Fixtures.UpdateRepoParams + param.PoolBalancerType = params.PoolBalancerTypePack + org, err := s.Runner.UpdateOrganization(s.Fixtures.AdminContext, s.Fixtures.StoreOrgs["test-org-1"].ID, param) + + s.Fixtures.PoolMgrMock.AssertExpectations(s.T()) + s.Fixtures.PoolMgrCtrlMock.AssertExpectations(s.T()) + s.Require().Nil(err) + s.Require().Equal(params.PoolBalancerTypePack, org.PoolBalancerType) +} + func (s *OrgTestSuite) TestUpdateOrganizationErrUnauthorized() { _, err := s.Runner.UpdateOrganization(context.Background(), "dummy-org-id", s.Fixtures.UpdateRepoParams) diff --git a/runner/pool/enterprise.go b/runner/pool/enterprise.go index a3fcdad6..24685fcb 100644 --- a/runner/pool/enterprise.go +++ b/runner/pool/enterprise.go @@ -190,10 +190,6 @@ func (e *enterprise) GetJITConfig(ctx context.Context, instance string, pool par return ret, jitConfig.Runner, nil } -func (e *enterprise) GithubCLI() common.GithubClient { - return e.ghcli -} - func (e *enterprise) PoolType() params.PoolType { return params.EnterprisePool } @@ -370,14 +366,6 @@ func (e *enterprise) WebhookSecret() string { return e.cfg.WebhookSecret } -func (e *enterprise) FindPoolByTags(labels []string) (params.Pool, error) { - pool, err := e.store.FindEnterprisePoolByTags(e.ctx, e.id, labels) - if err != nil { - return params.Pool{}, errors.Wrap(err, "fetching suitable pool") - } - return pool, nil -} - func (e *enterprise) GetPoolByID(poolID string) (params.Pool, error) { pool, err := e.store.GetEnterprisePool(e.ctx, e.id, poolID) if err != nil { diff --git a/runner/pool/interfaces.go b/runner/pool/interfaces.go index 6cdf6fcb..3815a3ac 100644 --- a/runner/pool/interfaces.go +++ b/runner/pool/interfaces.go @@ -21,7 +21,6 @@ import ( commonParams "github.com/cloudbase/garm-provider-common/params" "github.com/cloudbase/garm/params" - "github.com/cloudbase/garm/runner/common" ) type poolHelper interface { @@ -35,8 +34,6 @@ type poolHelper interface { UninstallHook(ctx context.Context, url string) error GetHookInfo(ctx context.Context) (params.HookInfo, error) - GithubCLI() common.GithubClient - GetJITConfig(ctx context.Context, instanceName string, pool params.Pool, labels []string) (map[string]string, *github.Runner, error) FetchDbInstances() ([]params.Instance, error) @@ -44,7 +41,6 @@ type poolHelper interface { GithubURL() string JwtToken() string String() string - FindPoolByTags(labels []string) (params.Pool, error) GetPoolByID(poolID string) (params.Pool, error) ValidateOwner(job params.WorkflowJob) error UpdateState(param params.UpdatePoolStateParams) error diff --git a/runner/pool/organization.go b/runner/pool/organization.go index a3e465ee..65448953 100644 --- a/runner/pool/organization.go +++ b/runner/pool/organization.go @@ -202,10 +202,6 @@ func (o *organization) GetJITConfig(ctx context.Context, instance string, pool p return ret, runner, nil } -func (o *organization) GithubCLI() common.GithubClient { - return o.ghcli -} - func (o *organization) PoolType() params.PoolType { return params.OrganizationPool } @@ -384,14 +380,6 @@ func (o *organization) WebhookSecret() string { return o.cfg.WebhookSecret } -func (o *organization) FindPoolByTags(labels []string) (params.Pool, error) { - pool, err := o.store.FindOrganizationPoolByTags(o.ctx, o.id, labels) - if err != nil { - return params.Pool{}, errors.Wrap(err, "fetching suitable pool") - } - return pool, nil -} - func (o *organization) GetPoolByID(poolID string) (params.Pool, error) { pool, err := o.store.GetOrganizationPool(o.ctx, o.id, poolID) if err != nil { diff --git a/runner/pool/pool.go b/runner/pool/pool.go index 2d52ca02..f3b21852 100644 --- a/runner/pool/pool.go +++ b/runner/pool/pool.go @@ -1653,7 +1653,15 @@ func (r *basePoolManager) Stop() error { } func (r *basePoolManager) RefreshState(param params.UpdatePoolStateParams) error { - return r.helper.UpdateState(param) + if err := r.helper.UpdateState(param); err != nil { + return fmt.Errorf("failed to update pool state: %w", err) + } + // Update the tools as soon as state is updated. This should revive a stopped pool manager + // or stop one if the supplied credentials are not okay. + if err := r.updateTools(); err != nil { + return fmt.Errorf("failed to update tools: %w", err) + } + return nil } func (r *basePoolManager) WebhookSecret() string { diff --git a/runner/pool/repository.go b/runner/pool/repository.go index e37ed55b..3383aacf 100644 --- a/runner/pool/repository.go +++ b/runner/pool/repository.go @@ -161,10 +161,6 @@ func (r *repository) GetJITConfig(ctx context.Context, instance string, pool par return ret, runner, nil } -func (r *repository) GithubCLI() common.GithubClient { - return r.ghcli -} - func (r *repository) PoolType() params.PoolType { return params.RepositoryPool } @@ -341,14 +337,6 @@ func (r *repository) WebhookSecret() string { return r.cfg.WebhookSecret } -func (r *repository) FindPoolByTags(labels []string) (params.Pool, error) { - pool, err := r.store.FindRepositoryPoolByTags(r.ctx, r.id, labels) - if err != nil { - return params.Pool{}, errors.Wrap(err, "fetching suitable pool") - } - return pool, nil -} - func (r *repository) GetPoolByID(poolID string) (params.Pool, error) { pool, err := r.store.GetRepositoryPool(r.ctx, r.id, poolID) if err != nil { diff --git a/runner/repositories_test.go b/runner/repositories_test.go index 12f62478..8a1e8d9c 100644 --- a/runner/repositories_test.go +++ b/runner/repositories_test.go @@ -171,6 +171,27 @@ func (s *RepoTestSuite) TestCreateRepository() { s.Require().Equal(s.Fixtures.CreateRepoParams.Owner, repo.Owner) s.Require().Equal(s.Fixtures.CreateRepoParams.Name, repo.Name) s.Require().Equal(s.Fixtures.Credentials[s.Fixtures.CreateRepoParams.CredentialsName].Name, repo.CredentialsName) + s.Require().Equal(params.PoolBalancerTypeRoundRobin, repo.PoolBalancerType) +} + +func (s *RepoTestSuite) TestCreareRepositoryPoolBalancerTypePack() { + // setup mocks expectations + s.Fixtures.PoolMgrMock.On("Start").Return(nil) + s.Fixtures.PoolMgrCtrlMock.On("CreateRepoPoolManager", s.Fixtures.AdminContext, mock.AnythingOfType("params.Repository"), s.Fixtures.Providers, s.Fixtures.Store).Return(s.Fixtures.PoolMgrMock, nil) + + // call tested function + param := s.Fixtures.CreateRepoParams + param.PoolBalancerType = params.PoolBalancerTypePack + repo, err := s.Runner.CreateRepository(s.Fixtures.AdminContext, param) + + // assertions + s.Fixtures.PoolMgrMock.AssertExpectations(s.T()) + s.Fixtures.PoolMgrCtrlMock.AssertExpectations(s.T()) + s.Require().Nil(err) + s.Require().Equal(param.Owner, repo.Owner) + s.Require().Equal(param.Name, repo.Name) + s.Require().Equal(s.Fixtures.Credentials[s.Fixtures.CreateRepoParams.CredentialsName].Name, repo.CredentialsName) + s.Require().Equal(params.PoolBalancerTypePack, repo.PoolBalancerType) } func (s *RepoTestSuite) TestCreateRepositoryErrUnauthorized() { @@ -304,11 +325,27 @@ func (s *RepoTestSuite) TestUpdateRepository() { s.Require().Nil(err) s.Require().Equal(s.Fixtures.UpdateRepoParams.CredentialsName, repo.CredentialsName) s.Require().Equal(s.Fixtures.UpdateRepoParams.WebhookSecret, repo.WebhookSecret) + s.Require().Equal(params.PoolBalancerTypeRoundRobin, repo.PoolBalancerType) +} + +func (s *RepoTestSuite) TestUpdateRepositoryBalancingType() { + s.Fixtures.PoolMgrCtrlMock.On("UpdateRepoPoolManager", s.Fixtures.AdminContext, mock.AnythingOfType("params.Repository")).Return(s.Fixtures.PoolMgrMock, nil) + s.Fixtures.PoolMgrMock.On("Status").Return(params.PoolManagerStatus{IsRunning: true}, nil) + + updateRepoParams := s.Fixtures.UpdateRepoParams + updateRepoParams.PoolBalancerType = params.PoolBalancerTypePack + repo, err := s.Runner.UpdateRepository(s.Fixtures.AdminContext, s.Fixtures.StoreRepos["test-repo-1"].ID, updateRepoParams) + + s.Fixtures.PoolMgrCtrlMock.AssertExpectations(s.T()) + s.Fixtures.PoolMgrMock.AssertExpectations(s.T()) + s.Require().Nil(err) + s.Require().Equal(updateRepoParams.CredentialsName, repo.CredentialsName) + s.Require().Equal(updateRepoParams.WebhookSecret, repo.WebhookSecret) + s.Require().Equal(params.PoolBalancerTypePack, repo.PoolBalancerType) } func (s *RepoTestSuite) TestUpdateRepositoryErrUnauthorized() { _, err := s.Runner.UpdateRepository(context.Background(), "dummy-repo-id", s.Fixtures.UpdateRepoParams) - s.Require().Equal(runnerErrors.ErrUnauthorized, err) }