Re-create the listener when GitHub secret is updated

controllers/actions.github.com/autoscalingrunnerset_controller.go

@@ -246,7 +246,7 @@ func (r *AutoscalingRunnerSetReconciler) Reconcile(ctx context.Context, req ctrl
 
 	// Our listener pod is out of date, so we need to delete it to get a new recreate.
 	listenerValuesHashChanged := listener.Annotations[annotationKeyValuesHash] != autoscalingRunnerSet.Annotations[annotationKeyValuesHash]
-	listenerSpecHashChanged := listener.Annotations[annotationKeyRunnerSpecHash] != autoscalingRunnerSet.ListenerSpecHash()
+	listenerSpecHashChanged := listener.Annotations[annotationKeyRunnerSpecHash] != autoscalingRunnerSet.ListenerSpecHash(secret)
 	if listenerFound && (listenerValuesHashChanged || listenerSpecHashChanged) {
 		log.Info("RunnerScaleSetListener is out of date. Deleting it so that it is recreated", "name", listener.Name)
 		if err := r.Delete(ctx, listener); err != nil {
@@ -297,7 +297,7 @@ func (r *AutoscalingRunnerSetReconciler) Reconcile(ctx context.Context, req ctrl
 			return ctrl.Result{}, nil
 		}
 		log.Info("Creating a new AutoscalingListener for the runner set", "ephemeralRunnerSetName", latestRunnerSet.Name)
-		return r.createAutoScalingListenerForRunnerSet(ctx, autoscalingRunnerSet, latestRunnerSet, log)
+		return r.createAutoScalingListenerForRunnerSet(ctx, autoscalingRunnerSet, latestRunnerSet, secret, log)
 	}
 
 	// Update the status of autoscaling runner set.
@@ -643,7 +643,13 @@ func (r *AutoscalingRunnerSetReconciler) createEphemeralRunnerSet(ctx context.Co
 	return ctrl.Result{}, nil
 }
 
-func (r *AutoscalingRunnerSetReconciler) createAutoScalingListenerForRunnerSet(ctx context.Context, autoscalingRunnerSet *v1alpha1.AutoscalingRunnerSet, ephemeralRunnerSet *v1alpha1.EphemeralRunnerSet, log logr.Logger) (ctrl.Result, error) {
+func (r *AutoscalingRunnerSetReconciler) createAutoScalingListenerForRunnerSet(
+	ctx context.Context,
+	autoscalingRunnerSet *v1alpha1.AutoscalingRunnerSet,
+	ephemeralRunnerSet *v1alpha1.EphemeralRunnerSet,
+	githubSecret *corev1.Secret,
+	log logr.Logger,
+) (ctrl.Result, error) {
 	var imagePullSecrets []corev1.LocalObjectReference
 	for _, imagePullSecret := range r.DefaultRunnerScaleSetListenerImagePullSecrets {
 		imagePullSecrets = append(imagePullSecrets, corev1.LocalObjectReference{
@@ -651,7 +657,14 @@ func (r *AutoscalingRunnerSetReconciler) createAutoScalingListenerForRunnerSet(c
 		})
 	}
 
-	autoscalingListener, err := r.ResourceBuilder.newAutoScalingListener(autoscalingRunnerSet, ephemeralRunnerSet, r.ControllerNamespace, r.DefaultRunnerScaleSetListenerImage, imagePullSecrets)
+	autoscalingListener, err := r.ResourceBuilder.newAutoScalingListener(
+		autoscalingRunnerSet,
+		ephemeralRunnerSet,
+		githubSecret,
+		r.ControllerNamespace,
+		r.DefaultRunnerScaleSetListenerImage,
+		imagePullSecrets,
+	)
 	if err != nil {
 		log.Error(err, "Could not create AutoscalingListener spec")
 		return ctrl.Result{}, err

controllers/actions.github.com/autoscalingrunnerset_controller_test.go

@@ -476,6 +476,101 @@ var _ = Describe("Test AutoScalingRunnerSet controller", Ordered, func() {
 				autoscalingRunnerSetTestInterval,
 			).Should(BeEquivalentTo("testgroup2"), "AutoScalingRunnerSet should have the runner group in its annotation")
 		})
+
+		It("should re-create the listener when the github secret changes", func() {
+			// Wait till the listener is created
+			listener := new(v1alpha1.AutoscalingListener)
+			Eventually(
+				func() error {
+					return k8sClient.Get(ctx, client.ObjectKey{Name: scaleSetListenerName(autoscalingRunnerSet), Namespace: autoscalingRunnerSet.Namespace}, listener)
+				},
+				autoscalingRunnerSetTestTimeout,
+				autoscalingRunnerSetTestInterval,
+			).Should(Succeed(), "Listener should be created")
+
+			actionsClient, err := controller.actionsClientFor(ctx, autoscalingRunnerSet)
+			Expect(err).NotTo(HaveOccurred(), "failed to get actions client")
+
+			listenerCreationTimestamp := listener.ObjectMeta.CreationTimestamp
+			listenerHash := listener.ObjectMeta.Annotations[annotationKeyRunnerSpecHash]
+
+			githubSecret := new(corev1.Secret)
+			Eventually(
+				func() error {
+					return k8sClient.Get(
+						ctx,
+						client.ObjectKey{
+							Name:      configSecret.ObjectMeta.Name,
+							Namespace: configSecret.ObjectMeta.Namespace,
+						},
+						githubSecret,
+					)
+				},
+				autoscalingRunnerSetTestTimeout,
+				autoscalingRunnerSetTestInterval,
+			).Should(Succeed(), "Failed to fetch the github secret")
+
+			githubSecret.Data["update"] = []byte("update")
+			err = k8sClient.Update(ctx, githubSecret)
+			Expect(err).NotTo(HaveOccurred(), "failed to update the github secret")
+
+			updatedGitHubSecret := new(corev1.Secret)
+			Eventually(
+				func() error {
+					err := k8sClient.Get(
+						ctx,
+						client.ObjectKey{
+							Name:      configSecret.ObjectMeta.Name,
+							Namespace: configSecret.ObjectMeta.Namespace,
+						},
+						updatedGitHubSecret,
+					)
+					if err != nil {
+						return err
+					}
+
+					if _, ok := updatedGitHubSecret.Data["update"]; !ok {
+						return fmt.Errorf("secret update not yet present")
+					}
+					return nil
+				},
+				autoscalingRunnerSetTestTimeout,
+				autoscalingRunnerSetTestInterval,
+			).Should(Succeed(), "Failed to eventually figure out github secret data update")
+
+			updatedListener := new(v1alpha1.AutoscalingListener)
+			Eventually(
+				func() error {
+					err := k8sClient.Get(
+						ctx,
+						client.ObjectKey{
+							Name:      scaleSetListenerName(autoscalingRunnerSet),
+							Namespace: autoscalingRunnerSet.Namespace,
+						},
+						listener,
+					)
+					if err != nil {
+						return err
+					}
+
+					if updatedListener.CreationTimestamp == listenerCreationTimestamp {
+						return fmt.Errorf("creation timestamp not updated yet")
+					}
+					if updatedListener.Annotations[annotationKeyRunnerSpecHash] == listenerHash {
+						return fmt.Errorf("hash not updated yet")
+					}
+
+					return nil
+				},
+				autoscalingRunnerSetTestTimeout,
+				autoscalingRunnerSetTestInterval,
+			).Should(Succeed(), "Listener should be re-created")
+
+			actionsClientAfterUpdate, err := controller.actionsClientFor(ctx, autoscalingRunnerSet)
+			Expect(err).NotTo(HaveOccurred(), "failed to get actions client")
+
+			Expect(actionsClientAfterUpdate.(*fake.FakeClient).ID).NotTo(BeEquivalentTo(actionsClient.(*fake.FakeClient).ID), "expected new client to be used")
+		})
 	})
 
 	Context("When updating an AutoscalingRunnerSet with running or pending jobs", func() {

controllers/actions.github.com/resourcebuilder.go

@@ -78,7 +78,13 @@ func boolPtr(v bool) *bool {
 	return &v
 }
 
-func (b *ResourceBuilder) newAutoScalingListener(autoscalingRunnerSet *v1alpha1.AutoscalingRunnerSet, ephemeralRunnerSet *v1alpha1.EphemeralRunnerSet, namespace, image string, imagePullSecrets []corev1.LocalObjectReference) (*v1alpha1.AutoscalingListener, error) {
+func (b *ResourceBuilder) newAutoScalingListener(
+	autoscalingRunnerSet *v1alpha1.AutoscalingRunnerSet,
+	ephemeralRunnerSet *v1alpha1.EphemeralRunnerSet,
+	githubSecret *corev1.Secret,
+	namespace, image string,
+	imagePullSecrets []corev1.LocalObjectReference,
+) (*v1alpha1.AutoscalingListener, error) {
 	runnerScaleSetId, err := strconv.Atoi(autoscalingRunnerSet.Annotations[runnerScaleSetIdAnnotationKey])
 	if err != nil {
 		return nil, err
@@ -102,7 +108,7 @@ func (b *ResourceBuilder) newAutoScalingListener(autoscalingRunnerSet *v1alpha1.
 	})
 
 	annotations := map[string]string{
-		annotationKeyRunnerSpecHash: autoscalingRunnerSet.ListenerSpecHash(),
+		annotationKeyRunnerSpecHash: autoscalingRunnerSet.ListenerSpecHash(githubSecret),
 		annotationKeyValuesHash:     autoscalingRunnerSet.Annotations[annotationKeyValuesHash],
 	}
 

controllers/actions.github.com/resourcebuilder_test.go

@@ -59,7 +59,24 @@ func TestLabelPropagation(t *testing.T) {
 	assert.Equal(t, autoscalingRunnerSet.Annotations[AnnotationKeyGitHubRunnerScaleSetName], ephemeralRunnerSet.Annotations[AnnotationKeyGitHubRunnerScaleSetName])
 	assert.Equal(t, autoscalingRunnerSet.Labels["arbitrary-label"], ephemeralRunnerSet.Labels["arbitrary-label"])
 
-	listener, err := b.newAutoScalingListener(&autoscalingRunnerSet, ephemeralRunnerSet, autoscalingRunnerSet.Namespace, "test:latest", nil)
+	githubSecret := &corev1.Secret{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      "test-scale-set",
+			Namespace: "test-ns",
+		},
+		Data: map[string][]byte{
+			"github_token": []byte("github_token"),
+		},
+	}
+
+	listener, err := b.newAutoScalingListener(
+		&autoscalingRunnerSet,
+		ephemeralRunnerSet,
+		githubSecret,
+		autoscalingRunnerSet.Namespace,
+		"test:latest",
+		nil,
+	)
 	require.NoError(t, err)
 	assert.Equal(t, labelValueKubernetesPartOf, listener.Labels[LabelKeyKubernetesPartOf])
 	assert.Equal(t, "runner-scale-set-listener", listener.Labels[LabelKeyKubernetesComponent])
@@ -120,6 +137,16 @@ func TestGitHubURLTrimLabelValues(t *testing.T) {
 	organization := strings.Repeat("b", 64)
 	repository := strings.Repeat("c", 64)
 
+	githubSecret := &corev1.Secret{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      "test-scale-set",
+			Namespace: "test-ns",
+		},
+		Data: map[string][]byte{
+			"github_token": []byte("github_token"),
+		},
+	}
+
 	autoscalingRunnerSet := v1alpha1.AutoscalingRunnerSet{
 		ObjectMeta: metav1.ObjectMeta{
 			Name:      "test-scale-set",
@@ -151,7 +178,14 @@ func TestGitHubURLTrimLabelValues(t *testing.T) {
 		assert.True(t, strings.HasSuffix(ephemeralRunnerSet.Labels[LabelKeyGitHubOrganization], trimLabelVauleSuffix))
 		assert.True(t, strings.HasSuffix(ephemeralRunnerSet.Labels[LabelKeyGitHubRepository], trimLabelVauleSuffix))
 
-		listener, err := b.newAutoScalingListener(autoscalingRunnerSet, ephemeralRunnerSet, autoscalingRunnerSet.Namespace, "test:latest", nil)
+		listener, err := b.newAutoScalingListener(
+			autoscalingRunnerSet,
+			ephemeralRunnerSet,
+			githubSecret,
+			autoscalingRunnerSet.Namespace,
+			"test:latest",
+			nil,
+		)
 		require.NoError(t, err)
 		assert.Len(t, listener.Labels[LabelKeyGitHubEnterprise], 0)
 		assert.Len(t, listener.Labels[LabelKeyGitHubOrganization], 63)
@@ -174,7 +208,14 @@ func TestGitHubURLTrimLabelValues(t *testing.T) {
 		assert.Len(t, ephemeralRunnerSet.Labels[LabelKeyGitHubOrganization], 0)
 		assert.Len(t, ephemeralRunnerSet.Labels[LabelKeyGitHubRepository], 0)
 
-		listener, err := b.newAutoScalingListener(autoscalingRunnerSet, ephemeralRunnerSet, autoscalingRunnerSet.Namespace, "test:latest", nil)
+		listener, err := b.newAutoScalingListener(
+			autoscalingRunnerSet,
+			ephemeralRunnerSet,
+			githubSecret,
+			autoscalingRunnerSet.Namespace,
+			"test:latest",
+			nil,
+		)
 		require.NoError(t, err)
 		assert.Len(t, listener.Labels[LabelKeyGitHubEnterprise], 63)
 		assert.True(t, strings.HasSuffix(ephemeralRunnerSet.Labels[LabelKeyGitHubEnterprise], trimLabelVauleSuffix))