mirror of
https://github.com/actions-runner-controller/actions-runner-controller.git
synced 2026-09-30 09:51:33 +02:00
Track runner spec updates with an actionable revision
When the AutoscalingRunnerSet's runner spec changes, the EphemeralRunnerSet has to delete its idle and pending runners so they are rebuilt from the new spec. That was detected by hashing Spec.EphemeralRunnerSpec into the actions.github.com/integrity-hash annotation and comparing the annotation against a freshly computed hash. Replace it with a monotonic revision counter split across spec and status. The AutoscalingRunnerSet controller bumps Spec.ActionableRevision when it patches a new runner spec across; the EphemeralRunnerSet controller advances Status.AppliedActionableRevision only after the cleanup has actually succeeded. Because the applied marker lives in status and is written last, a controller that crashes part-way through the cleanup comes back with the applied revision still behind the spec revision and redoes the work, instead of skipping runners that are still running the old spec. The drift check uses apiequality.Semantic.DeepEqual rather than cmp.Equal or reflect.DeepEqual. Most PodSpec collection fields carry omitempty, so a template containing an explicitly empty value (`env: []`) is dropped when the EphemeralRunnerSet is written and reads back as nil. A strict comparison would report drift on every reconcile, bump the revision each time, and delete every idle and pending runner forever. helpers_drift_test.go pins that behaviour with a round-trip-through-JSON test. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
This commit is contained in:
co-authored by
Copilot App
parent
b830b658d6
commit
ec793dfe9f
@@ -36,12 +36,22 @@ type EphemeralRunnerSetSpec struct {
|
||||
// but does not apply to existing ephemeral runners.
|
||||
// +optional
|
||||
EphemeralRunnerMetadata *ResourceMeta `json:"ephemeralRunnerMetadata,omitempty"`
|
||||
// ActionableRevision is a restart-safe applied marker that increments whenever
|
||||
// Spec.EphemeralRunnerSpec changes, enabling detection of spec updates.
|
||||
// Unset defaults to 0.
|
||||
// +optional
|
||||
ActionableRevision int64 `json:"actionableRevision,omitempty"`
|
||||
}
|
||||
|
||||
// EphemeralRunnerSetStatus defines the observed state of EphemeralRunnerSet
|
||||
type EphemeralRunnerSetStatus struct {
|
||||
// +optional
|
||||
Phase EphemeralRunnerSetPhase `json:"phase"`
|
||||
// AppliedActionableRevision is a restart-safe applied marker tracking the last successfully
|
||||
// applied ActionableRevision value. Advances only after spec cleanup succeeds.
|
||||
// Unset defaults to 0.
|
||||
// +optional
|
||||
AppliedActionableRevision int64 `json:"appliedActionableRevision,omitempty"`
|
||||
}
|
||||
|
||||
// EphemeralRunnerSetPhase is the phase of the ephemeral runner set resource
|
||||
@@ -71,11 +81,6 @@ type EphemeralRunnerSet struct {
|
||||
Status EphemeralRunnerSetStatus `json:"status,omitempty"`
|
||||
}
|
||||
|
||||
// EphemeralRunnerSpecHash computes the hash value of the EphemeralRunnerSpec and returns it as a string.
|
||||
func (ers *EphemeralRunnerSet) EphemeralRunnerSpecHash() string {
|
||||
return ers.Spec.EphemeralRunnerSpec.Hash()
|
||||
}
|
||||
|
||||
func (ers *EphemeralRunnerSet) GitHubConfigSecret() string {
|
||||
return ers.Spec.EphemeralRunnerSpec.GitHubConfigSecret
|
||||
}
|
||||
|
||||
charts/gha-runner-scale-set-controller-experimental/crds/actions.github.com_ephemeralrunnersets.yaml
Generated
+14
@@ -46,6 +46,13 @@ spec:
|
||||
spec:
|
||||
description: EphemeralRunnerSetSpec defines the desired state of EphemeralRunnerSet
|
||||
properties:
|
||||
actionableRevision:
|
||||
description: |-
|
||||
ActionableRevision is a restart-safe applied marker that increments whenever
|
||||
Spec.EphemeralRunnerSpec changes, enabling detection of spec updates.
|
||||
Unset defaults to 0.
|
||||
format: int64
|
||||
type: integer
|
||||
ephemeralRunnerMetadata:
|
||||
description: |-
|
||||
EphemeralRunnerMetadata is the metadata to be applied to all ephemeral runners created by this set.
|
||||
@@ -8295,6 +8302,13 @@ spec:
|
||||
status:
|
||||
description: EphemeralRunnerSetStatus defines the observed state of EphemeralRunnerSet
|
||||
properties:
|
||||
appliedActionableRevision:
|
||||
description: |-
|
||||
AppliedActionableRevision is a restart-safe applied marker tracking the last successfully
|
||||
applied ActionableRevision value. Advances only after spec cleanup succeeds.
|
||||
Unset defaults to 0.
|
||||
format: int64
|
||||
type: integer
|
||||
phase:
|
||||
description: EphemeralRunnerSetPhase is the phase of the ephemeral runner set resource
|
||||
type: string
|
||||
|
||||
+14
@@ -46,6 +46,13 @@ spec:
|
||||
spec:
|
||||
description: EphemeralRunnerSetSpec defines the desired state of EphemeralRunnerSet
|
||||
properties:
|
||||
actionableRevision:
|
||||
description: |-
|
||||
ActionableRevision is a restart-safe applied marker that increments whenever
|
||||
Spec.EphemeralRunnerSpec changes, enabling detection of spec updates.
|
||||
Unset defaults to 0.
|
||||
format: int64
|
||||
type: integer
|
||||
ephemeralRunnerMetadata:
|
||||
description: |-
|
||||
EphemeralRunnerMetadata is the metadata to be applied to all ephemeral runners created by this set.
|
||||
@@ -8295,6 +8302,13 @@ spec:
|
||||
status:
|
||||
description: EphemeralRunnerSetStatus defines the observed state of EphemeralRunnerSet
|
||||
properties:
|
||||
appliedActionableRevision:
|
||||
description: |-
|
||||
AppliedActionableRevision is a restart-safe applied marker tracking the last successfully
|
||||
applied ActionableRevision value. Advances only after spec cleanup succeeds.
|
||||
Unset defaults to 0.
|
||||
format: int64
|
||||
type: integer
|
||||
phase:
|
||||
description: EphemeralRunnerSetPhase is the phase of the ephemeral runner set resource
|
||||
type: string
|
||||
|
||||
@@ -46,6 +46,13 @@ spec:
|
||||
spec:
|
||||
description: EphemeralRunnerSetSpec defines the desired state of EphemeralRunnerSet
|
||||
properties:
|
||||
actionableRevision:
|
||||
description: |-
|
||||
ActionableRevision is a restart-safe applied marker that increments whenever
|
||||
Spec.EphemeralRunnerSpec changes, enabling detection of spec updates.
|
||||
Unset defaults to 0.
|
||||
format: int64
|
||||
type: integer
|
||||
ephemeralRunnerMetadata:
|
||||
description: |-
|
||||
EphemeralRunnerMetadata is the metadata to be applied to all ephemeral runners created by this set.
|
||||
@@ -8295,6 +8302,13 @@ spec:
|
||||
status:
|
||||
description: EphemeralRunnerSetStatus defines the observed state of EphemeralRunnerSet
|
||||
properties:
|
||||
appliedActionableRevision:
|
||||
description: |-
|
||||
AppliedActionableRevision is a restart-safe applied marker tracking the last successfully
|
||||
applied ActionableRevision value. Advances only after spec cleanup succeeds.
|
||||
Unset defaults to 0.
|
||||
format: int64
|
||||
type: integer
|
||||
phase:
|
||||
description: EphemeralRunnerSetPhase is the phase of the ephemeral runner set resource
|
||||
type: string
|
||||
|
||||
@@ -288,10 +288,11 @@ func (r *AutoscalingRunnerSetReconciler) Reconcile(ctx context.Context, req ctrl
|
||||
return ctrl.Result{}, nil
|
||||
}
|
||||
|
||||
if ephemeralRunnerSet.Annotations[annotationKeyIntegrityHash] != desired.Annotations[annotationKeyIntegrityHash] {
|
||||
if ephemeralRunnerSetActionableSpecChanged(&ephemeralRunnerSet, desired) {
|
||||
original := ephemeralRunnerSet.DeepCopy()
|
||||
ephemeralRunnerSet.Spec.EphemeralRunnerMetadata = desired.Spec.EphemeralRunnerMetadata
|
||||
ephemeralRunnerSet.Spec.EphemeralRunnerSpec = desired.Spec.EphemeralRunnerSpec
|
||||
ephemeralRunnerSet.Spec.ActionableRevision = nextActionableRevision(&ephemeralRunnerSet)
|
||||
ephemeralRunnerSet.Labels = r.filterAndMergeLabels(ephemeralRunnerSet.Labels, desired.Labels)
|
||||
ephemeralRunnerSet.Annotations = r.mergeAnnotations(ephemeralRunnerSet.Annotations, desired.Annotations)
|
||||
|
||||
|
||||
@@ -743,7 +743,7 @@ var _ = Describe("Test AutoScalingRunnerSet controller", Ordered, func() {
|
||||
autoscalingRunnerSetTestInterval,
|
||||
).Should(Succeed(), "EphemeralRunnerSet should be created")
|
||||
originalRunnerSetUID := runnerSet.UID
|
||||
originalRunnerSetHash := runnerSet.Annotations[annotationKeyIntegrityHash]
|
||||
originalActionableRevision := runnerSet.Spec.ActionableRevision
|
||||
|
||||
patched := autoscalingRunnerSet.DeepCopy()
|
||||
patched.Spec.Template.Spec.Containers[0].Image = "ghcr.io/actions/runner:updated"
|
||||
@@ -757,7 +757,7 @@ 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.Spec.ActionableRevision).To(BeNumerically(">", originalActionableRevision), "ActionableRevision should increment for actionable spec changes")
|
||||
},
|
||||
autoscalingRunnerSetTestTimeout,
|
||||
autoscalingRunnerSetTestInterval,
|
||||
@@ -796,7 +796,7 @@ var _ = Describe("Test AutoScalingRunnerSet controller", Ordered, func() {
|
||||
autoscalingRunnerSetTestInterval,
|
||||
).Should(Succeed(), "EphemeralRunnerSet should be created")
|
||||
originalRunnerSetUID := runnerSet.UID
|
||||
originalRunnerSetHash := runnerSet.Annotations[annotationKeyIntegrityHash]
|
||||
originalActionableRevision := runnerSet.Spec.ActionableRevision
|
||||
|
||||
patched := autoscalingRunnerSet.DeepCopy()
|
||||
max := 20
|
||||
@@ -822,7 +822,7 @@ var _ = Describe("Test AutoScalingRunnerSet controller", Ordered, func() {
|
||||
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.Spec.ActionableRevision).To(Equal(originalActionableRevision), "ActionableRevision should not change for non-actionable updates")
|
||||
},
|
||||
time.Second*5,
|
||||
autoscalingRunnerSetTestInterval,
|
||||
|
||||
@@ -35,6 +35,7 @@ import (
|
||||
kerrors "k8s.io/apimachinery/pkg/api/errors"
|
||||
"k8s.io/apimachinery/pkg/runtime"
|
||||
"k8s.io/apimachinery/pkg/types"
|
||||
"k8s.io/client-go/util/retry"
|
||||
ctrl "sigs.k8s.io/controller-runtime"
|
||||
"sigs.k8s.io/controller-runtime/pkg/client"
|
||||
"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
|
||||
@@ -133,11 +134,15 @@ func (r *EphemeralRunnerSetReconciler) Reconcile(ctx context.Context, req ctrl.R
|
||||
return ctrl.Result{}, nil
|
||||
}
|
||||
|
||||
// If hash spec has changed, delete idle ephemeral runners
|
||||
// in order to apply the change to the runners that did not yet receive a job.
|
||||
ephemeralRunnerIntegrityHash := ephemeralRunnerSetIntegrityHash(&ephemeralRunnerSet)
|
||||
if ephemeralRunnerSet.Annotations[annotationKeyIntegrityHash] != ephemeralRunnerIntegrityHash {
|
||||
log.Info("EphemeralRunnerSpec has changed, deleting idle ephemeral runners to apply the new spec")
|
||||
// If the runner spec revision has advanced past the one that was last
|
||||
// successfully applied, delete idle and pending ephemeral runners so they are
|
||||
// rebuilt from the new spec.
|
||||
if ephemeralRunnerSet.Spec.ActionableRevision > ephemeralRunnerSet.Status.AppliedActionableRevision {
|
||||
log.Info(
|
||||
"EphemeralRunnerSpec revision has changed, deleting idle or pending ephemeral runners to apply the new spec",
|
||||
"specActionableRevision", ephemeralRunnerSet.Spec.ActionableRevision,
|
||||
"statusAppliedActionableRevision", ephemeralRunnerSet.Status.AppliedActionableRevision,
|
||||
)
|
||||
if _, err := r.cleanUpEphemeralRunners(ctx, &ephemeralRunnerSet, log); err != nil {
|
||||
log.Error(err, "Failed to clean up EphemeralRunners")
|
||||
return ctrl.Result{}, err
|
||||
@@ -148,18 +153,12 @@ func (r *EphemeralRunnerSetReconciler) Reconcile(ctx context.Context, req ctrl.R
|
||||
return ctrl.Result{}, err
|
||||
}
|
||||
|
||||
log.Info("Updating EphemeralRunnerSet with new spec hash")
|
||||
original := ephemeralRunnerSet.DeepCopy()
|
||||
if ephemeralRunnerSet.Annotations == nil {
|
||||
ephemeralRunnerSet.Annotations = make(map[string]string)
|
||||
}
|
||||
ephemeralRunnerSet.Annotations[annotationKeyIntegrityHash] = ephemeralRunnerIntegrityHash
|
||||
if err := r.Patch(ctx, &ephemeralRunnerSet, client.MergeFrom(original)); err != nil {
|
||||
log.Error(err, "Failed to update ephemeral runner set with new spec hash")
|
||||
if err := r.patchAppliedActionableRevisionStatus(ctx, req.NamespacedName, ephemeralRunnerSet.Spec.ActionableRevision); err != nil {
|
||||
log.Error(err, "Failed to update EphemeralRunnerSet applied actionable revision status")
|
||||
return ctrl.Result{}, err
|
||||
}
|
||||
|
||||
log.Info("Updated ephemeral runner set with new spec hash")
|
||||
log.Info("Updated EphemeralRunnerSet applied actionable revision status", "appliedActionableRevision", ephemeralRunnerSet.Spec.ActionableRevision)
|
||||
return ctrl.Result{}, nil
|
||||
}
|
||||
|
||||
@@ -245,6 +244,39 @@ func (r *EphemeralRunnerSetReconciler) Reconcile(ctx context.Context, req ctrl.R
|
||||
return ctrl.Result{}, r.updateStatus(ctx, &ephemeralRunnerSet, ephemeralRunnersByState, log)
|
||||
}
|
||||
|
||||
// patchAppliedActionableRevisionStatus records that the runner spec carried by
|
||||
// targetAppliedRevision has been fully applied.
|
||||
//
|
||||
// The marker lives in status rather than in an annotation on the spec, and it is
|
||||
// written only once the cleanup above has actually succeeded. If the controller
|
||||
// dies part-way through deleting the idle and pending runners, the applied
|
||||
// revision is still behind the spec revision when it comes back, so the work is
|
||||
// redone rather than skipped. Writing the marker first, or writing it together
|
||||
// with the spec, would let a crash leave runners alive that are running a spec
|
||||
// nobody will ever revisit.
|
||||
//
|
||||
// The object is re-fetched inside the retry rather than reusing the copy the
|
||||
// reconciler already has, because the cleanup can take long enough for that copy
|
||||
// to go stale, and a conflicting write must not be resolved by replaying an old
|
||||
// status.
|
||||
func (r *EphemeralRunnerSetReconciler) patchAppliedActionableRevisionStatus(ctx context.Context, key types.NamespacedName, targetAppliedRevision int64) error {
|
||||
return retry.RetryOnConflict(retry.DefaultBackoff, func() error {
|
||||
var latest v1alpha1.EphemeralRunnerSet
|
||||
if err := r.Get(ctx, key, &latest); err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
if latest.Status.AppliedActionableRevision >= targetAppliedRevision {
|
||||
return nil
|
||||
}
|
||||
|
||||
original := latest.DeepCopy()
|
||||
latest.Status.AppliedActionableRevision = targetAppliedRevision
|
||||
|
||||
return r.Status().Patch(ctx, &latest, client.MergeFrom(original))
|
||||
})
|
||||
}
|
||||
|
||||
func (r *EphemeralRunnerSetReconciler) updateStatus(ctx context.Context, ephemeralRunnerSet *v1alpha1.EphemeralRunnerSet, state *ephemeralRunnersByState, log logr.Logger) error {
|
||||
original := ephemeralRunnerSet.DeepCopy()
|
||||
var phase v1alpha1.EphemeralRunnerSetPhase
|
||||
@@ -257,7 +289,8 @@ func (r *EphemeralRunnerSetReconciler) updateStatus(ctx context.Context, ephemer
|
||||
phase = ephemeralRunnerSet.Status.Phase
|
||||
}
|
||||
desiredStatus := v1alpha1.EphemeralRunnerSetStatus{
|
||||
Phase: phase,
|
||||
Phase: phase,
|
||||
AppliedActionableRevision: ephemeralRunnerSet.Status.AppliedActionableRevision,
|
||||
}
|
||||
|
||||
// Update the status if needed.
|
||||
|
||||
@@ -1117,6 +1117,7 @@ var _ = Describe("Test EphemeralRunnerSet controller", func() {
|
||||
|
||||
updated = ers.DeepCopy()
|
||||
updated.Spec.EphemeralRunnerSpec.PodTemplateSpec.Spec.Containers[0].Image = "ghcr.io/actions/runner:new"
|
||||
updated.Spec.ActionableRevision = ers.Spec.ActionableRevision + 1
|
||||
err = k8sClient.Patch(ctx, updated, client.MergeFrom(ers))
|
||||
Expect(err).NotTo(HaveOccurred(), "failed to patch EphemeralRunnerSet with new spec")
|
||||
|
||||
@@ -1920,3 +1921,464 @@ func listEphemeralRunnersAndRemoveFinalizers(ctx context.Context, k8sClient clie
|
||||
list.Items = liveItems
|
||||
return nil
|
||||
}
|
||||
|
||||
var _ = Describe("Test EphemeralRunnerSet actionable revision cleanup", func() {
|
||||
var ctx context.Context
|
||||
var mgr ctrl.Manager
|
||||
var autoscalingNS *corev1.Namespace
|
||||
var configSecret *corev1.Secret
|
||||
|
||||
newRunner := func(name string, ers *v1alpha1.EphemeralRunnerSet) *v1alpha1.EphemeralRunner {
|
||||
controllerRef := true
|
||||
return &v1alpha1.EphemeralRunner{
|
||||
ObjectMeta: metav1.ObjectMeta{
|
||||
Name: name,
|
||||
Namespace: ers.Namespace,
|
||||
OwnerReferences: []metav1.OwnerReference{{
|
||||
APIVersion: v1alpha1.GroupVersion.String(),
|
||||
Kind: "EphemeralRunnerSet",
|
||||
Name: ers.Name,
|
||||
UID: ers.UID,
|
||||
Controller: &controllerRef,
|
||||
}},
|
||||
},
|
||||
Spec: ers.Spec.EphemeralRunnerSpec,
|
||||
}
|
||||
}
|
||||
|
||||
BeforeEach(func() {
|
||||
ctx = context.Background()
|
||||
autoscalingNS, mgr = createNamespace(GinkgoT(), k8sClient)
|
||||
configSecret = createDefaultSecret(GinkgoT(), k8sClient, autoscalingNS.Name)
|
||||
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(),
|
||||
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-success", Namespace: autoscalingNS.Name},
|
||||
Spec: v1alpha1.EphemeralRunnerSetSpec{
|
||||
ActionableRevision: 3,
|
||||
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())
|
||||
|
||||
current := new(v1alpha1.EphemeralRunnerSet)
|
||||
err = k8sClient.Get(ctx, request.NamespacedName, current)
|
||||
Expect(err).NotTo(HaveOccurred())
|
||||
|
||||
statusUpdated := current.DeepCopy()
|
||||
statusUpdated.Status.AppliedActionableRevision = 3
|
||||
statusUpdated.Status.Phase = v1alpha1.EphemeralRunnerSetPhaseRunning
|
||||
err = k8sClient.Status().Patch(ctx, statusUpdated, client.MergeFrom(current))
|
||||
Expect(err).NotTo(HaveOccurred())
|
||||
|
||||
idleRunner := newRunner("runner-a-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 = 101
|
||||
err = k8sClient.Status().Patch(ctx, idleUpdated, client.MergeFrom(idleCurrent))
|
||||
Expect(err).NotTo(HaveOccurred())
|
||||
|
||||
busyRunner := newRunner("runner-b-busy", statusUpdated)
|
||||
err = k8sClient.Create(ctx, busyRunner)
|
||||
Expect(err).NotTo(HaveOccurred())
|
||||
|
||||
busyCurrent := new(v1alpha1.EphemeralRunner)
|
||||
err = k8sClient.Get(ctx, client.ObjectKeyFromObject(busyRunner), busyCurrent)
|
||||
Expect(err).NotTo(HaveOccurred())
|
||||
busyUpdated := busyCurrent.DeepCopy()
|
||||
busyUpdated.Status.Phase = v1alpha1.EphemeralRunnerPhaseRunning
|
||||
busyUpdated.Status.RunnerID = 102
|
||||
busyUpdated.Status.JobID = "job-1"
|
||||
busyUpdated.Status.WorkflowRunID = 9001
|
||||
err = k8sClient.Status().Patch(ctx, busyUpdated, client.MergeFrom(busyCurrent))
|
||||
Expect(err).NotTo(HaveOccurred())
|
||||
|
||||
err = k8sClient.Get(ctx, request.NamespacedName, current)
|
||||
Expect(err).NotTo(HaveOccurred())
|
||||
specUpdated := current.DeepCopy()
|
||||
specUpdated.Spec.ActionableRevision = 4
|
||||
err = k8sClient.Patch(ctx, specUpdated, client.MergeFrom(current))
|
||||
Expect(err).NotTo(HaveOccurred())
|
||||
|
||||
Eventually(func() bool {
|
||||
_, err := controller.Reconcile(ctx, request)
|
||||
Expect(err).NotTo(HaveOccurred())
|
||||
|
||||
runner := new(v1alpha1.EphemeralRunner)
|
||||
return kerrors.IsNotFound(k8sClient.Get(ctx, types.NamespacedName{Namespace: autoscalingNS.Name, Name: "runner-a-idle"}, runner))
|
||||
}, ephemeralRunnerSetTestTimeout, ephemeralRunnerSetTestInterval).Should(BeTrue())
|
||||
|
||||
Consistently(func() error {
|
||||
runner := new(v1alpha1.EphemeralRunner)
|
||||
if err := k8sClient.Get(ctx, types.NamespacedName{Namespace: autoscalingNS.Name, Name: "runner-b-busy"}, runner); err != nil {
|
||||
return err
|
||||
}
|
||||
if runner.Status.RunnerID != 102 {
|
||||
return fmt.Errorf("expected busy runner ID 102, got %d", runner.Status.RunnerID)
|
||||
}
|
||||
if !runner.HasJob() {
|
||||
return fmt.Errorf("expected runner-b-busy to keep its assigned job")
|
||||
}
|
||||
return nil
|
||||
}, time.Second, ephemeralRunnerSetTestInterval).Should(Succeed())
|
||||
|
||||
Eventually(func() int64 {
|
||||
_, err := controller.Reconcile(ctx, request)
|
||||
Expect(err).NotTo(HaveOccurred())
|
||||
|
||||
updatedSet := new(v1alpha1.EphemeralRunnerSet)
|
||||
if err := k8sClient.Get(ctx, request.NamespacedName, updatedSet); err != nil {
|
||||
return 0
|
||||
}
|
||||
return updatedSet.Status.AppliedActionableRevision
|
||||
}, ephemeralRunnerSetTestTimeout, ephemeralRunnerSetTestInterval).Should(Equal(int64(4)))
|
||||
})
|
||||
|
||||
It("keeps applied actionable revision at 3 when cleanup fails", 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(fmt.Errorf("remove failed")))),
|
||||
)),
|
||||
},
|
||||
}
|
||||
|
||||
ephemeralRunnerSet := &v1alpha1.EphemeralRunnerSet{
|
||||
ObjectMeta: metav1.ObjectMeta{Name: "test-actionable-revision-error", Namespace: autoscalingNS.Name},
|
||||
Spec: v1alpha1.EphemeralRunnerSetSpec{
|
||||
ActionableRevision: 3,
|
||||
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())
|
||||
|
||||
current := new(v1alpha1.EphemeralRunnerSet)
|
||||
err = k8sClient.Get(ctx, request.NamespacedName, current)
|
||||
Expect(err).NotTo(HaveOccurred())
|
||||
|
||||
statusUpdated := current.DeepCopy()
|
||||
statusUpdated.Status.AppliedActionableRevision = 3
|
||||
err = k8sClient.Status().Patch(ctx, statusUpdated, client.MergeFrom(current))
|
||||
Expect(err).NotTo(HaveOccurred())
|
||||
|
||||
idleRunner := newRunner("runner-a-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 = 101
|
||||
err = k8sClient.Status().Patch(ctx, idleUpdated, client.MergeFrom(idleCurrent))
|
||||
Expect(err).NotTo(HaveOccurred())
|
||||
|
||||
err = k8sClient.Get(ctx, request.NamespacedName, current)
|
||||
Expect(err).NotTo(HaveOccurred())
|
||||
specUpdated := current.DeepCopy()
|
||||
specUpdated.Spec.ActionableRevision = 4
|
||||
err = k8sClient.Patch(ctx, specUpdated, client.MergeFrom(current))
|
||||
Expect(err).NotTo(HaveOccurred())
|
||||
|
||||
// The reconciler reads through the manager's cache, so retry until the
|
||||
// bumped actionable revision is observed and cleanup is attempted.
|
||||
Eventually(func() error {
|
||||
_, err := controller.Reconcile(ctx, request)
|
||||
return err
|
||||
}, ephemeralRunnerSetTestTimeout, ephemeralRunnerSetTestInterval).Should(MatchError(ContainSubstring("remove failed")))
|
||||
|
||||
Consistently(func() int64 {
|
||||
updatedSet := new(v1alpha1.EphemeralRunnerSet)
|
||||
if err := k8sClient.Get(ctx, request.NamespacedName, updatedSet); err != nil {
|
||||
return 0
|
||||
}
|
||||
return updatedSet.Status.AppliedActionableRevision
|
||||
}, time.Second, ephemeralRunnerSetTestInterval).Should(Equal(int64(3)))
|
||||
})
|
||||
|
||||
It("deletes unregistered pending runner during actionable revision cleanup after restart with no cache", func() {
|
||||
controller := &EphemeralRunnerSetReconciler{
|
||||
Client: mgr.GetClient(),
|
||||
Scheme: mgr.GetScheme(),
|
||||
Log: logf.Log,
|
||||
ResourceBuilder: ResourceBuilder{
|
||||
ResourceCache: newTestResourceCache(), // fresh empty cache simulating restart
|
||||
SecretResolver: secretresolver.New(mgr.GetClient(), fake.NewMultiClient()),
|
||||
},
|
||||
}
|
||||
|
||||
ephemeralRunnerSet := &v1alpha1.EphemeralRunnerSet{
|
||||
ObjectMeta: metav1.ObjectMeta{Name: "test-restart-no-cache", Namespace: autoscalingNS.Name},
|
||||
Spec: v1alpha1.EphemeralRunnerSetSpec{
|
||||
ActionableRevision: 4, // spec has been bumped
|
||||
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:updated"}}}},
|
||||
},
|
||||
},
|
||||
}
|
||||
|
||||
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())
|
||||
|
||||
current := new(v1alpha1.EphemeralRunnerSet)
|
||||
err = k8sClient.Get(ctx, request.NamespacedName, current)
|
||||
Expect(err).NotTo(HaveOccurred())
|
||||
|
||||
statusUpdated := current.DeepCopy()
|
||||
statusUpdated.Status.AppliedActionableRevision = 3 // status is behind
|
||||
err = k8sClient.Status().Patch(ctx, statusUpdated, client.MergeFrom(current))
|
||||
Expect(err).NotTo(HaveOccurred())
|
||||
|
||||
pendingRunner := newRunner("runner-restart-pending", statusUpdated)
|
||||
err = k8sClient.Create(ctx, pendingRunner)
|
||||
Expect(err).NotTo(HaveOccurred())
|
||||
|
||||
Eventually(func(g Gomega) {
|
||||
cachedSet := new(v1alpha1.EphemeralRunnerSet)
|
||||
err := controller.Get(ctx, request.NamespacedName, cachedSet)
|
||||
g.Expect(err).NotTo(HaveOccurred())
|
||||
g.Expect(cachedSet.Status.AppliedActionableRevision).To(Equal(int64(3)))
|
||||
|
||||
cachedRunner := new(v1alpha1.EphemeralRunner)
|
||||
err = controller.Get(ctx, types.NamespacedName{Namespace: autoscalingNS.Name, Name: "runner-restart-pending"}, cachedRunner)
|
||||
g.Expect(err).NotTo(HaveOccurred())
|
||||
g.Expect(cachedRunner.Status.RunnerID).To(BeZero())
|
||||
g.Expect(cachedRunner.Status.Phase).To(BeEmpty())
|
||||
}, ephemeralRunnerSetTestTimeout, ephemeralRunnerSetTestInterval).Should(Succeed())
|
||||
|
||||
// Reconcile with fresh cache (simulating restart). Actionable revision cleanup deletes pending runners.
|
||||
Eventually(func() bool {
|
||||
_, err := controller.Reconcile(ctx, request)
|
||||
Expect(err).NotTo(HaveOccurred())
|
||||
|
||||
runner := new(v1alpha1.EphemeralRunner)
|
||||
return kerrors.IsNotFound(k8sClient.Get(ctx, types.NamespacedName{Namespace: autoscalingNS.Name, Name: "runner-restart-pending"}, runner))
|
||||
}, ephemeralRunnerSetTestTimeout, ephemeralRunnerSetTestInterval).Should(BeTrue())
|
||||
|
||||
// AppliedActionableRevision should advance after cleanup completes.
|
||||
Eventually(func() int64 {
|
||||
_, err := controller.Reconcile(ctx, request)
|
||||
Expect(err).NotTo(HaveOccurred())
|
||||
|
||||
updatedSet := new(v1alpha1.EphemeralRunnerSet)
|
||||
if err := k8sClient.Get(ctx, request.NamespacedName, updatedSet); err != nil {
|
||||
return 0
|
||||
}
|
||||
return updatedSet.Status.AppliedActionableRevision
|
||||
}, ephemeralRunnerSetTestTimeout, ephemeralRunnerSetTestInterval).Should(Equal(int64(4)))
|
||||
})
|
||||
|
||||
It("preserves AppliedActionableRevision during status-only phase updates", 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()),
|
||||
)),
|
||||
},
|
||||
}
|
||||
|
||||
// Setup: Create ERS with an actionable revision
|
||||
ephemeralRunnerSet := &v1alpha1.EphemeralRunnerSet{
|
||||
ObjectMeta: metav1.ObjectMeta{Name: "test-preserve-applied-revision", Namespace: autoscalingNS.Name},
|
||||
Spec: v1alpha1.EphemeralRunnerSetSpec{
|
||||
ActionableRevision: 5,
|
||||
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())
|
||||
|
||||
// Set AppliedActionableRevision to 5
|
||||
current := new(v1alpha1.EphemeralRunnerSet)
|
||||
err = k8sClient.Get(ctx, request.NamespacedName, current)
|
||||
Expect(err).NotTo(HaveOccurred())
|
||||
|
||||
statusUpdated := current.DeepCopy()
|
||||
statusUpdated.Status.AppliedActionableRevision = 5
|
||||
statusUpdated.Status.Phase = v1alpha1.EphemeralRunnerSetPhaseRunning
|
||||
err = k8sClient.Status().Patch(ctx, statusUpdated, client.MergeFrom(current))
|
||||
Expect(err).NotTo(HaveOccurred())
|
||||
|
||||
// Create a runner that will cause phase change (outdated runner).
|
||||
ephemeralRunner := &v1alpha1.EphemeralRunner{
|
||||
ObjectMeta: metav1.ObjectMeta{
|
||||
Name: "test-runner-outdated",
|
||||
Namespace: autoscalingNS.Name,
|
||||
Labels: map[string]string{
|
||||
LabelKeyGitHubScaleSetName: ephemeralRunnerSet.Name,
|
||||
LabelKeyGitHubScaleSetNamespace: ephemeralRunnerSet.Namespace,
|
||||
},
|
||||
OwnerReferences: []metav1.OwnerReference{
|
||||
{
|
||||
APIVersion: v1alpha1.GroupVersion.String(),
|
||||
Kind: "EphemeralRunnerSet",
|
||||
Name: ephemeralRunnerSet.Name,
|
||||
UID: ephemeralRunnerSet.UID,
|
||||
Controller: func(b bool) *bool { return &b }(true),
|
||||
BlockOwnerDeletion: func(b bool) *bool { return &b }(true),
|
||||
},
|
||||
},
|
||||
},
|
||||
Spec: 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:old"}}}},
|
||||
},
|
||||
}
|
||||
err = k8sClient.Create(ctx, ephemeralRunner)
|
||||
Expect(err).NotTo(HaveOccurred())
|
||||
|
||||
runnerStatusUpdated := ephemeralRunner.DeepCopy()
|
||||
runnerStatusUpdated.Status.Phase = v1alpha1.EphemeralRunnerPhaseOutdated
|
||||
runnerStatusUpdated.Status.RunnerID = 123
|
||||
runnerStatusUpdated.Status.JobRequestID = 456
|
||||
err = k8sClient.Status().Patch(ctx, runnerStatusUpdated, client.MergeFrom(ephemeralRunner))
|
||||
Expect(err).NotTo(HaveOccurred())
|
||||
|
||||
Eventually(func(g Gomega) {
|
||||
cachedSet := new(v1alpha1.EphemeralRunnerSet)
|
||||
err := controller.Get(ctx, request.NamespacedName, cachedSet)
|
||||
g.Expect(err).NotTo(HaveOccurred())
|
||||
g.Expect(cachedSet.Status.AppliedActionableRevision).To(Equal(int64(5)))
|
||||
|
||||
cachedRunner := new(v1alpha1.EphemeralRunner)
|
||||
err = controller.Get(ctx, types.NamespacedName{Namespace: autoscalingNS.Name, Name: "test-runner-outdated"}, cachedRunner)
|
||||
g.Expect(err).NotTo(HaveOccurred())
|
||||
g.Expect(cachedRunner.Status.Phase).To(Equal(v1alpha1.EphemeralRunnerPhaseOutdated))
|
||||
}, ephemeralRunnerSetTestTimeout, ephemeralRunnerSetTestInterval).Should(Succeed())
|
||||
|
||||
// Verify: Phase changed to Outdated, but AppliedActionableRevision preserved
|
||||
Eventually(func(g Gomega) {
|
||||
_, err := controller.Reconcile(ctx, request)
|
||||
g.Expect(err).NotTo(HaveOccurred())
|
||||
|
||||
updatedSet := new(v1alpha1.EphemeralRunnerSet)
|
||||
err = k8sClient.Get(ctx, request.NamespacedName, updatedSet)
|
||||
g.Expect(err).NotTo(HaveOccurred())
|
||||
g.Expect(updatedSet.Status.Phase).To(Equal(v1alpha1.EphemeralRunnerSetPhaseOutdated), "phase should change to Outdated")
|
||||
g.Expect(updatedSet.Status.AppliedActionableRevision).To(Equal(int64(5)), "AppliedActionableRevision should be preserved")
|
||||
}, ephemeralRunnerSetTestTimeout, ephemeralRunnerSetTestInterval).Should(Succeed())
|
||||
})
|
||||
})
|
||||
|
||||
@@ -1,10 +1,44 @@
|
||||
package actionsgithubcom
|
||||
|
||||
import (
|
||||
"github.com/actions/actions-runner-controller/apis/actions.github.com/v1alpha1"
|
||||
corev1 "k8s.io/api/core/v1"
|
||||
apiequality "k8s.io/apimachinery/pkg/api/equality"
|
||||
)
|
||||
|
||||
// ephemeralRunnerSetActionableSpecChanged reports whether the runner spec the
|
||||
// EphemeralRunnerSet is running differs from the one derived from the
|
||||
// AutoscalingRunnerSet, in a way that requires re-applying it to the runners.
|
||||
//
|
||||
// Semantic.DeepEqual is used rather than cmp.Equal or reflect.DeepEqual because
|
||||
// it treats a nil slice/map as equal to an empty one. That matters here: most
|
||||
// PodSpec collection fields carry omitempty, so a template containing an
|
||||
// explicitly empty value (`env: []`) is dropped when the EphemeralRunnerSet is
|
||||
// written and reads back as nil. A strict comparison would report drift on every
|
||||
// single reconcile, bumping ActionableRevision each time and making the
|
||||
// EphemeralRunnerSet controller delete every idle and pending runner, forever.
|
||||
// Semantic also knows how to compare resource.Quantity, and unlike cmp.Equal it
|
||||
// cannot panic on types with unexported fields.
|
||||
func ephemeralRunnerSetActionableSpecChanged(current, desired *v1alpha1.EphemeralRunnerSet) bool {
|
||||
if current == nil || desired == nil {
|
||||
return current != desired
|
||||
}
|
||||
|
||||
return !apiequality.Semantic.DeepEqual(current.Spec.EphemeralRunnerSpec, desired.Spec.EphemeralRunnerSpec)
|
||||
}
|
||||
|
||||
func nextActionableRevision(current *v1alpha1.EphemeralRunnerSet) int64 {
|
||||
if current == nil {
|
||||
return 1
|
||||
}
|
||||
|
||||
if current.Spec.ActionableRevision > current.Status.AppliedActionableRevision {
|
||||
return current.Spec.ActionableRevision + 1
|
||||
}
|
||||
|
||||
return current.Status.AppliedActionableRevision + 1
|
||||
}
|
||||
|
||||
// listenerPodSpecRequiresRecreation reports whether the live listener pod must be
|
||||
// deleted and rebuilt to match the desired spec.
|
||||
//
|
||||
|
||||
@@ -1,9 +1,112 @@
|
||||
package actionsgithubcom
|
||||
|
||||
import (
|
||||
"fmt"
|
||||
"testing"
|
||||
|
||||
"github.com/actions/actions-runner-controller/apis/actions.github.com/v1alpha1"
|
||||
corev1 "k8s.io/api/core/v1"
|
||||
"k8s.io/apimachinery/pkg/api/resource"
|
||||
)
|
||||
|
||||
// benchmarkEphemeralRunnerSet builds a runner set roughly the size of a real
|
||||
// ARC deployment: a runner container, a dind sidecar, an init container,
|
||||
// resource limits, volumes and a dozen environment variables.
|
||||
func benchmarkEphemeralRunnerSet() *v1alpha1.EphemeralRunnerSet {
|
||||
env := make([]corev1.EnvVar, 0, 12)
|
||||
for i := range 12 {
|
||||
env = append(env, corev1.EnvVar{Name: fmt.Sprintf("VAR_%d", i), Value: fmt.Sprintf("value-%d", i)})
|
||||
}
|
||||
q := resource.MustParse
|
||||
|
||||
return &v1alpha1.EphemeralRunnerSet{
|
||||
Spec: v1alpha1.EphemeralRunnerSetSpec{
|
||||
Replicas: 10,
|
||||
PatchID: 7,
|
||||
EphemeralRunnerSpec: v1alpha1.EphemeralRunnerSpec{
|
||||
GitHubConfigURL: "https://github.com/octo-org",
|
||||
GitHubConfigSecret: "gh-config",
|
||||
RunnerScaleSetID: 42,
|
||||
PodTemplateSpec: corev1.PodTemplateSpec{
|
||||
Spec: corev1.PodSpec{
|
||||
ServiceAccountName: "runner-sa",
|
||||
RestartPolicy: corev1.RestartPolicyNever,
|
||||
NodeSelector: map[string]string{"kubernetes.io/os": "linux", "node.kubernetes.io/pool": "runners"},
|
||||
Tolerations: []corev1.Toleration{{
|
||||
Key: "dedicated", Operator: corev1.TolerationOpEqual,
|
||||
Value: "runners", Effect: corev1.TaintEffectNoSchedule,
|
||||
}},
|
||||
Volumes: []corev1.Volume{
|
||||
{Name: "work", VolumeSource: corev1.VolumeSource{EmptyDir: &corev1.EmptyDirVolumeSource{}}},
|
||||
{Name: "dind-sock", VolumeSource: corev1.VolumeSource{EmptyDir: &corev1.EmptyDirVolumeSource{}}},
|
||||
},
|
||||
InitContainers: []corev1.Container{{
|
||||
Name: "init-dind", Image: "docker:dind",
|
||||
Command: []string{"cp"},
|
||||
Args: []string{"-r", "/usr/local/bin/.", "/dind"},
|
||||
VolumeMounts: []corev1.VolumeMount{{Name: "dind-sock", MountPath: "/dind"}},
|
||||
}},
|
||||
Containers: []corev1.Container{
|
||||
{
|
||||
Name: "runner",
|
||||
Image: "ghcr.io/actions/actions-runner:2.337.0",
|
||||
Command: []string{"/home/runner/run.sh"},
|
||||
Env: env,
|
||||
VolumeMounts: []corev1.VolumeMount{
|
||||
{Name: "work", MountPath: "/home/runner/_work"},
|
||||
{Name: "dind-sock", MountPath: "/var/run"},
|
||||
},
|
||||
Resources: corev1.ResourceRequirements{
|
||||
Requests: corev1.ResourceList{corev1.ResourceCPU: q("500m"), corev1.ResourceMemory: q("1Gi")},
|
||||
Limits: corev1.ResourceList{corev1.ResourceCPU: q("2"), corev1.ResourceMemory: q("4Gi")},
|
||||
},
|
||||
},
|
||||
{
|
||||
Name: "dind", Image: "docker:dind", Env: env[:6],
|
||||
VolumeMounts: []corev1.VolumeMount{{Name: "dind-sock", MountPath: "/var/run"}},
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
}
|
||||
}
|
||||
|
||||
// BenchmarkEphemeralRunnerSetActionableSpecChanged measures the drift check that
|
||||
// runs on every AutoscalingRunnerSet reconcile. Reconciles are driven by
|
||||
// EphemeralRunnerSet status churn via Owns(), so this executes constantly and
|
||||
// its allocation count feeds directly into controller GC pressure.
|
||||
//
|
||||
// For reference on the machine this was written on: Semantic.DeepEqual is around
|
||||
// 65us/241 allocs, versus 332us/405 allocs for cmp.Equal. If this regresses by an
|
||||
// order of magnitude, something switched the comparison back to a reflection
|
||||
// heavy implementation.
|
||||
func BenchmarkEphemeralRunnerSetActionableSpecChanged(b *testing.B) {
|
||||
b.Run("no drift", func(b *testing.B) {
|
||||
current, desired := benchmarkEphemeralRunnerSet(), benchmarkEphemeralRunnerSet()
|
||||
b.ReportAllocs()
|
||||
b.ResetTimer()
|
||||
for range b.N {
|
||||
if ephemeralRunnerSetActionableSpecChanged(current, desired) {
|
||||
b.Fatal("expected no drift")
|
||||
}
|
||||
}
|
||||
})
|
||||
|
||||
b.Run("drift", func(b *testing.B) {
|
||||
current, desired := benchmarkEphemeralRunnerSet(), benchmarkEphemeralRunnerSet()
|
||||
desired.Spec.EphemeralRunnerSpec.PodTemplateSpec.Spec.Containers[0].Image = "ghcr.io/actions/actions-runner:2.338.0"
|
||||
b.ReportAllocs()
|
||||
b.ResetTimer()
|
||||
for range b.N {
|
||||
if !ephemeralRunnerSetActionableSpecChanged(current, desired) {
|
||||
b.Fatal("expected drift")
|
||||
}
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
// BenchmarkListenerPodSpecRequiresRecreation measures the listener pod drift
|
||||
// check, which also runs on every AutoscalingListener reconcile.
|
||||
func BenchmarkListenerPodSpecRequiresRecreation(b *testing.B) {
|
||||
|
||||
@@ -0,0 +1,147 @@
|
||||
package actionsgithubcom
|
||||
|
||||
import (
|
||||
"encoding/json"
|
||||
"testing"
|
||||
|
||||
"github.com/actions/actions-runner-controller/apis/actions.github.com/v1alpha1"
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
corev1 "k8s.io/api/core/v1"
|
||||
)
|
||||
|
||||
// roundTripThroughAPIServer simulates what happens when the controller writes an
|
||||
// EphemeralRunnerSet and then reads it back: the Go value is marshalled to JSON
|
||||
// (dropping fields tagged omitempty, including explicitly-empty slices and maps)
|
||||
// and decoded again. Fields that were empty-but-non-nil come back nil.
|
||||
func roundTripThroughAPIServer(t *testing.T, spec v1alpha1.EphemeralRunnerSpec) v1alpha1.EphemeralRunnerSpec {
|
||||
t.Helper()
|
||||
raw, err := json.Marshal(spec)
|
||||
require.NoError(t, err)
|
||||
var out v1alpha1.EphemeralRunnerSpec
|
||||
require.NoError(t, json.Unmarshal(raw, &out))
|
||||
return out
|
||||
}
|
||||
|
||||
// TestEphemeralRunnerSetActionableSpecChanged_EmptySliceRoundTrip is a
|
||||
// regression test for a permanent drift loop.
|
||||
//
|
||||
// A user may legitimately write `env: []` (or an empty nodeSelector, tolerations,
|
||||
// volumes, ...) in the AutoscalingRunnerSet template. Those fields carry
|
||||
// `omitempty`, so when the controller writes the derived EphemeralRunnerSet the
|
||||
// empty slice is dropped entirely and reads back as nil. If the drift check
|
||||
// treats nil and empty as different it reports drift forever: every reconcile
|
||||
// bumps ActionableRevision, which makes the EphemeralRunnerSet controller delete
|
||||
// every idle and pending runner, permanently.
|
||||
func TestEphemeralRunnerSetActionableSpecChanged_EmptySliceRoundTrip(t *testing.T) {
|
||||
desiredSpec := v1alpha1.EphemeralRunnerSpec{
|
||||
GitHubConfigURL: "https://github.com/owner/repo",
|
||||
GitHubConfigSecret: "gh-config",
|
||||
RunnerScaleSetID: 42,
|
||||
PodTemplateSpec: corev1.PodTemplateSpec{
|
||||
Spec: corev1.PodSpec{
|
||||
Containers: []corev1.Container{{
|
||||
Name: "runner",
|
||||
Image: "ghcr.io/actions/actions-runner:2.337.0",
|
||||
// Explicitly empty, exactly as a user writing `env: []` would produce.
|
||||
Env: []corev1.EnvVar{},
|
||||
}},
|
||||
NodeSelector: map[string]string{},
|
||||
Tolerations: []corev1.Toleration{},
|
||||
},
|
||||
},
|
||||
}
|
||||
|
||||
desired := &v1alpha1.EphemeralRunnerSet{
|
||||
Spec: v1alpha1.EphemeralRunnerSetSpec{EphemeralRunnerSpec: desiredSpec},
|
||||
}
|
||||
|
||||
// The live object is what the API server hands back after the controller
|
||||
// persisted exactly this desired spec.
|
||||
current := &v1alpha1.EphemeralRunnerSet{
|
||||
Spec: v1alpha1.EphemeralRunnerSetSpec{
|
||||
EphemeralRunnerSpec: roundTripThroughAPIServer(t, desiredSpec),
|
||||
},
|
||||
}
|
||||
|
||||
// Sanity check that the round trip really does produce the nil/empty split,
|
||||
// otherwise this test would pass vacuously.
|
||||
require.NotNil(t, desired.Spec.EphemeralRunnerSpec.PodTemplateSpec.Spec.Containers[0].Env)
|
||||
require.Nil(t, current.Spec.EphemeralRunnerSpec.PodTemplateSpec.Spec.Containers[0].Env)
|
||||
|
||||
assert.False(t,
|
||||
ephemeralRunnerSetActionableSpecChanged(current, desired),
|
||||
"an empty slice that was dropped by omitempty on write must not be reported as drift; "+
|
||||
"reporting drift here bumps ActionableRevision on every reconcile and deletes idle runners forever",
|
||||
)
|
||||
}
|
||||
|
||||
// TestEphemeralRunnerSetActionableSpecChanged_RealChangeStillDetected guards the
|
||||
// opposite direction: relaxing nil-vs-empty must not blind us to genuine drift.
|
||||
func TestEphemeralRunnerSetActionableSpecChanged_RealChangeStillDetected(t *testing.T) {
|
||||
base := func() *v1alpha1.EphemeralRunnerSet {
|
||||
return &v1alpha1.EphemeralRunnerSet{
|
||||
Spec: v1alpha1.EphemeralRunnerSetSpec{
|
||||
EphemeralRunnerSpec: v1alpha1.EphemeralRunnerSpec{
|
||||
GitHubConfigURL: "https://github.com/owner/repo",
|
||||
RunnerScaleSetID: 42,
|
||||
PodTemplateSpec: corev1.PodTemplateSpec{
|
||||
Spec: corev1.PodSpec{
|
||||
Containers: []corev1.Container{{
|
||||
Name: "runner",
|
||||
Image: "ghcr.io/actions/actions-runner:2.337.0",
|
||||
Env: []corev1.EnvVar{{Name: "A", Value: "1"}},
|
||||
}},
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
}
|
||||
}
|
||||
|
||||
tests := map[string]func(*v1alpha1.EphemeralRunnerSet){
|
||||
"image changed": func(e *v1alpha1.EphemeralRunnerSet) {
|
||||
e.Spec.EphemeralRunnerSpec.PodTemplateSpec.Spec.Containers[0].Image = "ghcr.io/actions/actions-runner:2.338.0"
|
||||
},
|
||||
"env value changed": func(e *v1alpha1.EphemeralRunnerSet) {
|
||||
e.Spec.EphemeralRunnerSpec.PodTemplateSpec.Spec.Containers[0].Env[0].Value = "2"
|
||||
},
|
||||
"env var removed": func(e *v1alpha1.EphemeralRunnerSet) {
|
||||
e.Spec.EphemeralRunnerSpec.PodTemplateSpec.Spec.Containers[0].Env = nil
|
||||
},
|
||||
"env var added": func(e *v1alpha1.EphemeralRunnerSet) {
|
||||
e.Spec.EphemeralRunnerSpec.PodTemplateSpec.Spec.Containers[0].Env = append(
|
||||
e.Spec.EphemeralRunnerSpec.PodTemplateSpec.Spec.Containers[0].Env,
|
||||
corev1.EnvVar{Name: "B", Value: "2"},
|
||||
)
|
||||
},
|
||||
"scale set id changed": func(e *v1alpha1.EphemeralRunnerSet) {
|
||||
e.Spec.EphemeralRunnerSpec.RunnerScaleSetID = 43
|
||||
},
|
||||
"config url changed": func(e *v1alpha1.EphemeralRunnerSet) {
|
||||
e.Spec.EphemeralRunnerSpec.GitHubConfigURL = "https://github.com/owner/other"
|
||||
},
|
||||
"container removed": func(e *v1alpha1.EphemeralRunnerSet) {
|
||||
e.Spec.EphemeralRunnerSpec.PodTemplateSpec.Spec.Containers = nil
|
||||
},
|
||||
}
|
||||
|
||||
for name, mutate := range tests {
|
||||
t.Run(name, func(t *testing.T) {
|
||||
current, desired := base(), base()
|
||||
mutate(desired)
|
||||
assert.True(t, ephemeralRunnerSetActionableSpecChanged(current, desired),
|
||||
"genuine drift must still be detected")
|
||||
})
|
||||
}
|
||||
|
||||
t.Run("identical specs report no drift", func(t *testing.T) {
|
||||
assert.False(t, ephemeralRunnerSetActionableSpecChanged(base(), base()))
|
||||
})
|
||||
|
||||
t.Run("nil handling", func(t *testing.T) {
|
||||
assert.False(t, ephemeralRunnerSetActionableSpecChanged(nil, nil))
|
||||
assert.True(t, ephemeralRunnerSetActionableSpecChanged(nil, base()))
|
||||
assert.True(t, ephemeralRunnerSetActionableSpecChanged(base(), nil))
|
||||
})
|
||||
}
|
||||
@@ -864,8 +864,6 @@ func (b *ResourceBuilder) newEphemeralRunnerSet(autoscalingRunnerSet *v1alpha1.A
|
||||
Spec: spec,
|
||||
}
|
||||
|
||||
newEphemeralRunnerSet.Annotations[annotationKeyIntegrityHash] = ephemeralRunnerSetIntegrityHash(newEphemeralRunnerSet)
|
||||
|
||||
if err := b.setControllerReference(autoscalingRunnerSet, newEphemeralRunnerSet); err != nil {
|
||||
return nil, fmt.Errorf("failed to set controller reference for ephemeral runner set: %w", err)
|
||||
}
|
||||
@@ -874,17 +872,6 @@ func (b *ResourceBuilder) newEphemeralRunnerSet(autoscalingRunnerSet *v1alpha1.A
|
||||
return newEphemeralRunnerSet, nil
|
||||
}
|
||||
|
||||
func ephemeralRunnerSetIntegrityHash(ers *v1alpha1.EphemeralRunnerSet) string {
|
||||
type data struct {
|
||||
EphemeralRunnerSpec v1alpha1.EphemeralRunnerSpec `json:"ephemeralRunnerSpec"`
|
||||
}
|
||||
|
||||
d := data{
|
||||
EphemeralRunnerSpec: ers.Spec.EphemeralRunnerSpec,
|
||||
}
|
||||
return hash.ComputeTemplateHash(&d)
|
||||
}
|
||||
|
||||
func (b *ResourceBuilder) newAutoscalingListenerProxySecret(autoscalingListener *v1alpha1.AutoscalingListener, data map[string][]byte) (*corev1.Secret, error) {
|
||||
newProxySecret := &corev1.Secret{
|
||||
ObjectMeta: metav1.ObjectMeta{
|
||||
|
||||
@@ -115,7 +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.NotEmpty(t, ephemeralRunnerSet.Annotations[annotationKeyIntegrityHash])
|
||||
assert.NotContains(t, ephemeralRunnerSet.Annotations, annotationKeyIntegrityHash)
|
||||
assert.Equal(t, autoscalingRunnerSet.Name, ephemeralRunnerSet.Labels[LabelKeyGitHubScaleSetName])
|
||||
assert.Equal(t, autoscalingRunnerSet.Namespace, ephemeralRunnerSet.Labels[LabelKeyGitHubScaleSetNamespace])
|
||||
assert.Equal(t, "", ephemeralRunnerSet.Labels[LabelKeyGitHubEnterprise])
|
||||
@@ -132,7 +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.NotEmpty(t, ephemeralRunnerSet.Annotations[annotationKeyIntegrityHash])
|
||||
assert.NotEmpty(t, listener.Annotations[annotationKeyIntegrityHash])
|
||||
assert.Equal(t, autoscalingRunnerSet.Name, listener.Labels[LabelKeyGitHubScaleSetName])
|
||||
assert.Equal(t, autoscalingRunnerSet.Namespace, listener.Labels[LabelKeyGitHubScaleSetNamespace])
|
||||
assert.Equal(t, "", listener.Labels[LabelKeyGitHubEnterprise])
|
||||
|
||||
Reference in New Issue
Block a user