diff --git a/controllers/actions.github.com/autoscalinglistener_controller.go b/controllers/actions.github.com/autoscalinglistener_controller.go index c0aa81114c..38e8d64fdb 100644 --- a/controllers/actions.github.com/autoscalinglistener_controller.go +++ b/controllers/actions.github.com/autoscalinglistener_controller.go @@ -105,6 +105,7 @@ func (r *AutoscalingListenerReconciler) Reconcile(ctx context.Context, req ctrl. } log.Info("Successfully removed finalizer after cleanup") + r.ResourceCache.Delete(&autoscalingListener) return ctrl.Result{}, nil } @@ -500,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, @@ -685,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 @@ -768,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) @@ -781,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 d48d613e6c..67f7ecf5e7 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,7 +50,9 @@ var _ = Describe("Test AutoScalingListener controller", func() { scalefake.NewMultiClient(), ) + resourceCache = newTestResourceCache() rb := ResourceBuilder{ + ResourceCache: resourceCache, SecretResolver: secretResolver, } @@ -230,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") }) }) @@ -250,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 @@ -342,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") }) }) @@ -593,6 +632,7 @@ var _ = Describe("Test AutoScalingListener customization", func() { secretResolver := secretresolver.New(mgr.GetClient(), scalefake.NewMultiClient()) rb := ResourceBuilder{ + ResourceCache: newTestResourceCache(), SecretResolver: secretResolver, } @@ -922,6 +962,7 @@ var _ = Describe("Test AutoScalingListener controller with proxy", func() { secretResolver := secretresolver.New(mgr.GetClient(), scalefake.NewMultiClient()) rb := ResourceBuilder{ + ResourceCache: newTestResourceCache(), SecretResolver: secretResolver, } @@ -1127,6 +1168,7 @@ var _ = Describe("Test AutoScalingListener controller with template modification secretResolver := secretresolver.New(mgr.GetClient(), scalefake.NewMultiClient()) rb := ResourceBuilder{ + ResourceCache: newTestResourceCache(), SecretResolver: secretResolver, } @@ -1232,6 +1274,7 @@ var _ = Describe("Test GitHub Server TLS configuration", func() { secretResolver := secretresolver.New(mgr.GetClient(), scalefake.NewMultiClient()) rb := ResourceBuilder{ + ResourceCache: newTestResourceCache(), SecretResolver: secretResolver, } diff --git a/controllers/actions.github.com/autoscalingrunnerset_controller.go b/controllers/actions.github.com/autoscalingrunnerset_controller.go index e48f75ff15..08c9ebc960 100644 --- a/controllers/actions.github.com/autoscalingrunnerset_controller.go +++ b/controllers/actions.github.com/autoscalingrunnerset_controller.go @@ -108,6 +108,7 @@ func (r *AutoscalingRunnerSetReconciler) Reconcile(ctx context.Context, req ctrl } log.Info("Successfully removed finalizer after cleanup") + r.ResourceCache.Delete(&autoscalingRunnerSet) return ctrl.Result{}, nil } @@ -748,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") @@ -772,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 11a03dc482..3cbe719fc8 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,6 +75,7 @@ var _ = Describe("Test AutoScalingRunnerSet controller", Ordered, func() { ControllerNamespace: autoscalingNS.Name, DefaultRunnerScaleSetListenerImage: "ghcr.io/actions/arc", ResourceBuilder: ResourceBuilder{ + ResourceCache: resourceCache, SecretResolver: secretresolver.New(mgr.GetClient(), scalefake.NewMultiClient( scalefake.WithClient( scalefake.NewClient( @@ -251,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)) @@ -270,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 @@ -320,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") }) }) @@ -961,6 +994,7 @@ var _ = Describe("Test AutoScalingController updates", Ordered, func() { ControllerNamespace: autoscalingNS.Name, DefaultRunnerScaleSetListenerImage: "ghcr.io/actions/arc", ResourceBuilder: ResourceBuilder{ + ResourceCache: newTestResourceCache(), SecretResolver: secretresolver.New(mgr.GetClient(), multiClient), }, } @@ -1078,6 +1112,7 @@ var _ = Describe("Test AutoscalingController creation failures", Ordered, func() ControllerNamespace: autoscalingNS.Name, DefaultRunnerScaleSetListenerImage: "ghcr.io/actions/arc", ResourceBuilder: ResourceBuilder{ + ResourceCache: newTestResourceCache(), SecretResolver: secretresolver.New(mgr.GetClient(), scalefake.NewMultiClient()), }, } @@ -1205,6 +1240,7 @@ var _ = Describe("Test client optional configuration", Ordered, func() { ControllerNamespace: autoscalingNS.Name, DefaultRunnerScaleSetListenerImage: "ghcr.io/actions/arc", ResourceBuilder: ResourceBuilder{ + ResourceCache: newTestResourceCache(), SecretResolver: secretresolver.New(mgr.GetClient(), multiclient.NewScaleset()), }, } @@ -1400,6 +1436,7 @@ var _ = Describe("Test client optional configuration", Ordered, func() { ControllerNamespace: autoscalingNS.Name, DefaultRunnerScaleSetListenerImage: "ghcr.io/actions/arc", ResourceBuilder: ResourceBuilder{ + ResourceCache: newTestResourceCache(), SecretResolver: secretresolver.New(mgr.GetClient(), scalefake.NewMultiClient( scalefake.WithClient( scalefake.NewClient( @@ -1647,6 +1684,7 @@ var _ = Describe("Test external permissions cleanup", Ordered, func() { ControllerNamespace: autoscalingNS.Name, DefaultRunnerScaleSetListenerImage: "ghcr.io/actions/arc", ResourceBuilder: ResourceBuilder{ + ResourceCache: newTestResourceCache(), SecretResolver: secretresolver.New(mgr.GetClient(), scalefake.NewMultiClient()), }, } @@ -1807,6 +1845,7 @@ var _ = Describe("Test external permissions cleanup", Ordered, func() { ControllerNamespace: autoscalingNS.Name, DefaultRunnerScaleSetListenerImage: "ghcr.io/actions/arc", ResourceBuilder: ResourceBuilder{ + ResourceCache: newTestResourceCache(), SecretResolver: secretresolver.New(mgr.GetClient(), scalefake.NewMultiClient()), }, } @@ -2017,6 +2056,7 @@ var _ = Describe("Test resource version and build version mismatch", func() { ControllerNamespace: autoscalingNS.Name, DefaultRunnerScaleSetListenerImage: "ghcr.io/actions/arc", ResourceBuilder: ResourceBuilder{ + ResourceCache: newTestResourceCache(), SecretResolver: secretresolver.New(mgr.GetClient(), scalefake.NewMultiClient()), }, } diff --git a/controllers/actions.github.com/ephemeralrunner_controller.go b/controllers/actions.github.com/ephemeralrunner_controller.go index e78ede68ff..256ed311c4 100644 --- a/controllers/actions.github.com/ephemeralrunner_controller.go +++ b/controllers/actions.github.com/ephemeralrunner_controller.go @@ -151,7 +151,7 @@ func (r *EphemeralRunnerReconciler) Reconcile(ctx context.Context, req ctrl.Requ } } - log.Info("Successfully removed finalizer after cleanup") + r.ResourceCache.Delete(&ephemeralRunner) return ctrl.Result{}, nil } diff --git a/controllers/actions.github.com/ephemeralrunner_controller_test.go b/controllers/actions.github.com/ephemeralrunner_controller_test.go index 80c27134a7..74f9fe9923 100644 --- a/controllers/actions.github.com/ephemeralrunner_controller_test.go +++ b/controllers/actions.github.com/ephemeralrunner_controller_test.go @@ -100,17 +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: resourceCache, SecretResolver: secretresolver.New(mgr.GetClient(), scalefake.NewMultiClient( scalefake.WithClient( scalefake.NewClient( @@ -651,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{ @@ -670,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) { @@ -777,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() { @@ -1216,6 +1233,7 @@ var _ = Describe("EphemeralRunner", func() { Scheme: mgr.GetScheme(), Log: logf.Log, ResourceBuilder: ResourceBuilder{ + ResourceCache: newTestResourceCache(), SecretResolver: secretresolver.New( mgr.GetClient(), scalefake.NewMultiClient( @@ -1302,6 +1320,7 @@ var _ = Describe("EphemeralRunner", func() { Scheme: mgr.GetScheme(), Log: logf.Log, ResourceBuilder: ResourceBuilder{ + ResourceCache: newTestResourceCache(), SecretResolver: secretresolver.New(mgr.GetClient(), scalefake.NewMultiClient( scalefake.WithClient( scalefake.NewClient( @@ -1326,6 +1345,7 @@ var _ = Describe("EphemeralRunner", func() { It("uses an actions client with proxy transport", func() { // Use an actual client controller.ResourceBuilder = ResourceBuilder{ + ResourceCache: newTestResourceCache(), SecretResolver: secretresolver.New( mgr.GetClient(), multiclient.NewScaleset(), @@ -1485,6 +1505,7 @@ var _ = Describe("EphemeralRunner", func() { Scheme: mgr.GetScheme(), Log: logf.Log, ResourceBuilder: ResourceBuilder{ + ResourceCache: newTestResourceCache(), SecretResolver: secretresolver.New(mgr.GetClient(), scalefake.NewMultiClient()), }, } @@ -1519,6 +1540,7 @@ var _ = Describe("EphemeralRunner", func() { // Use an actual client controller.ResourceBuilder = ResourceBuilder{ + ResourceCache: newTestResourceCache(), SecretResolver: secretresolver.New( mgr.GetClient(), multiclient.NewScaleset(), diff --git a/controllers/actions.github.com/ephemeralrunnerset_controller.go b/controllers/actions.github.com/ephemeralrunnerset_controller.go index 919a46419f..c4f6d8c025 100644 --- a/controllers/actions.github.com/ephemeralrunnerset_controller.go +++ b/controllers/actions.github.com/ephemeralrunnerset_controller.go @@ -117,6 +117,7 @@ func (r *EphemeralRunnerSetReconciler) Reconcile(ctx context.Context, req ctrl.R } log.Info("Successfully removed finalizer after cleanup") + r.ResourceCache.Delete(&ephemeralRunnerSet) return ctrl.Result{}, nil } diff --git a/controllers/actions.github.com/ephemeralrunnerset_controller_test.go b/controllers/actions.github.com/ephemeralrunnerset_controller_test.go index 76cf98ab88..331e8e596c 100644 --- a/controllers/actions.github.com/ephemeralrunnerset_controller_test.go +++ b/controllers/actions.github.com/ephemeralrunnerset_controller_test.go @@ -133,17 +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: resourceCache, SecretResolver: secretresolver.New(mgr.GetClient(), fake.NewMultiClient( fake.WithClient( fake.NewClient( @@ -275,21 +278,6 @@ var _ = Describe("Test EphemeralRunnerSet controller", func() { ephemeralRunnerSetTestTimeout, ephemeralRunnerSetTestInterval, ).Should(BeEquivalentTo(5), "5 EphemeralRunner should be created") - - // Check if the status stays running - Eventually( - func() (v1alpha1.EphemeralRunnerSetPhase, error) { - runnerSet := new(v1alpha1.EphemeralRunnerSet) - err := k8sClient.Get(ctx, client.ObjectKey{Name: ephemeralRunnerSet.Name, Namespace: ephemeralRunnerSet.Namespace}, runnerSet) - if err != nil { - return "", err - } - - return runnerSet.Status.Phase, nil - }, - ephemeralRunnerSetTestTimeout, - ephemeralRunnerSetTestInterval, - ).Should(BeEquivalentTo(v1alpha1.EphemeralRunnerSetPhaseRunning), "EphemeralRunnerSet status should be running") }) }) @@ -298,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() @@ -374,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") }) }) @@ -1378,6 +1376,7 @@ var _ = Describe("EphemeralRunner phase metrics", func() { Log: logf.Log, PublishMetrics: true, ResourceBuilder: ResourceBuilder{ + ResourceCache: newTestResourceCache(), SecretResolver: secretresolver.New(k8sClient, fake.NewMultiClient( fake.WithClient( fake.NewClient( @@ -1509,6 +1508,7 @@ var _ = Describe("Test EphemeralRunnerSet controller with proxy settings", func( Scheme: mgr.GetScheme(), Log: logf.Log, ResourceBuilder: ResourceBuilder{ + ResourceCache: newTestResourceCache(), SecretResolver: secretresolver.New(mgr.GetClient(), multiclient.NewScaleset()), }, } @@ -1827,6 +1827,7 @@ var _ = Describe("Test EphemeralRunnerSet controller with custom root CA", func( Scheme: mgr.GetScheme(), Log: logf.Log, ResourceBuilder: ResourceBuilder{ + ResourceCache: newTestResourceCache(), SecretResolver: secretresolver.New(mgr.GetClient(), multiclient.NewScaleset()), }, } diff --git a/controllers/actions.github.com/resourcebuilder.go b/controllers/actions.github.com/resourcebuilder.go index fd456b56f1..a75d414c57 100644 --- a/controllers/actions.github.com/resourcebuilder.go +++ b/controllers/actions.github.com/resourcebuilder.go @@ -95,7 +95,8 @@ type SecretResolver interface { type ResourceBuilder struct { ExcludeLabelPropagationPrefixes []string SecretResolver - Scheme *runtime.Scheme + Scheme *runtime.Scheme + ResourceCache *ResourceCache } func (b *ResourceBuilder) setSchemeIfUnset(scheme *runtime.Scheme) { @@ -121,6 +122,25 @@ func (b *ResourceBuilder) newAutoscalingListener(autoscalingRunnerSet *v1alpha1. return nil, err } + cacheKeyObject := &v1alpha1.AutoscalingListener{ + ObjectMeta: metav1.ObjectMeta{ + Name: scaleSetListenerName(autoscalingRunnerSet), + Namespace: namespace, + }, + } + inputDependency := resourceCacheInputObject("autoscaling-listener-inputs", struct { + Namespace string + Image string + ImagePullSecrets []corev1.LocalObjectReference + }{ + Namespace: namespace, + Image: image, + ImagePullSecrets: imagePullSecrets, + }) + if cached, ok := b.ResourceCache.autoscalingListener.Get(autoscalingRunnerSet, cacheKeyObject, ephemeralRunnerSet, inputDependency); ok { + return cached, nil + } + effectiveMinRunners := 0 effectiveMaxRunners := math.MaxInt32 if autoscalingRunnerSet.Spec.MaxRunners != nil { @@ -174,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, @@ -182,10 +206,24 @@ func (b *ResourceBuilder) newAutoscalingListener(autoscalingRunnerSet *v1alpha1. }, Spec: spec, } + 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), + }, + } +} + type listenerMetricsServerConfig struct { addr string endpoint string @@ -267,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, @@ -307,6 +349,16 @@ func (b *ResourceBuilder) newScaleSetListenerPod( roleBinding *rbacv1.RoleBinding, metricsConfig *listenerMetricsServerConfig, ) (*corev1.Pod, error) { + cacheKeyObject := &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: autoscalingListener.Name, + Namespace: autoscalingListener.Namespace, + }, + } + if cached, ok := b.ResourceCache.listenerPod.Get(autoscalingListener, cacheKeyObject, podConfig, serviceAccount, role, roleBinding); ok { + return cached, nil + } + envs := []corev1.EnvVar{ { Name: "LISTENER_CONFIG_PATH", @@ -414,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, @@ -443,6 +495,7 @@ func (b *ResourceBuilder) newScaleSetListenerPod( if autoscalingListener.Spec.Template != nil { mergeListenerPodWithTemplate(newRunnerScaleSetListenerPod, autoscalingListener.Spec.Template) } + b.ResourceCache.listenerPod.Upsert(autoscalingListener, newRunnerScaleSetListenerPod, podConfig, serviceAccount, role, roleBinding) return newRunnerScaleSetListenerPod, nil } @@ -597,7 +650,21 @@ func mergeListenerContainer(base, from *corev1.Container) { } func (b *ResourceBuilder) newScaleSetListenerServiceAccount(autoscalingListener *v1alpha1.AutoscalingListener) (*corev1.ServiceAccount, error) { + cacheKeyObject := &corev1.ServiceAccount{ + ObjectMeta: metav1.ObjectMeta{ + Name: autoscalingListener.Name, + Namespace: autoscalingListener.Namespace, + }, + } + 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, @@ -619,6 +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) } + b.ResourceCache.listenerServiceAccount.Upsert(autoscalingListener, base) return base, nil } @@ -640,6 +708,16 @@ func scaleSetListenerServiceAccountIntegrityHash(sa *corev1.ServiceAccount) stri } func (b *ResourceBuilder) newScaleSetListenerRole(autoscalingListener *v1alpha1.AutoscalingListener) *rbacv1.Role { + cacheKeyObject := &rbacv1.Role{ + ObjectMeta: metav1.ObjectMeta{ + Name: autoscalingListener.Name, + Namespace: autoscalingListener.Spec.AutoscalingRunnerSetNamespace, + }, + } + if cached, ok := b.ResourceCache.listenerRole.Get(autoscalingListener, cacheKeyObject); ok { + return cached + } + labels := b.filterAndMergeLabels(autoscalingListener.Labels, map[string]string{ LabelKeyGitHubScaleSetNamespace: autoscalingListener.Spec.AutoscalingRunnerSetNamespace, LabelKeyGitHubScaleSetName: autoscalingListener.Spec.AutoscalingRunnerSetName, @@ -654,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, @@ -664,6 +746,7 @@ func (b *ResourceBuilder) newScaleSetListenerRole(autoscalingListener *v1alpha1. } newRole.Annotations[annotationKeyIntegrityHash] = scaleSetRoleIntegrityHash(newRole) + b.ResourceCache.listenerRole.Upsert(autoscalingListener, newRole) return newRole } @@ -681,6 +764,16 @@ func scaleSetRoleIntegrityHash(role *rbacv1.Role) string { } func (b *ResourceBuilder) newScaleSetListenerRoleBinding(autoscalingListener *v1alpha1.AutoscalingListener, listenerRole *rbacv1.Role, serviceAccount *corev1.ServiceAccount) *rbacv1.RoleBinding { + cacheKeyObject := &rbacv1.RoleBinding{ + ObjectMeta: metav1.ObjectMeta{ + Name: autoscalingListener.Name, + Namespace: autoscalingListener.Spec.AutoscalingRunnerSetNamespace, + }, + } + if cached, ok := b.ResourceCache.listenerRoleBinding.Get(autoscalingListener, cacheKeyObject, listenerRole, serviceAccount); ok { + return cached + } + roleRef := rbacv1.RoleRef{ Kind: "Role", Name: listenerRole.Name, @@ -708,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, @@ -719,6 +816,7 @@ func (b *ResourceBuilder) newScaleSetListenerRoleBinding(autoscalingListener *v1 } newRoleBinding.Annotations[annotationKeyIntegrityHash] = scaleSetListenerRoleBindingIntegrityHash(newRoleBinding) + b.ResourceCache.listenerRoleBinding.Upsert(autoscalingListener, newRoleBinding, listenerRole, serviceAccount) return newRoleBinding } @@ -743,6 +841,16 @@ func (b *ResourceBuilder) newEphemeralRunnerSet(autoscalingRunnerSet *v1alpha1.A return nil, err } + cacheKeyObject := &v1alpha1.EphemeralRunnerSet{ + ObjectMeta: metav1.ObjectMeta{ + Name: autoscalingRunnerSet.Name, + Namespace: autoscalingRunnerSet.Namespace, + }, + } + if cached, ok := b.ResourceCache.ephemeralRunnerSet.Get(autoscalingRunnerSet, cacheKeyObject); ok { + return cached, nil + } + spec := v1alpha1.EphemeralRunnerSetSpec{ Replicas: 0, EphemeralRunnerSpec: v1alpha1.EphemeralRunnerSpec{ @@ -781,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, @@ -796,6 +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) } + 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 d08851173a..6097308d71 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) @@ -171,6 +173,7 @@ func TestMetadataPropagation(t *testing.T) { ephemeralRunner, err := b.newEphemeralRunner(ephemeralRunnerSet) require.NoError(t, err) + assert.ElementsMatch(t, []string{ephemeralRunnerFinalizerName, ephemeralRunnerActionsFinalizerName}, ephemeralRunner.Finalizers) for _, key := range commonLabelKeys { if key == LabelKeyKubernetesComponent { @@ -257,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) @@ -281,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) @@ -322,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) @@ -421,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 new file mode 100644 index 0000000000..2a4b1c1035 --- /dev/null +++ b/controllers/actions.github.com/resourcecache.go @@ -0,0 +1,306 @@ +package actionsgithubcom + +import ( + "reflect" + "slices" + "strings" + "sync" + + "github.com/actions/actions-runner-controller/apis/actions.github.com/v1alpha1" + "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" +) + +const ( + resourceCacheInitialEntries = 4096 + resourceCacheInitialMainUIDEntries = 4096 + resourceCacheInitialOwnerEntries = 8 + resourceCacheMaxDependencyRefs = 4 +) + +type ResourceCacheObjectRef struct { + ObjectType schema.GroupVersionKind + Namespace string + Name string + UID types.UID + ResourceVersion string +} + +type ResourceCacheKey struct { + MainUID types.UID + Namespace string + Name string +} + +type ResourceCacheValue[T client.Object] struct { + MainObject ResourceCacheObjectRef + ResourceVersion string + dependencyKey resourceCacheDependencyKey + Object T +} + +type resourceCacheDependencyKey struct { + count int + refs [resourceCacheMaxDependencyRefs]ResourceCacheObjectRef +} + +type ResourceCache struct { + autoscalingListener *resourceCacheState[*v1alpha1.AutoscalingListener] + ephemeralRunnerSet *resourceCacheState[*v1alpha1.EphemeralRunnerSet] + listenerPod *resourceCacheState[*corev1.Pod] + listenerServiceAccount *resourceCacheState[*corev1.ServiceAccount] + listenerRole *resourceCacheState[*rbacv1.Role] + listenerRoleBinding *resourceCacheState[*rbacv1.RoleBinding] +} + +func NewResourceCache() ResourceCache { + return ResourceCache{ + autoscalingListener: newResourceCacheState[*v1alpha1.AutoscalingListener](), + ephemeralRunnerSet: newResourceCacheState[*v1alpha1.EphemeralRunnerSet](), + listenerPod: newResourceCacheState[*corev1.Pod](), + listenerServiceAccount: newResourceCacheState[*corev1.ServiceAccount](), + listenerRole: newResourceCacheState[*rbacv1.Role](), + listenerRoleBinding: newResourceCacheState[*rbacv1.RoleBinding](), + } +} + +type resourceCacheState[T client.Object] struct { + 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], resourceCacheInitialEntries), + entriesByMainUID: make(map[types.UID]map[ResourceCacheKey]struct{}, resourceCacheInitialMainUIDEntries), + } +} + +func (s *resourceCacheState[T]) Get( + mainObject client.Object, + desiredObject T, + dependencies ...client.Object, +) (T, bool) { + 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 + } + + 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( + mainObject client.Object, + 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.dependencyKey.Equal(dependencyKey) { + s.mu.RUnlock() + return previous, false + } + s.mu.RUnlock() + + s.mu.Lock() + defer s.mu.Unlock() + + previous, ok = s.entries[key] + if ok && previous.MainObject == mainObjectRef && previous.ResourceVersion == resourceVersion && previous.dependencyKey.Equal(dependencyKey) { + return previous, false + } + + value := ResourceCacheValue[T]{ + MainObject: mainObjectRef, + ResourceVersion: resourceVersion, + dependencyKey: dependencyKey, + Object: desiredObject, + } + s.entries[key] = value + s.indexKeyLocked(key) + return value, true +} + +func (c *ResourceCache) Delete(mainObject client.Object) { + if mainObject == nil { + return + } + + c.autoscalingListener.Delete(mainObject) + c.ephemeralRunnerSet.Delete(mainObject) + c.listenerPod.Delete(mainObject) + c.listenerServiceAccount.Delete(mainObject) + c.listenerRole.Delete(mainObject) + c.listenerRoleBinding.Delete(mainObject) +} + +func (s *resourceCacheState[T]) Delete(mainObject client.Object) { + if s == nil || mainObject == nil { + return + } + + uid := mainObject.GetUID() + if uid == "" { + return + } + + s.mu.Lock() + defer s.mu.Unlock() + + for key := range s.entriesByMainUID[uid] { + delete(s.entries, key) + } + delete(s.entriesByMainUID, uid) +} + +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 + } + keys[key] = struct{}{} +} + +func newResourceCacheKey(mainObject client.Object, desiredObject client.Object) ResourceCacheKey { + return ResourceCacheKey{ + MainUID: mainObject.GetUID(), + Namespace: desiredObject.GetNamespace(), + Name: resourceCacheObjectName(desiredObject), + } +} + +func newResourceCacheDependencyKey(objects ...client.Object) (resourceCacheDependencyKey, bool) { + if len(objects) > resourceCacheMaxDependencyRefs { + return resourceCacheDependencyKey{}, false + } + + key := resourceCacheDependencyKey{count: len(objects)} + for i, object := range objects { + if isNilResourceCacheObject(object) { + return resourceCacheDependencyKey{}, false + } + key.refs[i] = newResourceCacheObjectRef(object) + } + slices.SortFunc(key.refs[:key.count], func(a, b ResourceCacheObjectRef) int { + return compareResourceCacheObjectRefs(a, b) + }) + return key, true +} + +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 i := range k.count { + if k.refs[i] != other.refs[i] { + return false + } + } + + return true +} + +func newResourceCacheObjectRef(object client.Object) ResourceCacheObjectRef { + resourceVersion := object.GetResourceVersion() + if resourceVersion == "" { + resourceVersion = object.GetAnnotations()[annotationKeyIntegrityHash] + } + if resourceVersion == "" { + resourceVersion = hash.ComputeTemplateHash(object) + } + + return ResourceCacheObjectRef{ + ObjectType: object.GetObjectKind().GroupVersionKind(), + Namespace: object.GetNamespace(), + Name: resourceCacheObjectName(object), + UID: object.GetUID(), + ResourceVersion: resourceVersion, + } +} + +func compareResourceCacheObjectRefs(a, b ResourceCacheObjectRef) int { + if c := compareGroupVersionKinds(a.ObjectType, b.ObjectType); c != 0 { + return c + } + if c := strings.Compare(a.Namespace, b.Namespace); c != 0 { + return c + } + if c := strings.Compare(a.Name, b.Name); c != 0 { + return c + } + if c := strings.Compare(string(a.UID), string(b.UID)); c != 0 { + return c + } + return strings.Compare(a.ResourceVersion, b.ResourceVersion) +} + +func compareGroupVersionKinds(a, b schema.GroupVersionKind) int { + if c := strings.Compare(a.Group, b.Group); c != 0 { + return c + } + if c := strings.Compare(a.Version, b.Version); c != 0 { + return c + } + return strings.Compare(a.Kind, b.Kind) +} + +func resourceCacheObjectName(object client.Object) string { + if object.GetName() != "" { + return object.GetName() + } + 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 new file mode 100644 index 0000000000..608ed9d95d --- /dev/null +++ b/controllers/actions.github.com/resourcecache_test.go @@ -0,0 +1,292 @@ +package actionsgithubcom + +import ( + "fmt" + "testing" + + "github.com/actions/actions-runner-controller/apis/actions.github.com/v1alpha1" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + 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" +) + +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{ + Name: "listener", + Namespace: "controller-ns", + UID: "listener-uid", + ResourceVersion: "10", + }, + } + desiredPod := &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "listener", + Namespace: "controller-ns", + ResourceVersion: "1", + Labels: map[string]string{ + "app": "listener", + }, + }, + } + configSecret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "listener-config", + Namespace: "controller-ns", + UID: "config-secret-uid", + ResourceVersion: "1", + }, + } + serviceAccount := &corev1.ServiceAccount{ + ObjectMeta: metav1.ObjectMeta{ + Name: "listener", + Namespace: "controller-ns", + UID: "service-account-uid", + ResourceVersion: "1", + }, + } + role := &rbacv1.Role{ + ObjectMeta: metav1.ObjectMeta{ + Name: "listener", + Namespace: "scale-set-ns", + UID: "role-uid", + ResourceVersion: "1", + }, + } + + cache := NewResourceCache() + value, replaced := cache.listenerPod.Upsert(mainObject, desiredPod, configSecret, serviceAccount, role) + assert.True(t, replaced) + _, ok := cache.listenerPod.Get(mainObject, desiredPod, configSecret, serviceAccount, role) + assert.True(t, ok) + assert.Equal(t, "1", value.ResourceVersion) + + _, replaced = cache.listenerPod.Upsert(mainObject, desiredPod, role, configSecret, serviceAccount) + assert.False(t, replaced, "dependency ordering should not affect the cache value") + _, ok = cache.listenerPod.Get(mainObject, desiredPod, configSecret, serviceAccount, role) + assert.True(t, ok) + + configSecret.ResourceVersion = "2" + value, replaced = cache.listenerPod.Upsert(mainObject, desiredPod, configSecret, serviceAccount, role) + assert.True(t, replaced) + 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) + + assert.Same(t, desiredPod, value.Object) +} + +func TestResourceCacheDeleteRemovesMainObjectEntries(t *testing.T) { + mainObject := &v1alpha1.AutoscalingListener{ + ObjectMeta: metav1.ObjectMeta{ + Name: "listener", + Namespace: "controller-ns", + UID: "listener-uid", + }, + } + otherMainObject := &v1alpha1.AutoscalingListener{ + ObjectMeta: metav1.ObjectMeta{ + Name: "other-listener", + Namespace: "controller-ns", + UID: "other-listener-uid", + }, + } + listenerPod := &corev1.Pod{ObjectMeta: metav1.ObjectMeta{Name: "listener", Namespace: "controller-ns"}} + listenerServiceAccount := &corev1.ServiceAccount{ObjectMeta: metav1.ObjectMeta{Name: "listener", Namespace: "controller-ns"}} + otherListenerPod := &corev1.Pod{ObjectMeta: metav1.ObjectMeta{Name: "other-listener", Namespace: "controller-ns"}} + + cache := NewResourceCache() + cache.listenerPod.Upsert(mainObject, listenerPod) + cache.listenerServiceAccount.Upsert(mainObject, listenerServiceAccount) + cache.listenerPod.Upsert(otherMainObject, otherListenerPod) + + cache.Delete(mainObject) + + _, ok := cache.listenerPod.Get(mainObject, listenerPod) + assert.False(t, ok) + _, ok = cache.listenerServiceAccount.Get(mainObject, listenerServiceAccount) + assert.False(t, ok) + _, ok = cache.listenerPod.Get(otherMainObject, otherListenerPod) + assert.True(t, ok) +} + +func TestResourceCacheDeletePanicsWithNilCache(t *testing.T) { + var cache *ResourceCache + assert.Panics(t, func() { + cache.Delete(&v1alpha1.AutoscalingListener{}) + }) +} + +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{ + Name: "listener", + Namespace: "controller-ns", + UID: "listener-uid", + Annotations: map[string]string{ + annotationKeyIntegrityHash: "listener-hash", + }, + }, + Spec: v1alpha1.AutoscalingListenerSpec{ + Image: "listener:latest", + AutoscalingRunnerSetName: "scale-set", + AutoscalingRunnerSetNamespace: "scale-set-ns", + EphemeralRunnerSetName: "scale-set", + }, + } + podConfig := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "listener-config", + Namespace: "controller-ns", + UID: "config-secret-uid", + ResourceVersion: "11", + Annotations: map[string]string{ + annotationKeyIntegrityHash: "config-hash", + }, + }, + } + serviceAccount := &corev1.ServiceAccount{ + ObjectMeta: metav1.ObjectMeta{ + Name: "listener", + Namespace: "controller-ns", + UID: "service-account-uid", + ResourceVersion: "12", + Annotations: map[string]string{ + annotationKeyIntegrityHash: "service-account-hash", + }, + }, + } + role := &rbacv1.Role{ + ObjectMeta: metav1.ObjectMeta{ + Name: "listener", + Namespace: "scale-set-ns", + UID: "role-uid", + ResourceVersion: "13", + Annotations: map[string]string{ + annotationKeyIntegrityHash: "role-hash", + }, + }, + } + roleBinding := &rbacv1.RoleBinding{ + ObjectMeta: metav1.ObjectMeta{ + Name: "listener", + Namespace: "scale-set-ns", + UID: "role-binding-uid", + ResourceVersion: "14", + Annotations: map[string]string{ + annotationKeyIntegrityHash: "role-binding-hash", + }, + }, + } + + cache := NewResourceCache() + b := ResourceBuilder{ResourceCache: &cache} + listenerPod, err := b.newScaleSetListenerPod(listener, podConfig, serviceAccount, role, roleBinding, nil) + require.NoError(t, err) + + cachedPod, ok := b.ResourceCache.listenerPod.Get(listener, listenerPod, podConfig, serviceAccount, role, roleBinding) + require.True(t, ok) + assert.IsType(t, &corev1.Pod{}, cachedPod) + + role.ResourceVersion = "changed" + _, ok = b.ResourceCache.listenerPod.Get(listener, listenerPod, podConfig, serviceAccount, role, roleBinding) + assert.False(t, ok) +} + +func TestResourceBuilderCachesEphemeralRunnerSet(t *testing.T) { + autoscalingRunnerSet := v1alpha1.AutoscalingRunnerSet{ + ObjectMeta: metav1.ObjectMeta{ + Name: "scale-set", + Namespace: "default", + UID: "scale-set-uid", + Annotations: map[string]string{ + runnerScaleSetIDAnnotationKey: "1", + }, + }, + Spec: v1alpha1.AutoscalingRunnerSetSpec{ + GitHubConfigUrl: "https://github.com/actions/actions-runner-controller", + }, + } + + cache := NewResourceCache() + b := ResourceBuilder{ResourceCache: &cache} + runnerSet, err := b.newEphemeralRunnerSet(&autoscalingRunnerSet) + require.NoError(t, err) + + cachedRunnerSet, ok := b.ResourceCache.ephemeralRunnerSet.Get(&autoscalingRunnerSet, runnerSet) + require.True(t, ok) + assert.Equal(t, runnerSet.Spec, cachedRunnerSet.Spec) + assert.Same(t, runnerSet, cachedRunnerSet) + + fromBuilder, err := b.newEphemeralRunnerSet(&autoscalingRunnerSet) + require.NoError(t, err) + assert.Same(t, runnerSet, fromBuilder) + + autoscalingRunnerSet.Annotations[runnerScaleSetIDAnnotationKey] = "2" + _, ok = b.ResourceCache.ephemeralRunnerSet.Get(&autoscalingRunnerSet, runnerSet) + assert.False(t, ok) +} diff --git a/main.go b/main.go index 72c65f6bbb..5ec4a56645 100644 --- a/main.go +++ b/main.go @@ -218,6 +218,7 @@ func main() { } actionsgithubcom.SetListenerEntrypoint(os.Getenv("LISTENER_ENTRYPOINT")) + resourceCache := actionsgithubcom.NewResourceCache() var webhookServer webhook.Server if port != 0 { @@ -303,6 +304,7 @@ func main() { ExcludeLabelPropagationPrefixes: excludeLabelPropagationPrefixes, SecretResolver: secretResolver, Scheme: mgr.GetScheme(), + ResourceCache: &resourceCache, } log.Info("Resource builder initializing")