feat(runners): add support to disable default labels (Linux) #3491

jgutierrezglez · 2023-09-21T10:27:29Z

In case the pool of runners deployed using this module are fully available for an wide GH org (no repository restrictions) any workflow configured to run in runners that contain just default labels in the runs-on definition:

e.g.

runs-on: self-hosted
runs-on: Linux

can end-up running in this pool without knowing it. That's why I have decided to remove the default labels from our runners and just rely on unique custom labels, and I believed the best way to do it by adding a runner_enable_default_labels variable - that by default is true (so, it doesn't change the current behavior), but it can help other people to deal with similar issues like the one described above.

npalm · 2023-09-21T12:36:26Z

I think I missed GitHub changes this. In the past the default labels are added by GitHub without control. When we introduced the JIT config the default labels are not added by default. But is this the same for non JIT?

jgutierrezglez · 2023-10-04T12:44:25Z

I think I missed GitHub changes this. In the past the default labels are added by GitHub without control. When we introduced the JIT config the default labels are not added by default. But is this the same for non JIT?

We're using JIT - and the default labels are added by default, that's why we have created this new variable and set it to false in the cases where we just want to use custom labels

npalm

Need to find some time to test.

Changes impacted the runner module should also reflected to the multi-runner module. See comment
I think this option requires that JIT config is enabled. Can you hadd this to the docs of the introduced variable.

main.tf

sdarwin · 2023-11-01T23:25:35Z

For the reasons explained in the original post, this feature would be great, so you can enable runners without repository restrictions.

runs-on: self-hosted won't accidentally trigger a launch if only custom labels have been added.

Maybe I am mistaken (and I might be!) but here are a few quick comments:

This appears to be the pull request adding the feature: Add --no-default-labels config option to self-hosted runners actions/runner#2443
It doesn't mention JIT config. Isn't JIT completely orthogonal to default labels? Unrelated. So remove JIT from this PR.
The feature uses the flag --no-default-labels. Where is the flag --no-default-labels in this PR?

github-actions · 2023-12-02T01:47:12Z

This pull request has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed if no further activity occurs. Thank you for your contributions.

npalm · 2023-12-08T08:08:02Z

Sorry we had no time yet to dig in this PR. I do my best to check the PR in the coming week. Sorry for the delay.

github-actions · 2024-02-11T01:47:28Z

This pull request has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed if no further activity occurs. Thank you for your contributions.

npalm

@jgutierrezglez thanks for this great PR. I have tested several cases

Default runner - works, requires a suggested change on defaults
Multi runner (linux)
Multi runner (linux) - with JIT
Multi runner (windows)

For the multi runner I needed to make the follwoing adjusments:

mutli-runner module runners.tf

Pass parametr into runner module:

runner_disable_default_labels        = each.value.runner_config.runner_disable_default_labels

and avoid setting defaults if labels are disabled.

  runner_labels                        = each.value.runner_config.runner_disable_default_labels ?  sort(distinct(each.value.runner_config.runner_extra_labels)) :  sort(distinct(concat(["self-hosted", each.value.runner_config.runner_os, each.value.runner_config.runner_architecture], each.value.runner_config.runner_extra_labels)))

I would say the PR is getting close to merge. To get it faster passed you could split the PR in two parts. First the linux part adding the feature, next another PR to fix the windows part.

variables.tf

modules/runners/templates/start-runner.ps1

Co-authored-by: Niek Palm <npalm@users.noreply.github.com>

… between linux and windows changes

jgutierrezglez · 2024-11-15T12:45:12Z

@npalm #4261 PR to address the Windows fix

sdarwin · 2024-11-15T12:59:35Z

@jgutierrezglez , this is a question rather than definitely something needs to change, but why does it say $${extra_flags} instead of ${extra_flags} in start-runner.sh? What effect does that have? isn't extra_flags a regular variable within the bash script that would have one $?

jgutierrezglez · 2024-11-15T13:17:20Z

@jgutierrezglez , this is a question rather than definitely something needs to change, but why does it say $${extra_flags} instead of ${extra_flags} in start-runner.sh? What effect does that have? isn't extra_flags a regular variable within the bash script that would have one $?

