public_git
/
actions-runner-controller
mirror of https://github.com/actions-runner-controller/actions-runner-controller.git
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

Refactor resource naming removing unnecessary calculations (#4076)

This commit is contained in:
Nikola Jokic 2025-05-15 10:56:34 +02:00 committed by GitHub
parent 389d842a30
commit 43f1cd0dac
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 24 additions and 129 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

22
controllers/actions.github.com/autoscalinglistener_controller.go
View File

@ -139,9 +139,9 @@ func (r *AutoscalingListenerReconciler) Reconcile(ctx context.Context, req ctrl.
// Create a mirror secret in the same namespace as the AutoscalingListener // Create a mirror secret in the same namespace as the AutoscalingListener
mirrorSecret := new(corev1.Secret) mirrorSecret := new(corev1.Secret)
if err := r.Get(ctx, types.NamespacedName{Namespace: autoscalingListener.Namespace, Name: scaleSetListenerSecretMirrorName(autoscalingListener)}, mirrorSecret); err != nil { if err := r.Get(ctx, types.NamespacedName{Namespace: autoscalingListener.Namespace, Name: autoscalingListener.Name}, mirrorSecret); err != nil {
if !kerrors.IsNotFound(err) { if !kerrors.IsNotFound(err) {
log.Error(err, "Unable to get listener secret mirror", "namespace", autoscalingListener.Namespace, "name", scaleSetListenerSecretMirrorName(autoscalingListener)) log.Error(err, "Unable to get listener secret mirror", "namespace", autoscalingListener.Namespace, "name", autoscalingListener.Name)
return ctrl.Result{}, err return ctrl.Result{}, err
} }
@ -160,9 +160,9 @@ func (r *AutoscalingListenerReconciler) Reconcile(ctx context.Context, req ctrl.
// Make sure the runner scale set listener service account is created for the listener pod in the controller namespace // Make sure the runner scale set listener service account is created for the listener pod in the controller namespace
serviceAccount := new(corev1.ServiceAccount) serviceAccount := new(corev1.ServiceAccount)
if err := r.Get(ctx, types.NamespacedName{Namespace: autoscalingListener.Namespace, Name: scaleSetListenerServiceAccountName(autoscalingListener)}, serviceAccount); err != nil { if err := r.Get(ctx, types.NamespacedName{Namespace: autoscalingListener.Namespace, Name: autoscalingListener.Name}, serviceAccount); err != nil {
if !kerrors.IsNotFound(err) { if !kerrors.IsNotFound(err) {
log.Error(err, "Unable to get listener service accounts", "namespace", autoscalingListener.Namespace, "name", scaleSetListenerServiceAccountName(autoscalingListener)) log.Error(err, "Unable to get listener service accounts", "namespace", autoscalingListener.Namespace, "name", autoscalingListener.Name)
return ctrl.Result{}, err return ctrl.Result{}, err
} }
@ -175,9 +175,9 @@ func (r *AutoscalingListenerReconciler) Reconcile(ctx context.Context, req ctrl.
// Make sure the runner scale set listener role is created in the AutoscalingRunnerSet namespace // Make sure the runner scale set listener role is created in the AutoscalingRunnerSet namespace
listenerRole := new(rbacv1.Role) listenerRole := new(rbacv1.Role)
if err := r.Get(ctx, types.NamespacedName{Namespace: autoscalingListener.Spec.AutoscalingRunnerSetNamespace, Name: scaleSetListenerRoleName(autoscalingListener)}, listenerRole); err != nil { if err := r.Get(ctx, types.NamespacedName{Namespace: autoscalingListener.Spec.AutoscalingRunnerSetNamespace, Name: autoscalingListener.Name}, listenerRole); err != nil {
if !kerrors.IsNotFound(err) { if !kerrors.IsNotFound(err) {
log.Error(err, "Unable to get listener role", "namespace", autoscalingListener.Spec.AutoscalingRunnerSetNamespace, "name", scaleSetListenerRoleName(autoscalingListener)) log.Error(err, "Unable to get listener role", "namespace", autoscalingListener.Spec.AutoscalingRunnerSetNamespace, "name", autoscalingListener.Name)
return ctrl.Result{}, err return ctrl.Result{}, err
} }
@ -197,9 +197,9 @@ func (r *AutoscalingListenerReconciler) Reconcile(ctx context.Context, req ctrl.
// Make sure the runner scale set listener role binding is created // Make sure the runner scale set listener role binding is created
listenerRoleBinding := new(rbacv1.RoleBinding) listenerRoleBinding := new(rbacv1.RoleBinding)
if err := r.Get(ctx, types.NamespacedName{Namespace: autoscalingListener.Spec.AutoscalingRunnerSetNamespace, Name: scaleSetListenerRoleName(autoscalingListener)}, listenerRoleBinding); err != nil { if err := r.Get(ctx, types.NamespacedName{Namespace: autoscalingListener.Spec.AutoscalingRunnerSetNamespace, Name: autoscalingListener.Name}, listenerRoleBinding); err != nil {
if !kerrors.IsNotFound(err) { if !kerrors.IsNotFound(err) {
log.Error(err, "Unable to get listener role binding", "namespace", autoscalingListener.Spec.AutoscalingRunnerSetNamespace, "name", scaleSetListenerRoleName(autoscalingListener)) log.Error(err, "Unable to get listener role binding", "namespace", autoscalingListener.Spec.AutoscalingRunnerSetNamespace, "name", autoscalingListener.Name)
return ctrl.Result{}, err return ctrl.Result{}, err
} }
@ -330,7 +330,7 @@ func (r *AutoscalingListenerReconciler) cleanupResources(ctx context.Context, au
} }
listenerRoleBinding := new(rbacv1.RoleBinding) listenerRoleBinding := new(rbacv1.RoleBinding)
err = r.Get(ctx, types.NamespacedName{Namespace: autoscalingListener.Spec.AutoscalingRunnerSetNamespace, Name: scaleSetListenerRoleName(autoscalingListener)}, listenerRoleBinding) err = r.Get(ctx, types.NamespacedName{Namespace: autoscalingListener.Spec.AutoscalingRunnerSetNamespace, Name: autoscalingListener.Name}, listenerRoleBinding)
switch { switch {
case err == nil: case err == nil:
if listenerRoleBinding.DeletionTimestamp.IsZero() { if listenerRoleBinding.DeletionTimestamp.IsZero() {
@ -346,7 +346,7 @@ func (r *AutoscalingListenerReconciler) cleanupResources(ctx context.Context, au
logger.Info("Listener role binding is deleted") logger.Info("Listener role binding is deleted")
listenerRole := new(rbacv1.Role) listenerRole := new(rbacv1.Role)
err = r.Get(ctx, types.NamespacedName{Namespace: autoscalingListener.Spec.AutoscalingRunnerSetNamespace, Name: scaleSetListenerRoleName(autoscalingListener)}, listenerRole) err = r.Get(ctx, types.NamespacedName{Namespace: autoscalingListener.Spec.AutoscalingRunnerSetNamespace, Name: autoscalingListener.Name}, listenerRole)
switch { switch {
case err == nil: case err == nil:
if listenerRole.DeletionTimestamp.IsZero() { if listenerRole.DeletionTimestamp.IsZero() {
@ -363,7 +363,7 @@ func (r *AutoscalingListenerReconciler) cleanupResources(ctx context.Context, au
logger.Info("Cleaning up the listener service account") logger.Info("Cleaning up the listener service account")
listenerSa := new(corev1.ServiceAccount) listenerSa := new(corev1.ServiceAccount)
err = r.Get(ctx, types.NamespacedName{Name: scaleSetListenerServiceAccountName(autoscalingListener), Namespace: autoscalingListener.Namespace}, listenerSa) err = r.Get(ctx, types.NamespacedName{Name: autoscalingListener.Name, Namespace: autoscalingListener.Namespace}, listenerSa)
switch { switch {
case err == nil: case err == nil:
if listenerSa.DeletionTimestamp.IsZero() { if listenerSa.DeletionTimestamp.IsZero() {

99
controllers/actions.github.com/autoscalinglistener_controller_test.go
View File

@ -134,37 +134,25 @@ var _ = Describe("Test AutoScalingListener controller", func() {
autoscalingListenerTestTimeout, autoscalingListenerTestTimeout,
autoscalingListenerTestInterval).Should(BeEquivalentTo(autoscalingListenerFinalizerName), "AutoScalingListener should have a finalizer") autoscalingListenerTestInterval).Should(BeEquivalentTo(autoscalingListenerFinalizerName), "AutoScalingListener should have a finalizer")
// Check if secret is created
mirrorSecret := new(corev1.Secret)
Eventually(
func() (string, error) {
err := k8sClient.Get(ctx, client.ObjectKey{Name: scaleSetListenerSecretMirrorName(autoscalingListener), Namespace: autoscalingListener.Namespace}, mirrorSecret)
if err != nil {
return "", err
}
return string(mirrorSecret.Data["github_token"]), nil
},
autoscalingListenerTestTimeout,
autoscalingListenerTestInterval).Should(BeEquivalentTo(autoscalingListenerTestGitHubToken), "Mirror secret should be created")
// Check if service account is created // Check if service account is created
serviceAccount := new(corev1.ServiceAccount) serviceAccount := new(corev1.ServiceAccount)
Eventually( Eventually(
func() (string, error) { func() (string, error) {
err := k8sClient.Get(ctx, client.ObjectKey{Name: scaleSetListenerServiceAccountName(autoscalingListener), Namespace: autoscalingListener.Namespace}, serviceAccount) err := k8sClient.Get(ctx, client.ObjectKey{Name: autoscalingListener.Name, Namespace: autoscalingListener.Namespace}, serviceAccount)
if err != nil { if err != nil {
return "", err return "", err
} }
return serviceAccount.Name, nil return serviceAccount.Name, nil
}, },
autoscalingListenerTestTimeout, autoscalingListenerTestTimeout,
autoscalingListenerTestInterval).Should(BeEquivalentTo(scaleSetListenerServiceAccountName(autoscalingListener)), "Service account should be created") autoscalingListenerTestInterval,
).Should(BeEquivalentTo(autoscalingListener.Name), "Service account should be created")
// Check if role is created // Check if role is created
role := new(rbacv1.Role) role := new(rbacv1.Role)
Eventually( Eventually(
func() ([]rbacv1.PolicyRule, error) { func() ([]rbacv1.PolicyRule, error) {
err := k8sClient.Get(ctx, client.ObjectKey{Name: scaleSetListenerRoleName(autoscalingListener), Namespace: autoscalingListener.Spec.AutoscalingRunnerSetNamespace}, role) err := k8sClient.Get(ctx, client.ObjectKey{Name: autoscalingListener.Name, Namespace: autoscalingListener.Spec.AutoscalingRunnerSetNamespace}, role)
if err != nil { if err != nil {
return nil, err return nil, err
} }
@ -178,7 +166,7 @@ var _ = Describe("Test AutoScalingListener controller", func() {
roleBinding := new(rbacv1.RoleBinding) roleBinding := new(rbacv1.RoleBinding)
Eventually( Eventually(
func() (string, error) { func() (string, error) {
err := k8sClient.Get(ctx, client.ObjectKey{Name: scaleSetListenerRoleName(autoscalingListener), Namespace: autoscalingListener.Spec.AutoscalingRunnerSetNamespace}, roleBinding) err := k8sClient.Get(ctx, client.ObjectKey{Name: autoscalingListener.Name, Namespace: autoscalingListener.Spec.AutoscalingRunnerSetNamespace}, roleBinding)
if err != nil { if err != nil {
return "", err return "", err
} }
@ -186,7 +174,7 @@ var _ = Describe("Test AutoScalingListener controller", func() {
return roleBinding.RoleRef.Name, nil return roleBinding.RoleRef.Name, nil
}, },
autoscalingListenerTestTimeout, autoscalingListenerTestTimeout,
autoscalingListenerTestInterval).Should(BeEquivalentTo(scaleSetListenerRoleName(autoscalingListener)), "Rolebinding should be created") autoscalingListenerTestInterval).Should(BeEquivalentTo(autoscalingListener.Name), "Rolebinding should be created")
// Check if pod is created // Check if pod is created
pod := new(corev1.Pod) pod := new(corev1.Pod)
@ -248,7 +236,7 @@ var _ = Describe("Test AutoScalingListener controller", func() {
Eventually( Eventually(
func() bool { func() bool {
roleBinding := new(rbacv1.RoleBinding) roleBinding := new(rbacv1.RoleBinding)
err := k8sClient.Get(ctx, client.ObjectKey{Name: scaleSetListenerRoleName(autoscalingListener), Namespace: autoscalingListener.Spec.AutoscalingRunnerSetNamespace}, roleBinding) err := k8sClient.Get(ctx, client.ObjectKey{Name: autoscalingListener.Name, Namespace: autoscalingListener.Spec.AutoscalingRunnerSetNamespace}, roleBinding)
return kerrors.IsNotFound(err) return kerrors.IsNotFound(err)
}, },
autoscalingListenerTestTimeout, autoscalingListenerTestTimeout,
@ -259,7 +247,7 @@ var _ = Describe("Test AutoScalingListener controller", func() {
Eventually( Eventually(
func() bool { func() bool {
role := new(rbacv1.Role) role := new(rbacv1.Role)
err := k8sClient.Get(ctx, client.ObjectKey{Name: scaleSetListenerRoleName(autoscalingListener), Namespace: autoscalingListener.Spec.AutoscalingRunnerSetNamespace}, role) err := k8sClient.Get(ctx, client.ObjectKey{Name: autoscalingListener.Name, Namespace: autoscalingListener.Spec.AutoscalingRunnerSetNamespace}, role)
return kerrors.IsNotFound(err) return kerrors.IsNotFound(err)
}, },
autoscalingListenerTestTimeout, autoscalingListenerTestTimeout,
@ -340,7 +328,7 @@ var _ = Describe("Test AutoScalingListener controller", func() {
role := new(rbacv1.Role) role := new(rbacv1.Role)
Eventually( Eventually(
func() ([]rbacv1.PolicyRule, error) { func() ([]rbacv1.PolicyRule, error) {
err := k8sClient.Get(ctx, client.ObjectKey{Name: scaleSetListenerRoleName(autoscalingListener), Namespace: autoscalingListener.Spec.AutoscalingRunnerSetNamespace}, role) err := k8sClient.Get(ctx, client.ObjectKey{Name: autoscalingListener.Name, Namespace: autoscalingListener.Spec.AutoscalingRunnerSetNamespace}, role)
if err != nil { if err != nil {
return nil, err return nil, err
} }
@ -397,75 +385,6 @@ var _ = Describe("Test AutoScalingListener controller", func() {
autoscalingListenerTestInterval, autoscalingListenerTestInterval,
).ShouldNot(BeEquivalentTo(oldPodUID), "Pod should be re-created") ).ShouldNot(BeEquivalentTo(oldPodUID), "Pod should be re-created")
}) })
It("It should update mirror secrets to match secret used by AutoScalingRunnerSet", func() {
// Waiting for the pod is created
pod := new(corev1.Pod)
Eventually(
func() (string, error) {
err := k8sClient.Get(ctx, client.ObjectKey{Name: autoscalingListener.Name, Namespace: autoscalingListener.Namespace}, pod)
if err != nil {
return "", err
}
return pod.Name, nil
},
autoscalingListenerTestTimeout,
autoscalingListenerTestInterval).Should(BeEquivalentTo(autoscalingListener.Name), "Pod should be created")
// Update the secret
updatedSecret := configSecret.DeepCopy()
updatedSecret.Data["github_token"] = []byte(autoscalingListenerTestGitHubToken + "_updated")
err := k8sClient.Update(ctx, updatedSecret)
Expect(err).NotTo(HaveOccurred(), "failed to update test secret")
updatedPod := pod.DeepCopy()
// Ignore status running and consult the container state
updatedPod.Status.Phase = corev1.PodRunning
updatedPod.Status.ContainerStatuses = []corev1.ContainerStatus{
{
Name: autoscalingListenerContainerName,
State: corev1.ContainerState{
Terminated: &corev1.ContainerStateTerminated{
ExitCode: 1,
},
},
},
}
err = k8sClient.Status().Update(ctx, updatedPod)
Expect(err).NotTo(HaveOccurred(), "failed to update test pod to failed")
// Check if mirror secret is updated with right data
mirrorSecret := new(corev1.Secret)
Eventually(
func() (map[string][]byte, error) {
err := k8sClient.Get(ctx, client.ObjectKey{Name: scaleSetListenerSecretMirrorName(autoscalingListener), Namespace: autoscalingListener.Namespace}, mirrorSecret)
if err != nil {
return nil, err
}
return mirrorSecret.Data, nil
},
autoscalingListenerTestTimeout,
autoscalingListenerTestInterval).Should(BeEquivalentTo(updatedSecret.Data), "Mirror secret should be updated")
// Check if we re-created a new pod
Eventually(
func() error {
latestPod := new(corev1.Pod)
err := k8sClient.Get(ctx, client.ObjectKey{Name: autoscalingListener.Name, Namespace: autoscalingListener.Namespace}, latestPod)
if err != nil {
return err
}
if latestPod.UID == pod.UID {
return fmt.Errorf("Pod should be recreated")
}
return nil
},
autoscalingListenerTestTimeout,
autoscalingListenerTestInterval).Should(Succeed(), "Pod should be recreated")
})
}) })
}) })

32
controllers/actions.github.com/resourcebuilder.go
View File

@ -429,7 +429,7 @@ func mergeListenerContainer(base, from *corev1.Container) {
func (b *ResourceBuilder) newScaleSetListenerServiceAccount(autoscalingListener *v1alpha1.AutoscalingListener) *corev1.ServiceAccount { func (b *ResourceBuilder) newScaleSetListenerServiceAccount(autoscalingListener *v1alpha1.AutoscalingListener) *corev1.ServiceAccount {
return &corev1.ServiceAccount{ return &corev1.ServiceAccount{
ObjectMeta: metav1.ObjectMeta{ ObjectMeta: metav1.ObjectMeta{
Name: scaleSetListenerServiceAccountName(autoscalingListener), Name: autoscalingListener.Name,
Namespace: autoscalingListener.Namespace, Namespace: autoscalingListener.Namespace,
Labels: b.mergeLabels(autoscalingListener.Labels, map[string]string{ Labels: b.mergeLabels(autoscalingListener.Labels, map[string]string{
LabelKeyGitHubScaleSetNamespace: autoscalingListener.Spec.AutoscalingRunnerSetNamespace, LabelKeyGitHubScaleSetNamespace: autoscalingListener.Spec.AutoscalingRunnerSetNamespace,
@ -444,7 +444,7 @@ func (b *ResourceBuilder) newScaleSetListenerRole(autoscalingListener *v1alpha1.
rulesHash := hash.ComputeTemplateHash(&rules) rulesHash := hash.ComputeTemplateHash(&rules)
newRole := &rbacv1.Role{ newRole := &rbacv1.Role{
ObjectMeta: metav1.ObjectMeta{ ObjectMeta: metav1.ObjectMeta{
Name: scaleSetListenerRoleName(autoscalingListener), Name: autoscalingListener.Name,
Namespace: autoscalingListener.Spec.AutoscalingRunnerSetNamespace, Namespace: autoscalingListener.Spec.AutoscalingRunnerSetNamespace,
Labels: b.mergeLabels(autoscalingListener.Labels, map[string]string{ Labels: b.mergeLabels(autoscalingListener.Labels, map[string]string{
LabelKeyGitHubScaleSetNamespace: autoscalingListener.Spec.AutoscalingRunnerSetNamespace, LabelKeyGitHubScaleSetNamespace: autoscalingListener.Spec.AutoscalingRunnerSetNamespace,
@ -478,7 +478,7 @@ func (b *ResourceBuilder) newScaleSetListenerRoleBinding(autoscalingListener *v1
newRoleBinding := &rbacv1.RoleBinding{ newRoleBinding := &rbacv1.RoleBinding{
ObjectMeta: metav1.ObjectMeta{ ObjectMeta: metav1.ObjectMeta{
Name: scaleSetListenerRoleName(autoscalingListener), Name: autoscalingListener.Name,
Namespace: autoscalingListener.Spec.AutoscalingRunnerSetNamespace, Namespace: autoscalingListener.Spec.AutoscalingRunnerSetNamespace,
Labels: b.mergeLabels(autoscalingListener.Labels, map[string]string{ Labels: b.mergeLabels(autoscalingListener.Labels, map[string]string{
LabelKeyGitHubScaleSetNamespace: autoscalingListener.Spec.AutoscalingRunnerSetNamespace, LabelKeyGitHubScaleSetNamespace: autoscalingListener.Spec.AutoscalingRunnerSetNamespace,
@ -501,7 +501,7 @@ func (b *ResourceBuilder) newScaleSetListenerSecretMirror(autoscalingListener *v
newListenerSecret := &corev1.Secret{ newListenerSecret := &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{ ObjectMeta: metav1.ObjectMeta{
Name: scaleSetListenerSecretMirrorName(autoscalingListener), Name: autoscalingListener.Name,
Namespace: autoscalingListener.Namespace, Namespace: autoscalingListener.Namespace,
Labels: b.mergeLabels(autoscalingListener.Labels, map[string]string{ Labels: b.mergeLabels(autoscalingListener.Labels, map[string]string{
LabelKeyGitHubScaleSetNamespace: autoscalingListener.Spec.AutoscalingRunnerSetNamespace, LabelKeyGitHubScaleSetNamespace: autoscalingListener.Spec.AutoscalingRunnerSetNamespace,
@ -713,30 +713,6 @@ func scaleSetListenerName(autoscalingRunnerSet *v1alpha1.AutoscalingRunnerSet) s
return fmt.Sprintf("%v-%v-listener", autoscalingRunnerSet.Name, namespaceHash) return fmt.Sprintf("%v-%v-listener", autoscalingRunnerSet.Name, namespaceHash)
} }
func scaleSetListenerServiceAccountName(autoscalingListener *v1alpha1.AutoscalingListener) string {
namespaceHash := hash.FNVHashString(autoscalingListener.Spec.AutoscalingRunnerSetNamespace)
if len(namespaceHash) > 8 {
namespaceHash = namespaceHash[:8]
}
return fmt.Sprintf("%v-%v-listener", autoscalingListener.Spec.AutoscalingRunnerSetName, namespaceHash)
}
func scaleSetListenerRoleName(autoscalingListener *v1alpha1.AutoscalingListener) string {
namespaceHash := hash.FNVHashString(autoscalingListener.Spec.AutoscalingRunnerSetNamespace)
if len(namespaceHash) > 8 {
namespaceHash = namespaceHash[:8]
}
return fmt.Sprintf("%v-%v-listener", autoscalingListener.Spec.AutoscalingRunnerSetName, namespaceHash)
}
func scaleSetListenerSecretMirrorName(autoscalingListener *v1alpha1.AutoscalingListener) string {
namespaceHash := hash.FNVHashString(autoscalingListener.Spec.AutoscalingRunnerSetNamespace)
if len(namespaceHash) > 8 {
namespaceHash = namespaceHash[:8]
}
return fmt.Sprintf("%v-%v-listener", autoscalingListener.Spec.AutoscalingRunnerSetName, namespaceHash)
}
func proxyListenerSecretName(autoscalingListener *v1alpha1.AutoscalingListener) string { func proxyListenerSecretName(autoscalingListener *v1alpha1.AutoscalingListener) string {
namespaceHash := hash.FNVHashString(autoscalingListener.Spec.AutoscalingRunnerSetNamespace) namespaceHash := hash.FNVHashString(autoscalingListener.Spec.AutoscalingRunnerSetNamespace)
if len(namespaceHash) > 8 { if len(namespaceHash) > 8 {