Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

propogate arbitrary labels from runnersets to all created resources #3157

Merged
merged 1 commit into from
Apr 23, 2024

Conversation

Lazyshot
Copy link
Contributor

@Lazyshot Lazyshot commented Dec 15, 2023

Within our organization we have policies in place that require all resources to have specific labels for security and cost attribution purposes (e.g. team, business unit, etc.).

In order to support such a requirement, this change would have any arbitrary labels that are set at the
AutoscalingRunnerSet level also be assigned to any subsequent resources the controller will create.

Similar behavior could be valuable for annotations as well, but I personally do not have any need for it.

@Lazyshot Lazyshot changed the title propogate of arbitrary labels from runnersets to all created resources propogate arbitrary labels from runnersets to all created resources Jan 18, 2024
@nikola-jokic nikola-jokic added the gha-runner-scale-set Related to the gha-runner-scale-set mode label Mar 19, 2024
@MartinLesterSynamedia
Copy link

This is a really important feature for us.
We have many VMs added as runners right now and we want to migrate them to use this system.
There are 100s of jobs that reference our enterprise runners using the same tag so we want this tag on all available runners.
Further we want to be able to create variations e.g. Enterprise, small and Enterprise, large
Being restricted to 1 label, that is also the name of the scale set seems really limiting.

Please approve this.

@@ -741,3 +737,15 @@ func trimLabelValue(val string) string {
}
return val
}

func mergeLabels(labels ...map[string]string) map[string]string {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we do something like:

func mergeLabels(base, overwrites map[string]string) map[string]string

It would be much easier to understand by reading the function definition.

Copy link
Contributor

@nikola-jokic nikola-jokic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a suggestion. Please fix merge conflict ☺️

@Lazyshot
Copy link
Contributor Author

This is a really important feature for us. We have many VMs added as runners right now and we want to migrate them to use this system. There are 100s of jobs that reference our enterprise runners using the same tag so we want this tag on all available runners. Further we want to be able to create variations e.g. Enterprise, small and Enterprise, large Being restricted to 1 label, that is also the name of the scale set seems really limiting.

Please approve this.

To be clear this is not intended to touch runner labels, but instead only the kubernetes resource labels.

@MartinLesterSynamedia
Copy link

This is a really important feature for us. We have many VMs added as runners ...

To be clear this is not intended to touch runner labels, but instead only the kubernetes resource labels.

Dang. Am I missing something?
Currently our runners have runs-on lines like [ Enterprise, ubuntu, docker, docker-compose ] so their job gets allocated to a VM with the correct capabilities. As far as I can tell there is no way to get an ephemeral runner to pick up a job with a runs-on with more than 1 label. The best I could do is create a new scale set for each possible label set e.g. Enterprise-Ubuntu, Enterprise-Ubuntu-Docker, etc. I couldn't even find a way to add labels to a scale set or it's runners in the UI.

Sadly this requires all our engineers update their jobs to be able to use the new ephemeral runners and I wont be able silently replace our VMs with ephemeral

Copy link
Contributor

@nikola-jokic nikola-jokic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@nikola-jokic nikola-jokic merged commit 109750f into actions:master Apr 23, 2024
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
gha-runner-scale-set Related to the gha-runner-scale-set mode
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants