Skip to content

Conversation

@nikola-jokic
Copy link
Collaborator

@nikola-jokic nikola-jokic commented Nov 3, 2025

GitHub URL, namespace and runner group should provide a unique hash part of the name. Since all listeners go into a single namespace, if the scale set is named the same belonging to a different runner group, the collisions could occur.
This change uses combination of the namespace, GitHub URL, and the runner group along with the name should provide a unique combination especially during testing canary release in the same namespace.

Fixes #3587

Copilot AI review requested due to automatic review settings November 3, 2025 12:02
@github-actions
Copy link
Contributor

github-actions bot commented Nov 3, 2025

Hello! Thank you for your contribution.

Please review our contribution guidelines to understand the project's testing and code conventions.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR refactors resource naming functions to improve uniqueness by incorporating additional attributes into hash generation. The changes aim to prevent naming collisions when multiple AutoscalingRunnerSets in different contexts share the same name.

  • Introduced hashSuffix helper function that combines namespace, runner group, and config URL for hash generation
  • Updated scaleSetListenerName to use the enhanced hash suffix
  • Simplified proxyListenerSecretName to derive name from listener name rather than constructing it independently

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

namespaceHash = namespaceHash[:8]
}
return fmt.Sprintf("%v-%v-listener-proxy", autoscalingListener.Spec.AutoscalingRunnerSetName, namespaceHash)
return autoscalingListener.Name + "-proxy"
Copy link

Copilot AI Nov 3, 2025

Choose a reason for hiding this comment

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

The proxy secret naming has changed from being based on AutoscalingRunnerSetName to being based on autoscalingListener.Name. This is a breaking change that will cause existing proxy secrets to be orphaned and new ones to be created with different names. Consider implementing a migration strategy or documenting this breaking change for users upgrading.

Copilot uses AI. Check for mistakes.
func scaleSetListenerName(autoscalingRunnerSet *v1alpha1.AutoscalingRunnerSet) string {
namespaceHash := hash.FNVHashString(autoscalingRunnerSet.Namespace)
func hashSuffix(namespace, runnerGroup, configURL string) string {
namespaceHash := hash.FNVHashString(namespace + "@" + runnerGroup + "@" + configURL)
Copy link

Copilot AI Nov 3, 2025

Choose a reason for hiding this comment

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

The hash input format uses '@' as a delimiter between components. If any of these fields (namespace, runnerGroup, or configURL) can contain '@' characters, this could lead to hash collisions. Consider using a more robust delimiter strategy or including field separators that cannot appear in the input values.

Suggested change
namespaceHash := hash.FNVHashString(namespace + "@" + runnerGroup + "@" + configURL)
data, err := json.Marshal([]string{namespace, runnerGroup, configURL})
if err != nil {
panic(fmt.Sprintf("failed to marshal hash input: %v", err))
}
namespaceHash := hash.FNVHashString(string(data))

Copilot uses AI. Check for mistakes.
@nikola-jokic nikola-jokic changed the title Nikola jokic/hash name collision change Use combination of namespace, GitHub URL, and runner group when hashing the listener name Nov 3, 2025
@nikola-jokic nikola-jokic added the gha-runner-scale-set Related to the gha-runner-scale-set mode label Nov 3, 2025
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.

Listener pod names conflict when using the same runnerScaleSetName in multiple orgs.

2 participants