Thanks for the catch, there is no real reason to use $${extra_flags} and I just corrected

modules/runners/templates/start-runner.sh

npalm

Made two tiny documentation suggestions. Rest looks good and tested for linux (amazon and ubuntu).

Maybe worth to add a small section in the docs to disuss the choice betwoon using default labels and disable them.

Besides those small notes good to go 🚀

variables.tf

modules/multi-runner/variables.tf

Co-authored-by: Niek Palm <npalm@users.noreply.github.com>

jgutierrezglez · 2024-11-18T05:48:34Z

Made two tiny documentation suggestions. Rest looks good and tested for linux (amazon and ubuntu).

Maybe worth to add a small section in the docs to disuss the choice betwoon using default labels and disable them.

Besides those small notes good to go 🚀

I added a small note under "Configuration considerations" - let me know what you think about it

npalm

Tested and all good, one tiny change requested

main.tf

Co-authored-by: Niek Palm <npalm@users.noreply.github.com>

@@ Description Adding support for no default labels on Windows, see #3491

@jgutierrezglez

🤖 I have created a release *beep* *boop* --- ## [5.20.0](v5.19.0...v5.20.0) (2024-11-19) ### Features * **runners:** add support to disable default labels (Linux) ([#3491](#3491)) ([772e1a5](772e1a5)) @jgutierrezglez * **runners:** add support to disable default labels (Windows) ([#4261](#4261)) ([ad9bcc4](ad9bcc4)) @jgutierrezglez ### Bug Fixes * **lambda:** bump cross-spawn from 7.0.3 to 7.0.6 in /lambdas ([#4273](#4273)) ([dcec236](dcec236)) * **lambda:** bump the aws group in /lambdas with 7 updates ([#4266](#4266)) ([849549e](849549e)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: forest-releaser[bot] <80285352+forest-releaser[bot]@users.noreply.github.com>

jgutierrezglez force-pushed the enable_default_labels_option branch from b296740 to eb9a4e6 Compare September 21, 2023 10:30

jgutierrezglez changed the base branch from develop to main September 21, 2023 10:32

npalm self-requested a review September 21, 2023 12:36

jgutierrezglez changed the title ~~feat: Adding runner_enable_default_labels variable~~ feat: Adding the possibility of removing the default labels Oct 4, 2023

npalm reviewed Oct 5, 2023

View reviewed changes

main.tf Outdated Show resolved Hide resolved

jgutierrezglez force-pushed the enable_default_labels_option branch from 9b858e0 to e945d21 Compare October 9, 2023 11:22

npalm self-requested a review October 14, 2023 08:42

github-actions bot added the Stale label Dec 2, 2023

npalm removed the Stale label Dec 8, 2023

jgutierrezglez force-pushed the enable_default_labels_option branch 2 times, most recently from ed603ef to c0ebeb7 Compare December 11, 2023 16:55

jgutierrezglez marked this pull request as draft December 11, 2023 16:57

jgutierrezglez force-pushed the enable_default_labels_option branch 6 times, most recently from cd60c15 to 78a52a2 Compare December 12, 2023 14:39

jgutierrezglez marked this pull request as ready for review December 12, 2023 14:41

github-actions bot added the Stale label Feb 11, 2024

npalm removed the Stale label Feb 12, 2024

jgutierrezglez force-pushed the enable_default_labels_option branch 3 times, most recently from 5d77fea to 01b73bf Compare March 11, 2024 16:53

npalm requested changes Nov 15, 2024

View reviewed changes

variables.tf Outdated Show resolved Hide resolved

variables.tf Show resolved Hide resolved

modules/runners/templates/start-runner.ps1 Outdated Show resolved Hide resolved

jgutierrezglez and others added 3 commits November 15, 2024 13:27

Update variables.tf

333d134

Co-authored-by: Niek Palm <npalm@users.noreply.github.com>

docs: auto update terraform docs

009c5d2

Revert modules/runners/templates/start-runner.ps1 changes => split PR…

