Merge remote-tracking branch 'origin/master' into nikola-jokic/remove-annotation-fingerprint

controllers/actions.github.com/autoscalinglistener_controller.go

@@ -394,8 +394,9 @@ func (r *AutoscalingListenerReconciler) Reconcile(ctx context.Context, req ctrl.
 		labelsModified := !maps.Equal(listenerConfigSecret.Labels, desiredLabels)
 		desiredAnnotations := r.mergeAnnotations(listenerConfigSecret.Annotations, desiredSecret.Annotations)
 		annotationsModified := !maps.Equal(listenerConfigSecret.Annotations, desiredAnnotations)
+		dataModified := !reflect.DeepEqual(listenerConfigSecret.Data, desiredSecret.Data)
 
-		if labelsModified || annotationsModified {
+		if labelsModified || annotationsModified || dataModified {
 			updatedSecret := listenerConfigSecret.DeepCopy()
 			if labelsModified {
 				updatedSecret.Labels = desiredLabels
@@ -403,6 +404,9 @@ func (r *AutoscalingListenerReconciler) Reconcile(ctx context.Context, req ctrl.
 			if annotationsModified {
 				updatedSecret.Annotations = desiredAnnotations
 			}
+			if dataModified {
+				updatedSecret.Data = desiredSecret.Data
+			}
 			log.Info("Updating listener config secret", "namespace", updatedSecret.Namespace, "name", updatedSecret.Name)
 			if err := r.Patch(ctx, updatedSecret, client.MergeFrom(&listenerConfigSecret)); err != nil {
 				return ctrl.Result{}, fmt.Errorf("failed to update listener config secret: %w", err)

controllers/actions.github.com/autoscalinglistener_controller_test.go

@@ -423,6 +423,63 @@ var _ = Describe("Test AutoScalingListener controller", func() {
 			).Should(BeEquivalentTo(rulesForListenerRole([]string{updated.Spec.EphemeralRunnerSetName})), "Role should be updated")
 		})
 
+		It("updates listener scaler configuration and recreates the listener pod", func() {
+			pod := new(corev1.Pod)
+			Eventually(
+				func() error {
+					return k8sClient.Get(ctx, client.ObjectKey{Name: autoscalingListener.Name, Namespace: autoscalingListener.Namespace}, pod)
+				},
+				autoscalingListenerTestTimeout,
+				autoscalingListenerTestInterval,
+			).Should(Succeed(), "Listener pod should be created")
+			oldPodUID := 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")
+
+			qps := 75
+			burst := 150
+			updated := current.DeepCopy()
+			updated.Spec.ListenerConfig = &v1alpha1.ListenerConfig{
+				Scaler: &v1alpha1.ScalerConfig{
+					QPS:   &qps,
+					Burst: &burst,
+				},
+			}
+			err = k8sClient.Patch(ctx, updated, client.MergeFrom(current))
+			Expect(err).NotTo(HaveOccurred(), "failed to update listener scaler configuration")
+
+			secret := new(corev1.Secret)
+			Eventually(
+				func(g Gomega) {
+					err := k8sClient.Get(ctx, client.ObjectKey{Name: scaleSetListenerConfigName(autoscalingListener), Namespace: autoscalingListener.Namespace}, secret)
+					g.Expect(err).NotTo(HaveOccurred(), "failed to get listener config Secret")
+
+					var config ghalistenerconfig.Config
+					err = json.Unmarshal(secret.Data["config.json"], &config)
+					g.Expect(err).NotTo(HaveOccurred(), "failed to parse listener configuration file")
+					g.Expect(config.ListenerConfig.GetScaler()).NotTo(BeNil())
+					g.Expect(config.ListenerConfig.GetScaler().QPS).NotTo(BeNil())
+					g.Expect(config.ListenerConfig.GetScaler().Burst).NotTo(BeNil())
+					g.Expect(*config.ListenerConfig.GetScaler().QPS).To(Equal(qps))
+					g.Expect(*config.ListenerConfig.GetScaler().Burst).To(Equal(burst))
+				},
+				autoscalingListenerTestTimeout,
+				autoscalingListenerTestInterval,
+			).Should(Succeed(), "Listener config Secret should be updated")
+
+			Eventually(
+				func() (types.UID, error) {
+					pod := new(corev1.Pod)
+					err := k8sClient.Get(ctx, client.ObjectKey{Name: autoscalingListener.Name, Namespace: autoscalingListener.Namespace}, pod)
+					return pod.UID, err
+				},
+				autoscalingListenerTestTimeout,
+				autoscalingListenerTestInterval,
+			).ShouldNot(Equal(oldPodUID), "Listener pod should be recreated with the updated configuration")
+		})
+
 		It("propagates updated listener metadata to owned resources", func() {
 			assertPropagatedMetadata := func(expected string) {
 				Eventually(

controllers/actions.github.com/options.go

@@ -10,9 +10,25 @@ import (
 // Options is the optional configuration for the controllers, which can be
 // set via command-line flags or environment variables.
 type Options struct {
-	// RunnerMaxConcurrentReconciles is the maximum number of concurrent Reconciles which can be run
-	// by the EphemeralRunnerController.
-	RunnerMaxConcurrentReconciles int
+	// DefaultMaxConcurrentReconciles is the maximum number of concurrent Reconciles
+	// applied to every controller that does not have its own value set below.
+	DefaultMaxConcurrentReconciles int
+
+	// AutoscalingRunnerSetMaxConcurrentReconciles is the maximum number of concurrent Reconciles
+	// which can be run by the AutoscalingRunnerSetController. Zero means DefaultMaxConcurrentReconciles.
+	AutoscalingRunnerSetMaxConcurrentReconciles int
+
+	// AutoscalingListenerMaxConcurrentReconciles is the maximum number of concurrent Reconciles
+	// which can be run by the AutoscalingListenerController. Zero means DefaultMaxConcurrentReconciles.
+	AutoscalingListenerMaxConcurrentReconciles int
+
+	// EphemeralRunnerSetMaxConcurrentReconciles is the maximum number of concurrent Reconciles
+	// which can be run by the EphemeralRunnerSetController. Zero means DefaultMaxConcurrentReconciles.
+	EphemeralRunnerSetMaxConcurrentReconciles int
+
+	// EphemeralRunnerMaxConcurrentReconciles is the maximum number of concurrent Reconciles
+	// which can be run by the EphemeralRunnerController. Zero means DefaultMaxConcurrentReconciles.
+	EphemeralRunnerMaxConcurrentReconciles int
 }
 
 // OptionsWithDefault returns the default options.
@@ -20,10 +36,31 @@ type Options struct {
 // rather than having to correlate those in multiple places.
 func OptionsWithDefault() Options {
 	return Options{
-		RunnerMaxConcurrentReconciles: 2,
+		DefaultMaxConcurrentReconciles: 1,
 	}
 }
 
+// Resolve returns a copy of the options where a DefaultMaxConcurrentReconciles
+// of zero or less is replaced by 1, and every per-controller
+// MaxConcurrentReconciles that is left at zero is replaced by
+// DefaultMaxConcurrentReconciles.
+func (o Options) Resolve() Options {
+	if o.DefaultMaxConcurrentReconciles <= 0 {
+		o.DefaultMaxConcurrentReconciles = 1
+	}
+	orDefault := func(n int) int {
+		if n > 0 {
+			return n
+		}
+		return o.DefaultMaxConcurrentReconciles
+	}
+	o.AutoscalingRunnerSetMaxConcurrentReconciles = orDefault(o.AutoscalingRunnerSetMaxConcurrentReconciles)
+	o.AutoscalingListenerMaxConcurrentReconciles = orDefault(o.AutoscalingListenerMaxConcurrentReconciles)
+	o.EphemeralRunnerSetMaxConcurrentReconciles = orDefault(o.EphemeralRunnerSetMaxConcurrentReconciles)
+	o.EphemeralRunnerMaxConcurrentReconciles = orDefault(o.EphemeralRunnerMaxConcurrentReconciles)
+	return o
+}
+
 type Option func(*controller.Options)
 
 // WithMaxConcurrentReconciles sets the maximum number of concurrent Reconciles which can be run.

controllers/actions.github.com/options_test.go

@@ -0,0 +1,43 @@
+package actionsgithubcom
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestOptionsResolve(t *testing.T) {
+	t.Parallel()
+
+	t.Run("unset per-controller values fall back to the default", func(t *testing.T) {
+		got := OptionsWithDefault().Resolve()
+		assert.Equal(t, 1, got.DefaultMaxConcurrentReconciles)
+		assert.Equal(t, 1, got.AutoscalingRunnerSetMaxConcurrentReconciles)
+		assert.Equal(t, 1, got.AutoscalingListenerMaxConcurrentReconciles)
+		assert.Equal(t, 1, got.EphemeralRunnerSetMaxConcurrentReconciles)
+		assert.Equal(t, 1, got.EphemeralRunnerMaxConcurrentReconciles)
+	})
+
+	t.Run("a default of zero or less is replaced by 1", func(t *testing.T) {
+		for _, n := range []int{0, -1} {
+			opts := Options{DefaultMaxConcurrentReconciles: n}
+			got := opts.Resolve()
+			assert.Equal(t, 1, got.DefaultMaxConcurrentReconciles)
+			assert.Equal(t, 1, got.AutoscalingRunnerSetMaxConcurrentReconciles)
+			assert.Equal(t, 1, got.AutoscalingListenerMaxConcurrentReconciles)
+			assert.Equal(t, 1, got.EphemeralRunnerSetMaxConcurrentReconciles)
+			assert.Equal(t, 1, got.EphemeralRunnerMaxConcurrentReconciles)
+		}
+	})
+
+	t.Run("a changed default applies to every unset controller", func(t *testing.T) {
+		opts := OptionsWithDefault()
+		opts.DefaultMaxConcurrentReconciles = 4
+		opts.EphemeralRunnerMaxConcurrentReconciles = 20
+		got := opts.Resolve()
+		assert.Equal(t, 4, got.AutoscalingRunnerSetMaxConcurrentReconciles)
+		assert.Equal(t, 4, got.AutoscalingListenerMaxConcurrentReconciles)
+		assert.Equal(t, 4, got.EphemeralRunnerSetMaxConcurrentReconciles)
+		assert.Equal(t, 20, got.EphemeralRunnerMaxConcurrentReconciles)
+	})
+}

controllers/actions.github.com/resourcebuilder.go

@@ -161,6 +161,7 @@ func (b *ResourceBuilder) newAutoscalingListener(autoscalingRunnerSet *v1alpha1.
 		RoleMetadata:                  autoscalingRunnerSet.Spec.ListenerRoleMetadata,
 		RoleBindingMetadata:           autoscalingRunnerSet.Spec.ListenerRoleBindingMetadata,
 		ConfigSecretMetadata:          autoscalingRunnerSet.Spec.ListenerConfigSecretMetadata,
+		ListenerConfig:                autoscalingRunnerSet.Spec.ListenerConfig,
 	}
 
 	labels := b.filterAndMergeLabels(autoscalingRunnerSet.Labels, map[string]string{
@@ -258,6 +259,7 @@ func (b *ResourceBuilder) newScaleSetListenerConfig(autoscalingListener *v1alpha
 		MetricsAddr:                 metricsAddr,
 		MetricsEndpoint:             metricsEndpoint,
 		Metrics:                     autoscalingListener.Spec.Metrics,
+		ListenerConfig:              autoscalingListener.Spec.ListenerConfig,
 	}
 
 	vault := autoscalingListener.Spec.VaultConfig