From 1d14a26325f7533879248e271e1191ebad34ba74 Mon Sep 17 00:00:00 2001 From: Mario Constanti Date: Tue, 21 May 2024 07:27:07 +0200 Subject: [PATCH 1/5] feat: garm pools do not force default labels Signed-off-by: Mario Constanti --- database/sql/enterprise_test.go | 2 +- database/sql/instances_test.go | 2 +- database/sql/organizations_test.go | 2 +- database/sql/pools_test.go | 2 +- database/sql/repositories_test.go | 2 +- runner/enterprises_test.go | 2 +- runner/organizations_test.go | 2 +- runner/pools.go | 8 ++---- runner/pools_test.go | 11 +++++++- runner/repositories_test.go | 4 +-- runner/runner.go | 40 ------------------------------ 11 files changed, 21 insertions(+), 56 deletions(-) diff --git a/database/sql/enterprise_test.go b/database/sql/enterprise_test.go index 4c51d2ac..9e1a86dd 100644 --- a/database/sql/enterprise_test.go +++ b/database/sql/enterprise_test.go @@ -152,7 +152,7 @@ func (s *EnterpriseTestSuite) SetupTest() { Flavor: "test-flavor", OSType: "linux", OSArch: "amd64", - Tags: []string{"self-hosted", "arm64", "linux"}, + Tags: []string{"amd64-linux-runner"}, }, CreateInstanceParams: params.CreateInstanceParams{ Name: "test-instance-name", diff --git a/database/sql/instances_test.go b/database/sql/instances_test.go index 82b5cb46..de37033d 100644 --- a/database/sql/instances_test.go +++ b/database/sql/instances_test.go @@ -97,7 +97,7 @@ func (s *InstancesTestSuite) SetupTest() { Image: "test-image", Flavor: "test-flavor", OSType: "linux", - Tags: []string{"self-hosted", "amd64", "linux"}, + Tags: []string{"amd64", "linux"}, } entity, err := org.GetEntity() s.Require().Nil(err) diff --git a/database/sql/organizations_test.go b/database/sql/organizations_test.go index 049a061e..11be72d0 100644 --- a/database/sql/organizations_test.go +++ b/database/sql/organizations_test.go @@ -153,7 +153,7 @@ func (s *OrgTestSuite) SetupTest() { Flavor: "test-flavor", OSType: "linux", OSArch: "amd64", - Tags: []string{"self-hosted", "arm64", "linux"}, + Tags: []string{"amd64-linux-runner"}, }, CreateInstanceParams: params.CreateInstanceParams{ Name: "test-instance-name", diff --git a/database/sql/pools_test.go b/database/sql/pools_test.go index 97dbdf71..e6cf7f4a 100644 --- a/database/sql/pools_test.go +++ b/database/sql/pools_test.go @@ -88,7 +88,7 @@ func (s *PoolsTestSuite) SetupTest() { Image: fmt.Sprintf("test-image-%d", i), Flavor: "test-flavor", OSType: "linux", - Tags: []string{"self-hosted", "amd64", "linux"}, + Tags: []string{"amd64-linux-runner"}, }, ) if err != nil { diff --git a/database/sql/repositories_test.go b/database/sql/repositories_test.go index a588263b..6eb20a2c 100644 --- a/database/sql/repositories_test.go +++ b/database/sql/repositories_test.go @@ -165,7 +165,7 @@ func (s *RepoTestSuite) SetupTest() { Flavor: "test-flavor", OSType: "windows", OSArch: "amd64", - Tags: []string{"self-hosted", "arm64", "windows"}, + Tags: []string{"arm64-windows-runner"}, }, CreateInstanceParams: params.CreateInstanceParams{ Name: "test-instance", diff --git a/runner/enterprises_test.go b/runner/enterprises_test.go index 501d96a8..22946ae6 100644 --- a/runner/enterprises_test.go +++ b/runner/enterprises_test.go @@ -121,7 +121,7 @@ func (s *EnterpriseTestSuite) SetupTest() { Flavor: "test", OSType: "linux", OSArch: "arm64", - Tags: []string{"self-hosted", "arm64", "linux"}, + Tags: []string{"arm64-linux-runner"}, RunnerBootstrapTimeout: 0, }, CreateInstanceParams: params.CreateInstanceParams{ diff --git a/runner/organizations_test.go b/runner/organizations_test.go index 7954d2e7..4d439b76 100644 --- a/runner/organizations_test.go +++ b/runner/organizations_test.go @@ -122,7 +122,7 @@ func (s *OrgTestSuite) SetupTest() { Flavor: "test", OSType: "linux", OSArch: "arm64", - Tags: []string{"self-hosted", "arm64", "linux"}, + Tags: []string{"arm64-linux-runner"}, RunnerBootstrapTimeout: 0, }, CreateInstanceParams: params.CreateInstanceParams{ diff --git a/runner/pools.go b/runner/pools.go index aab423ff..7daa3fa5 100644 --- a/runner/pools.go +++ b/runner/pools.go @@ -99,12 +99,8 @@ func (r *Runner) UpdatePoolByID(ctx context.Context, poolID string, param params return params.Pool{}, runnerErrors.NewBadRequestError("min_idle_runners cannot be larger than max_runners") } - if param.Tags != nil && len(param.Tags) > 0 { - newTags, err := r.processTags(string(pool.OSArch), pool.OSType, param.Tags) - if err != nil { - return params.Pool{}, errors.Wrap(err, "processing tags") - } - param.Tags = newTags + if len(param.Tags) == 0 { + return params.Pool{}, runnerErrors.NewBadRequestError("tags cannot be empty") } entity, err := pool.GithubEntity() diff --git a/runner/pools_test.go b/runner/pools_test.go index 49ca5a5c..08bcfbb8 100644 --- a/runner/pools_test.go +++ b/runner/pools_test.go @@ -91,7 +91,7 @@ func (s *PoolTestSuite) SetupTest() { Image: fmt.Sprintf("test-image-%d", i), Flavor: "test-flavor", OSType: "linux", - Tags: []string{"self-hosted", "amd64", "linux"}, + Tags: []string{"amd64-linux-runner"}, RunnerBootstrapTimeout: 0, }, ) @@ -113,6 +113,9 @@ func (s *PoolTestSuite) SetupTest() { MinIdleRunners: &minIdleRunners, Image: "test-images-updated", Flavor: "test-flavor-updated", + Tags: []string{ + "amd64-linux-runner", + }, }, CreateInstanceParams: params.CreateInstanceParams{ Name: "test-instance-name", @@ -199,8 +202,14 @@ func (s *PoolTestSuite) TestDeletePoolByIDRunnersFailed() { func (s *PoolTestSuite) TestUpdatePoolByID() { pool, err := s.Runner.UpdatePoolByID(s.Fixtures.AdminContext, s.Fixtures.Pools[0].ID, s.Fixtures.UpdatePoolParams) + var tags []string + for _, tag := range pool.Tags { + tags = append(tags, tag.Name) + } + s.Require().Nil(err) s.Require().Equal(*s.Fixtures.UpdatePoolParams.MaxRunners, pool.MaxRunners) + s.Require().Equal(s.Fixtures.UpdatePoolParams.Tags, tags) s.Require().Equal(*s.Fixtures.UpdatePoolParams.MinIdleRunners, pool.MinIdleRunners) s.Require().Equal(s.Fixtures.UpdatePoolParams.Image, pool.Image) s.Require().Equal(s.Fixtures.UpdatePoolParams.Flavor, pool.Flavor) diff --git a/runner/repositories_test.go b/runner/repositories_test.go index 717e795d..d0a6ab61 100644 --- a/runner/repositories_test.go +++ b/runner/repositories_test.go @@ -121,7 +121,7 @@ func (s *RepoTestSuite) SetupTest() { Flavor: "test", OSType: "linux", OSArch: "arm64", - Tags: []string{"self-hosted", "arm64", "linux"}, + Tags: []string{"arm64-linux-runner"}, RunnerBootstrapTimeout: 0, }, CreateInstanceParams: params.CreateInstanceParams{ @@ -177,7 +177,7 @@ func (s *RepoTestSuite) TestCreateRepository() { s.Require().Equal(params.PoolBalancerTypeRoundRobin, repo.PoolBalancerType) } -func (s *RepoTestSuite) TestCreareRepositoryPoolBalancerTypePack() { +func (s *RepoTestSuite) TestCreateRepositoryPoolBalancerTypePack() { // 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) diff --git a/runner/runner.go b/runner/runner.go index 9873695d..6f37c55f 100644 --- a/runner/runner.go +++ b/runner/runner.go @@ -786,49 +786,9 @@ func (r *Runner) appendTagsToCreatePoolParams(param params.CreatePoolParams) (pa return params.CreatePoolParams{}, runnerErrors.NewBadRequestError("no such provider %s", param.ProviderName) } - newTags, err := r.processTags(string(param.OSArch), param.OSType, param.Tags) - if err != nil { - return params.CreatePoolParams{}, fmt.Errorf("failed to process tags: %w", err) - } - - param.Tags = newTags - return param, nil } -func (r *Runner) processTags(osArch string, osType commonParams.OSType, tags []string) ([]string, error) { - // github automatically adds the "self-hosted" tag as well as the OS type (linux, windows, etc) - // and architecture (arm, x64, etc) to all self hosted runners. When a workflow job comes in, we try - // to find a pool based on the labels that are set in the workflow. If we don't explicitly define these - // default tags for each pool, and the user targets these labels, we won't be able to match any pools. - // The downside is that all pools with the same OS and arch will have these default labels. Users should - // set distinct and unique labels on each pool, and explicitly target those labels, or risk assigning - // the job to the wrong worker type. - ghArch, err := util.ResolveToGithubArch(osArch) - if err != nil { - return nil, errors.Wrap(err, "invalid arch") - } - - ghOSType, err := util.ResolveToGithubTag(osType) - if err != nil { - return nil, errors.Wrap(err, "invalid os type") - } - - labels := []string{ - "self-hosted", - ghArch, - ghOSType, - } - - for _, val := range tags { - if val != "self-hosted" && val != ghArch && val != ghOSType { - labels = append(labels, val) - } - } - - return labels, nil -} - func (r *Runner) GetInstance(ctx context.Context, instanceName string) (params.Instance, error) { if !auth.IsAdmin(ctx) { return params.Instance{}, runnerErrors.ErrUnauthorized From 40cdf5b639fdc06e4c6aecd17a3902fde12f3f90 Mon Sep 17 00:00:00 2001 From: Mario Constanti Date: Tue, 21 May 2024 11:25:13 +0200 Subject: [PATCH 2/5] doc: remove self-hosted label from docs Signed-off-by: Mario Constanti --- doc/external_provider.md | 3 --- doc/quickstart.md | 14 +++++++------- doc/using_garm.md | 18 +++++++++--------- 3 files changed, 16 insertions(+), 19 deletions(-) diff --git a/doc/external_provider.md b/doc/external_provider.md index 5c544324..0b40257f 100644 --- a/doc/external_provider.md +++ b/doc/external_provider.md @@ -188,9 +188,6 @@ Here is a sample of that: "image": "8ed8a690-69b6-49eb-982f-dcb466895e2d", "labels": [ "ubuntu", - "self-hosted", - "x64", - "linux", "openstack", "runner-controller-id:f9286791-1589-4f39-a106-5b68c2a18af4", "runner-pool-id:9dcf590a-1192-4a9c-b3e4-e0902974c2c0" diff --git a/doc/quickstart.md b/doc/quickstart.md index 18e214d6..fac82a55 100644 --- a/doc/quickstart.md +++ b/doc/quickstart.md @@ -521,7 +521,7 @@ gabriel@rossak:~$ garm-cli pool add \ | Max Runners | 5 | | Min Idle Runners | 0 | | Runner Bootstrap Timeout | 20 | -| Tags | self-hosted, amd64, Linux, ubuntu, generic | +| Tags | ubuntu, generic | | Belongs to | gsamfira/scripts | | Level | repo | | Enabled | true | @@ -535,11 +535,11 @@ If we list the pool we should see it: ```bash gabriel@rock:~$ garm-cli pool ls -a -+--------------------------------------+---------------------------+--------------+-----------------------------------------+------------------+-------+---------+---------------+----------+ -| ID | IMAGE | FLAVOR | TAGS | BELONGS TO | LEVEL | ENABLED | RUNNER PREFIX | PRIORITY | -+--------------------------------------+---------------------------+--------------+-----------------------------------------+------------------+-------+---------+---------------+----------+ -| 344e4a72-2035-4a18-a3d5-87bd3874b56c | ubuntu:22.04 | default | self-hosted amd64 Linux ubuntu generic | gsamfira/scripts | repo | true | garm | 0 | -+--------------------------------------+---------------------------+--------------+-----------------------------------------+------------------+-------+---------+---------------+----------+ ++--------------------------------------+---------------------------+--------------+-----------------+------------------+-------+---------+---------------+----------+ +| ID | IMAGE | FLAVOR | TAGS | BELONGS TO | LEVEL | ENABLED | RUNNER PREFIX | PRIORITY | ++--------------------------------------+---------------------------+--------------+-----------------+------------------+-------+---------+---------------+----------+ +| 344e4a72-2035-4a18-a3d5-87bd3874b56c | ubuntu:22.04 | default | ubuntu generic | gsamfira/scripts | repo | true | garm | 0 | ++--------------------------------------+---------------------------+--------------+-----------------+------------------+-------+---------+---------------+----------+ ``` This pool is enabled, but the `min-idle-runners` option is set to 0. This means that it will not create any lingering runners. It will only create runners when a job is started. If your provider is slow to boot up new instances, you may want to set this to a value higher than 0. @@ -573,7 +573,7 @@ gabriel@rossak:~$ garm-cli pool update 344e4a72-2035-4a18-a3d5-87bd3874b56c --mi | Max Runners | 5 | | Min Idle Runners | 1 | | Runner Bootstrap Timeout | 20 | -| Tags | self-hosted, amd64, Linux, ubuntu, generic | +| Tags | ubuntu, generic | | Belongs to | gsamfira/scripts | | Level | repo | | Enabled | true | diff --git a/doc/using_garm.md b/doc/using_garm.md index b65d7679..564a8bf8 100644 --- a/doc/using_garm.md +++ b/doc/using_garm.md @@ -296,7 +296,7 @@ garm-cli pool add \ | Max Runners | 5 | | Min Idle Runners | 1 | | Runner Bootstrap Timeout | 20 | -| Tags | self-hosted, x64, Linux, ubuntu, incus | +| Tags | ubuntu, incus | | Belongs to | gabriel-samfira/garm | | Level | repo | | Enabled | false | @@ -328,11 +328,11 @@ To list pools created for a repository you can run: ```bash ubuntu@garm:~$ garm-cli pool list --repo=be3a0673-56af-4395-9ebf-4521fea67567 -+--------------------------------------+---------------------------+---------+------------------------------------+------------+-------+---------+---------------+ -| ID | IMAGE | FLAVOR | TAGS | BELONGS TO | LEVEL | ENABLED | RUNNER PREFIX | -+--------------------------------------+---------------------------+---------+------------------------------------+------------+-------+---------+---------------+ -| 9daa34aa-a08a-4f29-a782-f54950d8521a | images:ubuntu/22.04/cloud | default | self-hosted x64 Linux ubuntu incus | | | false | garm | -+--------------------------------------+---------------------------+---------+------------------------------------+------------+-------+---------+---------------+ ++--------------------------------------+---------------------------+---------+--------------+------------+-------+---------+---------------+ +| ID | IMAGE | FLAVOR | TAGS | BELONGS TO | LEVEL | ENABLED | RUNNER PREFIX | ++--------------------------------------+---------------------------+---------+--------------+------------+-------+---------+---------------+ +| 9daa34aa-a08a-4f29-a782-f54950d8521a | images:ubuntu/22.04/cloud | default | ubuntu incus | | | false | garm | ++--------------------------------------+---------------------------+---------+--------------+------------+-------+---------+---------------+ ``` If you want to list pools for an organization or enterprise, you can use the `--org` or `--enterprise` options respectively. @@ -355,7 +355,7 @@ ubuntu@garm:~$ garm-cli pool show 9daa34aa-a08a-4f29-a782-f54950d8521a | Max Runners | 5 | | Min Idle Runners | 1 | | Runner Bootstrap Timeout | 20 | -| Tags | self-hosted, x64, Linux, ubuntu, incus | +| Tags | ubuntu, incus | | Belongs to | gabriel-samfira/garm | | Level | repo | | Enabled | false | @@ -385,7 +385,7 @@ ubuntu@garm:~$ garm-cli pool update 9daa34aa-a08a-4f29-a782-f54950d8521a --enabl | Max Runners | 5 | | Min Idle Runners | 1 | | Runner Bootstrap Timeout | 20 | -| Tags | self-hosted, x64, Linux, ubuntu, incus | +| Tags | ubuntu, incus | | Belongs to | gabriel-samfira/garm | | Level | repo | | Enabled | false | @@ -419,7 +419,7 @@ ubuntu@garm:~$ garm-cli pool update 9daa34aa-a08a-4f29-a782-f54950d8521a --enabl | Max Runners | 5 | | Min Idle Runners | 1 | | Runner Bootstrap Timeout | 20 | -| Tags | self-hosted, x64, Linux, ubuntu, incus | +| Tags | ubuntu, incus | | Belongs to | gabriel-samfira/garm | | Level | repo | | Enabled | true | From c4f023b6a80afdf7ef93bd4f8e36f3e4e8c3a805 Mon Sep 17 00:00:00 2001 From: Mario Constanti Date: Tue, 21 May 2024 12:06:54 +0200 Subject: [PATCH 3/5] doc: add some notes about default labels Signed-off-by: Mario Constanti --- doc/labels.md | 15 +++++++++++++++ 1 file changed, 15 insertions(+) create mode 100644 doc/labels.md diff --git a/doc/labels.md b/doc/labels.md new file mode 100644 index 00000000..04165c8c --- /dev/null +++ b/doc/labels.md @@ -0,0 +1,15 @@ +# Labels and Tags + +Github Runners can be tagged with labels. These labels can be used to restrict the jobs that can run on a runner. For example, you can have a runner with the label `linux` and another with the label `windows`. You can then restrict a job to run only on a runner with the label `linux`. + +Whenever a new runner register themselves on Github, the runner knows its own labels as the labels are defined in the pool specification as tags. + +Beside the custom labels, Github also has some predefined labels that are appended by the runner script per default. +These are: +```yaml +[ 'self-hosted', '$OS_TYPE', '$OS_ARCH' ] +``` + +With Version **TBD** of `garm-provider-common`, the runner script will register themselves with a new command line flag, called `--no-default-labels`. If this flag is set, the runner will not append any default label. + +As all labels can be defined in the pool specification, it's still possible to add the default labels manually. From 27e081eb36d0b5e6561842252b63038a8281ee1f Mon Sep 17 00:00:00 2001 From: Mario Constanti Date: Wed, 22 May 2024 06:03:06 +0200 Subject: [PATCH 4/5] remove required tags during update Signed-off-by: Mario Constanti --- runner/pools.go | 4 ---- runner/pools_test.go | 6 ------ 2 files changed, 10 deletions(-) diff --git a/runner/pools.go b/runner/pools.go index 7daa3fa5..f2eb3c25 100644 --- a/runner/pools.go +++ b/runner/pools.go @@ -99,10 +99,6 @@ func (r *Runner) UpdatePoolByID(ctx context.Context, poolID string, param params return params.Pool{}, runnerErrors.NewBadRequestError("min_idle_runners cannot be larger than max_runners") } - if len(param.Tags) == 0 { - return params.Pool{}, runnerErrors.NewBadRequestError("tags cannot be empty") - } - entity, err := pool.GithubEntity() if err != nil { return params.Pool{}, errors.Wrap(err, "getting entity") diff --git a/runner/pools_test.go b/runner/pools_test.go index 08bcfbb8..918598d1 100644 --- a/runner/pools_test.go +++ b/runner/pools_test.go @@ -202,14 +202,8 @@ func (s *PoolTestSuite) TestDeletePoolByIDRunnersFailed() { func (s *PoolTestSuite) TestUpdatePoolByID() { pool, err := s.Runner.UpdatePoolByID(s.Fixtures.AdminContext, s.Fixtures.Pools[0].ID, s.Fixtures.UpdatePoolParams) - var tags []string - for _, tag := range pool.Tags { - tags = append(tags, tag.Name) - } - s.Require().Nil(err) s.Require().Equal(*s.Fixtures.UpdatePoolParams.MaxRunners, pool.MaxRunners) - s.Require().Equal(s.Fixtures.UpdatePoolParams.Tags, tags) s.Require().Equal(*s.Fixtures.UpdatePoolParams.MinIdleRunners, pool.MinIdleRunners) s.Require().Equal(s.Fixtures.UpdatePoolParams.Image, pool.Image) s.Require().Equal(s.Fixtures.UpdatePoolParams.Flavor, pool.Flavor) From debb9696ce984de718a7b76451821ce6cda7c5a5 Mon Sep 17 00:00:00 2001 From: Mario Constanti Date: Wed, 22 May 2024 06:04:22 +0200 Subject: [PATCH 5/5] docs: document released provider-common version Signed-off-by: Mario Constanti --- doc/labels.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/labels.md b/doc/labels.md index 04165c8c..2eb92dd4 100644 --- a/doc/labels.md +++ b/doc/labels.md @@ -10,6 +10,6 @@ These are: [ 'self-hosted', '$OS_TYPE', '$OS_ARCH' ] ``` -With Version **TBD** of `garm-provider-common`, the runner script will register themselves with a new command line flag, called `--no-default-labels`. If this flag is set, the runner will not append any default label. +With Version `v0.1.2` of `garm-provider-common`, the runner script will register themselves with a new command line flag, called `--no-default-labels`. If this flag is set, the runner will not append any default label. As all labels can be defined in the pool specification, it's still possible to add the default labels manually.