From 6b0e8992086ae9b58a106275248e8f847753c116 Mon Sep 17 00:00:00 2001 From: Nikola Jokic Date: Wed, 17 Jun 2026 00:56:20 +0200 Subject: [PATCH] wip --- .../v1alpha1/autoscalinglistener_types.go | 18 +-- .../v1alpha1/autoscalingrunnerset_types.go | 20 +-- .../v1alpha1/ephemeralrunner_types.go | 9 +- ...tions.github.com_autoscalinglisteners.yaml | 10 -- ...ions.github.com_autoscalingrunnersets.yaml | 4 - .../actions.github.com_ephemeralrunners.yaml | 4 - ...ctions.github.com_ephemeralrunnersets.yaml | 4 - ...tions.github.com_autoscalinglisteners.yaml | 10 -- ...ions.github.com_autoscalingrunnersets.yaml | 4 - .../actions.github.com_ephemeralrunners.yaml | 4 - ...ctions.github.com_ephemeralrunnersets.yaml | 4 - ...tions.github.com_autoscalinglisteners.yaml | 10 -- ...ions.github.com_autoscalingrunnersets.yaml | 4 - .../actions.github.com_ephemeralrunners.yaml | 4 - ...ctions.github.com_ephemeralrunnersets.yaml | 4 - .../autoscalinglistener_controller.go | 32 ++-- .../autoscalingrunnerset_controller.go | 24 +-- .../autoscalingrunnerset_controller_test.go | 138 ++++++++++++++---- .../actions.github.com/resourcebuilder.go | 21 +++ controllers/actions.github.com/utils.go | 10 ++ 20 files changed, 194 insertions(+), 144 deletions(-) diff --git a/apis/actions.github.com/v1alpha1/autoscalinglistener_types.go b/apis/actions.github.com/v1alpha1/autoscalinglistener_types.go index 20f46135..8f86bb42 100644 --- a/apis/actions.github.com/v1alpha1/autoscalinglistener_types.go +++ b/apis/actions.github.com/v1alpha1/autoscalinglistener_types.go @@ -24,34 +24,34 @@ import ( // AutoscalingListenerSpec defines the desired state of AutoscalingListener type AutoscalingListenerSpec struct { - // +required + // +optional GitHubConfigURL string `json:"githubConfigUrl,omitempty"` - // +required + // +optional GitHubConfigSecret string `json:"githubConfigSecret,omitempty"` - // +required + // +optional // +kubebuilder:validation:Minimum:=1 RunnerScaleSetID int `json:"runnerScaleSetId,omitempty"` - // +required + // +optional AutoscalingRunnerSetNamespace string `json:"autoscalingRunnerSetNamespace,omitempty"` - // +required + // +optional AutoscalingRunnerSetName string `json:"autoscalingRunnerSetName,omitempty"` - // +required + // +optional EphemeralRunnerSetName string `json:"ephemeralRunnerSetName,omitempty"` // +kubebuilder:validation:Minimum:=0 - // +required + // +optional MaxRunners int `json:"maxRunners"` // +kubebuilder:validation:Minimum:=0 - // +required + // +optional MinRunners int `json:"minRunners"` - // +required + // +optional Image string `json:"image,omitempty"` // +optional diff --git a/apis/actions.github.com/v1alpha1/autoscalingrunnerset_types.go b/apis/actions.github.com/v1alpha1/autoscalingrunnerset_types.go index 23a3230c..804efc9c 100644 --- a/apis/actions.github.com/v1alpha1/autoscalingrunnerset_types.go +++ b/apis/actions.github.com/v1alpha1/autoscalingrunnerset_types.go @@ -56,10 +56,10 @@ type AutoscalingRunnerSet struct { // AutoscalingRunnerSetSpec defines the desired state of AutoscalingRunnerSet type AutoscalingRunnerSetSpec struct { - // +required + // +optional GitHubConfigUrl string `json:"githubConfigUrl,omitempty"` - // +required + // +optional GitHubConfigSecret string `json:"githubConfigSecret,omitempty"` // +optional @@ -80,7 +80,7 @@ type AutoscalingRunnerSetSpec struct { // +optional VaultConfig *VaultConfig `json:"vaultConfig,omitempty"` - // +required + // +optional Template corev1.PodTemplateSpec `json:"template,omitempty"` // +optional @@ -346,20 +346,6 @@ const ( AutoscalingRunnerSetPhaseOutdated AutoscalingRunnerSetPhase = "Outdated" ) -func (ars *AutoscalingRunnerSet) Hash() string { - type data struct { - Spec *AutoscalingRunnerSetSpec - Labels map[string]string - } - - d := &data{ - Spec: ars.Spec.DeepCopy(), - Labels: ars.Labels, - } - - return hash.ComputeTemplateHash(d) -} - func (ars *AutoscalingRunnerSet) ListenerSpecHash() string { arsSpec := ars.Spec.DeepCopy() spec := arsSpec diff --git a/apis/actions.github.com/v1alpha1/ephemeralrunner_types.go b/apis/actions.github.com/v1alpha1/ephemeralrunner_types.go index 5ce38f82..e53f257a 100644 --- a/apis/actions.github.com/v1alpha1/ephemeralrunner_types.go +++ b/apis/actions.github.com/v1alpha1/ephemeralrunner_types.go @@ -105,17 +105,17 @@ func (er *EphemeralRunner) VaultProxy() *ProxyConfig { // EphemeralRunnerSpec defines the desired state of EphemeralRunner type EphemeralRunnerSpec struct { - // +required + // +optional GitHubConfigURL string `json:"githubConfigUrl,omitempty"` - // +required + // +optional GitHubConfigSecret string `json:"githubConfigSecret,omitempty"` // +optional GitHubServerTLS *TLSConfig `json:"githubServerTLS,omitempty"` - // +required - RunnerScaleSetID int `json:"runnerScaleSetId,omitempty"` + // +optional + RunnerScaleSetID int `json:"runnerScaleSetId"` // +optional Proxy *ProxyConfig `json:"proxy,omitempty"` @@ -129,6 +129,7 @@ type EphemeralRunnerSpec struct { // +optional EphemeralRunnerConfigSecretMetadata *ResourceMeta `json:"ephemeralRunnerConfigSecretMetadata,omitempty"` + // +optional corev1.PodTemplateSpec `json:",inline"` } diff --git a/charts/gha-runner-scale-set-controller-experimental/crds/actions.github.com_autoscalinglisteners.yaml b/charts/gha-runner-scale-set-controller-experimental/crds/actions.github.com_autoscalinglisteners.yaml index 92652986..84e243f9 100644 --- a/charts/gha-runner-scale-set-controller-experimental/crds/actions.github.com_autoscalinglisteners.yaml +++ b/charts/gha-runner-scale-set-controller-experimental/crds/actions.github.com_autoscalinglisteners.yaml @@ -8802,16 +8802,6 @@ spec: It is used to identify which vault integration should be used to resolve secrets. type: string type: object - required: - - autoscalingRunnerSetName - - autoscalingRunnerSetNamespace - - ephemeralRunnerSetName - - githubConfigSecret - - githubConfigUrl - - image - - maxRunners - - minRunners - - runnerScaleSetId type: object status: description: AutoscalingListenerStatus defines the observed state of AutoscalingListener diff --git a/charts/gha-runner-scale-set-controller-experimental/crds/actions.github.com_autoscalingrunnersets.yaml b/charts/gha-runner-scale-set-controller-experimental/crds/actions.github.com_autoscalingrunnersets.yaml index 122b52de..b303ff14 100644 --- a/charts/gha-runner-scale-set-controller-experimental/crds/actions.github.com_autoscalingrunnersets.yaml +++ b/charts/gha-runner-scale-set-controller-experimental/crds/actions.github.com_autoscalingrunnersets.yaml @@ -16541,10 +16541,6 @@ spec: It is used to identify which vault integration should be used to resolve secrets. type: string type: object - required: - - githubConfigSecret - - githubConfigUrl - - template type: object status: description: AutoscalingRunnerSetStatus defines the observed state of AutoscalingRunnerSet diff --git a/charts/gha-runner-scale-set-controller-experimental/crds/actions.github.com_ephemeralrunners.yaml b/charts/gha-runner-scale-set-controller-experimental/crds/actions.github.com_ephemeralrunners.yaml index 0b413f41..8248263e 100644 --- a/charts/gha-runner-scale-set-controller-experimental/crds/actions.github.com_ephemeralrunners.yaml +++ b/charts/gha-runner-scale-set-controller-experimental/crds/actions.github.com_ephemeralrunners.yaml @@ -8297,10 +8297,6 @@ spec: It is used to identify which vault integration should be used to resolve secrets. type: string type: object - required: - - githubConfigSecret - - githubConfigUrl - - runnerScaleSetId type: object status: description: EphemeralRunnerStatus defines the observed state of EphemeralRunner diff --git a/charts/gha-runner-scale-set-controller-experimental/crds/actions.github.com_ephemeralrunnersets.yaml b/charts/gha-runner-scale-set-controller-experimental/crds/actions.github.com_ephemeralrunnersets.yaml index 6ff8ca2d..04c274bb 100644 --- a/charts/gha-runner-scale-set-controller-experimental/crds/actions.github.com_ephemeralrunnersets.yaml +++ b/charts/gha-runner-scale-set-controller-experimental/crds/actions.github.com_ephemeralrunnersets.yaml @@ -8306,10 +8306,6 @@ spec: It is used to identify which vault integration should be used to resolve secrets. type: string type: object - required: - - githubConfigSecret - - githubConfigUrl - - runnerScaleSetId type: object patchID: description: PatchID is the unique identifier for the patch issued by the listener app diff --git a/charts/gha-runner-scale-set-controller/crds/actions.github.com_autoscalinglisteners.yaml b/charts/gha-runner-scale-set-controller/crds/actions.github.com_autoscalinglisteners.yaml index 92652986..84e243f9 100644 --- a/charts/gha-runner-scale-set-controller/crds/actions.github.com_autoscalinglisteners.yaml +++ b/charts/gha-runner-scale-set-controller/crds/actions.github.com_autoscalinglisteners.yaml @@ -8802,16 +8802,6 @@ spec: It is used to identify which vault integration should be used to resolve secrets. type: string type: object - required: - - autoscalingRunnerSetName - - autoscalingRunnerSetNamespace - - ephemeralRunnerSetName - - githubConfigSecret - - githubConfigUrl - - image - - maxRunners - - minRunners - - runnerScaleSetId type: object status: description: AutoscalingListenerStatus defines the observed state of AutoscalingListener diff --git a/charts/gha-runner-scale-set-controller/crds/actions.github.com_autoscalingrunnersets.yaml b/charts/gha-runner-scale-set-controller/crds/actions.github.com_autoscalingrunnersets.yaml index 122b52de..b303ff14 100644 --- a/charts/gha-runner-scale-set-controller/crds/actions.github.com_autoscalingrunnersets.yaml +++ b/charts/gha-runner-scale-set-controller/crds/actions.github.com_autoscalingrunnersets.yaml @@ -16541,10 +16541,6 @@ spec: It is used to identify which vault integration should be used to resolve secrets. type: string type: object - required: - - githubConfigSecret - - githubConfigUrl - - template type: object status: description: AutoscalingRunnerSetStatus defines the observed state of AutoscalingRunnerSet diff --git a/charts/gha-runner-scale-set-controller/crds/actions.github.com_ephemeralrunners.yaml b/charts/gha-runner-scale-set-controller/crds/actions.github.com_ephemeralrunners.yaml index 0b413f41..8248263e 100644 --- a/charts/gha-runner-scale-set-controller/crds/actions.github.com_ephemeralrunners.yaml +++ b/charts/gha-runner-scale-set-controller/crds/actions.github.com_ephemeralrunners.yaml @@ -8297,10 +8297,6 @@ spec: It is used to identify which vault integration should be used to resolve secrets. type: string type: object - required: - - githubConfigSecret - - githubConfigUrl - - runnerScaleSetId type: object status: description: EphemeralRunnerStatus defines the observed state of EphemeralRunner diff --git a/charts/gha-runner-scale-set-controller/crds/actions.github.com_ephemeralrunnersets.yaml b/charts/gha-runner-scale-set-controller/crds/actions.github.com_ephemeralrunnersets.yaml index 6ff8ca2d..04c274bb 100644 --- a/charts/gha-runner-scale-set-controller/crds/actions.github.com_ephemeralrunnersets.yaml +++ b/charts/gha-runner-scale-set-controller/crds/actions.github.com_ephemeralrunnersets.yaml @@ -8306,10 +8306,6 @@ spec: It is used to identify which vault integration should be used to resolve secrets. type: string type: object - required: - - githubConfigSecret - - githubConfigUrl - - runnerScaleSetId type: object patchID: description: PatchID is the unique identifier for the patch issued by the listener app diff --git a/config/crd/bases/actions.github.com_autoscalinglisteners.yaml b/config/crd/bases/actions.github.com_autoscalinglisteners.yaml index 92652986..84e243f9 100644 --- a/config/crd/bases/actions.github.com_autoscalinglisteners.yaml +++ b/config/crd/bases/actions.github.com_autoscalinglisteners.yaml @@ -8802,16 +8802,6 @@ spec: It is used to identify which vault integration should be used to resolve secrets. type: string type: object - required: - - autoscalingRunnerSetName - - autoscalingRunnerSetNamespace - - ephemeralRunnerSetName - - githubConfigSecret - - githubConfigUrl - - image - - maxRunners - - minRunners - - runnerScaleSetId type: object status: description: AutoscalingListenerStatus defines the observed state of AutoscalingListener diff --git a/config/crd/bases/actions.github.com_autoscalingrunnersets.yaml b/config/crd/bases/actions.github.com_autoscalingrunnersets.yaml index 122b52de..b303ff14 100644 --- a/config/crd/bases/actions.github.com_autoscalingrunnersets.yaml +++ b/config/crd/bases/actions.github.com_autoscalingrunnersets.yaml @@ -16541,10 +16541,6 @@ spec: It is used to identify which vault integration should be used to resolve secrets. type: string type: object - required: - - githubConfigSecret - - githubConfigUrl - - template type: object status: description: AutoscalingRunnerSetStatus defines the observed state of AutoscalingRunnerSet diff --git a/config/crd/bases/actions.github.com_ephemeralrunners.yaml b/config/crd/bases/actions.github.com_ephemeralrunners.yaml index 0b413f41..8248263e 100644 --- a/config/crd/bases/actions.github.com_ephemeralrunners.yaml +++ b/config/crd/bases/actions.github.com_ephemeralrunners.yaml @@ -8297,10 +8297,6 @@ spec: It is used to identify which vault integration should be used to resolve secrets. type: string type: object - required: - - githubConfigSecret - - githubConfigUrl - - runnerScaleSetId type: object status: description: EphemeralRunnerStatus defines the observed state of EphemeralRunner diff --git a/config/crd/bases/actions.github.com_ephemeralrunnersets.yaml b/config/crd/bases/actions.github.com_ephemeralrunnersets.yaml index 6ff8ca2d..04c274bb 100644 --- a/config/crd/bases/actions.github.com_ephemeralrunnersets.yaml +++ b/config/crd/bases/actions.github.com_ephemeralrunnersets.yaml @@ -8306,10 +8306,6 @@ spec: It is used to identify which vault integration should be used to resolve secrets. type: string type: object - required: - - githubConfigSecret - - githubConfigUrl - - runnerScaleSetId type: object patchID: description: PatchID is the unique identifier for the patch issued by the listener app diff --git a/controllers/actions.github.com/autoscalinglistener_controller.go b/controllers/actions.github.com/autoscalinglistener_controller.go index dc8fc5bf..431bdbdd 100644 --- a/controllers/actions.github.com/autoscalinglistener_controller.go +++ b/controllers/actions.github.com/autoscalinglistener_controller.go @@ -171,7 +171,7 @@ func (r *AutoscalingListenerReconciler) Reconcile(ctx context.Context, req ctrl. shouldUpdate = true } desiredAnnotations := r.filterAndMergeAnnotations(serviceAccount.Annotations, desiredServiceAccount.Annotations) - if !maps.Equal(serviceAccount.Annotations, desiredAnnotations) { + if !r.annotationsEqual(serviceAccount.Annotations, desiredAnnotations) { updatedServiceAccount.Annotations = desiredAnnotations shouldUpdate = true } @@ -215,7 +215,7 @@ func (r *AutoscalingListenerReconciler) Reconcile(ctx context.Context, req ctrl. shouldUpdate = true } desiredAnnotations := r.filterAndMergeAnnotations(listenerRole.Annotations, desiredRole.Annotations) - if !maps.Equal(listenerRole.Annotations, desiredAnnotations) { + if !r.annotationsEqual(listenerRole.Annotations, desiredAnnotations) { updatedRole.Annotations = desiredAnnotations shouldUpdate = true } @@ -258,7 +258,7 @@ func (r *AutoscalingListenerReconciler) Reconcile(ctx context.Context, req ctrl. shouldUpdate = true } desiredAnnotations := r.filterAndMergeAnnotations(listenerRoleBinding.Annotations, desiredRoleBinding.Annotations) - if !maps.Equal(listenerRoleBinding.Annotations, desiredAnnotations) { + if !r.annotationsEqual(listenerRoleBinding.Annotations, desiredAnnotations) { updatedRoleBinding.Annotations = desiredAnnotations shouldUpdate = true } @@ -314,7 +314,7 @@ func (r *AutoscalingListenerReconciler) Reconcile(ctx context.Context, req ctrl. shouldUpdate = true } desiredAnnotations := r.filterAndMergeAnnotations(proxySecret.Annotations, desiredListenerProxy.Annotations) - if !maps.Equal(proxySecret.Annotations, desiredAnnotations) { + if !r.annotationsEqual(proxySecret.Annotations, desiredAnnotations) { updatedProxySecret.Annotations = desiredAnnotations shouldUpdate = true } @@ -401,7 +401,7 @@ func (r *AutoscalingListenerReconciler) Reconcile(ctx context.Context, req ctrl. shouldUpdate = true } desiredAnnotations := r.filterAndMergeAnnotations(listenerConfigSecret.Annotations, desiredSecret.Annotations) - if !maps.Equal(listenerConfigSecret.Annotations, desiredAnnotations) { + if !r.annotationsEqual(listenerConfigSecret.Annotations, desiredAnnotations) { updatedSecret.Annotations = desiredAnnotations shouldUpdate = true } @@ -467,9 +467,17 @@ func (r *AutoscalingListenerReconciler) Reconcile(ctx context.Context, req ctrl. return ctrl.Result{}, err } - shouldReCreate := desiredPod.Annotations[AnnotationKeyIntegrityHash] != listenerPod.Annotations[AnnotationKeyIntegrityHash] - if shouldReCreate { - log.Info("Listener pod dependency changed, recreating listener pod") + if desiredPod.Annotations[AnnotationKeyIntegrityHash] != listenerPod.Annotations[AnnotationKeyIntegrityHash] { + // Since the pod is controlled by a pod controller, we tag the pod with integrity hash. + // If the integrity hash is changed, that means the new spec is different. Keep in mind, the tagged hash + // is created by hashing only the fields this controller sets. + log.Info( + "Listener pod dependency changed, recreating listener pod", + "desiredSpec", + mustJSON(desiredPod.Spec), + "currentSpec", + mustJSON(listenerPod.Spec), + ) if err := r.deleteListenerPod(ctx, &autoscalingListener, &listenerPod, log); err != nil { return ctrl.Result{}, err } @@ -486,7 +494,7 @@ func (r *AutoscalingListenerReconciler) Reconcile(ctx context.Context, req ctrl. shouldUpdate = true } desiredAnnotations := r.filterAndMergeAnnotations(listenerPod.Annotations, desiredPod.Annotations) - if !maps.Equal(listenerPod.Annotations, desiredAnnotations) { + if !r.annotationsEqual(listenerPod.Annotations, desiredAnnotations) { updatedPod.Annotations = desiredAnnotations shouldUpdate = true } @@ -519,7 +527,11 @@ func (r *AutoscalingListenerReconciler) Reconcile(ctx context.Context, req ctrl. return ctrl.Result{}, err } - log.Info("Creating listener pod", "namespace", desiredPod.Namespace, "name", desiredPod.Name) + log.Info( + "Creating listener pod", + "namespace", desiredPod.Namespace, + "name", desiredPod.Name, + ) if err := r.Create(ctx, desiredPod); err != nil { log.Error(err, "Unable to create listener pod", "namespace", desiredPod.Namespace, "name", desiredPod.Name) return ctrl.Result{}, err diff --git a/controllers/actions.github.com/autoscalingrunnerset_controller.go b/controllers/actions.github.com/autoscalingrunnerset_controller.go index a9cc7506..a392ac81 100644 --- a/controllers/actions.github.com/autoscalingrunnerset_controller.go +++ b/controllers/actions.github.com/autoscalingrunnerset_controller.go @@ -142,13 +142,11 @@ func (r *AutoscalingRunnerSetReconciler) Reconcile(ctx context.Context, req ctrl } // Something has changed, we need to re-apply the pending phase and change hash annotation to trigger the update of runner scale set and listener. - if targetHash := autoscalingRunnerSet.Hash(); autoscalingRunnerSet.Annotations[AnnotationKeyIntegrityHash] != targetHash { - // TODO: apply the version label + if targetHash := autoscalingRunnerSetIntegrityHash(&autoscalingRunnerSet); autoscalingRunnerSet.Annotations[AnnotationKeyIntegrityHash] != targetHash { original := autoscalingRunnerSet.DeepCopy() if autoscalingRunnerSet.Annotations == nil { autoscalingRunnerSet.Annotations = map[string]string{} } - autoscalingRunnerSet.Annotations[AnnotationKeyIntegrityHash] = targetHash if err := r.Patch(ctx, &autoscalingRunnerSet, client.MergeFrom(original)); err != nil { log.Error(err, "Failed to update autoscaling runner set with new change hash and pending phase") return ctrl.Result{}, err @@ -291,7 +289,8 @@ func (r *AutoscalingRunnerSetReconciler) Reconcile(ctx context.Context, req ctrl return ctrl.Result{}, nil } - if ephemeralRunnerSet.Annotations[AnnotationKeyIntegrityHash] != desired.Annotations[AnnotationKeyIntegrityHash] { + integrityDiff := desired.Annotations[AnnotationKeyIntegrityHash] != ephemeralRunnerSetIntegrityHash(&ephemeralRunnerSet) + if integrityDiff { original := ephemeralRunnerSet.DeepCopy() ephemeralRunnerSet.Spec.EphemeralRunnerMetadata = desired.Spec.EphemeralRunnerMetadata ephemeralRunnerSet.Spec.EphemeralRunnerSpec = desired.Spec.EphemeralRunnerSpec @@ -310,7 +309,7 @@ func (r *AutoscalingRunnerSetReconciler) Reconcile(ctx context.Context, req ctrl ephemeralRunnerMetadataModified := !cmp.Equal(ephemeralRunnerSet.Spec.EphemeralRunnerMetadata, desired.Spec.EphemeralRunnerMetadata) ephemeralRunnerLabelsModified := !maps.Equal(ephemeralRunnerSet.Labels, desired.Labels) - ephemeralRunnerAnnotationsModified := !maps.Equal(ephemeralRunnerSet.Annotations, desired.Annotations) + ephemeralRunnerAnnotationsModified := !r.annotationsEqual(ephemeralRunnerSet.Annotations, desired.Annotations) if ephemeralRunnerLabelsModified || ephemeralRunnerAnnotationsModified || ephemeralRunnerMetadataModified { original := ephemeralRunnerSet.DeepCopy() @@ -358,14 +357,17 @@ func (r *AutoscalingRunnerSetReconciler) Reconcile(ctx context.Context, req ctrl } if !cmp.Equal(listener.Spec, desired.Spec) || - !cmp.Equal(listener.Labels, desired.Labels) || - !cmp.Equal(listener.Annotations, desired.Annotations) { - log.Info("Deleting AutoscalingListener to re-create with updated spec") - if err := r.Delete(ctx, &listener); err != nil { - log.Error(err, "Failed to delete AutoscalingListener for re-creation") + !maps.Equal(listener.Labels, desired.Labels) || + !r.annotationsEqual(listener.Annotations, desired.Annotations) { + log.Info("Updating listener") + listener.Spec = desired.Spec + listener.Annotations = r.filterAndMergeAnnotations(listener.Annotations, desired.Annotations) + listener.Labels = r.filterAndMergeLabels(listener.Labels, desired.Labels) + if err := r.Update(ctx, &listener); err != nil { + log.Error(err, "Failed to update AutoscalingListener with new spec") return ctrl.Result{}, err } - log.Info("Deleted AutoscalingListener, will re-create on next reconcile") + log.Info("Successfully updated AutoscalingListener with new spec") return ctrl.Result{}, nil } } diff --git a/controllers/actions.github.com/autoscalingrunnerset_controller_test.go b/controllers/actions.github.com/autoscalingrunnerset_controller_test.go index d529a1ab..fe4c1842 100644 --- a/controllers/actions.github.com/autoscalingrunnerset_controller_test.go +++ b/controllers/actions.github.com/autoscalingrunnerset_controller_test.go @@ -461,7 +461,14 @@ var _ = Describe("Test AutoScalingRunnerSet controller", Ordered, func() { listener := new(v1alpha1.AutoscalingListener) Eventually( func() error { - return k8sClient.Get(ctx, client.ObjectKey{Name: scaleSetListenerName(autoscalingRunnerSet), Namespace: autoscalingRunnerSet.Namespace}, listener) + return k8sClient.Get( + ctx, + client.ObjectKey{ + Name: scaleSetListenerName(autoscalingRunnerSet), + Namespace: autoscalingRunnerSet.Namespace, + }, + listener, + ) }, autoscalingRunnerSetTestTimeout, autoscalingRunnerSetTestInterval, @@ -472,13 +479,21 @@ var _ = Describe("Test AutoScalingRunnerSet controller", Ordered, func() { runnerSet := new(v1alpha1.EphemeralRunnerSet) Eventually( func() error { - return k8sClient.Get(ctx, client.ObjectKey{Name: autoscalingRunnerSet.Name, Namespace: autoscalingRunnerSet.Namespace}, runnerSet) + return k8sClient.Get( + ctx, + client.ObjectKey{ + Name: autoscalingRunnerSet.Name, + Namespace: autoscalingRunnerSet.Namespace, + }, + runnerSet, + ) }, autoscalingRunnerSetTestTimeout, autoscalingRunnerSetTestInterval, ).Should(Succeed(), "EphemeralRunnerSet should be created") originalRunnerSetUID := runnerSet.UID originalRunnerSetHash := runnerSet.Annotations[AnnotationKeyIntegrityHash] + originalResourceVersion := runnerSet.ResourceVersion patched := autoscalingRunnerSet.DeepCopy() patched.Spec.Template.Spec.Containers[0].Image = "ghcr.io/actions/runner:updated" @@ -492,7 +507,8 @@ var _ = Describe("Test AutoScalingRunnerSet controller", Ordered, func() { g.Expect(err).NotTo(HaveOccurred(), "failed to get EphemeralRunnerSet") g.Expect(current.UID).To(Equal(originalRunnerSetUID), "EphemeralRunnerSet should be updated in place") g.Expect(current.Spec.EphemeralRunnerSpec.PodTemplateSpec.Spec.Containers[0].Image).To(Equal("ghcr.io/actions/runner:updated")) - g.Expect(current.Annotations[AnnotationKeyIntegrityHash]).NotTo(Equal(originalRunnerSetHash), "EphemeralRunnerSet spec hash should change") + g.Expect(current.Annotations[AnnotationKeyIntegrityHash]).To(Equal(originalRunnerSetHash), "EphemeralRunnerSet hash integrity key should not be modified") + g.Expect(current.ResourceVersion).NotTo(Equal(originalResourceVersion), "EphemeralRunnerSet ResourceVersion should change after update") }, autoscalingRunnerSetTestTimeout, autoscalingRunnerSetTestInterval, @@ -504,34 +520,50 @@ var _ = Describe("Test AutoScalingRunnerSet controller", Ordered, func() { err := k8sClient.Get(ctx, client.ObjectKey{Name: scaleSetListenerName(autoscalingRunnerSet), Namespace: autoscalingRunnerSet.Namespace}, current) g.Expect(err).NotTo(HaveOccurred(), "failed to get Listener") g.Expect(current.UID).To(Equal(originalListenerUID), "Listener should not be recreated") - g.Expect(current.ResourceVersion).To(Equal(originalListenerResourceVersion), "Listener should not be updated") + g.Expect(current.ResourceVersion).To(Equal(originalListenerResourceVersion), "Listener ResourceVersion should not change after update") }, - time.Second*5, + autoscalingRunnerSetTestTimeout, autoscalingRunnerSetTestInterval, ).Should(Succeed()) }) - It("recreates only the Listener when max runners changes", func() { + It("Updates only the Listener when max runners changes", func() { listener := new(v1alpha1.AutoscalingListener) Eventually( func() error { - return k8sClient.Get(ctx, client.ObjectKey{Name: scaleSetListenerName(autoscalingRunnerSet), Namespace: autoscalingRunnerSet.Namespace}, listener) + return k8sClient.Get( + ctx, + client.ObjectKey{ + Name: scaleSetListenerName(autoscalingRunnerSet), + Namespace: autoscalingRunnerSet.Namespace, + }, + listener, + ) }, autoscalingRunnerSetTestTimeout, autoscalingRunnerSetTestInterval, ).Should(Succeed(), "Listener should be created") originalListenerUID := listener.UID + originalListenerResourceVersion := listener.ResourceVersion + originalListenerIntegrityHash := listener.Annotations[AnnotationKeyIntegrityHash] runnerSet := new(v1alpha1.EphemeralRunnerSet) Eventually( func() error { - return k8sClient.Get(ctx, client.ObjectKey{Name: autoscalingRunnerSet.Name, Namespace: autoscalingRunnerSet.Namespace}, runnerSet) + return k8sClient.Get( + ctx, + client.ObjectKey{ + Name: autoscalingRunnerSet.Name, + Namespace: autoscalingRunnerSet.Namespace, + }, + runnerSet, + ) }, autoscalingRunnerSetTestTimeout, autoscalingRunnerSetTestInterval, ).Should(Succeed(), "EphemeralRunnerSet should be created") - originalRunnerSetUID := runnerSet.UID - originalRunnerSetHash := runnerSet.Annotations[AnnotationKeyIntegrityHash] + originalERSRunnerSetUID := runnerSet.UID + originalERSResourceVersion := runnerSet.ResourceVersion patched := autoscalingRunnerSet.DeepCopy() max := 20 @@ -544,8 +576,10 @@ var _ = Describe("Test AutoScalingRunnerSet controller", Ordered, func() { current := new(v1alpha1.AutoscalingListener) err := k8sClient.Get(ctx, client.ObjectKey{Name: scaleSetListenerName(autoscalingRunnerSet), Namespace: autoscalingRunnerSet.Namespace}, current) g.Expect(err).NotTo(HaveOccurred(), "failed to get Listener") - g.Expect(current.UID).NotTo(Equal(originalListenerUID), "Listener should be recreated") + g.Expect(current.UID).To(Equal(originalListenerUID), "Listener should be updated") + g.Expect(current.Annotations[AnnotationKeyIntegrityHash]).To(Equal(originalListenerIntegrityHash), "Listener hash integrity key should not be modified") g.Expect(current.Spec.MaxRunners).To(Equal(max)) + g.Expect(current.ResourceVersion).NotTo(Equal(originalListenerResourceVersion), "Listener ResourceVersion should change after update") }, autoscalingRunnerSetTestTimeout, autoscalingRunnerSetTestInterval, @@ -556,8 +590,8 @@ var _ = Describe("Test AutoScalingRunnerSet controller", Ordered, func() { current := new(v1alpha1.EphemeralRunnerSet) err := k8sClient.Get(ctx, client.ObjectKey{Name: autoscalingRunnerSet.Name, Namespace: autoscalingRunnerSet.Namespace}, current) g.Expect(err).NotTo(HaveOccurred(), "failed to get EphemeralRunnerSet") - g.Expect(current.UID).To(Equal(originalRunnerSetUID), "EphemeralRunnerSet should not be recreated") - g.Expect(current.Annotations[AnnotationKeyIntegrityHash]).To(Equal(originalRunnerSetHash), "EphemeralRunnerSet spec should not change") + g.Expect(current.UID).To(Equal(originalERSRunnerSetUID), "EphemeralRunnerSet should not be recreated") + g.Expect(current.ResourceVersion).To(Equal(originalERSResourceVersion), "EphemeralRunnerSet spec should not change") }, time.Second*5, autoscalingRunnerSetTestInterval, @@ -568,7 +602,14 @@ var _ = Describe("Test AutoScalingRunnerSet controller", Ordered, func() { runnerSet := new(v1alpha1.EphemeralRunnerSet) Eventually( func() (string, error) { - err := k8sClient.Get(ctx, client.ObjectKey{Name: autoscalingRunnerSet.Name, Namespace: autoscalingRunnerSet.Namespace}, runnerSet) + err := k8sClient.Get( + ctx, + client.ObjectKey{ + Name: autoscalingRunnerSet.Name, + Namespace: autoscalingRunnerSet.Namespace, + }, + runnerSet, + ) if err != nil { return "", err } @@ -586,7 +627,14 @@ var _ = Describe("Test AutoScalingRunnerSet controller", Ordered, func() { Eventually( func() (string, error) { current := new(v1alpha1.EphemeralRunnerSet) - err := k8sClient.Get(ctx, client.ObjectKey{Name: autoscalingRunnerSet.Name, Namespace: autoscalingRunnerSet.Namespace}, current) + err := k8sClient.Get( + ctx, + client.ObjectKey{ + Name: autoscalingRunnerSet.Name, + Namespace: autoscalingRunnerSet.Namespace, + }, + current, + ) if err != nil { return "", err } @@ -601,7 +649,14 @@ var _ = Describe("Test AutoScalingRunnerSet controller", Ordered, func() { runnerSet := new(v1alpha1.EphemeralRunnerSet) Eventually( func() (string, error) { - err := k8sClient.Get(ctx, client.ObjectKey{Name: autoscalingRunnerSet.Name, Namespace: autoscalingRunnerSet.Namespace}, runnerSet) + err := k8sClient.Get( + ctx, + client.ObjectKey{ + Name: autoscalingRunnerSet.Name, + Namespace: autoscalingRunnerSet.Namespace, + }, + runnerSet, + ) if err != nil { return "", err } @@ -614,6 +669,8 @@ var _ = Describe("Test AutoScalingRunnerSet controller", Ordered, func() { patched := autoscalingRunnerSet.DeepCopy() patched.Spec.EphemeralRunnerSetMetadata.Annotations["arc.test/metadata-annotation"] = "updated" patched.Spec.EphemeralRunnerSetMetadata.Annotations["arc.test/new-metadata-annotation"] = "added" + originalERSIntegrityHash := runnerSet.Annotations[AnnotationKeyIntegrityHash] + patched.Spec.EphemeralRunnerSetMetadata.Annotations[AnnotationKeyIntegrityHash] = "must-not-be-modified" err := k8sClient.Patch(ctx, patched, client.MergeFrom(autoscalingRunnerSet)) Expect(err).NotTo(HaveOccurred(), "failed to patch AutoScalingRunnerSet EphemeralRunnerSet metadata") @@ -624,6 +681,7 @@ var _ = Describe("Test AutoScalingRunnerSet controller", Ordered, func() { g.Expect(err).NotTo(HaveOccurred(), "failed to get EphemeralRunnerSet") g.Expect(current.Annotations["arc.test/metadata-annotation"]).To(Equal("updated")) g.Expect(current.Annotations["arc.test/new-metadata-annotation"]).To(Equal("added")) + g.Expect(current.Annotations[AnnotationKeyIntegrityHash]).To(Equal(originalERSIntegrityHash), "EphemeralRunnerSet hash integrity key should not be modified") }, autoscalingRunnerSetTestTimeout, autoscalingRunnerSetTestInterval, @@ -634,7 +692,14 @@ var _ = Describe("Test AutoScalingRunnerSet controller", Ordered, func() { runnerSet := new(v1alpha1.EphemeralRunnerSet) Eventually( func(g Gomega) { - err := k8sClient.Get(ctx, client.ObjectKey{Name: autoscalingRunnerSet.Name, Namespace: autoscalingRunnerSet.Namespace}, runnerSet) + err := k8sClient.Get( + ctx, + client.ObjectKey{ + Name: autoscalingRunnerSet.Name, + Namespace: autoscalingRunnerSet.Namespace, + }, + runnerSet, + ) g.Expect(err).NotTo(HaveOccurred(), "failed to get EphemeralRunnerSet") g.Expect(runnerSet.Spec.EphemeralRunnerMetadata).NotTo(BeNil()) g.Expect(runnerSet.Spec.EphemeralRunnerMetadata.Labels["arc.test/runner-metadata-label"]).To(Equal("initial")) @@ -719,22 +784,38 @@ var _ = Describe("Test AutoScalingRunnerSet controller", Ordered, func() { listener := new(v1alpha1.AutoscalingListener) Eventually( func() error { - return k8sClient.Get(ctx, client.ObjectKey{Name: scaleSetListenerName(autoscalingRunnerSet), Namespace: autoscalingRunnerSet.Namespace}, listener) + return k8sClient.Get( + ctx, + client.ObjectKey{ + Name: scaleSetListenerName(autoscalingRunnerSet), + Namespace: autoscalingRunnerSet.Namespace, + }, + listener, + ) }, autoscalingRunnerSetTestTimeout, autoscalingRunnerSetTestInterval, ).Should(Succeed(), "Listener should be created") originalListenerUID := listener.UID + originalListenerIntegrityHash := listener.Annotations[AnnotationKeyIntegrityHash] runnerSet := new(v1alpha1.EphemeralRunnerSet) Eventually( func() error { - return k8sClient.Get(ctx, client.ObjectKey{Name: autoscalingRunnerSet.Name, Namespace: autoscalingRunnerSet.Namespace}, runnerSet) + return k8sClient.Get( + ctx, + client.ObjectKey{ + Name: autoscalingRunnerSet.Name, + Namespace: autoscalingRunnerSet.Namespace, + }, + runnerSet, + ) }, autoscalingRunnerSetTestTimeout, autoscalingRunnerSetTestInterval, ).Should(Succeed(), "EphemeralRunnerSet should be created") - originalRunnerSetUID := runnerSet.UID + originalEphemeralRunnerSetUID := runnerSet.UID + originalEphemeralRunnerSetIntegrityHash := runnerSet.Annotations[AnnotationKeyIntegrityHash] patched := autoscalingRunnerSet.DeepCopy() patched.Spec.GitHubConfigSecret = updatedSecret.Name @@ -757,8 +838,9 @@ var _ = Describe("Test AutoScalingRunnerSet controller", Ordered, func() { current := new(v1alpha1.EphemeralRunnerSet) err := k8sClient.Get(ctx, client.ObjectKey{Name: autoscalingRunnerSet.Name, Namespace: autoscalingRunnerSet.Namespace}, current) g.Expect(err).NotTo(HaveOccurred(), "failed to get EphemeralRunnerSet") - g.Expect(current.UID).To(Equal(originalRunnerSetUID), "EphemeralRunnerSet should be updated in place") + g.Expect(current.UID).To(Equal(originalEphemeralRunnerSetUID), "EphemeralRunnerSet should be updated in place") g.Expect(current.Spec.EphemeralRunnerSpec.GitHubConfigSecret).To(Equal(updatedSecret.Name)) + g.Expect(current.Annotations[AnnotationKeyIntegrityHash]).To(Equal(originalEphemeralRunnerSetIntegrityHash), "EphemeralRunnerSet hash integrity key should not be modified") }, autoscalingRunnerSetTestTimeout, autoscalingRunnerSetTestInterval, @@ -769,8 +851,9 @@ var _ = Describe("Test AutoScalingRunnerSet controller", Ordered, func() { current := new(v1alpha1.AutoscalingListener) err := k8sClient.Get(ctx, client.ObjectKey{Name: scaleSetListenerName(autoscalingRunnerSet), Namespace: autoscalingRunnerSet.Namespace}, current) g.Expect(err).NotTo(HaveOccurred(), "failed to get Listener") - g.Expect(current.UID).NotTo(Equal(originalListenerUID), "Listener should be recreated") - g.Expect(current.Spec.GitHubConfigSecret).To(Equal(updatedSecret.Name)) + g.Expect(current.UID).To(Equal(originalListenerUID), "Listener should be updated in place") + g.Expect(updatedSecret.Name).To(Equal(current.Spec.GitHubConfigSecret)) + g.Expect(current.Annotations[AnnotationKeyIntegrityHash]).To(Equal(originalListenerIntegrityHash), "Listener hash integrity key should not be modified") }, autoscalingRunnerSetTestTimeout, autoscalingRunnerSetTestInterval, @@ -1211,10 +1294,11 @@ var _ = Describe("Test AutoscalingController creation failures", Ordered, func() }, }, Spec: v1alpha1.AutoscalingRunnerSetSpec{ - GitHubConfigUrl: "https://github.com/owner/repo", - MaxRunners: &max, - MinRunners: &min, - RunnerGroup: "testgroup", + GitHubConfigUrl: "https://github.com/owner/repo", + GitHubConfigSecret: "secret1", + MaxRunners: &max, + MinRunners: &min, + RunnerGroup: "testgroup", Template: corev1.PodTemplateSpec{ Spec: corev1.PodSpec{ Containers: []corev1.Container{ diff --git a/controllers/actions.github.com/resourcebuilder.go b/controllers/actions.github.com/resourcebuilder.go index bdce614a..e08be51a 100644 --- a/controllers/actions.github.com/resourcebuilder.go +++ b/controllers/actions.github.com/resourcebuilder.go @@ -115,6 +115,10 @@ func (b *ResourceBuilder) setControllerReference(owner client.Object, object cli return ctrl.SetControllerReference(owner, object, b.Scheme) } +func autoscalingRunnerSetIntegrityHash(ars *v1alpha1.AutoscalingRunnerSet) string { + return hash.ComputeTemplateHash(&ars.Spec) +} + func (b *ResourceBuilder) newAutoscalingListener(autoscalingRunnerSet *v1alpha1.AutoscalingRunnerSet, ephemeralRunnerSet *v1alpha1.EphemeralRunnerSet, namespace, image string, imagePullSecrets []corev1.LocalObjectReference) (*v1alpha1.AutoscalingListener, error) { runnerScaleSetID, err := strconv.Atoi(autoscalingRunnerSet.Annotations[runnerScaleSetIDAnnotationKey]) if err != nil { @@ -1144,3 +1148,20 @@ func (b *ResourceBuilder) filterAndMergeAnnotations(base, overwrite map[string]s return result } + +// compareAnnotations compares two maps of annotations, ignoring the integrity hash annotation. +func (b *ResourceBuilder) annotationsEqual(m1, m2 map[string]string) bool { + if len(m1) != len(m2) { + return false + } + + for k, v1 := range m1 { + if k == AnnotationKeyIntegrityHash { + continue + } + if v2, ok := m2[k]; !ok || v1 != v2 { + return false + } + } + return true +} diff --git a/controllers/actions.github.com/utils.go b/controllers/actions.github.com/utils.go index a77b24ba..da87eb7b 100644 --- a/controllers/actions.github.com/utils.go +++ b/controllers/actions.github.com/utils.go @@ -1,6 +1,8 @@ package actionsgithubcom import ( + "encoding/json" + "k8s.io/apimachinery/pkg/util/rand" ) @@ -25,3 +27,11 @@ func RandStringRunes(n int) string { } return string(b) } + +func mustJSON(v any) string { + val, err := json.Marshal(v) + if err != nil { + panic(err) + } + return string(val) +}