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

Trim gha-runner-scale-set to gha-rs in names and remove role type suffixes #2706

Merged
merged 6 commits into from
Aug 9, 2023

Conversation

nikola-jokic
Copy link
Member

@nikola-jokic nikola-jokic commented Jun 28, 2023

Fixes #2697

This PR tries to mitigate the problem when service account names are used as labels. The suffix being added by supported charts can be too long (the -gha-runner-scale-set suffix takes 21 characters).

Summary:

  • All suffixes with gha-runner-scale-set are replaced with gha-rs
  • Resource types are removed from the name suffix (for example -role)

Change

By trimming down the suffix, we can allow longer names for clusters with such limitations.

gha-runner-scale-set-controller

Templates Resource Kind Name Reserved Length Description Notes
deployment.yaml Deployment [installation name]-gha-rs-controller 18 The resource running controller-manager The pods created by this resource will have replica set suffix and the pod suffix
serviceaccount.yaml ServiceAccount [installation name]-gha-rs-controller 18 Created if serviceAccount.create in values.yaml is set to true The name can be customized in values.yaml
manager_cluster_role.yaml ClusterRole [installation name]-gha-rs-controller 18 ClusterRole for the controller manager Created if the value of flags.watchSingleNamespace is empty
manager_cluster_role_binding.yaml ClusterRoleBinding [installation name]-gha-rs-controller 18 ClusterRoleBinding for the controller manager Created if the value of flags.watchSingleNamespace is empty
manager_single_namespace_controller_role.yaml Role [installation name]-gha-rs-controller-single-namespace 35 Role for the controller manager Created if the value of flags.watchSingleNamespace is set
manager_single_namespace_controller_role_binding.yaml RoleBinding [installation name]-gha-rs-controller-single-namespace 35 RoleBinding for the controller manager Created if the value of flags.watchSingleNamespace is set
manager_single_namespace_watch_role.yaml Role [installation name]-gha-rs-controller-single-namespace-watch 41 Role for the controller manager for the namespace configured Created if the value of flags.watchSingleNamespace is set
manager_single_namespace_watch_role_binding.yaml RoleBinding [installation name]-gha-rs-controller-single-namespace-watch 41 RoleBinding for the controller manager for the namespace configured Created if the value of flags.watchSingleNamespace is set
manager_listener_role.yaml Role [installation name]-gha-rs-controller-listener 26 Role for the listener Always created
manager_listener_role_binding.yaml RoleBinding [installation name]-gha-rs-controller-listener 26 RoleBinding for the listener Always created binding listener role with the service account (either created by serviceaccount.yaml or configured with values.yaml)

gha-runner-scale-set

Templates Resource Kind Name Reserved Length Description Notes
autoscalingrunnerset.yaml AutoscalingRunnerSet [installation name] 0 Top level resource working with scale sets The name is limited to 45 characters in length
no_permission_service_account.yaml ServiceAccount [installation name]-gha-rs-no-permission 21 Service account mounted to the runner container. Created if container mode is not "kubernetes" and template.spec.serviceAccountName is not specified
githubsecret.yaml Secret [installation name]-gha-rs-github-secret 20 Secret containing values needed to authenticate to the GitHub API Created if githubConfigSecret is an object. If string is provided, this secret will not be created.
manager_role.yaml Role [installation name]-gha-rs-manager 15 Role provided to the manager to be able to reconcile on resources in the autoscaling runner set's namespace Always created
manager_role_binding.yaml RoleBinding [installation name]-gha-rs-manager 15 Binding manager_role to the manager service account. Always created
kube_mode_role.yaml Role [installation name]-gha-rs-kube-mode 17 Role providing necessary permissions for the hook Created when container mode is set to "kubernetes" and template.spec.serviceAccount is not provided.
kube_mode_serviceaccount.yaml ServiceAccount [installation name]-gha-rs-kube-mode 17 Service account bound to the runner pod. Created when container mode is set to "kubernetes" and template.spec.serviceAccount is not provided.

Note

This can potentially break the external tooling targeting resources by names and labels.

@nikola-jokic nikola-jokic requested review from mumoshu, toast-gear and a team as code owners June 28, 2023 12:25
@nikola-jokic nikola-jokic added the gha-runner-scale-set Related to the gha-runner-scale-set mode label Jun 30, 2023
@Link- Link- self-requested a review July 3, 2023 08:22
@Link-
Copy link
Collaborator

Link- commented Jul 3, 2023

Todo (before merge)

  • Update PR description to provide more context on the problem and how this change fixes it
  • Update docs.github.com with a table with all the resource we create and their fixed names
  • Add a note about this potentially breaking change to our release notes

Table to fill

Chart Resource Kind Name Reserved Length Description Notes

Link-
Link- previously requested changes Jul 3, 2023
Copy link
Collaborator

@Link- Link- left a comment

Choose a reason for hiding this comment

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

Thank you @nikola-jokic - technically this looks good, we need to make sure to address the list of TODOs I posted above before merging this

@nikola-jokic nikola-jokic added this to the 0.5.0 milestone Jul 3, 2023
@nikola-jokic nikola-jokic merged commit 6a75bc0 into master Aug 9, 2023
16 checks passed
@nikola-jokic nikola-jokic deleted the nikola-jokic/short-naming branch August 9, 2023 09:11
@davhdavh
Copy link

I think you failed to take into account how helm does naming in the default templates...
If the installation name is the same as the given name, it will not double include it...
Since the new name is now different than the helm name, the end result is double naming...
IE, the new name after upgrading to 0.5.0 is gha-runner-scale-set-controller-gha-rs-controller

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.

ServiceAccount name for Runner Scale Set is too long and breaks Cillium CNI
4 participants