6d90ee4

… between linux and windows changes

jgutierrezglez changed the title ~~feat: Adding the possibility of removing the default labels~~ feat: Adding the possibility of removing the default labels (Linux) Nov 15, 2024

Replacing $${extra_flags} by ${extra_flags}

38dd523

npalm reviewed Nov 15, 2024

View reviewed changes

modules/runners/templates/start-runner.sh Outdated Show resolved Hide resolved

pass labels via multi-runner module

6af69a5

npalm changed the title ~~feat: Adding the possibility of removing the default labels (Linux)~~ feat(runners): add support to disable default labels (Linux) Nov 15, 2024

npalm requested changes Nov 15, 2024

View reviewed changes

variables.tf Outdated Show resolved Hide resolved

modules/multi-runner/variables.tf Outdated Show resolved Hide resolved

jgutierrezglez and others added 5 commits November 18, 2024 06:30

Update variables.tf

cde9b8a

Co-authored-by: Niek Palm <npalm@users.noreply.github.com>

Update modules/multi-runner/variables.tf

d77fa95

Co-authored-by: Niek Palm <npalm@users.noreply.github.com>

docs: auto update terraform docs

901f229

Merge branch 'main' into enable_default_labels_option

c9185d6

Docs: Updating configuration considerations

611fa17

jgutierrezglez force-pushed the enable_default_labels_option branch from 9e14e8b to 611fa17 Compare November 18, 2024 05:47

npalm self-requested a review November 18, 2024 12:09

npalm requested changes Nov 18, 2024

View reviewed changes

main.tf Outdated Show resolved Hide resolved

jgutierrezglez and others added 2 commits November 18, 2024 17:47

Update main.tf

338639a

Co-authored-by: Niek Palm <npalm@users.noreply.github.com>

Merge branch 'main' into enable_default_labels_option

e3f9058

npalm approved these changes Nov 18, 2024

View reviewed changes

npalm merged commit 772e1a5 into philips-labs:main Nov 18, 2024
42 checks passed

forest-releaser bot mentioned this pull request Nov 18, 2024

chore(main): release 5.20.0 #4272

Merged

npalm mentioned this pull request Nov 19, 2024

feat(runners): add support to disable default labels (Windows) #4261

Merged

npalm pushed a commit that referenced this pull request Nov 19, 2024

feat(runners): add support to disable default labels (Windows) (#4261)

ad9bcc4

@@ Description Adding support for no default labels on Windows, see #3491

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(runners): add support to disable default labels (Linux) #3491

feat(runners): add support to disable default labels (Linux) #3491

jgutierrezglez commented Sep 21, 2023

npalm commented Sep 21, 2023

jgutierrezglez commented Oct 4, 2023

npalm left a comment

sdarwin commented Nov 1, 2023

github-actions bot commented Dec 2, 2023

npalm commented Dec 8, 2023

github-actions bot commented Feb 11, 2024

npalm left a comment •

edited

Loading

jgutierrezglez commented Nov 15, 2024

sdarwin commented Nov 15, 2024

jgutierrezglez commented Nov 15, 2024

npalm left a comment

jgutierrezglez commented Nov 18, 2024

npalm left a comment

feat(runners): add support to disable default labels (Linux) #3491

feat(runners): add support to disable default labels (Linux) #3491

Conversation

jgutierrezglez commented Sep 21, 2023

npalm commented Sep 21, 2023

jgutierrezglez commented Oct 4, 2023

npalm left a comment

Choose a reason for hiding this comment

sdarwin commented Nov 1, 2023

github-actions bot commented Dec 2, 2023

npalm commented Dec 8, 2023

github-actions bot commented Feb 11, 2024

npalm left a comment • edited Loading

Choose a reason for hiding this comment

jgutierrezglez commented Nov 15, 2024

sdarwin commented Nov 15, 2024

jgutierrezglez commented Nov 15, 2024

npalm left a comment

Choose a reason for hiding this comment

jgutierrezglez commented Nov 18, 2024

npalm left a comment

Choose a reason for hiding this comment

npalm left a comment •

edited

Loading