From daca163ca10da766678c6504a121ecd8d638f57e Mon Sep 17 00:00:00 2001 From: Nikola Jokic Date: Wed, 22 Jul 2026 14:48:54 +0200 Subject: [PATCH] wip --- .github/workflows/arc-publish-chart.yaml | 2 +- .github/workflows/arc-validate-chart.yaml | 2 +- .github/workflows/gha-publish-chart.yaml | 16 +- .github/workflows/gha-validate-chart.yaml | 4 +- .github/workflows/global-publish-canary.yaml | 8 +- .github/workflows/go.yaml | 2 +- .../autoscalinglistener_controller.go | 18 +- .../autoscalinglistener_controller_test.go | 194 ++++---------- .../ephemeralrunner_controller.go | 44 ++-- controllers/actions.github.com/helpers.go | 24 -- .../actions.github.com/helpers_test.go | 197 -------------- .../actions.github.com/resourcebuilder.go | 53 ++-- .../resourcebuilder_test.go | 27 ++ .../actions.github.com/resourcecache.go | 60 +---- .../actions.github.com/resourcecache_test.go | 244 +----------------- 15 files changed, 161 insertions(+), 734 deletions(-) diff --git a/.github/workflows/arc-publish-chart.yaml b/.github/workflows/arc-publish-chart.yaml index 5d1983a2..9e4fde5a 100644 --- a/.github/workflows/arc-publish-chart.yaml +++ b/.github/workflows/arc-publish-chart.yaml @@ -45,7 +45,7 @@ jobs: fetch-depth: 0 - name: Set up Helm - uses: azure/setup-helm@9bc31f4ebc9c6b171d7bfbaa5d006ae7abdb4310 + uses: azure/setup-helm@dda3372f752e03dde6b3237bc9431cdc2f7a02a2 with: version: ${{ env.HELM_VERSION }} diff --git a/.github/workflows/arc-validate-chart.yaml b/.github/workflows/arc-validate-chart.yaml index e9ea3ba0..f905306c 100644 --- a/.github/workflows/arc-validate-chart.yaml +++ b/.github/workflows/arc-validate-chart.yaml @@ -45,7 +45,7 @@ jobs: fetch-depth: 0 - name: Set up Helm - uses: azure/setup-helm@9bc31f4ebc9c6b171d7bfbaa5d006ae7abdb4310 + uses: azure/setup-helm@dda3372f752e03dde6b3237bc9431cdc2f7a02a2 with: version: ${{ env.HELM_VERSION }} diff --git a/.github/workflows/gha-publish-chart.yaml b/.github/workflows/gha-publish-chart.yaml index 8bcb4076..af42962b 100644 --- a/.github/workflows/gha-publish-chart.yaml +++ b/.github/workflows/gha-publish-chart.yaml @@ -82,10 +82,10 @@ jobs: echo "repository_owner=$(echo ${{ github.repository_owner }} | tr '[:upper:]' '[:lower:]')" >> $GITHUB_OUTPUT - name: Set up QEMU - uses: docker/setup-qemu-action@96fe6ef7f33517b61c61be40b68a1882f3264fb8 + uses: docker/setup-qemu-action@06116385d9baf250c9f4dcb4858b16962ea869c3 - name: Set up Docker Buildx - uses: docker/setup-buildx-action@bb05f3f5519dd87d3ba754cc423b652a5edd6d2c + uses: docker/setup-buildx-action@d7f5e7f509e45cec5c76c4d5afdd7de93d0b3df5 with: # Pinning v0.9.1 for Buildx and BuildKit v0.10.6 # BuildKit v0.11 which has a bug causing intermittent @@ -94,14 +94,14 @@ jobs: driver-opts: image=moby/buildkit:v0.10.6 - name: Login to GitHub Container Registry - uses: docker/login-action@af1e73f918a031802d376d3c8bbc3fe56130a9b0 + uses: docker/login-action@650006c6eb7dba73a995cc03b0b2d7f5ca915bee with: registry: ghcr.io username: ${{ github.actor }} password: ${{ secrets.GITHUB_TOKEN }} - name: Build & push controller image - uses: docker/build-push-action@53b7df96c91f9c12dcc8a07bcb9ccacbed38856a + uses: docker/build-push-action@f9f3042f7e2789586610d6e8b85c8f03e5195baf with: context: . file: Dockerfile @@ -149,7 +149,7 @@ jobs: echo "repository_owner=$(echo ${{ github.repository_owner }} | tr '[:upper:]' '[:lower:]')" >> $GITHUB_OUTPUT - name: Set up Helm - uses: azure/setup-helm@9bc31f4ebc9c6b171d7bfbaa5d006ae7abdb4310 + uses: azure/setup-helm@dda3372f752e03dde6b3237bc9431cdc2f7a02a2 with: version: ${{ env.HELM_VERSION }} @@ -197,7 +197,7 @@ jobs: echo "repository_owner=$(echo ${{ github.repository_owner }} | tr '[:upper:]' '[:lower:]')" >> $GITHUB_OUTPUT - name: Set up Helm - uses: azure/setup-helm@9bc31f4ebc9c6b171d7bfbaa5d006ae7abdb4310 + uses: azure/setup-helm@dda3372f752e03dde6b3237bc9431cdc2f7a02a2 with: version: ${{ env.HELM_VERSION }} @@ -244,7 +244,7 @@ jobs: echo "repository_owner=$(echo ${{ github.repository_owner }} | tr '[:upper:]' '[:lower:]')" >> $GITHUB_OUTPUT - name: Set up Helm - uses: azure/setup-helm@9bc31f4ebc9c6b171d7bfbaa5d006ae7abdb4310 + uses: azure/setup-helm@dda3372f752e03dde6b3237bc9431cdc2f7a02a2 with: version: ${{ env.HELM_VERSION }} @@ -293,7 +293,7 @@ jobs: echo "repository_owner=$(echo ${{ github.repository_owner }} | tr '[:upper:]' '[:lower:]')" >> $GITHUB_OUTPUT - name: Set up Helm - uses: azure/setup-helm@9bc31f4ebc9c6b171d7bfbaa5d006ae7abdb4310 + uses: azure/setup-helm@dda3372f752e03dde6b3237bc9431cdc2f7a02a2 with: version: ${{ env.HELM_VERSION }} diff --git a/.github/workflows/gha-validate-chart.yaml b/.github/workflows/gha-validate-chart.yaml index 8e6ed541..949432ce 100644 --- a/.github/workflows/gha-validate-chart.yaml +++ b/.github/workflows/gha-validate-chart.yaml @@ -41,7 +41,7 @@ jobs: fetch-depth: 0 - name: Set up Helm - uses: azure/setup-helm@9bc31f4ebc9c6b171d7bfbaa5d006ae7abdb4310 + uses: azure/setup-helm@dda3372f752e03dde6b3237bc9431cdc2f7a02a2 with: version: ${{ env.HELM_VERSION }} @@ -79,7 +79,7 @@ jobs: fetch-depth: 0 - name: Set up Helm - uses: azure/setup-helm@9bc31f4ebc9c6b171d7bfbaa5d006ae7abdb4310 + uses: azure/setup-helm@dda3372f752e03dde6b3237bc9431cdc2f7a02a2 with: version: ${{ env.HELM_VERSION }} diff --git a/.github/workflows/global-publish-canary.yaml b/.github/workflows/global-publish-canary.yaml index 0a177565..7adce966 100644 --- a/.github/workflows/global-publish-canary.yaml +++ b/.github/workflows/global-publish-canary.yaml @@ -93,7 +93,7 @@ jobs: uses: actions/checkout@v7 - name: Login to GitHub Container Registry - uses: docker/login-action@af1e73f918a031802d376d3c8bbc3fe56130a9b0 + uses: docker/login-action@650006c6eb7dba73a995cc03b0b2d7f5ca915bee with: registry: ghcr.io username: ${{ github.actor }} @@ -110,16 +110,16 @@ jobs: echo "repository_owner=$(echo ${{ github.repository_owner }} | tr '[:upper:]' '[:lower:]')" >> $GITHUB_OUTPUT - name: Set up QEMU - uses: docker/setup-qemu-action@96fe6ef7f33517b61c61be40b68a1882f3264fb8 + uses: docker/setup-qemu-action@06116385d9baf250c9f4dcb4858b16962ea869c3 - name: Set up Docker Buildx - uses: docker/setup-buildx-action@bb05f3f5519dd87d3ba754cc423b652a5edd6d2c + uses: docker/setup-buildx-action@d7f5e7f509e45cec5c76c4d5afdd7de93d0b3df5 with: version: latest # Unstable builds - run at your own risk - name: Build and Push - uses: docker/build-push-action@53b7df96c91f9c12dcc8a07bcb9ccacbed38856a + uses: docker/build-push-action@f9f3042f7e2789586610d6e8b85c8f03e5195baf with: context: . file: ./Dockerfile diff --git a/.github/workflows/go.yaml b/.github/workflows/go.yaml index 402b6c2a..6a30459f 100644 --- a/.github/workflows/go.yaml +++ b/.github/workflows/go.yaml @@ -48,7 +48,7 @@ jobs: go-version-file: "go.mod" cache: false - name: golangci-lint - uses: golangci/golangci-lint-action@ba0d7d2ec06a0ea1cb5fa41b2e4a3ab91d21278a + uses: golangci/golangci-lint-action@82606bf257cbaff209d206a39f5134f0cfbfd2ee with: only-new-issues: true version: v2.11.2 diff --git a/controllers/actions.github.com/autoscalinglistener_controller.go b/controllers/actions.github.com/autoscalinglistener_controller.go index d473b843..266d9d60 100644 --- a/controllers/actions.github.com/autoscalinglistener_controller.go +++ b/controllers/actions.github.com/autoscalinglistener_controller.go @@ -209,7 +209,7 @@ func (r *AutoscalingListenerReconciler) Reconcile(ctx context.Context, req ctrl. desiredRole := r.newScaleSetListenerRole(&autoscalingListener) desiredLabels := r.filterAndMergeLabels(listenerRole.Labels, desiredRole.Labels) labelsModified := !maps.Equal(listenerRole.Labels, desiredLabels) - desiredAnnotations := desiredRole.Annotations + desiredAnnotations := r.mergeAnnotations(listenerRole.Annotations, desiredRole.Annotations) annotationsModified := !maps.Equal(listenerRole.Annotations, desiredAnnotations) rulesModified := !reflect.DeepEqual(listenerRole.Rules, desiredRole.Rules) if labelsModified || annotationsModified || rulesModified { @@ -251,7 +251,7 @@ func (r *AutoscalingListenerReconciler) Reconcile(ctx context.Context, req ctrl. ) desiredLabels := r.filterAndMergeLabels(listenerRoleBinding.Labels, desiredRoleBinding.Labels) labelsModified := !maps.Equal(listenerRoleBinding.Labels, desiredLabels) - desiredAnnotations := desiredRoleBinding.Annotations + desiredAnnotations := r.mergeAnnotations(listenerRoleBinding.Annotations, desiredRoleBinding.Annotations) annotationsModified := !maps.Equal(listenerRoleBinding.Annotations, desiredAnnotations) if labelsModified || annotationsModified { updatedRoleBinding := listenerRoleBinding.DeepCopy() @@ -306,7 +306,7 @@ func (r *AutoscalingListenerReconciler) Reconcile(ctx context.Context, req ctrl. } desiredLabels := r.filterAndMergeLabels(proxySecret.Labels, desiredListenerProxy.Labels) labelsModified := !maps.Equal(proxySecret.Labels, desiredLabels) - desiredAnnotations := desiredListenerProxy.Annotations + desiredAnnotations := r.mergeAnnotations(proxySecret.Annotations, desiredListenerProxy.Annotations) annotationsModified := !maps.Equal(proxySecret.Annotations, desiredAnnotations) if labelsModified || annotationsModified { updatedProxySecret := proxySecret.DeepCopy() @@ -392,7 +392,7 @@ func (r *AutoscalingListenerReconciler) Reconcile(ctx context.Context, req ctrl. } desiredLabels := r.filterAndMergeLabels(listenerConfigSecret.Labels, desiredSecret.Labels) labelsModified := !maps.Equal(listenerConfigSecret.Labels, desiredLabels) - desiredAnnotations := desiredSecret.Annotations + desiredAnnotations := r.mergeAnnotations(listenerConfigSecret.Annotations, desiredSecret.Annotations) annotationsModified := !maps.Equal(listenerConfigSecret.Annotations, desiredAnnotations) if labelsModified || annotationsModified { @@ -463,11 +463,6 @@ func (r *AutoscalingListenerReconciler) Reconcile(ctx context.Context, req ctrl. return ctrl.Result{}, err } - desiredLabels := r.filterAndMergeLabels(listenerPod.Labels, desiredPod.Labels) - labelsModified := !maps.Equal(listenerPod.Labels, desiredLabels) - desiredAnnotations := r.mergeAnnotations(listenerPod.Annotations, desiredPod.Annotations) - annotationsModified := !maps.Equal(listenerPod.Annotations, desiredAnnotations) - shouldReCreate := listenerPodSpecRequiresRecreation(&listenerPod, desiredPod) if shouldReCreate { log.Info("Listener pod dependency changed, recreating listener pod") @@ -479,6 +474,11 @@ func (r *AutoscalingListenerReconciler) Reconcile(ctx context.Context, req ctrl. return ctrl.Result{}, nil } + desiredLabels := r.filterAndMergeLabels(listenerPod.Labels, desiredPod.Labels) + labelsModified := !maps.Equal(listenerPod.Labels, desiredLabels) + desiredAnnotations := r.mergeAnnotations(listenerPod.Annotations, desiredPod.Annotations) + annotationsModified := !maps.Equal(listenerPod.Annotations, desiredAnnotations) + if labelsModified || annotationsModified { updatedPod := listenerPod.DeepCopy() if labelsModified { diff --git a/controllers/actions.github.com/autoscalinglistener_controller_test.go b/controllers/actions.github.com/autoscalinglistener_controller_test.go index 9d328703..67f7ecf5 100644 --- a/controllers/actions.github.com/autoscalinglistener_controller_test.go +++ b/controllers/actions.github.com/autoscalinglistener_controller_test.go @@ -401,8 +401,6 @@ var _ = Describe("Test AutoScalingListener controller", func() { autoscalingListenerTestInterval, ).Should(BeEquivalentTo(autoscalingListener.Name), "Pod should be created") - oldPodUID := string(pod.UID) - // Update the AutoScalingListener updated := autoscalingListener.DeepCopy() updated.Spec.EphemeralRunnerSetName = "test-ers-updated" @@ -423,20 +421,6 @@ var _ = Describe("Test AutoScalingListener controller", func() { autoscalingListenerTestTimeout, autoscalingListenerTestInterval, ).Should(BeEquivalentTo(rulesForListenerRole([]string{updated.Spec.EphemeralRunnerSetName})), "Role should be updated") - - Eventually( - func() (string, error) { - pod := new(corev1.Pod) - err := k8sClient.Get(ctx, client.ObjectKey{Name: autoscalingListener.Name, Namespace: autoscalingListener.Namespace}, pod) - if err != nil { - return "", err - } - - return string(pod.UID), nil - }, - autoscalingListenerTestTimeout, - autoscalingListenerTestInterval, - ).Should(BeEquivalentTo(oldPodUID), "Pod should not be re-created when only listener role rules change") }) It("propagates updated listener metadata to owned resources", func() { @@ -447,25 +431,37 @@ var _ = Describe("Test AutoScalingListener controller", func() { err := k8sClient.Get(ctx, client.ObjectKey{Name: autoscalingListener.Name, Namespace: autoscalingListener.Namespace}, serviceAccount) g.Expect(err).NotTo(HaveOccurred(), "failed to get ServiceAccount") g.Expect(serviceAccount.Labels["arc.test/listener-label"]).To(Equal(expected)) - g.Expect(serviceAccount.Annotations["arc.test/service-account-annotation"]).To(Equal("initial")) + g.Expect(serviceAccount.Annotations["arc.test/service-account-annotation"]).To(Equal(expected)) + if expected == "updated" { + g.Expect(serviceAccount.Annotations["arc.test/new-service-account-annotation"]).To(Equal("added")) + } role := new(rbacv1.Role) err = k8sClient.Get(ctx, client.ObjectKey{Name: autoscalingListener.Name, Namespace: autoscalingListener.Spec.AutoscalingRunnerSetNamespace}, role) g.Expect(err).NotTo(HaveOccurred(), "failed to get Role") g.Expect(role.Labels["arc.test/listener-label"]).To(Equal(expected)) - g.Expect(role.Annotations["arc.test/role-annotation"]).To(Equal("initial")) + g.Expect(role.Annotations["arc.test/role-annotation"]).To(Equal(expected)) + if expected == "updated" { + g.Expect(role.Annotations["arc.test/new-role-annotation"]).To(Equal("added")) + } roleBinding := new(rbacv1.RoleBinding) err = k8sClient.Get(ctx, client.ObjectKey{Name: autoscalingListener.Name, Namespace: autoscalingListener.Spec.AutoscalingRunnerSetNamespace}, roleBinding) g.Expect(err).NotTo(HaveOccurred(), "failed to get RoleBinding") g.Expect(roleBinding.Labels["arc.test/listener-label"]).To(Equal(expected)) - g.Expect(roleBinding.Annotations["arc.test/role-binding-annotation"]).To(Equal("initial")) + g.Expect(roleBinding.Annotations["arc.test/role-binding-annotation"]).To(Equal(expected)) + if expected == "updated" { + g.Expect(roleBinding.Annotations["arc.test/new-role-binding-annotation"]).To(Equal("added")) + } secret := new(corev1.Secret) err = k8sClient.Get(ctx, client.ObjectKey{Name: scaleSetListenerConfigName(autoscalingListener), Namespace: autoscalingListener.Namespace}, secret) g.Expect(err).NotTo(HaveOccurred(), "failed to get config Secret") - g.Expect(secret.Labels["arc.test/config-secret-label"]).To(Equal("initial")) - g.Expect(secret.Annotations["arc.test/config-secret-annotation"]).To(Equal("initial")) + g.Expect(secret.Labels["arc.test/config-secret-label"]).To(Equal(expected)) + g.Expect(secret.Annotations["arc.test/config-secret-annotation"]).To(Equal(expected)) + if expected == "updated" { + g.Expect(secret.Annotations["arc.test/new-config-secret-annotation"]).To(Equal("added")) + } pod := new(corev1.Pod) err = k8sClient.Get(ctx, client.ObjectKey{Name: autoscalingListener.Name, Namespace: autoscalingListener.Namespace}, pod) @@ -479,84 +475,45 @@ var _ = Describe("Test AutoScalingListener controller", func() { assertPropagatedMetadata("initial") - pod := new(corev1.Pod) - Eventually( - func() (string, error) { - err := k8sClient.Get(ctx, client.ObjectKey{Name: autoscalingListener.Name, Namespace: autoscalingListener.Namespace}, pod) - if err != nil { - return "", err - } - - return string(pod.UID), nil - }, - autoscalingListenerTestTimeout, - autoscalingListenerTestInterval, - ).ShouldNot(BeEmpty(), "Pod should be created") - - oldPodUID := string(pod.UID) - - serviceAccount := new(corev1.ServiceAccount) - err := k8sClient.Get(ctx, client.ObjectKey{Name: autoscalingListener.Name, Namespace: autoscalingListener.Namespace}, serviceAccount) - Expect(err).NotTo(HaveOccurred(), "failed to get ServiceAccount") - - updatedServiceAccount := serviceAccount.DeepCopy() - if updatedServiceAccount.Annotations == nil { - updatedServiceAccount.Annotations = make(map[string]string) - } - updatedServiceAccount.Annotations["arc.test/third-party-service-account-annotation"] = "preserved" - err = k8sClient.Patch(ctx, updatedServiceAccount, client.MergeFrom(serviceAccount)) - Expect(err).NotTo(HaveOccurred(), "failed to patch third-party ServiceAccount annotation") - - updatedPod := pod.DeepCopy() - if updatedPod.Annotations == nil { - updatedPod.Annotations = make(map[string]string) - } - updatedPod.Annotations["arc.test/third-party-pod-annotation"] = "preserved" - err = k8sClient.Patch(ctx, updatedPod, client.MergeFrom(pod)) - Expect(err).NotTo(HaveOccurred(), "failed to patch third-party Pod annotation") - current := new(v1alpha1.AutoscalingListener) - err = k8sClient.Get(ctx, client.ObjectKey{Name: autoscalingListener.Name, Namespace: autoscalingListener.Namespace}, current) + err := k8sClient.Get(ctx, client.ObjectKey{Name: autoscalingListener.Name, Namespace: autoscalingListener.Namespace}, current) Expect(err).NotTo(HaveOccurred(), "failed to get AutoScalingListener") updated := current.DeepCopy() updated.Labels = map[string]string{ "arc.test/listener-label": "updated", } + updated.Spec.ServiceAccountMetadata = &v1alpha1.ResourceMeta{ + Annotations: map[string]string{ + "arc.test/service-account-annotation": "updated", + "arc.test/new-service-account-annotation": "added", + }, + } + updated.Spec.RoleMetadata = &v1alpha1.ResourceMeta{ + Annotations: map[string]string{ + "arc.test/role-annotation": "updated", + "arc.test/new-role-annotation": "added", + }, + } + updated.Spec.RoleBindingMetadata = &v1alpha1.ResourceMeta{ + Annotations: map[string]string{ + "arc.test/role-binding-annotation": "updated", + "arc.test/new-role-binding-annotation": "added", + }, + } + updated.Spec.ConfigSecretMetadata = &v1alpha1.ResourceMeta{ + Labels: map[string]string{ + "arc.test/config-secret-label": "updated", + }, + Annotations: map[string]string{ + "arc.test/config-secret-annotation": "updated", + "arc.test/new-config-secret-annotation": "added", + }, + } err = k8sClient.Patch(ctx, updated, client.MergeFrom(current)) - Expect(err).NotTo(HaveOccurred(), "failed to patch AutoScalingListener labels") + Expect(err).NotTo(HaveOccurred(), "failed to patch AutoScalingListener metadata") assertPropagatedMetadata("updated") - - Eventually( - func(g Gomega) { - serviceAccount := new(corev1.ServiceAccount) - err := k8sClient.Get(ctx, client.ObjectKey{Name: autoscalingListener.Name, Namespace: autoscalingListener.Namespace}, serviceAccount) - g.Expect(err).NotTo(HaveOccurred(), "failed to get ServiceAccount") - g.Expect(serviceAccount.Annotations).To(HaveKeyWithValue("arc.test/third-party-service-account-annotation", "preserved")) - - pod := new(corev1.Pod) - err = k8sClient.Get(ctx, client.ObjectKey{Name: autoscalingListener.Name, Namespace: autoscalingListener.Namespace}, pod) - g.Expect(err).NotTo(HaveOccurred(), "failed to get Pod") - g.Expect(pod.Annotations).To(HaveKeyWithValue("arc.test/third-party-pod-annotation", "preserved")) - }, - autoscalingListenerTestTimeout, - autoscalingListenerTestInterval, - ).Should(Succeed()) - - Eventually( - func() (string, error) { - pod := new(corev1.Pod) - err := k8sClient.Get(ctx, client.ObjectKey{Name: autoscalingListener.Name, Namespace: autoscalingListener.Namespace}, pod) - if err != nil { - return "", err - } - - return string(pod.UID), nil - }, - autoscalingListenerTestTimeout, - autoscalingListenerTestInterval, - ).Should(BeEquivalentTo(oldPodUID), "Pod should be patched, not re-created, for metadata-only updates") }) It("It should re-create pod but persist config secret whenever listener container is terminated", func() { @@ -631,7 +588,6 @@ var _ = Describe("Test AutoScalingListener controller", func() { autoscalingListenerTestInterval, ).Should(BeEquivalentTo(oldSecretUID), "Config secret should persist (not be re-created)") }) - }) }) @@ -1141,64 +1097,6 @@ var _ = Describe("Test AutoScalingListener controller with proxy", func() { autoscalingListenerTestInterval, ).Should(Succeed(), "failed to delete secret with proxy details") }) - - It("should re-create listener pod when proxy dependency changes", func() { - proxy := &v1alpha1.ProxyConfig{ - HTTP: &v1alpha1.ProxyServerConfig{ - Url: "http://localhost:8080", - }, - NoProxy: []string{"example.com"}, - } - - createRunnerSetAndListener(proxy) - - pod := new(corev1.Pod) - Eventually( - func() (string, error) { - err := k8sClient.Get( - ctx, - client.ObjectKey{Name: autoscalingListener.Name, Namespace: autoscalingListener.Namespace}, - pod, - ) - if err != nil { - return "", err - } - - return string(pod.UID), nil - }, - autoscalingListenerTestTimeout, - autoscalingListenerTestInterval, - ).ShouldNot(BeEmpty(), "Pod should be created") - - oldPodUID := string(pod.UID) - - current := new(v1alpha1.AutoscalingListener) - err := k8sClient.Get(ctx, client.ObjectKey{Name: autoscalingListener.Name, Namespace: autoscalingListener.Namespace}, current) - Expect(err).NotTo(HaveOccurred(), "failed to get AutoScalingListener") - - updated := current.DeepCopy() - updated.Spec.Proxy.NoProxy = []string{"example.com", "example.org"} - err = k8sClient.Patch(ctx, updated, client.MergeFrom(current)) - Expect(err).NotTo(HaveOccurred(), "failed to patch AutoScalingListener proxy") - - Eventually( - func() (string, error) { - pod := new(corev1.Pod) - err := k8sClient.Get( - ctx, - client.ObjectKey{Name: autoscalingListener.Name, Namespace: autoscalingListener.Namespace}, - pod, - ) - if err != nil { - return "", err - } - - return string(pod.UID), nil - }, - autoscalingListenerTestTimeout, - autoscalingListenerTestInterval, - ).Should(BeEquivalentTo(oldPodUID), "Pod should not be re-created when proxy metadata updates do not change pod spec") - }) }) var _ = Describe("Test AutoScalingListener controller with template modification", func() { diff --git a/controllers/actions.github.com/ephemeralrunner_controller.go b/controllers/actions.github.com/ephemeralrunner_controller.go index 3371b585..256ed311 100644 --- a/controllers/actions.github.com/ephemeralrunner_controller.go +++ b/controllers/actions.github.com/ephemeralrunner_controller.go @@ -186,8 +186,8 @@ func (r *EphemeralRunnerReconciler) Reconcile(ctx context.Context, req ctrl.Requ log.Info("Successfully added finalizers") } - var secret corev1.Secret - if err := r.Get(ctx, req.NamespacedName, &secret); err != nil { + secret := new(corev1.Secret) + if err := r.Get(ctx, req.NamespacedName, secret); err != nil { if !kerrors.IsNotFound(err) { log.Error(err, "Failed to fetch secret") return ctrl.Result{}, err @@ -203,7 +203,7 @@ func (r *EphemeralRunnerReconciler) Reconcile(ctx context.Context, req ctrl.Requ return ctrl.Result{}, fmt.Errorf("failed to create secret: %w", err) } log.Info("Created new ephemeral runner secret for jitconfig.") - secret = *jitSecret + secret = jitSecret case errors.Is(err, retryableError): log.Info("Encountered retryable error, requeueing", "error", err.Error()) @@ -227,7 +227,7 @@ func (r *EphemeralRunnerReconciler) Reconcile(ctx context.Context, req ctrl.Requ if err != nil { log.Error(err, "Runner config secret is corrupted: missing runnerId") log.Info("Deleting corrupted runner config secret") - if err := r.Delete(ctx, &secret); err != nil { + if err := r.Delete(ctx, secret); err != nil { return ctrl.Result{}, fmt.Errorf("failed to delete the corrupted runner config secret") } log.Info("Corrupted runner config secret has been deleted") @@ -273,15 +273,15 @@ func (r *EphemeralRunnerReconciler) Reconcile(ctx context.Context, req ctrl.Requ }, nil } - var pod corev1.Pod - if err := r.Get(ctx, req.NamespacedName, &pod); err != nil { + pod := new(corev1.Pod) + if err := r.Get(ctx, req.NamespacedName, pod); err != nil { if !kerrors.IsNotFound(err) { log.Error(err, "Failed to fetch the pod") return ctrl.Result{}, err } log.Info("Ephemeral runner pod does not exist. Creating new ephemeral runner") - result, err := r.createPod(ctx, &ephemeralRunner, &secret, log) + result, err := r.createPod(ctx, &ephemeralRunner, secret, log) switch { case err == nil: return result, nil @@ -329,7 +329,7 @@ func (r *EphemeralRunnerReconciler) Reconcile(ctx context.Context, req ctrl.Requ } } - cs := runnerContainerStatus(&pod) + cs := runnerContainerStatus(pod) switch { case pod.Status.Phase == corev1.PodFailed: // All containers are stopped log.Info( @@ -342,7 +342,7 @@ func (r *EphemeralRunnerReconciler) Reconcile(ctx context.Context, req ctrl.Requ // Therefore, we should try to restart it. if cs == nil || cs.State.Terminated == nil { log.Info("Runner container does not have state set, deleting pod as failed so it can be restarted") - return ctrl.Result{}, r.deleteEphemeralRunnerOrPod(ctx, &ephemeralRunner, &pod, log) + return ctrl.Result{}, r.deleteEphemeralRunnerOrPod(ctx, &ephemeralRunner, pod, log) } switch cs.State.Terminated.ExitCode { @@ -352,7 +352,7 @@ func (r *EphemeralRunnerReconciler) Reconcile(ctx context.Context, req ctrl.Requ // If the runner container exits with 0, we assume that the runner has finished successfully. // If side-car container exits with non-zero, it shouldn't affect the runner. Runner exit code // drives the controller's inference of whether the job has succeeded or failed. - if err := r.markAsSucceeded(ctx, &ephemeralRunner, &pod, log); err != nil { + if err := r.markAsSucceeded(ctx, &ephemeralRunner, pod, log); err != nil { log.Error(err, "Failed to set ephemeral runner to phase Succeeded") return ctrl.Result{}, err } @@ -374,14 +374,14 @@ func (r *EphemeralRunnerReconciler) Reconcile(ctx context.Context, req ctrl.Requ "Ephemeral runner container has failed, and runner container termination exit code is non-zero", "containerTerminatedState", cs.State.Terminated, ) - return ctrl.Result{}, r.deleteEphemeralRunnerOrPod(ctx, &ephemeralRunner, &pod, log) + return ctrl.Result{}, r.deleteEphemeralRunnerOrPod(ctx, &ephemeralRunner, pod, log) - case initContainerFailed(&pod): + case initContainerFailed(pod): log.Info( "Pod has a failed init container, deleting pod as failed so it can be restarted", "initContainerStatuses", pod.Status.InitContainerStatuses, ) - return ctrl.Result{}, r.deleteEphemeralRunnerOrPod(ctx, &ephemeralRunner, &pod, log) + return ctrl.Result{}, r.deleteEphemeralRunnerOrPod(ctx, &ephemeralRunner, pod, log) case cs == nil: // starting, no container state yet @@ -390,7 +390,7 @@ func (r *EphemeralRunnerReconciler) Reconcile(ctx context.Context, req ctrl.Requ case cs.State.Terminated == nil: // container is not terminated and pod phase is not failed, so runner is still running log.Info("Runner container is still running; updating ephemeral runner status") - if err := r.updateRunStatusFromPod(ctx, &ephemeralRunner, &pod, log); err != nil { + if err := r.updateRunStatusFromPod(ctx, &ephemeralRunner, pod, log); err != nil { log.Info("Failed to update ephemeral runner status. Requeue to not miss this event") return ctrl.Result{}, err } @@ -405,11 +405,11 @@ func (r *EphemeralRunnerReconciler) Reconcile(ctx context.Context, req ctrl.Requ case cs.State.Terminated.ExitCode != 0: // failed log.Info("Ephemeral runner container failed", "exitCode", cs.State.Terminated.ExitCode) - return ctrl.Result{}, r.deleteEphemeralRunnerOrPod(ctx, &ephemeralRunner, &pod, log) + return ctrl.Result{}, r.deleteEphemeralRunnerOrPod(ctx, &ephemeralRunner, pod, log) default: // succeeded log.Info("Ephemeral runner has finished successfully, deleting ephemeral runner", "exitCode", cs.State.Terminated.ExitCode) - if err := r.markAsSucceeded(ctx, &ephemeralRunner, &pod, log); err != nil { + if err := r.markAsSucceeded(ctx, &ephemeralRunner, pod, log); err != nil { log.Error(err, "Failed to set ephemeral runner to phase Succeeded") return ctrl.Result{}, err } @@ -471,13 +471,13 @@ func (r *EphemeralRunnerReconciler) cleanupRunnerFromService(ctx context.Context func (r *EphemeralRunnerReconciler) cleanupResources(ctx context.Context, ephemeralRunner *v1alpha1.EphemeralRunner, log logr.Logger) error { log.Info("Cleaning up the runner pod") - var pod corev1.Pod - err := r.Get(ctx, types.NamespacedName{Namespace: ephemeralRunner.Namespace, Name: ephemeralRunner.Name}, &pod) + pod := new(corev1.Pod) + err := r.Get(ctx, types.NamespacedName{Namespace: ephemeralRunner.Namespace, Name: ephemeralRunner.Name}, pod) switch { case err == nil: if pod.DeletionTimestamp.IsZero() { log.Info("Deleting the runner pod") - if err := r.Delete(ctx, &pod); err != nil && !kerrors.IsNotFound(err) { + if err := r.Delete(ctx, pod); err != nil && !kerrors.IsNotFound(err) { return fmt.Errorf("failed to delete pod: %w", err) } log.Info("Deleted the runner pod") @@ -491,13 +491,13 @@ func (r *EphemeralRunnerReconciler) cleanupResources(ctx context.Context, epheme } log.Info("Cleaning up the runner jitconfig secret") - var secret corev1.Secret - err = r.Get(ctx, types.NamespacedName{Namespace: ephemeralRunner.Namespace, Name: ephemeralRunner.Name}, &secret) + secret := new(corev1.Secret) + err = r.Get(ctx, types.NamespacedName{Namespace: ephemeralRunner.Namespace, Name: ephemeralRunner.Name}, secret) switch { case err == nil: if secret.DeletionTimestamp.IsZero() { log.Info("Deleting the jitconfig secret") - if err := r.Delete(ctx, &secret); err != nil && !kerrors.IsNotFound(err) { + if err := r.Delete(ctx, secret); err != nil && !kerrors.IsNotFound(err) { return fmt.Errorf("failed to delete secret: %w", err) } log.Info("Deleted jitconfig secret") diff --git a/controllers/actions.github.com/helpers.go b/controllers/actions.github.com/helpers.go index 8d90f5ae..72188ae6 100644 --- a/controllers/actions.github.com/helpers.go +++ b/controllers/actions.github.com/helpers.go @@ -5,13 +5,11 @@ import ( "github.com/google/go-cmp/cmp" corev1 "k8s.io/api/core/v1" apiequality "k8s.io/apimachinery/pkg/api/equality" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) var ( _ = ephemeralRunnerSetActionableSpecChanged _ = nextActionableRevision - _ = listenerPodCanonicalEqual ) func ephemeralRunnerSetActionableSpecChanged(current, desired *v1alpha1.EphemeralRunnerSet) bool { @@ -34,28 +32,6 @@ func nextActionableRevision(current *v1alpha1.EphemeralRunnerSet) int64 { return current.Status.AppliedActionableRevision + 1 } -func listenerPodCanonicalForComparison(pod *corev1.Pod) *corev1.Pod { - if pod == nil { - return nil - } - - canonical := pod.DeepCopy() - canonical.UID = "" - canonical.ResourceVersion = "" - canonical.ManagedFields = nil - canonical.CreationTimestamp = metav1.Time{} - canonical.DeletionTimestamp = nil - canonical.Finalizers = nil - canonical.Generation = 0 - canonical.Status = corev1.PodStatus{} - - return canonical -} - -func listenerPodCanonicalEqual(current, desired *corev1.Pod) bool { - return cmp.Equal(listenerPodCanonicalForComparison(current), listenerPodCanonicalForComparison(desired)) -} - func listenerPodSpecRequiresRecreation(current, desired *corev1.Pod) bool { if current == nil || desired == nil { return current != desired diff --git a/controllers/actions.github.com/helpers_test.go b/controllers/actions.github.com/helpers_test.go index f426db37..b7989586 100644 --- a/controllers/actions.github.com/helpers_test.go +++ b/controllers/actions.github.com/helpers_test.go @@ -2,11 +2,8 @@ package actionsgithubcom import ( "context" - "testing" - "github.com/actions/actions-runner-controller/apis/actions.github.com/v1alpha1" "github.com/onsi/ginkgo/v2" - "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "golang.org/x/sync/errgroup" corev1 "k8s.io/api/core/v1" @@ -88,197 +85,3 @@ func createDefaultSecret(t ginkgo.GinkgoTInterface, client client.Client, namesp return secret } - -func TestEphemeralRunnerSetActionableSpecChanged(t *testing.T) { - base := func() *v1alpha1.EphemeralRunnerSet { - return &v1alpha1.EphemeralRunnerSet{ - ObjectMeta: metav1.ObjectMeta{ - Labels: map[string]string{"app": "arc"}, - Annotations: map[string]string{"note": "keep"}, - }, - Spec: v1alpha1.EphemeralRunnerSetSpec{ - Replicas: 1, - PatchID: 10, - EphemeralRunnerSpec: v1alpha1.EphemeralRunnerSpec{ - PodTemplateSpec: corev1.PodTemplateSpec{ - Spec: corev1.PodSpec{ - Containers: []corev1.Container{{Name: "runner", Image: "ghcr.io/actions/runner:old"}}, - }, - }, - }, - EphemeralRunnerMetadata: &v1alpha1.ResourceMeta{ - Labels: map[string]string{"meta-label": "v1"}, - Annotations: map[string]string{"meta-annotation": "v1"}, - }, - }, - } - } - - tests := []struct { - name string - mutate func(current, desired *v1alpha1.EphemeralRunnerSet) - want bool - }{ - { - name: "ephemeral runner image change is actionable", - mutate: func(_ *v1alpha1.EphemeralRunnerSet, desired *v1alpha1.EphemeralRunnerSet) { - desired.Spec.EphemeralRunnerSpec.PodTemplateSpec.Spec.Containers[0].Image = "ghcr.io/actions/runner:new" - }, - want: true, - }, - { - name: "ephemeral runner template change is actionable", - mutate: func(_ *v1alpha1.EphemeralRunnerSet, desired *v1alpha1.EphemeralRunnerSet) { - desired.Spec.EphemeralRunnerSpec.PodTemplateSpec.Spec.NodeSelector = map[string]string{"kubernetes.io/os": "linux"} - }, - want: true, - }, - { - name: "replicas change is non-actionable", - mutate: func(_ *v1alpha1.EphemeralRunnerSet, desired *v1alpha1.EphemeralRunnerSet) { - desired.Spec.Replicas = 3 - }, - want: false, - }, - { - name: "patch id change is non-actionable", - mutate: func(_ *v1alpha1.EphemeralRunnerSet, desired *v1alpha1.EphemeralRunnerSet) { - desired.Spec.PatchID = 11 - }, - want: false, - }, - { - name: "set labels change is non-actionable", - mutate: func(_ *v1alpha1.EphemeralRunnerSet, desired *v1alpha1.EphemeralRunnerSet) { - desired.Labels["app"] = "changed" - }, - want: false, - }, - { - name: "set annotations change is non-actionable", - mutate: func(_ *v1alpha1.EphemeralRunnerSet, desired *v1alpha1.EphemeralRunnerSet) { - desired.Annotations["note"] = "changed" - }, - want: false, - }, - { - name: "ephemeral runner metadata change is non-actionable", - mutate: func(_ *v1alpha1.EphemeralRunnerSet, desired *v1alpha1.EphemeralRunnerSet) { - desired.Spec.EphemeralRunnerMetadata.Annotations["meta-annotation"] = "v2" - }, - want: false, - }, - { - name: "nil metadata transition is non-actionable", - mutate: func(current, desired *v1alpha1.EphemeralRunnerSet) { - current.Spec.EphemeralRunnerMetadata = nil - desired.Spec.EphemeralRunnerMetadata = &v1alpha1.ResourceMeta{Labels: map[string]string{"meta-label": "new"}} - }, - want: false, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - current := base() - desired := current.DeepCopy() - tt.mutate(current, desired) - - assert.Equal(t, tt.want, ephemeralRunnerSetActionableSpecChanged(current, desired)) - }) - } -} - -func TestNextActionableRevision(t *testing.T) { - tests := []struct { - name string - current *v1alpha1.EphemeralRunnerSet - want int64 - }{ - {name: "nil current starts at one", current: nil, want: 1}, - { - name: "spec revision ahead", - current: &v1alpha1.EphemeralRunnerSet{Spec: v1alpha1.EphemeralRunnerSetSpec{ActionableRevision: 3}, Status: v1alpha1.EphemeralRunnerSetStatus{AppliedActionableRevision: 2}}, - want: 4, - }, - { - name: "applied revision ahead", - current: &v1alpha1.EphemeralRunnerSet{Spec: v1alpha1.EphemeralRunnerSetSpec{ActionableRevision: 2}, Status: v1alpha1.EphemeralRunnerSetStatus{AppliedActionableRevision: 7}}, - want: 8, - }, - { - name: "equal revisions", - current: &v1alpha1.EphemeralRunnerSet{Spec: v1alpha1.EphemeralRunnerSetSpec{ActionableRevision: 5}, Status: v1alpha1.EphemeralRunnerSetStatus{AppliedActionableRevision: 5}}, - want: 6, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - assert.Equal(t, tt.want, nextActionableRevision(tt.current)) - }) - } -} - -func TestListenerPodCanonicalEqual(t *testing.T) { - base := &corev1.Pod{ - ObjectMeta: metav1.ObjectMeta{ - Name: "listener", - Namespace: "controller-ns", - UID: "uid-1", - ResourceVersion: "100", - Annotations: map[string]string{"keep": "v"}, - Labels: map[string]string{"app": "listener"}, - ManagedFields: []metav1.ManagedFieldsEntry{{Manager: "kube-controller-manager"}}, - }, - Spec: corev1.PodSpec{ - ServiceAccountName: "listener-sa", - Containers: []corev1.Container{{Name: "listener", Image: "ghcr.io/actions/listener:v1"}}, - }, - Status: corev1.PodStatus{Phase: corev1.PodRunning}, - } - - tests := []struct { - name string - mutate func(current, desired *corev1.Pod) - want bool - }{ - { - name: "ignores runtime fields", - mutate: func(current, desired *corev1.Pod) { - current.UID = "uid-current" - desired.UID = "uid-desired" - current.ResourceVersion = "101" - desired.ResourceVersion = "202" - current.ManagedFields = []metav1.ManagedFieldsEntry{{Manager: "a"}} - desired.ManagedFields = []metav1.ManagedFieldsEntry{{Manager: "b"}} - current.Status.Phase = corev1.PodPending - desired.Status.Phase = corev1.PodFailed - }, - want: true, - }, - { - name: "spec change is not equal", - mutate: func(_ *corev1.Pod, desired *corev1.Pod) { - desired.Spec.Containers[0].Image = "ghcr.io/actions/listener:v2" - }, - want: false, - }, - { - name: "non-legacy annotation change is not equal", - mutate: func(_ *corev1.Pod, desired *corev1.Pod) { - desired.Annotations["keep"] = "different" - }, - want: false, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - current := base.DeepCopy() - desired := base.DeepCopy() - tt.mutate(current, desired) - assert.Equal(t, tt.want, listenerPodCanonicalEqual(current, desired)) - }) - } -} diff --git a/controllers/actions.github.com/resourcebuilder.go b/controllers/actions.github.com/resourcebuilder.go index cda96f18..c660a94f 100644 --- a/controllers/actions.github.com/resourcebuilder.go +++ b/controllers/actions.github.com/resourcebuilder.go @@ -128,8 +128,7 @@ func (b *ResourceBuilder) newAutoscalingListener(autoscalingRunnerSet *v1alpha1. Image: image, ImagePullSecrets: imagePullSecrets, }) - metadataDependency := resourceCacheObjectMetadataInputObject(autoscalingRunnerSet) - if cached, ok := b.ResourceCache.autoscalingListener.Get(autoscalingRunnerSet, cacheKeyObject, ephemeralRunnerSet, inputDependency, metadataDependency); ok { + if cached, ok := b.ResourceCache.autoscalingListener.Get(autoscalingRunnerSet, cacheKeyObject, ephemeralRunnerSet, inputDependency); ok { return cached, nil } @@ -182,6 +181,7 @@ func (b *ResourceBuilder) newAutoscalingListener(autoscalingRunnerSet *v1alpha1. labels = b.filterAndMergeLabels(autoscalingRunnerSet.Spec.AutoscalingListenerMetadata.Labels, labels) annotations = b.mergeAnnotations(autoscalingRunnerSet.Spec.AutoscalingListenerMetadata.Annotations, annotations) } + autoscalingListener := &v1alpha1.AutoscalingListener{ TypeMeta: metav1.TypeMeta{ APIVersion: v1alpha1.GroupVersion.String(), @@ -195,7 +195,7 @@ func (b *ResourceBuilder) newAutoscalingListener(autoscalingRunnerSet *v1alpha1. }, Spec: spec, } - b.ResourceCache.autoscalingListener.Upsert(autoscalingRunnerSet, autoscalingListener, ephemeralRunnerSet, inputDependency, metadataDependency) + b.ResourceCache.autoscalingListener.Upsert(autoscalingRunnerSet, autoscalingListener, ephemeralRunnerSet, inputDependency) return autoscalingListener, nil } @@ -213,20 +213,6 @@ func resourceCacheInputObject(name string, value any) client.Object { } } -func resourceCacheObjectMetadataInputObject(object client.Object) client.Object { - return resourceCacheInputObject(resourceCacheObjectName(object)+"-metadata", struct { - Namespace string - Name string - Labels map[string]string - Annotations map[string]string - }{ - Namespace: object.GetNamespace(), - Name: object.GetName(), - Labels: object.GetLabels(), - Annotations: object.GetAnnotations(), - }) -} - type listenerMetricsServerConfig struct { addr string endpoint string @@ -306,6 +292,7 @@ func (b *ResourceBuilder) newScaleSetListenerConfig(autoscalingListener *v1alpha if autoscalingListener.Spec.ConfigSecretMetadata != nil && len(autoscalingListener.Spec.ConfigSecretMetadata.Annotations) > 0 { annotations = autoscalingListener.Spec.ConfigSecretMetadata.Annotations } + desiredSecret := &corev1.Secret{ TypeMeta: metav1.TypeMeta{ APIVersion: corev1.SchemeGroupVersion.String(), @@ -343,8 +330,7 @@ func (b *ResourceBuilder) newScaleSetListenerPod( Namespace: autoscalingListener.Namespace, }, } - metadataDependency := resourceCacheObjectMetadataInputObject(autoscalingListener) - if cached, ok := b.ResourceCache.listenerPod.Get(autoscalingListener, cacheKeyObject, podConfig, serviceAccount, role, roleBinding, metadataDependency); ok { + if cached, ok := b.ResourceCache.listenerPod.Get(autoscalingListener, cacheKeyObject, podConfig, serviceAccount, role, roleBinding); ok { return cached, nil } @@ -474,7 +460,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, metadataDependency) + b.ResourceCache.listenerPod.Upsert(autoscalingListener, newRunnerScaleSetListenerPod, podConfig, serviceAccount, role, roleBinding) return newRunnerScaleSetListenerPod, nil } @@ -603,8 +589,7 @@ func (b *ResourceBuilder) newScaleSetListenerServiceAccount(autoscalingListener Namespace: autoscalingListener.Namespace, }, } - metadataDependency := resourceCacheObjectMetadataInputObject(autoscalingListener) - if cached, ok := b.ResourceCache.listenerServiceAccount.Get(autoscalingListener, cacheKeyObject, metadataDependency); ok { + if cached, ok := b.ResourceCache.listenerServiceAccount.Get(autoscalingListener, cacheKeyObject); ok { return cached, nil } @@ -628,10 +613,11 @@ func (b *ResourceBuilder) newScaleSetListenerServiceAccount(autoscalingListener base.Labels = b.filterAndMergeLabels(autoscalingListener.Spec.ServiceAccountMetadata.Labels, base.Labels) base.Annotations = b.mergeAnnotations(autoscalingListener.Spec.ServiceAccountMetadata.Annotations, base.Annotations) } + 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, metadataDependency) + b.ResourceCache.listenerServiceAccount.Upsert(autoscalingListener, base) return base, nil } @@ -643,8 +629,7 @@ func (b *ResourceBuilder) newScaleSetListenerRole(autoscalingListener *v1alpha1. Namespace: autoscalingListener.Spec.AutoscalingRunnerSetNamespace, }, } - metadataDependency := resourceCacheObjectMetadataInputObject(autoscalingListener) - if cached, ok := b.ResourceCache.listenerRole.Get(autoscalingListener, cacheKeyObject, metadataDependency); ok { + if cached, ok := b.ResourceCache.listenerRole.Get(autoscalingListener, cacheKeyObject); ok { return cached } @@ -660,6 +645,7 @@ func (b *ResourceBuilder) newScaleSetListenerRole(autoscalingListener *v1alpha1. labels = b.filterAndMergeLabels(autoscalingListener.Spec.RoleMetadata.Labels, labels) annotations = b.mergeAnnotations(autoscalingListener.Spec.RoleMetadata.Annotations, nil) } + newRole := &rbacv1.Role{ TypeMeta: metav1.TypeMeta{ APIVersion: rbacv1.SchemeGroupVersion.String(), @@ -674,7 +660,7 @@ func (b *ResourceBuilder) newScaleSetListenerRole(autoscalingListener *v1alpha1. Rules: rulesForListenerRole([]string{autoscalingListener.Spec.EphemeralRunnerSetName}), } - b.ResourceCache.listenerRole.Upsert(autoscalingListener, newRole, metadataDependency) + b.ResourceCache.listenerRole.Upsert(autoscalingListener, newRole) return newRole } @@ -686,8 +672,7 @@ func (b *ResourceBuilder) newScaleSetListenerRoleBinding(autoscalingListener *v1 Namespace: autoscalingListener.Spec.AutoscalingRunnerSetNamespace, }, } - metadataDependency := resourceCacheObjectMetadataInputObject(autoscalingListener) - if cached, ok := b.ResourceCache.listenerRoleBinding.Get(autoscalingListener, cacheKeyObject, listenerRole, serviceAccount, metadataDependency); ok { + if cached, ok := b.ResourceCache.listenerRoleBinding.Get(autoscalingListener, cacheKeyObject, listenerRole, serviceAccount); ok { return cached } @@ -716,6 +701,7 @@ func (b *ResourceBuilder) newScaleSetListenerRoleBinding(autoscalingListener *v1 labels = b.filterAndMergeLabels(autoscalingListener.Spec.RoleBindingMetadata.Labels, labels) annotations = autoscalingListener.Spec.RoleBindingMetadata.Annotations } + newRoleBinding := &rbacv1.RoleBinding{ TypeMeta: metav1.TypeMeta{ APIVersion: rbacv1.SchemeGroupVersion.String(), @@ -731,7 +717,7 @@ func (b *ResourceBuilder) newScaleSetListenerRoleBinding(autoscalingListener *v1 Subjects: subjects, } - b.ResourceCache.listenerRoleBinding.Upsert(autoscalingListener, newRoleBinding, listenerRole, serviceAccount, metadataDependency) + b.ResourceCache.listenerRoleBinding.Upsert(autoscalingListener, newRoleBinding, listenerRole, serviceAccount) return newRoleBinding } @@ -748,8 +734,7 @@ func (b *ResourceBuilder) newEphemeralRunnerSet(autoscalingRunnerSet *v1alpha1.A Namespace: autoscalingRunnerSet.Namespace, }, } - metadataDependency := resourceCacheObjectMetadataInputObject(autoscalingRunnerSet) - if cached, ok := b.ResourceCache.ephemeralRunnerSet.Get(autoscalingRunnerSet, cacheKeyObject, metadataDependency); ok { + if cached, ok := b.ResourceCache.ephemeralRunnerSet.Get(autoscalingRunnerSet, cacheKeyObject); ok { return cached, nil } @@ -789,6 +774,7 @@ func (b *ResourceBuilder) newEphemeralRunnerSet(autoscalingRunnerSet *v1alpha1.A labels = b.filterAndMergeLabels(autoscalingRunnerSet.Spec.EphemeralRunnerSetMetadata.Labels, labels) annotations = b.mergeAnnotations(autoscalingRunnerSet.Spec.EphemeralRunnerSetMetadata.Annotations, annotations) } + newEphemeralRunnerSet := &v1alpha1.EphemeralRunnerSet{ TypeMeta: metav1.TypeMeta{ APIVersion: v1alpha1.GroupVersion.String(), @@ -806,7 +792,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, metadataDependency) + b.ResourceCache.ephemeralRunnerSet.Upsert(autoscalingRunnerSet, newEphemeralRunnerSet) return newEphemeralRunnerSet, nil } @@ -845,6 +831,7 @@ func (b *ResourceBuilder) newEphemeralRunner(ephemeralRunnerSet *v1alpha1.Epheme labels = b.filterAndMergeLabels(ephemeralRunnerSet.Spec.EphemeralRunnerMetadata.Labels, labels) annotations = b.mergeAnnotations(ephemeralRunnerSet.Spec.EphemeralRunnerMetadata.Annotations, annotations) } + ephemeralRunner := &v1alpha1.EphemeralRunner{ ObjectMeta: metav1.ObjectMeta{ GenerateName: ephemeralRunnerSet.Name + "-runner-", @@ -871,6 +858,7 @@ func (b *ResourceBuilder) newEphemeralRunnerPod(runner *v1alpha1.EphemeralRunner annotations := make(map[string]string, len(runner.Annotations)+len(runner.Spec.Annotations)) maps.Copy(annotations, runner.Annotations) maps.Copy(annotations, runner.Spec.Annotations) + labels := make(map[string]string, len(runner.Labels)+len(runner.Spec.Labels)+2) maps.Copy(labels, runner.Labels) maps.Copy(labels, runner.Spec.Labels) @@ -940,6 +928,7 @@ func (b *ResourceBuilder) newEphemeralRunnerJitSecret(ephemeralRunner *v1alpha1. labels = b.filterAndMergeLabels(ephemeralRunner.Spec.EphemeralRunnerConfigSecretMetadata.Labels, nil) annotations = ephemeralRunner.Spec.EphemeralRunnerConfigSecretMetadata.Annotations } + jitSecret := &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ Name: ephemeralRunner.Name, diff --git a/controllers/actions.github.com/resourcebuilder_test.go b/controllers/actions.github.com/resourcebuilder_test.go index e71d079a..9e91790d 100644 --- a/controllers/actions.github.com/resourcebuilder_test.go +++ b/controllers/actions.github.com/resourcebuilder_test.go @@ -115,6 +115,7 @@ func TestMetadataPropagation(t *testing.T) { assert.Equal(t, labelValueKubernetesPartOf, ephemeralRunnerSet.Labels[LabelKeyKubernetesPartOf]) assert.Equal(t, "runner-set", ephemeralRunnerSet.Labels[LabelKeyKubernetesComponent]) assert.Equal(t, autoscalingRunnerSet.Labels[LabelKeyKubernetesVersion], ephemeralRunnerSet.Labels[LabelKeyKubernetesVersion]) + assert.NotContains(t, ephemeralRunnerSet.Annotations, "actions.github.com/integrity-hash") assert.Equal(t, autoscalingRunnerSet.Name, ephemeralRunnerSet.Labels[LabelKeyGitHubScaleSetName]) assert.Equal(t, autoscalingRunnerSet.Namespace, ephemeralRunnerSet.Labels[LabelKeyGitHubScaleSetNamespace]) assert.Equal(t, "", ephemeralRunnerSet.Labels[LabelKeyGitHubEnterprise]) @@ -131,6 +132,7 @@ func TestMetadataPropagation(t *testing.T) { assert.Equal(t, labelValueKubernetesPartOf, listener.Labels[LabelKeyKubernetesPartOf]) assert.Equal(t, "runner-scale-set-listener", listener.Labels[LabelKeyKubernetesComponent]) assert.Equal(t, autoscalingRunnerSet.Labels[LabelKeyKubernetesVersion], listener.Labels[LabelKeyKubernetesVersion]) + assert.NotContains(t, listener.Annotations, "actions.github.com/integrity-hash") assert.Equal(t, autoscalingRunnerSet.Name, listener.Labels[LabelKeyGitHubScaleSetName]) assert.Equal(t, autoscalingRunnerSet.Namespace, listener.Labels[LabelKeyGitHubScaleSetNamespace]) assert.Equal(t, "", listener.Labels[LabelKeyGitHubEnterprise]) @@ -204,6 +206,31 @@ func TestMetadataPropagation(t *testing.T) { } } +func TestEphemeralRunnerSetProxySecretMetadata(t *testing.T) { + ephemeralRunnerSet := &v1alpha1.EphemeralRunnerSet{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-scale-set", + Namespace: "test-ns", + Labels: map[string]string{ + LabelKeyGitHubScaleSetName: "test-scale-set", + LabelKeyGitHubScaleSetNamespace: "test-ns", + }, + }, + } + + var b ResourceBuilder + proxySecret, err := b.newEphemeralRunnerSetProxySecret(ephemeralRunnerSet, map[string][]byte{ + "http_proxy": []byte("http://proxy.example.com"), + }) + require.NoError(t, err) + + assert.Equal(t, proxyEphemeralRunnerSetSecretName(ephemeralRunnerSet), proxySecret.Name) + assert.Equal(t, ephemeralRunnerSet.Namespace, proxySecret.Namespace) + assert.Equal(t, ephemeralRunnerSet.Labels[LabelKeyGitHubScaleSetName], proxySecret.Labels[LabelKeyGitHubScaleSetName]) + assert.Equal(t, ephemeralRunnerSet.Labels[LabelKeyGitHubScaleSetNamespace], proxySecret.Labels[LabelKeyGitHubScaleSetNamespace]) + assert.NotContains(t, proxySecret.Annotations, "actions.github.com/integrity-hash") +} + func TestGitHubURLTrimLabelValues(t *testing.T) { enterprise := strings.Repeat("a", 64) organization := strings.Repeat("b", 64) diff --git a/controllers/actions.github.com/resourcecache.go b/controllers/actions.github.com/resourcecache.go index ec99c6bc..ccbec79f 100644 --- a/controllers/actions.github.com/resourcecache.go +++ b/controllers/actions.github.com/resourcecache.go @@ -19,7 +19,7 @@ const ( resourceCacheInitialEntries = 4096 resourceCacheInitialMainUIDEntries = 4096 resourceCacheInitialOwnerEntries = 8 - resourceCacheMaxDependencyRefs = 5 + resourceCacheMaxDependencyRefs = 4 ) type ResourceCacheObjectRef struct { @@ -28,7 +28,6 @@ type ResourceCacheObjectRef struct { Name string UID types.UID ResourceVersion string - Generation int64 // Used for CR owner identity (main objects), zero for dependencies/desired objects } type ResourceCacheKey struct { @@ -100,15 +99,11 @@ func (s *resourceCacheState[T]) Get( } key := newResourceCacheKey(mainObject, desiredObject) - mainObjectRef := newResourceCacheMainObjectRef(mainObject) - resourceVersion := desiredObject.GetResourceVersion() - if resourceVersion == "" && !isResourceCacheLookupObject(desiredObject) { - resourceVersion = hash.ComputeTemplateHash(desiredObject) - } + mainObjectRef := newResourceCacheObjectRef(mainObject) s.mu.RLock() value, ok := s.entries[key] - if ok && value.MainObject == mainObjectRef && (resourceVersion == "" || value.ResourceVersion == resourceVersion) && value.dependencyKey.Equal(dependencyKey) { + if ok && value.MainObject == mainObjectRef && value.dependencyKey.Equal(dependencyKey) { s.mu.RUnlock() return value.Object, true } @@ -135,11 +130,8 @@ func (s *resourceCacheState[T]) Upsert( } key := newResourceCacheKey(mainObject, desiredObject) - mainObjectRef := newResourceCacheMainObjectRef(mainObject) + mainObjectRef := newResourceCacheObjectRef(mainObject) resourceVersion := desiredObject.GetResourceVersion() - if resourceVersion == "" { - resourceVersion = hash.ComputeTemplateHash(desiredObject) - } s.mu.RLock() previous, ok := s.entries[key] @@ -227,7 +219,7 @@ func newResourceCacheDependencyKey(objects ...client.Object) (resourceCacheDepen if isNilResourceCacheObject(object) { return resourceCacheDependencyKey{}, false } - key.refs[i] = newResourceCacheDependencyObjectRef(object) + key.refs[i] = newResourceCacheObjectRef(object) } slices.SortFunc(key.refs[:key.count], func(a, b ResourceCacheObjectRef) int { return compareResourceCacheObjectRefs(a, b) @@ -243,7 +235,7 @@ func (k resourceCacheDependencyKey) Equal(other resourceCacheDependencyKey) bool return false } - for i := range k.count { + for i := 0; i < k.count; i++ { if k.refs[i] != other.refs[i] { return false } @@ -252,17 +244,7 @@ func (k resourceCacheDependencyKey) Equal(other resourceCacheDependencyKey) bool return true } -func newResourceCacheMainObjectRef(object client.Object) ResourceCacheObjectRef { - return ResourceCacheObjectRef{ - ObjectType: object.GetObjectKind().GroupVersionKind(), - Namespace: object.GetNamespace(), - Name: resourceCacheObjectName(object), - UID: object.GetUID(), - Generation: object.GetGeneration(), - } -} - -func newResourceCacheDependencyObjectRef(object client.Object) ResourceCacheObjectRef { +func newResourceCacheObjectRef(object client.Object) ResourceCacheObjectRef { resourceVersion := object.GetResourceVersion() if resourceVersion == "" { resourceVersion = hash.ComputeTemplateHash(object) @@ -290,12 +272,6 @@ func compareResourceCacheObjectRefs(a, b ResourceCacheObjectRef) int { if c := strings.Compare(string(a.UID), string(b.UID)); c != 0 { return c } - if a.Generation != b.Generation { - if a.Generation < b.Generation { - return -1 - } - return 1 - } return strings.Compare(a.ResourceVersion, b.ResourceVersion) } @@ -325,25 +301,3 @@ func isNilResourceCacheObject[T client.Object](object T) bool { value := reflect.ValueOf(clientObject) return value.Kind() == reflect.Pointer && value.IsNil() } - -func isResourceCacheLookupObject(object client.Object) bool { - lookupObject, ok := object.DeepCopyObject().(client.Object) - if !ok { - return false - } - lookupObject.SetGenerateName(object.GetGenerateName()) - lookupObject.SetName("") - lookupObject.SetNamespace("") - lookupObject.SetResourceVersion("") - - objectValue := reflect.ValueOf(object) - if objectValue.Kind() != reflect.Pointer { - return false - } - zeroObject, ok := reflect.New(objectValue.Elem().Type()).Interface().(client.Object) - if !ok { - return false - } - - return hash.ComputeTemplateHash(lookupObject) == hash.ComputeTemplateHash(zeroObject) -} diff --git a/controllers/actions.github.com/resourcecache_test.go b/controllers/actions.github.com/resourcecache_test.go index aa54341a..e187ac5e 100644 --- a/controllers/actions.github.com/resourcecache_test.go +++ b/controllers/actions.github.com/resourcecache_test.go @@ -184,10 +184,9 @@ func TestResourceCacheIgnoresInvalidInputs(t *testing.T) { func TestResourceBuilderCachesListenerPodDependencies(t *testing.T) { listener := &v1alpha1.AutoscalingListener{ ObjectMeta: metav1.ObjectMeta{ - Name: "listener", - Namespace: "controller-ns", - UID: "listener-uid", - Annotations: map[string]string{"example.com/listener-hash": "listener-hash"}, + Name: "listener", + Namespace: "controller-ns", + UID: "listener-uid", }, Spec: v1alpha1.AutoscalingListenerSpec{ Image: "listener:latest", @@ -202,7 +201,6 @@ func TestResourceBuilderCachesListenerPodDependencies(t *testing.T) { Namespace: "controller-ns", UID: "config-secret-uid", ResourceVersion: "11", - Annotations: map[string]string{"example.com/config-hash": "config-hash"}, }, } serviceAccount := &corev1.ServiceAccount{ @@ -211,7 +209,6 @@ func TestResourceBuilderCachesListenerPodDependencies(t *testing.T) { Namespace: "controller-ns", UID: "service-account-uid", ResourceVersion: "12", - Annotations: map[string]string{"example.com/service-account-hash": "service-account-hash"}, }, } role := &rbacv1.Role{ @@ -220,7 +217,6 @@ func TestResourceBuilderCachesListenerPodDependencies(t *testing.T) { Namespace: "scale-set-ns", UID: "role-uid", ResourceVersion: "13", - Annotations: map[string]string{"example.com/role-hash": "role-hash"}, }, } roleBinding := &rbacv1.RoleBinding{ @@ -229,7 +225,6 @@ func TestResourceBuilderCachesListenerPodDependencies(t *testing.T) { Namespace: "scale-set-ns", UID: "role-binding-uid", ResourceVersion: "14", - Annotations: map[string]string{"example.com/role-binding-hash": "role-binding-hash"}, }, } @@ -238,62 +233,15 @@ func TestResourceBuilderCachesListenerPodDependencies(t *testing.T) { listenerPod, err := b.newScaleSetListenerPod(listener, podConfig, serviceAccount, role, roleBinding, nil) require.NoError(t, err) - metadataDependency := resourceCacheObjectMetadataInputObject(listener) - cachedPod, ok := b.ResourceCache.listenerPod.Get(listener, listenerPod, podConfig, serviceAccount, role, roleBinding, metadataDependency) + cachedPod, ok := b.ResourceCache.listenerPod.Get(listener, listenerPod, podConfig, serviceAccount, role, roleBinding) require.True(t, ok) assert.IsType(t, &corev1.Pod{}, cachedPod) - lookupPod := &corev1.Pod{ - ObjectMeta: metav1.ObjectMeta{ - Name: listenerPod.Name, - Namespace: listenerPod.Namespace, - }, - } - cachedPod, ok = b.ResourceCache.listenerPod.Get(listener, lookupPod, podConfig, serviceAccount, role, roleBinding, metadataDependency) - require.True(t, ok, "name-only lookup object should hit the cached desired pod") - assert.Same(t, listenerPod, cachedPod) - role.ResourceVersion = "changed" - _, ok = b.ResourceCache.listenerPod.Get(listener, lookupPod, podConfig, serviceAccount, role, roleBinding, metadataDependency) + _, ok = b.ResourceCache.listenerPod.Get(listener, listenerPod, podConfig, serviceAccount, role, roleBinding) assert.False(t, ok) } -func TestResourceBuilderCachesListenerPodMetadataDependency(t *testing.T) { - listener := &v1alpha1.AutoscalingListener{ - ObjectMeta: metav1.ObjectMeta{ - Name: "listener", - Namespace: "controller-ns", - UID: "listener-uid", - Labels: map[string]string{ - "arc.test/listener-label": "initial", - }, - }, - 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"}} - serviceAccount := &corev1.ServiceAccount{ObjectMeta: metav1.ObjectMeta{Name: "listener", Namespace: "controller-ns", UID: "service-account-uid", ResourceVersion: "12"}} - role := &rbacv1.Role{ObjectMeta: metav1.ObjectMeta{Name: "listener", Namespace: "scale-set-ns", UID: "role-uid", ResourceVersion: "13"}} - roleBinding := &rbacv1.RoleBinding{ObjectMeta: metav1.ObjectMeta{Name: "listener", Namespace: "scale-set-ns", UID: "role-binding-uid", ResourceVersion: "14"}} - - cache := NewResourceCache() - b := ResourceBuilder{ResourceCache: &cache} - listenerPod, err := b.newScaleSetListenerPod(listener, podConfig, serviceAccount, role, roleBinding, nil) - require.NoError(t, err) - - lookupPod := &corev1.Pod{ObjectMeta: metav1.ObjectMeta{Name: listenerPod.Name, Namespace: listenerPod.Namespace}} - _, ok := b.ResourceCache.listenerPod.Get(listener, lookupPod, podConfig, serviceAccount, role, roleBinding, resourceCacheObjectMetadataInputObject(listener)) - assert.True(t, ok) - - listener.Labels["arc.test/listener-label"] = "updated" - _, ok = b.ResourceCache.listenerPod.Get(listener, lookupPod, podConfig, serviceAccount, role, roleBinding, resourceCacheObjectMetadataInputObject(listener)) - assert.False(t, ok, "cache miss when listener metadata used by the pod changes") -} - func TestResourceBuilderCachesEphemeralRunnerSet(t *testing.T) { autoscalingRunnerSet := v1alpha1.AutoscalingRunnerSet{ ObjectMeta: metav1.ObjectMeta{ @@ -314,184 +262,16 @@ func TestResourceBuilderCachesEphemeralRunnerSet(t *testing.T) { runnerSet, err := b.newEphemeralRunnerSet(&autoscalingRunnerSet) require.NoError(t, err) - metadataDependency := resourceCacheObjectMetadataInputObject(&autoscalingRunnerSet) - cachedRunnerSet, ok := b.ResourceCache.ephemeralRunnerSet.Get(&autoscalingRunnerSet, runnerSet, metadataDependency) - require.True(t, ok, "direct cache Get with returned object should hit") + cachedRunnerSet, ok := b.ResourceCache.ephemeralRunnerSet.Get(&autoscalingRunnerSet, runnerSet) + require.True(t, ok) assert.Equal(t, runnerSet.Spec, cachedRunnerSet.Spec) assert.Same(t, runnerSet, cachedRunnerSet) - lookupRunnerSet := &v1alpha1.EphemeralRunnerSet{ObjectMeta: metav1.ObjectMeta{Name: runnerSet.Name, Namespace: runnerSet.Namespace}} - cachedRunnerSet, ok = b.ResourceCache.ephemeralRunnerSet.Get(&autoscalingRunnerSet, lookupRunnerSet, metadataDependency) - require.True(t, ok, "name-only lookup object should hit the cached desired runner set") - 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, lookupRunnerSet, metadataDependency) - assert.True(t, ok, "cache should be valid when main object generation unchanged") -} - -func TestResourceBuilderCachesEphemeralRunnerSetMetadataDependency(t *testing.T) { - autoscalingRunnerSet := v1alpha1.AutoscalingRunnerSet{ - ObjectMeta: metav1.ObjectMeta{ - Name: "scale-set", - Namespace: "default", - UID: "scale-set-uid", - Labels: map[string]string{ - "arc.test/scale-set-label": "initial", - }, - 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) - - lookupRunnerSet := &v1alpha1.EphemeralRunnerSet{ObjectMeta: metav1.ObjectMeta{Name: runnerSet.Name, Namespace: runnerSet.Namespace}} - _, ok := b.ResourceCache.ephemeralRunnerSet.Get(&autoscalingRunnerSet, lookupRunnerSet, resourceCacheObjectMetadataInputObject(&autoscalingRunnerSet)) - assert.True(t, ok) - - autoscalingRunnerSet.Labels["arc.test/scale-set-label"] = "updated" - _, ok = b.ResourceCache.ephemeralRunnerSet.Get(&autoscalingRunnerSet, lookupRunnerSet, resourceCacheObjectMetadataInputObject(&autoscalingRunnerSet)) - assert.False(t, ok, "cache miss when autoscaling runner set metadata used by the runner set changes") -} - -func TestResourceCacheOwnerGenerationDoesNotAffectCacheEntry(t *testing.T) { - mainObject := &v1alpha1.AutoscalingListener{ - ObjectMeta: metav1.ObjectMeta{ - Name: "listener", - Namespace: "controller-ns", - UID: "listener-uid", - Generation: 5, - }, - } - desiredPod := &corev1.Pod{ - ObjectMeta: metav1.ObjectMeta{ - Name: "listener", - Namespace: "controller-ns", - }, - } - - cache := NewResourceCache() - _, replaced := cache.listenerPod.Upsert(mainObject, desiredPod) - assert.True(t, replaced) - _, ok := cache.listenerPod.Get(mainObject, desiredPod) - assert.True(t, ok) - - mainObjectCopy := mainObject.DeepCopy() - _, ok = cache.listenerPod.Get(mainObjectCopy, desiredPod) - assert.True(t, ok, "cache hit when owner generation unchanged") -} - -func TestResourceCacheOwnerGenerationChangeInvalidatesCacheEntry(t *testing.T) { - mainObject := &v1alpha1.AutoscalingListener{ - ObjectMeta: metav1.ObjectMeta{ - Name: "listener", - Namespace: "controller-ns", - UID: "listener-uid", - Generation: 5, - }, - } - desiredPod := &corev1.Pod{ - ObjectMeta: metav1.ObjectMeta{ - Name: "listener", - Namespace: "controller-ns", - }, - } - - cache := NewResourceCache() - _, replaced := cache.listenerPod.Upsert(mainObject, desiredPod) - assert.True(t, replaced) - _, ok := cache.listenerPod.Get(mainObject, desiredPod) - assert.True(t, ok) - - mainObjectWithNewGeneration := mainObject.DeepCopy() - mainObjectWithNewGeneration.Generation = 6 - _, ok = cache.listenerPod.Get(mainObjectWithNewGeneration, desiredPod) - assert.False(t, ok, "cache miss when owner generation changes") -} - -func TestResourceCacheDependencyResourceVersionChangeInvalidates(t *testing.T) { - mainObject := &v1alpha1.AutoscalingListener{ - ObjectMeta: metav1.ObjectMeta{ - Name: "listener", - Namespace: "controller-ns", - UID: "listener-uid", - }, - } - desiredPod := &corev1.Pod{ - ObjectMeta: metav1.ObjectMeta{ - Name: "listener", - Namespace: "controller-ns", - }, - } - dependency := &corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Name: "config", - Namespace: "controller-ns", - UID: "config-uid", - ResourceVersion: "1", - }, - } - - cache := NewResourceCache() - _, replaced := cache.listenerPod.Upsert(mainObject, desiredPod, dependency) - assert.True(t, replaced) - _, ok := cache.listenerPod.Get(mainObject, desiredPod, dependency) - assert.True(t, ok) - - dependencyWithNewResourceVersion := dependency.DeepCopy() - dependencyWithNewResourceVersion.ResourceVersion = "2" - _, ok = cache.listenerPod.Get(mainObject, desiredPod, dependencyWithNewResourceVersion) - assert.False(t, ok, "cache miss when dependency resourceVersion changes") -} - -func TestResourceCacheNoAnnotationFallback(t *testing.T) { - mainObject := &v1alpha1.AutoscalingListener{ - ObjectMeta: metav1.ObjectMeta{ - Name: "listener", - Namespace: "controller-ns", - UID: "listener-uid", - }, - } - desiredPodWithoutResourceVersion := &corev1.Pod{ - ObjectMeta: metav1.ObjectMeta{ - Name: "listener", - Namespace: "controller-ns", - }, - } - desiredPodWithDifferentAnnotation := &corev1.Pod{ - ObjectMeta: metav1.ObjectMeta{ - Name: "listener", - Namespace: "controller-ns", - Annotations: map[string]string{ - "unrelated-key": "unrelated-value", - }, - }, - } - - cache := NewResourceCache() - _, replaced := cache.listenerPod.Upsert(mainObject, desiredPodWithoutResourceVersion) - assert.True(t, replaced) - - _, ok := cache.listenerPod.Get(mainObject, desiredPodWithoutResourceVersion) - assert.True(t, ok, "cache hit with same pod object") - - _, ok = cache.listenerPod.Get(mainObject, desiredPodWithDifferentAnnotation) - assert.False(t, ok, "cache miss when pod changed - uses hash not annotation fallback") - - desiredPodIdentical := &corev1.Pod{ - ObjectMeta: metav1.ObjectMeta{ - Name: "listener", - Namespace: "controller-ns", - }, - } - _, ok = cache.listenerPod.Get(mainObject, desiredPodIdentical) - assert.True(t, ok, "cache hit when pod structure identical even if different instance") + _, ok = b.ResourceCache.ephemeralRunnerSet.Get(&autoscalingRunnerSet, runnerSet) + assert.False(t, ok) }