This commit is contained in:
Nikola Jokic
2026-07-22 22:45:09 +02:00
parent daca163ca1
commit c08a1147d0
3 changed files with 62 additions and 80 deletions
@@ -303,16 +303,12 @@ 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)
ephemeralRunnerReplicasModified := ephemeralRunnerSet.Spec.Replicas != desired.Spec.Replicas
ephemeralRunnerPatchIDModified := ephemeralRunnerSet.Spec.PatchID != desired.Spec.PatchID
if ephemeralRunnerLabelsModified || ephemeralRunnerAnnotationsModified || ephemeralRunnerMetadataModified || ephemeralRunnerReplicasModified || ephemeralRunnerPatchIDModified {
if ephemeralRunnerLabelsModified || ephemeralRunnerAnnotationsModified || ephemeralRunnerMetadataModified {
original := ephemeralRunnerSet.DeepCopy()
ephemeralRunnerSet.Labels = r.filterAndMergeLabels(ephemeralRunnerSet.Labels, desired.Labels)
ephemeralRunnerSet.Annotations = desired.Annotations
ephemeralRunnerSet.Spec.EphemeralRunnerMetadata = desired.Spec.EphemeralRunnerMetadata
ephemeralRunnerSet.Spec.Replicas = desired.Spec.Replicas
ephemeralRunnerSet.Spec.PatchID = desired.Spec.PatchID
log.Info("Updating ephemeral runner set metadata to match desired labels and annotations")
if err := r.Patch(ctx, &ephemeralRunnerSet, client.MergeFrom(original)); err != nil {
log.Error(err, "Failed to patch ephemeral runner set metadata to match desired labels and annotations")
@@ -759,8 +755,6 @@ func (r *AutoscalingRunnerSetReconciler) createEphemeralRunnerSet(ctx context.Co
log.Error(err, "Could not create EphemeralRunnerSet")
return ctrl.Result{}, err
}
desiredRunnerSet.Spec.ActionableRevision = 1
log.Info("Creating a new EphemeralRunnerSet resource")
if err := r.Create(ctx, desiredRunnerSet); err != nil {
log.Error(err, "Failed to create EphemeralRunnerSet resource")
@@ -140,7 +140,7 @@ func (r *EphemeralRunnerSetReconciler) Reconcile(ctx context.Context, req ctrl.R
"specActionableRevision", ephemeralRunnerSet.Spec.ActionableRevision,
"statusAppliedActionableRevision", ephemeralRunnerSet.Status.AppliedActionableRevision,
)
if err := r.cleanUpIdleAndPendingEphemeralRunners(ctx, &ephemeralRunnerSet, log); err != nil {
if _, err := r.cleanUpEphemeralRunners(ctx, &ephemeralRunnerSet, log); err != nil {
log.Error(err, "Failed to clean up EphemeralRunners")
return ctrl.Result{}, err
}
@@ -422,60 +422,6 @@ func (r *EphemeralRunnerSetReconciler) cleanUpEphemeralRunners(ctx context.Conte
return false, nil
}
func (r *EphemeralRunnerSetReconciler) cleanUpIdleAndPendingEphemeralRunners(ctx context.Context, ephemeralRunnerSet *v1alpha1.EphemeralRunnerSet, log logr.Logger) error {
ephemeralRunnerList := new(v1alpha1.EphemeralRunnerList)
err := r.List(ctx, ephemeralRunnerList, client.InNamespace(ephemeralRunnerSet.Namespace), client.MatchingFields{resourceOwnerKey: ephemeralRunnerSet.Name})
if err != nil {
return fmt.Errorf("failed to list child ephemeral runners: %w", err)
}
ephemeralRunnerState := newEphemeralRunnersByStates(ephemeralRunnerList)
if len(ephemeralRunnerState.running) == 0 && len(ephemeralRunnerState.pending) == 0 {
return nil
}
actionsClient, err := r.GetActionsService(ctx, ephemeralRunnerSet)
if err != nil {
return err
}
log.Info("Cleanup pending or idle ephemeral runners", "pending", len(ephemeralRunnerState.pending), "running", len(ephemeralRunnerState.running))
var errs []error
for _, ephemeralRunner := range ephemeralRunnerState.pending {
log.Info("Removing the pending ephemeral runner from the service", "name", ephemeralRunner.Name)
_, err := r.deleteEphemeralRunnerWithActionsClient(ctx, ephemeralRunner, actionsClient, log)
if err != nil {
errs = append(errs, err)
}
}
for _, ephemeralRunner := range ephemeralRunnerState.running {
if ephemeralRunner.HasJob() {
log.Info(
"Skipping ephemeral runner since it is running a job",
"name", ephemeralRunner.Name,
"workflowRunId", ephemeralRunner.Status.WorkflowRunID,
"jobId", ephemeralRunner.Status.JobID,
)
continue
}
log.Info("Removing the idle ephemeral runner from the service", "name", ephemeralRunner.Name)
_, err := r.deleteEphemeralRunnerWithActionsClient(ctx, ephemeralRunner, actionsClient, log)
if err != nil {
errs = append(errs, err)
}
}
if len(errs) > 0 {
mergedErrs := multierr.Combine(errs...)
log.Error(mergedErrs, "Failed to remove idle or pending ephemeral runners from the service")
return mergedErrs
}
return nil
}
func (r *EphemeralRunnerSetReconciler) cleanUpEphemeralRunnerSetProxySecret(ctx context.Context, ephemeralRunnerSet *v1alpha1.EphemeralRunnerSet, log logr.Logger) (done bool, err error) {
if ephemeralRunnerSet.Spec.EphemeralRunnerSpec.Proxy == nil {
return true, nil
@@ -1523,6 +1523,59 @@ var _ = Describe("Test EphemeralRunnerSet actionable revision cleanup", func() {
startManagers(GinkgoT(), mgr)
})
It("does not clean up runners on initial creation without an actionable revision", func() {
controller := &EphemeralRunnerSetReconciler{
Client: mgr.GetClient(),
Scheme: mgr.GetScheme(),
Log: logf.Log,
ResourceBuilder: ResourceBuilder{
ResourceCache: newTestResourceCache(),
SecretResolver: secretresolver.New(mgr.GetClient(), fake.NewMultiClient(
fake.WithClient(fake.NewClient(fake.WithRemoveRunner(nil))),
)),
},
}
ephemeralRunnerSet := &v1alpha1.EphemeralRunnerSet{
ObjectMeta: metav1.ObjectMeta{Name: "test-actionable-revision-initial", Namespace: autoscalingNS.Name},
Spec: v1alpha1.EphemeralRunnerSetSpec{
EphemeralRunnerSpec: v1alpha1.EphemeralRunnerSpec{
GitHubConfigURL: "https://github.com/owner/repo",
GitHubConfigSecret: configSecret.Name,
RunnerScaleSetID: 100,
PodTemplateSpec: corev1.PodTemplateSpec{Spec: corev1.PodSpec{Containers: []corev1.Container{{Name: "runner", Image: "ghcr.io/actions/runner"}}}},
},
},
}
err := k8sClient.Create(ctx, ephemeralRunnerSet)
Expect(err).NotTo(HaveOccurred())
request := ctrl.Request{NamespacedName: types.NamespacedName{Name: ephemeralRunnerSet.Name, Namespace: ephemeralRunnerSet.Namespace}}
_, err = controller.Reconcile(ctx, request)
Expect(err).NotTo(HaveOccurred())
pendingRunner := newRunner("runner-pending-initial", ephemeralRunnerSet)
err = k8sClient.Create(ctx, pendingRunner)
Expect(err).NotTo(HaveOccurred())
_, err = controller.Reconcile(ctx, request)
Expect(err).NotTo(HaveOccurred())
Consistently(func() error {
runner := new(v1alpha1.EphemeralRunner)
return k8sClient.Get(ctx, types.NamespacedName{Namespace: autoscalingNS.Name, Name: pendingRunner.Name}, runner)
}, time.Second, ephemeralRunnerSetTestInterval).Should(Succeed())
Consistently(func() int64 {
updatedSet := new(v1alpha1.EphemeralRunnerSet)
if err := k8sClient.Get(ctx, request.NamespacedName, updatedSet); err != nil {
return -1
}
return updatedSet.Status.AppliedActionableRevision
}, time.Second, ephemeralRunnerSetTestInterval).Should(Equal(int64(0)))
})
It("deletes runner-a-idle, keeps runner-b-busy, and advances applied actionable revision 3 to 4", func() {
controller := &EphemeralRunnerSetReconciler{
Client: mgr.GetClient(),
@@ -1698,7 +1751,7 @@ var _ = Describe("Test EphemeralRunnerSet actionable revision cleanup", func() {
}, time.Second, ephemeralRunnerSetTestInterval).Should(Equal(int64(3)))
})
It("deletes idle runner and advances revision after restart with no cache", func() {
It("does not delete unregistered pending runner after restart with no cache", func() {
controller := &EphemeralRunnerSetReconciler{
Client: mgr.GetClient(),
Scheme: mgr.GetScheme(),
@@ -1738,31 +1791,20 @@ var _ = Describe("Test EphemeralRunnerSet actionable revision cleanup", func() {
err = k8sClient.Status().Patch(ctx, statusUpdated, client.MergeFrom(current))
Expect(err).NotTo(HaveOccurred())
// Create idle runner (running but no job)
idleRunner := newRunner("runner-restart-idle", statusUpdated)
err = k8sClient.Create(ctx, idleRunner)
Expect(err).NotTo(HaveOccurred())
idleCurrent := new(v1alpha1.EphemeralRunner)
err = k8sClient.Get(ctx, client.ObjectKeyFromObject(idleRunner), idleCurrent)
Expect(err).NotTo(HaveOccurred())
idleUpdated := idleCurrent.DeepCopy()
idleUpdated.Status.Phase = v1alpha1.EphemeralRunnerPhaseRunning
idleUpdated.Status.RunnerID = 201
err = k8sClient.Status().Patch(ctx, idleUpdated, client.MergeFrom(idleCurrent))
pendingRunner := newRunner("runner-restart-pending", statusUpdated)
err = k8sClient.Create(ctx, pendingRunner)
Expect(err).NotTo(HaveOccurred())
// Reconcile with fresh cache (simulating restart)
_, err = controller.Reconcile(ctx, request)
Expect(err).NotTo(HaveOccurred())
// Idle runner should be deleted
Eventually(func() bool {
Consistently(func() error {
runner := new(v1alpha1.EphemeralRunner)
return kerrors.IsNotFound(k8sClient.Get(ctx, types.NamespacedName{Namespace: autoscalingNS.Name, Name: "runner-restart-idle"}, runner))
}, ephemeralRunnerSetTestTimeout, ephemeralRunnerSetTestInterval).Should(BeTrue())
return k8sClient.Get(ctx, types.NamespacedName{Namespace: autoscalingNS.Name, Name: "runner-restart-pending"}, runner)
}, time.Second, ephemeralRunnerSetTestInterval).Should(Succeed())
// AppliedActionableRevision should advance to 4
// AppliedActionableRevision should advance because there was nothing safe to clean up.
Eventually(func() int64 {
updatedSet := new(v1alpha1.EphemeralRunnerSet)
if err := k8sClient.Get(ctx, request.NamespacedName, updatedSet); err != nil {