From 4670f010ed011afd69550125721acba73e870bb1 Mon Sep 17 00:00:00 2001 From: Nikola Jokic Date: Wed, 15 Jul 2026 15:04:18 +0200 Subject: [PATCH] remove if checks, testing should catch nil references --- .../autoscalinglistener_controller.go | 4 + .../autoscalinglistener_controller_test.go | 42 +++- .../autoscalingrunnerset_controller.go | 2 + .../autoscalingrunnerset_controller_test.go | 36 +++- .../ephemeralrunner_controller_test.go | 20 +- .../ephemeralrunnerset_controller_test.go | 14 +- .../actions.github.com/resourcebuilder.go | 91 +++++---- .../resourcebuilder_test.go | 14 +- .../actions.github.com/resourcecache.go | 177 ++++++++++------ .../actions.github.com/resourcecache_test.go | 192 ++++++------------ 10 files changed, 341 insertions(+), 251 deletions(-) diff --git a/controllers/actions.github.com/autoscalinglistener_controller.go b/controllers/actions.github.com/autoscalinglistener_controller.go index af19a6f2..38e8d64f 100644 --- a/controllers/actions.github.com/autoscalinglistener_controller.go +++ b/controllers/actions.github.com/autoscalinglistener_controller.go @@ -501,6 +501,7 @@ func (r *AutoscalingListenerReconciler) Reconcile(ctx context.Context, req ctrl. return ctrl.Result{}, nil } + r.ResourceCache.listenerPod.Delete(&autoscalingListener) desiredPod, err := r.newScaleSetListenerPod( &autoscalingListener, &listenerConfigSecret, @@ -686,6 +687,7 @@ func (r *AutoscalingListenerReconciler) cleanupResources(ctx context.Context, au } func (r *AutoscalingListenerReconciler) createServiceAccountForListener(ctx context.Context, autoscalingListener *v1alpha1.AutoscalingListener, logger logr.Logger) (ctrl.Result, error) { + r.ResourceCache.listenerServiceAccount.Delete(autoscalingListener) newServiceAccount, err := r.newScaleSetListenerServiceAccount(autoscalingListener) if err != nil { return ctrl.Result{}, err @@ -769,6 +771,7 @@ func (r *AutoscalingListenerReconciler) createProxySecret(ctx context.Context, a } func (r *AutoscalingListenerReconciler) createRoleForListener(ctx context.Context, autoscalingListener *v1alpha1.AutoscalingListener, logger logr.Logger) (ctrl.Result, error) { + r.ResourceCache.listenerRole.Delete(autoscalingListener) newRole := r.newScaleSetListenerRole(autoscalingListener) logger.Info("Creating listener role", "namespace", newRole.Namespace, "name", newRole.Name, "rules", newRole.Rules) @@ -782,6 +785,7 @@ func (r *AutoscalingListenerReconciler) createRoleForListener(ctx context.Contex } func (r *AutoscalingListenerReconciler) createRoleBindingForListener(ctx context.Context, autoscalingListener *v1alpha1.AutoscalingListener, listenerRole *rbacv1.Role, serviceAccount *corev1.ServiceAccount, logger logr.Logger) (ctrl.Result, error) { + r.ResourceCache.listenerRoleBinding.Delete(autoscalingListener) newRoleBinding := r.newScaleSetListenerRoleBinding(autoscalingListener, listenerRole, serviceAccount) logger.Info("Creating listener role binding", diff --git a/controllers/actions.github.com/autoscalinglistener_controller_test.go b/controllers/actions.github.com/autoscalinglistener_controller_test.go index 83a499c4..67f7ecf5 100644 --- a/controllers/actions.github.com/autoscalinglistener_controller_test.go +++ b/controllers/actions.github.com/autoscalinglistener_controller_test.go @@ -38,6 +38,7 @@ var _ = Describe("Test AutoScalingListener controller", func() { var autoscalingRunnerSet *v1alpha1.AutoscalingRunnerSet var configSecret *corev1.Secret var autoscalingListener *v1alpha1.AutoscalingListener + var resourceCache *ResourceCache BeforeEach(func() { ctx = context.Background() @@ -49,8 +50,9 @@ var _ = Describe("Test AutoScalingListener controller", func() { scalefake.NewMultiClient(), ) + resourceCache = newTestResourceCache() rb := ResourceBuilder{ - ResourceCache: newTestResourceCache(), + ResourceCache: resourceCache, SecretResolver: secretResolver, } @@ -231,6 +233,17 @@ var _ = Describe("Test AutoScalingListener controller", func() { autoscalingListenerTestTimeout, autoscalingListenerTestInterval, ).Should(BeEquivalentTo(autoscalingListener.Name), "Pod should be created") + + Eventually( + func() bool { + return resourceCacheStateHasMainObjectEntries(resourceCache.listenerServiceAccount, created) && + resourceCacheStateHasMainObjectEntries(resourceCache.listenerRole, created) && + resourceCacheStateHasMainObjectEntries(resourceCache.listenerRoleBinding, created) && + resourceCacheStateHasMainObjectEntries(resourceCache.listenerPod, created) + }, + autoscalingListenerTestTimeout, + autoscalingListenerTestInterval, + ).Should(BeTrue(), "AutoScalingListener service account, role, role binding, and pod resources should be cached after reconciliation") }) }) @@ -251,8 +264,22 @@ var _ = Describe("Test AutoScalingListener controller", func() { autoscalingListenerTestInterval, ).Should(BeEquivalentTo(autoscalingListener.Name), "Pod should be created") + created := new(v1alpha1.AutoscalingListener) + err := k8sClient.Get(ctx, client.ObjectKey{Name: autoscalingListener.Name, Namespace: autoscalingListener.Namespace}, created) + Expect(err).NotTo(HaveOccurred(), "failed to get AutoScalingListener") + Eventually( + func() bool { + return resourceCacheStateHasMainObjectEntries(resourceCache.listenerServiceAccount, created) && + resourceCacheStateHasMainObjectEntries(resourceCache.listenerRole, created) && + resourceCacheStateHasMainObjectEntries(resourceCache.listenerRoleBinding, created) && + resourceCacheStateHasMainObjectEntries(resourceCache.listenerPod, created) + }, + autoscalingListenerTestTimeout, + autoscalingListenerTestInterval, + ).Should(BeTrue(), "AutoScalingListener service account, role, role binding, and pod resources should be cached before deletion") + // Delete the AutoScalingListener - err := k8sClient.Delete(ctx, autoscalingListener) + err = k8sClient.Delete(ctx, autoscalingListener) Expect(err).NotTo(HaveOccurred(), "failed to delete test AutoScalingListener") // Cleanup the listener pod @@ -343,6 +370,17 @@ var _ = Describe("Test AutoScalingListener controller", func() { autoscalingListenerTestTimeout, autoscalingListenerTestInterval, ).ShouldNot(Succeed(), "failed to delete AutoScalingListener") + + Eventually( + func() bool { + return resourceCacheStateHasMainObjectEntries(resourceCache.listenerServiceAccount, created) || + resourceCacheStateHasMainObjectEntries(resourceCache.listenerRole, created) || + resourceCacheStateHasMainObjectEntries(resourceCache.listenerRoleBinding, created) || + resourceCacheStateHasMainObjectEntries(resourceCache.listenerPod, created) + }, + autoscalingListenerTestTimeout, + autoscalingListenerTestInterval, + ).Should(BeFalse(), "AutoScalingListener service account, role, role binding, and pod resources should be removed from cache after deletion") }) }) diff --git a/controllers/actions.github.com/autoscalingrunnerset_controller.go b/controllers/actions.github.com/autoscalingrunnerset_controller.go index 9d56ffc7..08c9ebc9 100644 --- a/controllers/actions.github.com/autoscalingrunnerset_controller.go +++ b/controllers/actions.github.com/autoscalingrunnerset_controller.go @@ -749,6 +749,7 @@ func (r *AutoscalingRunnerSetReconciler) deleteRunnerScaleSet(ctx context.Contex } func (r *AutoscalingRunnerSetReconciler) createEphemeralRunnerSet(ctx context.Context, autoscalingRunnerSet *v1alpha1.AutoscalingRunnerSet, log logr.Logger) (ctrl.Result, error) { + r.ResourceCache.ephemeralRunnerSet.Delete(autoscalingRunnerSet) desiredRunnerSet, err := r.newEphemeralRunnerSet(autoscalingRunnerSet) if err != nil { log.Error(err, "Could not create EphemeralRunnerSet") @@ -773,6 +774,7 @@ func (r *AutoscalingRunnerSetReconciler) createAutoScalingListenerForRunnerSet(c }) } + r.ResourceCache.autoscalingListener.Delete(autoscalingRunnerSet) autoscalingListener, err := r.newAutoscalingListener( autoscalingRunnerSet, ephemeralRunnerSet, diff --git a/controllers/actions.github.com/autoscalingrunnerset_controller_test.go b/controllers/actions.github.com/autoscalingrunnerset_controller_test.go index 82177acb..3cbe719f 100644 --- a/controllers/actions.github.com/autoscalingrunnerset_controller_test.go +++ b/controllers/actions.github.com/autoscalingrunnerset_controller_test.go @@ -44,6 +44,7 @@ var _ = Describe("Test AutoScalingRunnerSet controller", Ordered, func() { var autoscalingNS *corev1.Namespace var autoscalingRunnerSet *v1alpha1.AutoscalingRunnerSet var configSecret *corev1.Secret + var resourceCache *ResourceCache var originalBuildVersion string buildVersion := "0.1.0" @@ -65,6 +66,7 @@ var _ = Describe("Test AutoScalingRunnerSet controller", Ordered, func() { // Track runner group mappings for dynamic responses runnerGroupMap := map[int]string{1: "testgroup"} // ID -> Name mapping runnerGroupMapLock := &sync.RWMutex{} // Thread-safe access + resourceCache = newTestResourceCache() controller = &AutoscalingRunnerSetReconciler{ Client: mgr.GetClient(), @@ -73,7 +75,7 @@ var _ = Describe("Test AutoScalingRunnerSet controller", Ordered, func() { ControllerNamespace: autoscalingNS.Name, DefaultRunnerScaleSetListenerImage: "ghcr.io/actions/arc", ResourceBuilder: ResourceBuilder{ - ResourceCache: newTestResourceCache(), + ResourceCache: resourceCache, SecretResolver: secretresolver.New(mgr.GetClient(), scalefake.NewMultiClient( scalefake.WithClient( scalefake.NewClient( @@ -252,6 +254,15 @@ var _ = Describe("Test AutoScalingRunnerSet controller", Ordered, func() { autoscalingRunnerSetTestInterval, ).Should(Succeed(), "Listener should be created") + Eventually( + func() bool { + return resourceCacheStateHasMainObjectEntries(resourceCache.ephemeralRunnerSet, created) && + resourceCacheStateHasMainObjectEntries(resourceCache.autoscalingListener, created) + }, + autoscalingRunnerSetTestTimeout, + autoscalingRunnerSetTestInterval, + ).Should(BeTrue(), "AutoScalingRunnerSet EphemeralRunnerSet and AutoScalingListener resources should be cached after reconciliation") + // Check if status is updated runnerSetList := new(v1alpha1.EphemeralRunnerSetList) err := k8sClient.List(ctx, runnerSetList, client.InNamespace(autoscalingRunnerSet.Namespace)) @@ -271,8 +282,20 @@ var _ = Describe("Test AutoScalingRunnerSet controller", Ordered, func() { autoscalingRunnerSetTestInterval, ).Should(Succeed(), "Listener should be created") + created := new(v1alpha1.AutoscalingRunnerSet) + err := k8sClient.Get(ctx, client.ObjectKey{Name: autoscalingRunnerSet.Name, Namespace: autoscalingRunnerSet.Namespace}, created) + Expect(err).NotTo(HaveOccurred(), "failed to get AutoScalingRunnerSet") + Eventually( + func() bool { + return resourceCacheStateHasMainObjectEntries(resourceCache.ephemeralRunnerSet, created) && + resourceCacheStateHasMainObjectEntries(resourceCache.autoscalingListener, created) + }, + autoscalingRunnerSetTestTimeout, + autoscalingRunnerSetTestInterval, + ).Should(BeTrue(), "AutoScalingRunnerSet EphemeralRunnerSet and AutoScalingListener resources should be cached before deletion") + // Delete the AutoScalingRunnerSet - err := k8sClient.Delete(ctx, autoscalingRunnerSet) + err = k8sClient.Delete(ctx, autoscalingRunnerSet) Expect(err).NotTo(HaveOccurred(), "failed to delete AutoScalingRunnerSet") // Check if the listener is deleted @@ -321,6 +344,15 @@ var _ = Describe("Test AutoScalingRunnerSet controller", Ordered, func() { autoscalingRunnerSetTestTimeout, autoscalingRunnerSetTestInterval, ).Should(Succeed(), "AutoScalingRunnerSet should be deleted") + + Eventually( + func() bool { + return resourceCacheStateHasMainObjectEntries(resourceCache.ephemeralRunnerSet, created) || + resourceCacheStateHasMainObjectEntries(resourceCache.autoscalingListener, created) + }, + autoscalingRunnerSetTestTimeout, + autoscalingRunnerSetTestInterval, + ).Should(BeFalse(), "AutoScalingRunnerSet EphemeralRunnerSet and AutoScalingListener resources should be removed from cache after deletion") }) }) diff --git a/controllers/actions.github.com/ephemeralrunner_controller_test.go b/controllers/actions.github.com/ephemeralrunner_controller_test.go index ddd666bb..74f9fe99 100644 --- a/controllers/actions.github.com/ephemeralrunner_controller_test.go +++ b/controllers/actions.github.com/ephemeralrunner_controller_test.go @@ -100,18 +100,20 @@ var _ = Describe("EphemeralRunner", func() { var configSecret *corev1.Secret var controller *EphemeralRunnerReconciler var ephemeralRunner *v1alpha1.EphemeralRunner + var resourceCache *ResourceCache BeforeEach(func() { ctx = context.Background() autoscalingNS, mgr = createNamespace(GinkgoT(), k8sClient) configSecret = createDefaultSecret(GinkgoT(), k8sClient, autoscalingNS.Name) + resourceCache = newTestResourceCache() controller = &EphemeralRunnerReconciler{ Client: mgr.GetClient(), Scheme: mgr.GetScheme(), Log: logf.Log, ResourceBuilder: ResourceBuilder{ - ResourceCache: newTestResourceCache(), + ResourceCache: resourceCache, SecretResolver: secretresolver.New(mgr.GetClient(), scalefake.NewMultiClient( scalefake.WithClient( scalefake.NewClient( @@ -652,6 +654,12 @@ var _ = Describe("EphemeralRunner", func() { return true, nil }).Should(BeEquivalentTo(true)) + created := new(v1alpha1.EphemeralRunner) + err := k8sClient.Get(ctx, client.ObjectKey{Name: ephemeralRunner.Name, Namespace: ephemeralRunner.Namespace}, created) + Expect(err).To(BeNil(), "failed to get ephemeral runner") + resourceCache.listenerPod.Upsert(created, &corev1.Pod{ObjectMeta: metav1.ObjectMeta{Name: "cached-runner-pod", Namespace: created.Namespace}}) + Expect(resourceCacheHasMainObjectEntries(resourceCache, created)).To(BeTrue(), "test setup should cache an EphemeralRunner-owned resource") + // create runner-linked pod runnerLinkedPod := &corev1.Pod{ ObjectMeta: metav1.ObjectMeta{ @@ -671,7 +679,7 @@ var _ = Describe("EphemeralRunner", func() { }, } - err := k8sClient.Create(ctx, runnerLinkedPod) + err = k8sClient.Create(ctx, runnerLinkedPod) Expect(err).To(BeNil(), "failed to create runner linked pod") Eventually( func() (bool, error) { @@ -778,6 +786,14 @@ var _ = Describe("EphemeralRunner", func() { ephemeralRunnerTimeout, ephemeralRunnerInterval, ).Should(BeEquivalentTo(true)) + + Eventually( + func() bool { + return resourceCacheHasMainObjectEntries(resourceCache, created) + }, + ephemeralRunnerTimeout, + ephemeralRunnerInterval, + ).Should(BeFalse(), "EphemeralRunner-owned resources should be removed from cache after deletion") }) It("It should eventually have runner id set", func() { diff --git a/controllers/actions.github.com/ephemeralrunnerset_controller_test.go b/controllers/actions.github.com/ephemeralrunnerset_controller_test.go index 2ee42ac2..331e8e59 100644 --- a/controllers/actions.github.com/ephemeralrunnerset_controller_test.go +++ b/controllers/actions.github.com/ephemeralrunnerset_controller_test.go @@ -133,18 +133,20 @@ var _ = Describe("Test EphemeralRunnerSet controller", func() { var autoscalingNS *corev1.Namespace var ephemeralRunnerSet *v1alpha1.EphemeralRunnerSet var configSecret *corev1.Secret + var resourceCache *ResourceCache BeforeEach(func() { ctx = context.Background() autoscalingNS, mgr = createNamespace(GinkgoT(), k8sClient) configSecret = createDefaultSecret(GinkgoT(), k8sClient, autoscalingNS.Name) + resourceCache = newTestResourceCache() controller := &EphemeralRunnerSetReconciler{ Client: mgr.GetClient(), Scheme: mgr.GetScheme(), Log: logf.Log, ResourceBuilder: ResourceBuilder{ - ResourceCache: newTestResourceCache(), + ResourceCache: resourceCache, SecretResolver: secretresolver.New(mgr.GetClient(), fake.NewMultiClient( fake.WithClient( fake.NewClient( @@ -284,6 +286,8 @@ var _ = Describe("Test EphemeralRunnerSet controller", func() { created := new(v1alpha1.EphemeralRunnerSet) err := k8sClient.Get(ctx, client.ObjectKey{Name: ephemeralRunnerSet.Name, Namespace: ephemeralRunnerSet.Namespace}, created) Expect(err).NotTo(HaveOccurred(), "failed to get EphemeralRunnerSet") + resourceCache.listenerPod.Upsert(created, &corev1.Pod{ObjectMeta: metav1.ObjectMeta{Name: "cached-runner-set-pod", Namespace: created.Namespace}}) + Expect(resourceCacheHasMainObjectEntries(resourceCache, created)).To(BeTrue(), "test setup should cache an EphemeralRunnerSet-owned resource") // Scale up the EphemeralRunnerSet updated := created.DeepCopy() @@ -360,6 +364,14 @@ var _ = Describe("Test EphemeralRunnerSet controller", func() { ephemeralRunnerSetTestTimeout, ephemeralRunnerSetTestInterval, ).Should(Succeed(), "EphemeralRunnerSet should be deleted") + + Eventually( + func() bool { + return resourceCacheHasMainObjectEntries(resourceCache, created) + }, + ephemeralRunnerSetTestTimeout, + ephemeralRunnerSetTestInterval, + ).Should(BeFalse(), "EphemeralRunnerSet-owned resources should be removed from cache after deletion") }) }) diff --git a/controllers/actions.github.com/resourcebuilder.go b/controllers/actions.github.com/resourcebuilder.go index 8d6ae26c..a75d414c 100644 --- a/controllers/actions.github.com/resourcebuilder.go +++ b/controllers/actions.github.com/resourcebuilder.go @@ -137,10 +137,8 @@ func (b *ResourceBuilder) newAutoscalingListener(autoscalingRunnerSet *v1alpha1. Image: image, ImagePullSecrets: imagePullSecrets, }) - if b.ResourceCache != nil { - if cached, ok := b.ResourceCache.autoscalingListener.Get(autoscalingRunnerSet, cacheKeyObject, ephemeralRunnerSet, inputDependency); ok { - return cached, nil - } + if cached, ok := b.ResourceCache.autoscalingListener.Get(autoscalingRunnerSet, cacheKeyObject, ephemeralRunnerSet, inputDependency); ok { + return cached, nil } effectiveMinRunners := 0 @@ -196,6 +194,10 @@ func (b *ResourceBuilder) newAutoscalingListener(autoscalingRunnerSet *v1alpha1. } autoscalingListener := &v1alpha1.AutoscalingListener{ + TypeMeta: metav1.TypeMeta{ + APIVersion: v1alpha1.GroupVersion.String(), + Kind: "AutoscalingListener", + }, ObjectMeta: metav1.ObjectMeta{ Name: scaleSetListenerName(autoscalingRunnerSet), Namespace: namespace, @@ -204,15 +206,17 @@ func (b *ResourceBuilder) newAutoscalingListener(autoscalingRunnerSet *v1alpha1. }, Spec: spec, } - if b.ResourceCache != nil { - b.ResourceCache.autoscalingListener.Upsert(autoscalingRunnerSet, autoscalingListener, ephemeralRunnerSet, inputDependency) - } + b.ResourceCache.autoscalingListener.Upsert(autoscalingRunnerSet, autoscalingListener, ephemeralRunnerSet, inputDependency) return autoscalingListener, nil } func resourceCacheInputObject(name string, value any) client.Object { return &corev1.ConfigMap{ + TypeMeta: metav1.TypeMeta{ + APIVersion: corev1.SchemeGroupVersion.String(), + Kind: "ConfigMap", + }, ObjectMeta: metav1.ObjectMeta{ Name: name, ResourceVersion: hash.ComputeTemplateHash(value), @@ -301,6 +305,10 @@ func (b *ResourceBuilder) newScaleSetListenerConfig(autoscalingListener *v1alpha } desiredSecret := &corev1.Secret{ + TypeMeta: metav1.TypeMeta{ + APIVersion: corev1.SchemeGroupVersion.String(), + Kind: "Secret", + }, ObjectMeta: metav1.ObjectMeta{ Name: scaleSetListenerConfigName(autoscalingListener), Namespace: autoscalingListener.Namespace, @@ -347,10 +355,8 @@ func (b *ResourceBuilder) newScaleSetListenerPod( Namespace: autoscalingListener.Namespace, }, } - if b.ResourceCache != nil { - if cached, ok := b.ResourceCache.listenerPod.Get(autoscalingListener, cacheKeyObject, podConfig, serviceAccount, role, roleBinding); ok { - return cached, nil - } + if cached, ok := b.ResourceCache.listenerPod.Get(autoscalingListener, cacheKeyObject, podConfig, serviceAccount, role, roleBinding); ok { + return cached, nil } envs := []corev1.EnvVar{ @@ -460,8 +466,8 @@ func (b *ResourceBuilder) newScaleSetListenerPod( newRunnerScaleSetListenerPod := &corev1.Pod{ TypeMeta: metav1.TypeMeta{ + APIVersion: corev1.SchemeGroupVersion.String(), Kind: "Pod", - APIVersion: "v1", }, ObjectMeta: metav1.ObjectMeta{ Name: autoscalingListener.Name, @@ -489,9 +495,7 @@ func (b *ResourceBuilder) newScaleSetListenerPod( if autoscalingListener.Spec.Template != nil { mergeListenerPodWithTemplate(newRunnerScaleSetListenerPod, autoscalingListener.Spec.Template) } - if b.ResourceCache != nil { - b.ResourceCache.listenerPod.Upsert(autoscalingListener, newRunnerScaleSetListenerPod, podConfig, serviceAccount, role, roleBinding) - } + b.ResourceCache.listenerPod.Upsert(autoscalingListener, newRunnerScaleSetListenerPod, podConfig, serviceAccount, role, roleBinding) return newRunnerScaleSetListenerPod, nil } @@ -652,13 +656,15 @@ func (b *ResourceBuilder) newScaleSetListenerServiceAccount(autoscalingListener Namespace: autoscalingListener.Namespace, }, } - if b.ResourceCache != nil { - if cached, ok := b.ResourceCache.listenerServiceAccount.Get(autoscalingListener, cacheKeyObject); ok { - return cached, nil - } + if cached, ok := b.ResourceCache.listenerServiceAccount.Get(autoscalingListener, cacheKeyObject); ok { + return cached, nil } base := &corev1.ServiceAccount{ + TypeMeta: metav1.TypeMeta{ + APIVersion: corev1.SchemeGroupVersion.String(), + Kind: "ServiceAccount", + }, ObjectMeta: metav1.ObjectMeta{ Name: autoscalingListener.Name, Namespace: autoscalingListener.Namespace, @@ -680,9 +686,7 @@ func (b *ResourceBuilder) newScaleSetListenerServiceAccount(autoscalingListener if err := b.setControllerReference(autoscalingListener, base); err != nil { return nil, fmt.Errorf("failed to set controller reference for listener service account: %w", err) } - if b.ResourceCache != nil { - b.ResourceCache.listenerServiceAccount.Upsert(autoscalingListener, base) - } + b.ResourceCache.listenerServiceAccount.Upsert(autoscalingListener, base) return base, nil } @@ -710,10 +714,8 @@ func (b *ResourceBuilder) newScaleSetListenerRole(autoscalingListener *v1alpha1. Namespace: autoscalingListener.Spec.AutoscalingRunnerSetNamespace, }, } - if b.ResourceCache != nil { - if cached, ok := b.ResourceCache.listenerRole.Get(autoscalingListener, cacheKeyObject); ok { - return cached - } + if cached, ok := b.ResourceCache.listenerRole.Get(autoscalingListener, cacheKeyObject); ok { + return cached } labels := b.filterAndMergeLabels(autoscalingListener.Labels, map[string]string{ @@ -730,6 +732,10 @@ func (b *ResourceBuilder) newScaleSetListenerRole(autoscalingListener *v1alpha1. } newRole := &rbacv1.Role{ + TypeMeta: metav1.TypeMeta{ + APIVersion: rbacv1.SchemeGroupVersion.String(), + Kind: "Role", + }, ObjectMeta: metav1.ObjectMeta{ Name: autoscalingListener.Name, Namespace: autoscalingListener.Spec.AutoscalingRunnerSetNamespace, @@ -740,9 +746,7 @@ func (b *ResourceBuilder) newScaleSetListenerRole(autoscalingListener *v1alpha1. } newRole.Annotations[annotationKeyIntegrityHash] = scaleSetRoleIntegrityHash(newRole) - if b.ResourceCache != nil { - b.ResourceCache.listenerRole.Upsert(autoscalingListener, newRole) - } + b.ResourceCache.listenerRole.Upsert(autoscalingListener, newRole) return newRole } @@ -766,10 +770,8 @@ func (b *ResourceBuilder) newScaleSetListenerRoleBinding(autoscalingListener *v1 Namespace: autoscalingListener.Spec.AutoscalingRunnerSetNamespace, }, } - if b.ResourceCache != nil { - if cached, ok := b.ResourceCache.listenerRoleBinding.Get(autoscalingListener, cacheKeyObject, listenerRole, serviceAccount); ok { - return cached - } + if cached, ok := b.ResourceCache.listenerRoleBinding.Get(autoscalingListener, cacheKeyObject, listenerRole, serviceAccount); ok { + return cached } roleRef := rbacv1.RoleRef{ @@ -799,6 +801,10 @@ func (b *ResourceBuilder) newScaleSetListenerRoleBinding(autoscalingListener *v1 } newRoleBinding := &rbacv1.RoleBinding{ + TypeMeta: metav1.TypeMeta{ + APIVersion: rbacv1.SchemeGroupVersion.String(), + Kind: "RoleBinding", + }, ObjectMeta: metav1.ObjectMeta{ Name: autoscalingListener.Name, Namespace: autoscalingListener.Spec.AutoscalingRunnerSetNamespace, @@ -810,9 +816,7 @@ func (b *ResourceBuilder) newScaleSetListenerRoleBinding(autoscalingListener *v1 } newRoleBinding.Annotations[annotationKeyIntegrityHash] = scaleSetListenerRoleBindingIntegrityHash(newRoleBinding) - if b.ResourceCache != nil { - b.ResourceCache.listenerRoleBinding.Upsert(autoscalingListener, newRoleBinding, listenerRole, serviceAccount) - } + b.ResourceCache.listenerRoleBinding.Upsert(autoscalingListener, newRoleBinding, listenerRole, serviceAccount) return newRoleBinding } @@ -843,10 +847,8 @@ func (b *ResourceBuilder) newEphemeralRunnerSet(autoscalingRunnerSet *v1alpha1.A Namespace: autoscalingRunnerSet.Namespace, }, } - if b.ResourceCache != nil { - if cached, ok := b.ResourceCache.ephemeralRunnerSet.Get(autoscalingRunnerSet, cacheKeyObject); ok { - return cached, nil - } + if cached, ok := b.ResourceCache.ephemeralRunnerSet.Get(autoscalingRunnerSet, cacheKeyObject); ok { + return cached, nil } spec := v1alpha1.EphemeralRunnerSetSpec{ @@ -887,7 +889,10 @@ func (b *ResourceBuilder) newEphemeralRunnerSet(autoscalingRunnerSet *v1alpha1.A } newEphemeralRunnerSet := &v1alpha1.EphemeralRunnerSet{ - TypeMeta: metav1.TypeMeta{}, + TypeMeta: metav1.TypeMeta{ + APIVersion: v1alpha1.GroupVersion.String(), + Kind: "EphemeralRunnerSet", + }, ObjectMeta: metav1.ObjectMeta{ Name: autoscalingRunnerSet.Name, Namespace: autoscalingRunnerSet.Namespace, @@ -902,9 +907,7 @@ func (b *ResourceBuilder) newEphemeralRunnerSet(autoscalingRunnerSet *v1alpha1.A if err := b.setControllerReference(autoscalingRunnerSet, newEphemeralRunnerSet); err != nil { return nil, fmt.Errorf("failed to set controller reference for ephemeral runner set: %w", err) } - if b.ResourceCache != nil { - b.ResourceCache.ephemeralRunnerSet.Upsert(autoscalingRunnerSet, newEphemeralRunnerSet) - } + b.ResourceCache.ephemeralRunnerSet.Upsert(autoscalingRunnerSet, newEphemeralRunnerSet) return newEphemeralRunnerSet, nil } diff --git a/controllers/actions.github.com/resourcebuilder_test.go b/controllers/actions.github.com/resourcebuilder_test.go index 4324401e..6097308d 100644 --- a/controllers/actions.github.com/resourcebuilder_test.go +++ b/controllers/actions.github.com/resourcebuilder_test.go @@ -102,11 +102,13 @@ func TestMetadataPropagation(t *testing.T) { }, } + cache := NewResourceCache() b := ResourceBuilder{ ExcludeLabelPropagationPrefixes: []string{ "example.com/", "directly.excluded.org/label", }, + ResourceCache: &cache, } ephemeralRunnerSet, err := b.newEphemeralRunnerSet(&autoscalingRunnerSet) require.NoError(t, err) @@ -258,7 +260,8 @@ func TestGitHubURLTrimLabelValues(t *testing.T) { GitHubConfigUrl: fmt.Sprintf("https://github.com/%s/%s", organization, repository), } - var b ResourceBuilder + cache := NewResourceCache() + b := ResourceBuilder{ResourceCache: &cache} ephemeralRunnerSet, err := b.newEphemeralRunnerSet(autoscalingRunnerSet) require.NoError(t, err) assert.Len(t, ephemeralRunnerSet.Labels[LabelKeyGitHubEnterprise], 0) @@ -282,7 +285,8 @@ func TestGitHubURLTrimLabelValues(t *testing.T) { GitHubConfigUrl: fmt.Sprintf("https://github.com/enterprises/%s", enterprise), } - var b ResourceBuilder + cache := NewResourceCache() + b := ResourceBuilder{ResourceCache: &cache} ephemeralRunnerSet, err := b.newEphemeralRunnerSet(autoscalingRunnerSet) require.NoError(t, err) assert.Len(t, ephemeralRunnerSet.Labels[LabelKeyGitHubEnterprise], 63) @@ -323,7 +327,8 @@ func TestOwnershipRelationships(t *testing.T) { } // Initialize ResourceBuilder - b := ResourceBuilder{} + cache := NewResourceCache() + b := ResourceBuilder{ResourceCache: &cache} // Create EphemeralRunnerSet ephemeralRunnerSet, err := b.newEphemeralRunnerSet(&autoscalingRunnerSet) @@ -422,7 +427,8 @@ func TestListenerPodNodeSelector(t *testing.T) { }, } - b := ResourceBuilder{} + cache := NewResourceCache() + b := ResourceBuilder{ResourceCache: &cache} ephemeralRunnerSet, err := b.newEphemeralRunnerSet(&autoscalingRunnerSet) require.NoError(t, err) diff --git a/controllers/actions.github.com/resourcecache.go b/controllers/actions.github.com/resourcecache.go index 1f631be0..2a4b1c10 100644 --- a/controllers/actions.github.com/resourcecache.go +++ b/controllers/actions.github.com/resourcecache.go @@ -10,14 +10,20 @@ import ( "github.com/actions/actions-runner-controller/hash" corev1 "k8s.io/api/core/v1" rbacv1 "k8s.io/api/rbac/v1" + "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/client" ) -var resourceCacheObjectTypes sync.Map +const ( + resourceCacheInitialEntries = 4096 + resourceCacheInitialMainUIDEntries = 4096 + resourceCacheInitialOwnerEntries = 8 + resourceCacheMaxDependencyRefs = 4 +) type ResourceCacheObjectRef struct { - ObjectType string + ObjectType schema.GroupVersionKind Namespace string Name string UID types.UID @@ -33,10 +39,15 @@ type ResourceCacheKey struct { type ResourceCacheValue[T client.Object] struct { MainObject ResourceCacheObjectRef ResourceVersion string - Dependencies []ResourceCacheObjectRef + dependencyKey resourceCacheDependencyKey Object T } +type resourceCacheDependencyKey struct { + count int + refs [resourceCacheMaxDependencyRefs]ResourceCacheObjectRef +} + type ResourceCache struct { autoscalingListener *resourceCacheState[*v1alpha1.AutoscalingListener] ephemeralRunnerSet *resourceCacheState[*v1alpha1.EphemeralRunnerSet] @@ -58,13 +69,15 @@ func NewResourceCache() ResourceCache { } type resourceCacheState[T client.Object] struct { - mu sync.RWMutex - entries map[ResourceCacheKey]ResourceCacheValue[T] + mu sync.RWMutex + entries map[ResourceCacheKey]ResourceCacheValue[T] + entriesByMainUID map[types.UID]map[ResourceCacheKey]struct{} } func newResourceCacheState[T client.Object]() *resourceCacheState[T] { return &resourceCacheState[T]{ - entries: make(map[ResourceCacheKey]ResourceCacheValue[T], 512), + entries: make(map[ResourceCacheKey]ResourceCacheValue[T], resourceCacheInitialEntries), + entriesByMainUID: make(map[types.UID]map[ResourceCacheKey]struct{}, resourceCacheInitialMainUIDEntries), } } @@ -73,17 +86,30 @@ func (s *resourceCacheState[T]) Get( desiredObject T, dependencies ...client.Object, ) (T, bool) { - key := newResourceCacheKey(mainObject, desiredObject) - - s.mu.RLock() - value, ok := s.entries[key] - s.mu.RUnlock() - if !ok || !value.Matches(mainObject, dependencies...) { - var zero T + var zero T + if s == nil || isNilResourceCacheObject(mainObject) || isNilResourceCacheObject(desiredObject) { + return zero, false + } + dependencyKey, ok := newResourceCacheDependencyKey(dependencies...) + if !ok { + return zero, false + } + if mainObject.GetUID() == "" { return zero, false } - return cloneResourceCacheObject(value.Object), true + key := newResourceCacheKey(mainObject, desiredObject) + mainObjectRef := newResourceCacheObjectRef(mainObject) + + s.mu.RLock() + value, ok := s.entries[key] + if ok && value.MainObject == mainObjectRef && value.dependencyKey.Equal(dependencyKey) { + s.mu.RUnlock() + return value.Object, true + } + s.mu.RUnlock() + + return zero, false } func (s *resourceCacheState[T]) Upsert( @@ -91,13 +117,25 @@ func (s *resourceCacheState[T]) Upsert( desiredObject T, dependencies ...client.Object, ) (ResourceCacheValue[T], bool) { + var zero ResourceCacheValue[T] + if s == nil || isNilResourceCacheObject(mainObject) || isNilResourceCacheObject(desiredObject) { + return zero, false + } + dependencyKey, ok := newResourceCacheDependencyKey(dependencies...) + if !ok { + return zero, false + } + if mainObject.GetUID() == "" { + return zero, false + } + key := newResourceCacheKey(mainObject, desiredObject) mainObjectRef := newResourceCacheObjectRef(mainObject) resourceVersion := desiredObject.GetResourceVersion() s.mu.RLock() previous, ok := s.entries[key] - if ok && previous.MainObject == mainObjectRef && previous.ResourceVersion == resourceVersion && previous.dependenciesMatch(dependencies...) { + if ok && previous.MainObject == mainObjectRef && previous.ResourceVersion == resourceVersion && previous.dependencyKey.Equal(dependencyKey) { s.mu.RUnlock() return previous, false } @@ -107,13 +145,18 @@ func (s *resourceCacheState[T]) Upsert( defer s.mu.Unlock() previous, ok = s.entries[key] - if ok && previous.MainObject == mainObjectRef && previous.ResourceVersion == resourceVersion && previous.dependenciesMatch(dependencies...) { + if ok && previous.MainObject == mainObjectRef && previous.ResourceVersion == resourceVersion && previous.dependencyKey.Equal(dependencyKey) { return previous, false } - dependencyRefs := newResourceCacheObjectRefs(dependencies...) - value := newResourceCacheValue(mainObjectRef, resourceVersion, dependencyRefs, cloneResourceCacheObject(desiredObject)) + value := ResourceCacheValue[T]{ + MainObject: mainObjectRef, + ResourceVersion: resourceVersion, + dependencyKey: dependencyKey, + Object: desiredObject, + } s.entries[key] = value + s.indexKeyLocked(key) return value, true } @@ -131,7 +174,7 @@ func (c *ResourceCache) Delete(mainObject client.Object) { } func (s *resourceCacheState[T]) Delete(mainObject client.Object) { - if mainObject == nil { + if s == nil || mainObject == nil { return } @@ -143,19 +186,19 @@ func (s *resourceCacheState[T]) Delete(mainObject client.Object) { s.mu.Lock() defer s.mu.Unlock() - for key := range s.entries { - if key.MainUID == uid { - delete(s.entries, key) - } + for key := range s.entriesByMainUID[uid] { + delete(s.entries, key) } + delete(s.entriesByMainUID, uid) } -func (v ResourceCacheValue[T]) Matches(mainObject client.Object, dependencies ...client.Object) bool { - if v.MainObject != newResourceCacheObjectRef(mainObject) { - return false +func (s *resourceCacheState[T]) indexKeyLocked(key ResourceCacheKey) { + keys, ok := s.entriesByMainUID[key.MainUID] + if !ok { + keys = make(map[ResourceCacheKey]struct{}, resourceCacheInitialOwnerEntries) + s.entriesByMainUID[key.MainUID] = keys } - - return v.dependenciesMatch(dependencies...) + keys[key] = struct{}{} } func newResourceCacheKey(mainObject client.Object, desiredObject client.Object) ResourceCacheKey { @@ -166,43 +209,34 @@ func newResourceCacheKey(mainObject client.Object, desiredObject client.Object) } } -func newResourceCacheValue[T client.Object]( - mainObjectRef ResourceCacheObjectRef, - resourceVersion string, - dependencyRefs []ResourceCacheObjectRef, - object T, -) ResourceCacheValue[T] { - return ResourceCacheValue[T]{ - MainObject: mainObjectRef, - ResourceVersion: resourceVersion, - Dependencies: dependencyRefs, - Object: object, +func newResourceCacheDependencyKey(objects ...client.Object) (resourceCacheDependencyKey, bool) { + if len(objects) > resourceCacheMaxDependencyRefs { + return resourceCacheDependencyKey{}, false } -} -func cloneResourceCacheObject[T client.Object](object T) T { - return object.DeepCopyObject().(T) -} - -func newResourceCacheObjectRefs(objects ...client.Object) []ResourceCacheObjectRef { - refs := make([]ResourceCacheObjectRef, 0, len(objects)) - for _, object := range objects { - refs = append(refs, newResourceCacheObjectRef(object)) + key := resourceCacheDependencyKey{count: len(objects)} + for i, object := range objects { + if isNilResourceCacheObject(object) { + return resourceCacheDependencyKey{}, false + } + key.refs[i] = newResourceCacheObjectRef(object) } - slices.SortFunc(refs, func(a, b ResourceCacheObjectRef) int { + slices.SortFunc(key.refs[:key.count], func(a, b ResourceCacheObjectRef) int { return compareResourceCacheObjectRefs(a, b) }) - return refs + return key, true } -func (v ResourceCacheValue[T]) dependenciesMatch(objects ...client.Object) bool { - if len(v.Dependencies) != len(objects) { +func (k resourceCacheDependencyKey) Equal(other resourceCacheDependencyKey) bool { + if k.count != other.count { + return false + } + if k.count > len(k.refs) || other.count > len(other.refs) { return false } - for _, object := range objects { - ref := newResourceCacheObjectRef(object) - if !slices.Contains(v.Dependencies, ref) { + for i := range k.count { + if k.refs[i] != other.refs[i] { return false } } @@ -212,12 +246,15 @@ func (v ResourceCacheValue[T]) dependenciesMatch(objects ...client.Object) bool func newResourceCacheObjectRef(object client.Object) ResourceCacheObjectRef { resourceVersion := object.GetResourceVersion() + if resourceVersion == "" { + resourceVersion = object.GetAnnotations()[annotationKeyIntegrityHash] + } if resourceVersion == "" { resourceVersion = hash.ComputeTemplateHash(object) } return ResourceCacheObjectRef{ - ObjectType: resourceCacheObjectType(object), + ObjectType: object.GetObjectKind().GroupVersionKind(), Namespace: object.GetNamespace(), Name: resourceCacheObjectName(object), UID: object.GetUID(), @@ -226,7 +263,7 @@ func newResourceCacheObjectRef(object client.Object) ResourceCacheObjectRef { } func compareResourceCacheObjectRefs(a, b ResourceCacheObjectRef) int { - if c := strings.Compare(a.ObjectType, b.ObjectType); c != 0 { + if c := compareGroupVersionKinds(a.ObjectType, b.ObjectType); c != 0 { return c } if c := strings.Compare(a.Namespace, b.Namespace); c != 0 { @@ -241,18 +278,14 @@ func compareResourceCacheObjectRefs(a, b ResourceCacheObjectRef) int { return strings.Compare(a.ResourceVersion, b.ResourceVersion) } -func resourceCacheObjectType(object client.Object) string { - t := reflect.TypeOf(object) - if t.Kind() == reflect.Pointer { - t = t.Elem() +func compareGroupVersionKinds(a, b schema.GroupVersionKind) int { + if c := strings.Compare(a.Group, b.Group); c != 0 { + return c } - if objectType, ok := resourceCacheObjectTypes.Load(t); ok { - return objectType.(string) + if c := strings.Compare(a.Version, b.Version); c != 0 { + return c } - - objectType := t.PkgPath() + "." + t.Name() - actual, _ := resourceCacheObjectTypes.LoadOrStore(t, objectType) - return actual.(string) + return strings.Compare(a.Kind, b.Kind) } func resourceCacheObjectName(object client.Object) string { @@ -261,3 +294,13 @@ func resourceCacheObjectName(object client.Object) string { } return object.GetGenerateName() } + +func isNilResourceCacheObject[T client.Object](object T) bool { + var clientObject client.Object = object + if clientObject == nil { + return true + } + + value := reflect.ValueOf(clientObject) + return value.Kind() == reflect.Pointer && value.IsNil() +} diff --git a/controllers/actions.github.com/resourcecache_test.go b/controllers/actions.github.com/resourcecache_test.go index 8bf576d5..608ed9d9 100644 --- a/controllers/actions.github.com/resourcecache_test.go +++ b/controllers/actions.github.com/resourcecache_test.go @@ -10,15 +10,35 @@ import ( corev1 "k8s.io/api/core/v1" rbacv1 "k8s.io/api/rbac/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "sigs.k8s.io/controller-runtime/pkg/client" ) -var benchmarkEphemeralRunnerSetSink *v1alpha1.EphemeralRunnerSet - func newTestResourceCache() *ResourceCache { cache := NewResourceCache() return &cache } +func resourceCacheHasMainObjectEntries(cache *ResourceCache, mainObject client.Object) bool { + return resourceCacheStateHasMainObjectEntries(cache.autoscalingListener, mainObject) || + resourceCacheStateHasMainObjectEntries(cache.ephemeralRunnerSet, mainObject) || + resourceCacheStateHasMainObjectEntries(cache.listenerPod, mainObject) || + resourceCacheStateHasMainObjectEntries(cache.listenerServiceAccount, mainObject) || + resourceCacheStateHasMainObjectEntries(cache.listenerRole, mainObject) || + resourceCacheStateHasMainObjectEntries(cache.listenerRoleBinding, mainObject) +} + +func resourceCacheStateHasMainObjectEntries[T client.Object](state *resourceCacheState[T], mainObject client.Object) bool { + uid := mainObject.GetUID() + if uid == "" { + return false + } + + state.mu.RLock() + defer state.mu.RUnlock() + + return len(state.entriesByMainUID[uid]) > 0 +} + func TestResourceCacheUpsertReplacesByDependencyResourceVersion(t *testing.T) { mainObject := &v1alpha1.AutoscalingListener{ ObjectMeta: metav1.ObjectMeta{ @@ -78,17 +98,14 @@ func TestResourceCacheUpsertReplacesByDependencyResourceVersion(t *testing.T) { configSecret.ResourceVersion = "2" value, replaced = cache.listenerPod.Upsert(mainObject, desiredPod, configSecret, serviceAccount, role) assert.True(t, replaced) - assert.Contains(t, value.Dependencies, ResourceCacheObjectRef{ - ObjectType: resourceCacheObjectType(configSecret), - Namespace: "controller-ns", - Name: "listener-config", - UID: "config-secret-uid", - ResourceVersion: "2", - }) + staleConfigSecret := configSecret.DeepCopy() + staleConfigSecret.ResourceVersion = "1" + _, ok = cache.listenerPod.Get(mainObject, desiredPod, staleConfigSecret, serviceAccount, role) + assert.False(t, ok) + _, ok = cache.listenerPod.Get(mainObject, desiredPod, configSecret, serviceAccount, role) + assert.True(t, ok) - desiredPod.Labels["mutated"] = "after-cache" - cachedPod := value.Object - assert.NotContains(t, cachedPod.Labels, "mutated") + assert.Same(t, desiredPod, value.Object) } func TestResourceCacheDeleteRemovesMainObjectEntries(t *testing.T) { @@ -132,6 +149,38 @@ func TestResourceCacheDeletePanicsWithNilCache(t *testing.T) { }) } +func TestResourceCacheIgnoresInvalidInputs(t *testing.T) { + cache := NewResourceCache() + desiredPod := &corev1.Pod{ObjectMeta: metav1.ObjectMeta{Name: "listener", Namespace: "controller-ns"}} + mainObjectWithoutUID := &v1alpha1.AutoscalingListener{ObjectMeta: metav1.ObjectMeta{Name: "listener", Namespace: "controller-ns"}} + mainObject := mainObjectWithoutUID.DeepCopy() + mainObject.UID = "listener-uid" + + _, replaced := cache.listenerPod.Upsert(mainObjectWithoutUID, desiredPod) + assert.False(t, replaced) + _, ok := cache.listenerPod.Get(mainObjectWithoutUID, desiredPod) + assert.False(t, ok) + + var nilDependency *corev1.Secret + assert.NotPanics(t, func() { + _, replaced = cache.listenerPod.Upsert(mainObject, desiredPod, nilDependency) + assert.False(t, replaced) + _, ok = cache.listenerPod.Get(mainObject, desiredPod, nilDependency) + assert.False(t, ok) + }) + + tooManyDependencies := make([]client.Object, resourceCacheMaxDependencyRefs+1) + for i := range tooManyDependencies { + tooManyDependencies[i] = &corev1.Secret{ObjectMeta: metav1.ObjectMeta{Name: fmt.Sprintf("dependency-%d", i), Namespace: "controller-ns"}} + } + assert.NotPanics(t, func() { + _, replaced = cache.listenerPod.Upsert(mainObject, desiredPod, tooManyDependencies...) + assert.False(t, replaced) + _, ok = cache.listenerPod.Get(mainObject, desiredPod, tooManyDependencies...) + assert.False(t, ok) + }) +} + func TestResourceBuilderCachesListenerPodDependencies(t *testing.T) { listener := &v1alpha1.AutoscalingListener{ ObjectMeta: metav1.ObjectMeta{ @@ -231,128 +280,13 @@ func TestResourceBuilderCachesEphemeralRunnerSet(t *testing.T) { cachedRunnerSet, ok := b.ResourceCache.ephemeralRunnerSet.Get(&autoscalingRunnerSet, runnerSet) require.True(t, ok) assert.Equal(t, runnerSet.Spec, cachedRunnerSet.Spec) - - runnerSet.Labels["mutated"] = "after-cache" - assert.NotContains(t, cachedRunnerSet.Labels, "mutated") + assert.Same(t, runnerSet, cachedRunnerSet) fromBuilder, err := b.newEphemeralRunnerSet(&autoscalingRunnerSet) require.NoError(t, err) - assert.NotContains(t, fromBuilder.Labels, "mutated") + assert.Same(t, runnerSet, fromBuilder) autoscalingRunnerSet.Annotations[runnerScaleSetIDAnnotationKey] = "2" _, ok = b.ResourceCache.ephemeralRunnerSet.Get(&autoscalingRunnerSet, runnerSet) assert.False(t, ok) } - -func BenchmarkNewEphemeralRunnerSetResourceCache(b *testing.B) { - autoscalingRunnerSet := newBenchmarkAutoscalingRunnerSet() - - b.Run("no_cache", func(b *testing.B) { - builder := ResourceBuilder{} - b.ReportAllocs() - b.ResetTimer() - - for i := 0; i < b.N; i++ { - runnerSet, err := builder.newEphemeralRunnerSet(autoscalingRunnerSet) - if err != nil { - b.Fatal(err) - } - benchmarkEphemeralRunnerSetSink = runnerSet - } - }) - - b.Run("cache_hit", func(b *testing.B) { - cache := NewResourceCache() - builder := ResourceBuilder{ResourceCache: &cache} - if _, err := builder.newEphemeralRunnerSet(autoscalingRunnerSet); err != nil { - b.Fatal(err) - } - - b.ReportAllocs() - b.ResetTimer() - - for i := 0; i < b.N; i++ { - runnerSet, err := builder.newEphemeralRunnerSet(autoscalingRunnerSet) - if err != nil { - b.Fatal(err) - } - benchmarkEphemeralRunnerSetSink = runnerSet - } - }) - - b.Run("cache_miss", func(b *testing.B) { - cache := NewResourceCache() - builder := ResourceBuilder{ResourceCache: &cache} - autoscalingRunnerSet := autoscalingRunnerSet.DeepCopy() - - b.ReportAllocs() - b.ResetTimer() - - for i := 0; i < b.N; i++ { - autoscalingRunnerSet.ResourceVersion = fmt.Sprint(i) - runnerSet, err := builder.newEphemeralRunnerSet(autoscalingRunnerSet) - if err != nil { - b.Fatal(err) - } - benchmarkEphemeralRunnerSetSink = runnerSet - } - }) -} - -func newBenchmarkAutoscalingRunnerSet() *v1alpha1.AutoscalingRunnerSet { - return &v1alpha1.AutoscalingRunnerSet{ - ObjectMeta: metav1.ObjectMeta{ - Name: "benchmark-scale-set", - Namespace: "benchmark-namespace", - UID: "benchmark-scale-set-uid", - ResourceVersion: "1", - Labels: map[string]string{ - LabelKeyKubernetesVersion: "0.12.0", - "example.com/label-1": "value-1", - "example.com/label-2": "value-2", - }, - Annotations: map[string]string{ - runnerScaleSetIDAnnotationKey: "123", - AnnotationKeyGitHubRunnerGroupName: "benchmark-runner-group", - AnnotationKeyGitHubRunnerScaleSetName: "benchmark-scale-set", - }, - }, - Spec: v1alpha1.AutoscalingRunnerSetSpec{ - GitHubConfigUrl: "https://github.com/actions/actions-runner-controller", - EphemeralRunnerSetMetadata: &v1alpha1.ResourceMeta{ - Labels: map[string]string{ - "example.com/runner-set-label": "runner-set-value", - }, - Annotations: map[string]string{ - "example.com/runner-set-annotation": "runner-set-value", - }, - }, - EphemeralRunnerMetadata: &v1alpha1.ResourceMeta{ - Labels: map[string]string{ - "example.com/runner-label": "runner-value", - }, - Annotations: map[string]string{ - "example.com/runner-annotation": "runner-value", - }, - }, - Template: corev1.PodTemplateSpec{ - ObjectMeta: metav1.ObjectMeta{ - Labels: map[string]string{ - "example.com/template-label": "template-value", - }, - }, - Spec: corev1.PodSpec{ - Containers: []corev1.Container{ - { - Name: v1alpha1.EphemeralRunnerContainerName, - Image: "ghcr.io/actions/actions-runner:latest", - Env: []corev1.EnvVar{ - {Name: "ACTIONS_RUNNER_REQUIRE_JOB_CONTAINER", Value: "false"}, - }, - }, - }, - }, - }, - }, - } -}