mirror of
https://github.com/mulgadc/spinifex.git
synced 2026-10-20 03:11:46 +00:00
fix(ecs): retry an owed awsvpc task ENI release (#1058)
Releasing an awsvpc task ENI was best effort: reclaimTaskENI logged a failure and returned, and nothing ever tried again. Its own comment said the ENI fields stayed on the record for a later retry, but no later retry existed, and the stopped-task sweep pruned the record an hour later without looking at those fields — deleting the only pointer to the leaked interface. An orphaned task ENI still references its security group, so DeleteSecurityGroup fails with DependencyViolation and no awsvpc stack can be destroyed without deleting the ENIs by hand. A non-empty ENIID on a stopped record now means exactly one thing: this ENI is still owed. reclaimTaskENI clears the identity when Release succeeds and leaves it when it does not, forceStopTask reclaims before its single write so the cleared fields land in it, and the stopped-task sweep gains the job of paying the debt off — retrying with a backoff that doubles from 30s to a 15m ceiling, capped at twenty attempts per pass because Release is a 30s NATS round trip and a bucket full of them would stall the sweep for the whole fleet. A record that still owes an ENI is never pruned, however far past retention it sits. It is the only pointer to the interface, so dropping it converts a recoverable leak into a permanent one. The placement rollback had the same hole from the other end: an attach failure cleared the ENI fields unconditionally and RunTask never persisted the record, so a failed release there leaked with no trace at all. It now clears only when the release succeeded, and otherwise persists the record as STOPPED so the sweep owns it like any other. Closes mulga-1ysjx.
This commit is contained in:
@@ -127,10 +127,9 @@ func eniCmdSubject(instanceID string) string {
|
||||
return fmt.Sprintf("ec2.cmd.%s", instanceID)
|
||||
}
|
||||
|
||||
// reclaimTaskENI releases an awsvpc task's ENI on the single-writer teardown path
|
||||
// (graceful stop + reaper both reach it). Best effort: a failure is logged and the
|
||||
// ENI fields stay on the record for a later retry, never blocking the STOPPED
|
||||
// transition. No-op for non-awsvpc tasks or tasks with no ENI.
|
||||
// reclaimTaskENI releases an awsvpc task's ENI on the single-writer teardown
|
||||
// path. Success clears the identity; failure leaves it, so a non-empty ENIID
|
||||
// always means "still owed" and the stopped-task sweep is what retries it.
|
||||
func (s *Service) reclaimTaskENI(ctx context.Context, accountID string, task *TaskRecord) {
|
||||
if s.eni == nil || task == nil || task.NetworkMode != NetworkModeAwsvpc || task.ENIID == "" {
|
||||
return
|
||||
@@ -138,7 +137,10 @@ func (s *Service) reclaimTaskENI(ctx context.Context, accountID string, task *Ta
|
||||
if err := s.eni.Release(ctx, accountID, task); err != nil {
|
||||
slog.ErrorContext(ctx, "ECS: task ENI release failed",
|
||||
"task", task.TaskID, "eni", task.ENIID, "err", err)
|
||||
return
|
||||
}
|
||||
task.ENIID = ""
|
||||
task.ENIAttachmentID = ""
|
||||
}
|
||||
|
||||
// isENINotFound reports whether err is an idempotent already-gone signal — a
|
||||
|
||||
@@ -26,6 +26,7 @@ type stubENI struct {
|
||||
releaseCalls int
|
||||
allocErr error
|
||||
attachErr error
|
||||
releaseErr error
|
||||
lastSubnet string
|
||||
lastSGs []string
|
||||
released []string
|
||||
@@ -58,6 +59,9 @@ func (s *stubENI) Attach(_ context.Context, _, _, _ string) (string, error) {
|
||||
|
||||
func (s *stubENI) Release(_ context.Context, _ string, rec *TaskRecord) error {
|
||||
s.releaseCalls++
|
||||
if s.releaseErr != nil {
|
||||
return s.releaseErr
|
||||
}
|
||||
s.released = append(s.released, rec.ENIID)
|
||||
return nil
|
||||
}
|
||||
@@ -173,6 +177,96 @@ func TestRunTask_Awsvpc_AttachFailure_RollsBack(t *testing.T) {
|
||||
assert.Equal(t, int64(0), aws.Int64Value(di.ContainerInstances[0].RunningTasksCount))
|
||||
}
|
||||
|
||||
// singleTaskRecord returns the sole task record in cluster, or found=false when
|
||||
// none exists. Tests that never learn a taskID (a rolled-back RunTask returns
|
||||
// no ARN) use this to reach the record straight from KV.
|
||||
func singleTaskRecord(t *testing.T, svc *Service, cluster string) (TaskRecord, bool) {
|
||||
t.Helper()
|
||||
kv, err := svc.bucket(context.Background(), testAccountID)
|
||||
require.NoError(t, err)
|
||||
keys, err := keysWithPrefix(context.Background(), kv, TasksPrefix(cluster))
|
||||
require.NoError(t, err)
|
||||
require.LessOrEqual(t, len(keys), 1, "test expects at most one task record")
|
||||
if len(keys) == 0 {
|
||||
return TaskRecord{}, false
|
||||
}
|
||||
var rec TaskRecord
|
||||
found, err := getJSON(context.Background(), kv, keys[0], &rec)
|
||||
require.NoError(t, err)
|
||||
return rec, found
|
||||
}
|
||||
|
||||
func TestReclaimTaskENI_ReleaseSucceeds_ClearsIdentity(t *testing.T) {
|
||||
svc, _ := newTestService(t)
|
||||
eni := &stubENI{}
|
||||
svc.eni = eni
|
||||
task := &TaskRecord{TaskID: "t-1", NetworkMode: NetworkModeAwsvpc, ENIID: "eni-1", ENIAttachmentID: "att-1"}
|
||||
|
||||
svc.reclaimTaskENI(context.Background(), testAccountID, task)
|
||||
|
||||
assert.Equal(t, 1, eni.releaseCalls)
|
||||
assert.Empty(t, task.ENIID, "a successful release clears the identity")
|
||||
assert.Empty(t, task.ENIAttachmentID)
|
||||
}
|
||||
|
||||
func TestReclaimTaskENI_ReleaseFails_LeavesIdentity(t *testing.T) {
|
||||
svc, _ := newTestService(t)
|
||||
eni := &stubENI{releaseErr: errors.New("nats timeout")}
|
||||
svc.eni = eni
|
||||
task := &TaskRecord{TaskID: "t-1", NetworkMode: NetworkModeAwsvpc, ENIID: "eni-1", ENIAttachmentID: "att-1"}
|
||||
|
||||
svc.reclaimTaskENI(context.Background(), testAccountID, task)
|
||||
|
||||
assert.Equal(t, 1, eni.releaseCalls)
|
||||
assert.Equal(t, "eni-1", task.ENIID, "a failed release is still owed, so the identity must survive for the sweep")
|
||||
assert.Equal(t, "att-1", task.ENIAttachmentID)
|
||||
}
|
||||
|
||||
func TestRunTask_Awsvpc_AttachFailure_ReleaseSucceeds_NoRecordPersisted(t *testing.T) {
|
||||
svc, _ := newTestService(t)
|
||||
eni := &stubENI{attachErr: errors.New("hot-plug timeout")}
|
||||
svc.eni = eni
|
||||
_, err := svc.CreateCluster(context.Background(), &ecs.CreateClusterInput{ClusterName: aws.String("web")}, testAccountID)
|
||||
require.NoError(t, err)
|
||||
registerAwsvpcTaskDef(t, svc, "app", 128, 256)
|
||||
registerInstance(t, svc, "web", "i-1", 1024, 2048)
|
||||
|
||||
out, err := svc.RunTask(context.Background(), awsvpcRunInput(), testAccountID)
|
||||
require.NoError(t, err)
|
||||
require.Len(t, out.Failures, 1)
|
||||
|
||||
_, found := singleTaskRecord(t, svc, "web")
|
||||
assert.False(t, found, "a release that succeeded leaves nothing for the sweep to find")
|
||||
}
|
||||
|
||||
func TestRunTask_Awsvpc_AttachFailure_ReleaseFails_PersistsStoppedRecord(t *testing.T) {
|
||||
svc, _ := newTestService(t)
|
||||
eni := &stubENI{attachErr: errors.New("hot-plug timeout"), releaseErr: errors.New("release timeout")}
|
||||
svc.eni = eni
|
||||
_, err := svc.CreateCluster(context.Background(), &ecs.CreateClusterInput{ClusterName: aws.String("web")}, testAccountID)
|
||||
require.NoError(t, err)
|
||||
registerAwsvpcTaskDef(t, svc, "app", 128, 256)
|
||||
registerInstance(t, svc, "web", "i-1", 1024, 2048)
|
||||
|
||||
out, err := svc.RunTask(context.Background(), awsvpcRunInput(), testAccountID)
|
||||
require.NoError(t, err)
|
||||
require.Len(t, out.Failures, 1)
|
||||
|
||||
rec, found := singleTaskRecord(t, svc, "web")
|
||||
require.True(t, found, "a release that failed must leave a record so the sweep can retry it")
|
||||
assert.Equal(t, TaskStatusStopped, rec.LastStatus)
|
||||
assert.Equal(t, TaskStatusStopped, rec.DesiredStatus)
|
||||
assert.Equal(t, "eni-stub", rec.ENIID, "the ENI identity must survive so the sweep owns the retry")
|
||||
assert.False(t, rec.StoppedAt.IsZero())
|
||||
|
||||
// The reservation was already given back to the instance.
|
||||
di, err := svc.DescribeContainerInstances(context.Background(), &ecs.DescribeContainerInstancesInput{
|
||||
Cluster: aws.String("web"), ContainerInstances: []*string{aws.String("i-1")},
|
||||
}, testAccountID)
|
||||
require.NoError(t, err)
|
||||
assert.Equal(t, int64(0), aws.Int64Value(di.ContainerInstances[0].RunningTasksCount))
|
||||
}
|
||||
|
||||
func TestRecordTaskState_Awsvpc_ReleasesENIOnStopOnce(t *testing.T) {
|
||||
svc, _ := newTestService(t)
|
||||
eni := &stubENI{}
|
||||
|
||||
@@ -402,6 +402,11 @@ type TaskRecord struct {
|
||||
ENIPrivateIP string `json:"eniPrivateIp,omitempty"`
|
||||
ENIMacAddress string `json:"eniMac,omitempty"`
|
||||
ENISubnetID string `json:"eniSubnetId,omitempty"`
|
||||
// ENIReleaseAttempts / ENIReleaseNextTry are the stopped-task sweep's retry
|
||||
// state once a release attempt has failed. Both stay zero on the common
|
||||
// path, so an untroubled record serialises exactly as it always has.
|
||||
ENIReleaseAttempts int `json:"eniReleaseAttempts,omitempty"`
|
||||
ENIReleaseNextTry time.Time `json:"eniReleaseNextTry,omitzero"`
|
||||
// ENIPublicIP / ENIEIPAllocationID hold the auto-assigned Elastic IP for an
|
||||
// awsvpc task whose service has AssignPublicIp=ENABLED. Set on the RUNNING
|
||||
// transition and released on STOPPED. Empty otherwise.
|
||||
|
||||
@@ -37,6 +37,17 @@ const (
|
||||
sweepInterval = 60 * time.Second
|
||||
stoppedTaskRetention = 1 * time.Hour
|
||||
|
||||
// eniReleaseRetriesPerPass bounds how many owed-ENI releases one sweep pass
|
||||
// attempts. Release is a 30s-budget NATS round trip, so a bucket full of them
|
||||
// would otherwise stall the sweep — and everything behind it — for the fleet.
|
||||
eniReleaseRetriesPerPass = 20
|
||||
|
||||
// eniReleaseBackoffBase/Max bound the retry spacing for a task whose ENI
|
||||
// release keeps failing: it starts at 30s and doubles up to a 15m ceiling
|
||||
// rather than hammering a control plane that is already struggling.
|
||||
eniReleaseBackoffBase = 30 * time.Second
|
||||
eniReleaseBackoffMax = 15 * time.Minute
|
||||
|
||||
// convergeInterval is how often the leader re-asserts the instance-role
|
||||
// policy. The document changes only across releases, so this trades a slower
|
||||
// pickup after an upgrade for a fleet-wide write every reconcile tick.
|
||||
|
||||
@@ -185,9 +185,10 @@ func (s *Service) forceStopTask(ctx context.Context, kv jetstream.KeyValue, acco
|
||||
task.DesiredStatus = TaskStatusStopped
|
||||
task.StoppedReason = reason
|
||||
task.StoppedAt = now
|
||||
// Release the auto-assigned EIP before persisting so the cleared public IP
|
||||
// lands in this single write.
|
||||
// Release the auto-assigned EIP and reclaim the ENI before persisting so the
|
||||
// cleared fields land in this single write.
|
||||
s.releaseTaskPublicIP(ctx, accountID, task)
|
||||
s.reclaimTaskENI(ctx, accountID, task)
|
||||
if perr := putJSON(ctx, kv, TaskKey(task.Cluster, task.TaskID), task); perr != nil {
|
||||
slog.ErrorContext(ctx, "ECS forceStopTask: persist failed", "task", task.TaskID, "err", perr)
|
||||
return
|
||||
@@ -195,7 +196,6 @@ func (s *Service) forceStopTask(ctx context.Context, kv jetstream.KeyValue, acco
|
||||
s.deregisterServiceTargets(ctx, kv, accountID, task)
|
||||
s.reclaimAssignInbox(ctx, kv, task.Cluster, task.ContainerInstanceID, task.TaskID)
|
||||
s.reclaimStopInbox(ctx, kv, task.Cluster, task.ContainerInstanceID, task.TaskID)
|
||||
s.reclaimTaskENI(ctx, accountID, task)
|
||||
if rerr := s.releaseReservation(ctx, kv, task.Cluster, task.ContainerInstanceID, task.TaskID, task.ReservedCPU, task.ReservedMemoryMiB, task.GPU); rerr != nil {
|
||||
slog.ErrorContext(ctx, "ECS forceStopTask: release reservation failed", "task", task.TaskID, "err", rerr)
|
||||
}
|
||||
|
||||
@@ -117,7 +117,22 @@ func (s *Service) provisionTaskENI(ctx context.Context, kv jetstream.KeyValue, a
|
||||
attachmentID, err := s.eni.Attach(ctx, accountID, rec.ContainerInstanceID, alloc.ENIID)
|
||||
if err != nil {
|
||||
s.reclaimTaskENI(ctx, accountID, rec)
|
||||
rec.ENIID, rec.ENIMacAddress, rec.ENIPrivateIP, rec.ENISubnetID = "", "", "", ""
|
||||
if rec.ENIID == "" {
|
||||
// Release succeeded, so nothing is owed; RunTask's caller never
|
||||
// persists a failed placement, so clearing here is the end of it.
|
||||
rec.ENIMacAddress, rec.ENIPrivateIP, rec.ENISubnetID = "", "", ""
|
||||
} else {
|
||||
// Release failed: persist this record as STOPPED so the sweep
|
||||
// retries it like any other owed ENI, rather than losing the
|
||||
// only pointer when RunTask drops the record on the floor.
|
||||
rec.LastStatus = TaskStatusStopped
|
||||
rec.DesiredStatus = TaskStatusStopped
|
||||
rec.StoppedReason = fmt.Sprintf("ENI attach failed: %s", err)
|
||||
rec.StoppedAt = time.Now().UTC()
|
||||
if perr := putJSON(ctx, kv, TaskKey(cluster, rec.TaskID), rec); perr != nil {
|
||||
slog.ErrorContext(ctx, "ECS RunTask: persist ENI rollback failed", "task", rec.TaskID, "err", perr)
|
||||
}
|
||||
}
|
||||
return rollback("RESOURCE:eni", err)
|
||||
}
|
||||
rec.ENIAttachmentID = attachmentID
|
||||
|
||||
@@ -35,7 +35,7 @@ func (sc *Scheduler) sweepStoppedTasks(ctx context.Context) (time.Duration, erro
|
||||
slog.Error("ECS sweep: open bucket failed", "bucket", bucket.name, "err", err)
|
||||
continue
|
||||
}
|
||||
pruned, due, serr := sc.svc.sweepStoppedBucket(ctx, kv, now, stoppedTaskRetention)
|
||||
pruned, due, serr := sc.svc.sweepStoppedBucket(ctx, kv, bucket.accountID, now, stoppedTaskRetention)
|
||||
if serr != nil {
|
||||
slog.Error("ECS sweep: bucket failed", "bucket", bucket.name, "err", serr)
|
||||
continue
|
||||
@@ -48,16 +48,16 @@ func (sc *Scheduler) sweepStoppedTasks(ctx context.Context) (time.Duration, erro
|
||||
return next, nil
|
||||
}
|
||||
|
||||
// sweepStoppedBucket deletes task records that have been STOPPED longer than
|
||||
// retention. A task missing its StoppedAt timestamp is never pruned (defensive:
|
||||
// it would otherwise look infinitely old). Returns the number deleted and when
|
||||
// the soonest record it kept falls due.
|
||||
func (s *Service) sweepStoppedBucket(ctx context.Context, kv jetstream.KeyValue, now time.Time, retention time.Duration) (int, time.Duration, error) {
|
||||
// sweepStoppedBucket deletes STOPPED task records past retention and retries
|
||||
// the ENI release for any that still owe one. A missing StoppedAt or a
|
||||
// non-empty ENIID both block deletion; the latter until the release succeeds.
|
||||
func (s *Service) sweepStoppedBucket(ctx context.Context, kv jetstream.KeyValue, accountID string, now time.Time, retention time.Duration) (int, time.Duration, error) {
|
||||
keys, err := keysWithPrefix(ctx, kv, "clusters/")
|
||||
if err != nil {
|
||||
return 0, 0, err
|
||||
}
|
||||
pruned := 0
|
||||
retried := 0
|
||||
var next time.Duration
|
||||
for _, k := range keys {
|
||||
if !strings.Contains(k, "/tasks/") {
|
||||
@@ -71,6 +71,26 @@ func (s *Service) sweepStoppedBucket(ctx context.Context, kv jetstream.KeyValue,
|
||||
if task.LastStatus != TaskStatusStopped || task.StoppedAt.IsZero() {
|
||||
continue
|
||||
}
|
||||
if task.ENIID != "" {
|
||||
due := !task.ENIReleaseNextTry.After(now)
|
||||
switch {
|
||||
case due && retried < eniReleaseRetriesPerPass:
|
||||
// Attempt the retry now; a failure recomputes ENIReleaseNextTry
|
||||
// itself, so only a leftover success needs no further deadline.
|
||||
retried++
|
||||
s.retryTaskENIRelease(ctx, kv, k, accountID, &task, now)
|
||||
if task.ENIID != "" && task.ENIReleaseNextTry.After(now) {
|
||||
next = reconciler.Earliest(next, task.ENIReleaseNextTry.Sub(now))
|
||||
}
|
||||
case due:
|
||||
// Due, but this pass's retry budget is spent: come back soon
|
||||
// rather than falling through to whatever else sets next.
|
||||
next = reconciler.Earliest(next, sweepInterval)
|
||||
default:
|
||||
next = reconciler.Earliest(next, task.ENIReleaseNextTry.Sub(now))
|
||||
}
|
||||
continue
|
||||
}
|
||||
if now.Sub(task.StoppedAt) <= retention {
|
||||
next = reconciler.Earliest(next, task.StoppedAt.Add(retention).Sub(now))
|
||||
continue
|
||||
@@ -83,3 +103,28 @@ func (s *Service) sweepStoppedBucket(ctx context.Context, kv jetstream.KeyValue,
|
||||
}
|
||||
return pruned, next, nil
|
||||
}
|
||||
|
||||
// retryTaskENIRelease makes one more attempt at a STOPPED task's owed ENI
|
||||
// release, then persists the outcome: success clears the identity and retry
|
||||
// state, failure bumps the attempt count and backs off the next try.
|
||||
func (s *Service) retryTaskENIRelease(ctx context.Context, kv jetstream.KeyValue, key, accountID string, task *TaskRecord, now time.Time) {
|
||||
s.reclaimTaskENI(ctx, accountID, task)
|
||||
if task.ENIID == "" {
|
||||
task.ENIReleaseAttempts = 0
|
||||
task.ENIReleaseNextTry = time.Time{}
|
||||
} else {
|
||||
task.ENIReleaseAttempts++
|
||||
task.ENIReleaseNextTry = now.Add(eniReleaseBackoff(task.ENIReleaseAttempts))
|
||||
}
|
||||
if perr := putJSON(ctx, kv, key, task); perr != nil {
|
||||
slog.Warn("ECS sweep: persist ENI retry state failed", "key", key, "err", perr)
|
||||
}
|
||||
}
|
||||
|
||||
// eniReleaseBackoff is the delay before the attempt after the attempts-th
|
||||
// failure, doubling from eniReleaseBackoffBase and capped at
|
||||
// eniReleaseBackoffMax.
|
||||
func eniReleaseBackoff(attempts int) time.Duration {
|
||||
shift := min(max(attempts-1, 0), 10)
|
||||
return min(eniReleaseBackoffBase<<shift, eniReleaseBackoffMax)
|
||||
}
|
||||
|
||||
@@ -1,6 +1,8 @@
|
||||
package handlers_ecs
|
||||
|
||||
import (
|
||||
"errors"
|
||||
"fmt"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
@@ -25,7 +27,7 @@ func TestSweepStoppedBucket_PrunesOnlyStale(t *testing.T) {
|
||||
put("running", TaskStatusRunning, time.Time{}) // not stopped -> keep
|
||||
put("nostamp", TaskStatusStopped, time.Time{}) // STOPPED but no timestamp -> keep (defensive)
|
||||
|
||||
pruned, due, err := svc.sweepStoppedBucket(t.Context(), kv, now, time.Hour)
|
||||
pruned, due, err := svc.sweepStoppedBucket(t.Context(), kv, testAccountID, now, time.Hour)
|
||||
require.NoError(t, err)
|
||||
assert.Equal(t, 1, pruned)
|
||||
// "fresh" is the only survivor with a clock on it, so the sweep must come back
|
||||
@@ -43,3 +45,147 @@ func TestSweepStoppedBucket_PrunesOnlyStale(t *testing.T) {
|
||||
assert.True(t, exists("running"), "RUNNING task should survive")
|
||||
assert.True(t, exists("nostamp"), "STOPPED task with no StoppedAt should survive")
|
||||
}
|
||||
|
||||
func TestSweepStoppedBucket_RetriesOwedENI_Succeeds(t *testing.T) {
|
||||
svc, _, kv := serviceTestRig(t)
|
||||
eni := &stubENI{}
|
||||
svc.eni = eni
|
||||
now := time.Now().UTC()
|
||||
|
||||
rec := TaskRecord{
|
||||
TaskID: "t-1", Cluster: "web", ARN: TaskARN(testRegion, testAccountID, "web", "t-1"),
|
||||
LastStatus: TaskStatusStopped, DesiredStatus: TaskStatusStopped,
|
||||
NetworkMode: NetworkModeAwsvpc, ENIID: "eni-1", ENIAttachmentID: "att-1",
|
||||
StoppedAt: now.Add(-2 * time.Hour), // well past retention
|
||||
}
|
||||
require.NoError(t, putJSON(t.Context(), kv, TaskKey("web", "t-1"), &rec))
|
||||
|
||||
pruned, due, err := svc.sweepStoppedBucket(t.Context(), kv, testAccountID, now, time.Hour)
|
||||
require.NoError(t, err)
|
||||
assert.Equal(t, 0, pruned, "an owed ENI is never pruned, however stale")
|
||||
assert.Zero(t, due)
|
||||
assert.Equal(t, 1, eni.releaseCalls)
|
||||
|
||||
var got TaskRecord
|
||||
found, gerr := getJSON(t.Context(), kv, TaskKey("web", "t-1"), &got)
|
||||
require.NoError(t, gerr)
|
||||
require.True(t, found)
|
||||
assert.Empty(t, got.ENIID, "a successful retry clears the identity")
|
||||
assert.Empty(t, got.ENIAttachmentID)
|
||||
assert.Equal(t, 0, got.ENIReleaseAttempts)
|
||||
assert.True(t, got.ENIReleaseNextTry.IsZero())
|
||||
}
|
||||
|
||||
func TestSweepStoppedBucket_RetryFails_BacksOff(t *testing.T) {
|
||||
svc, _, kv := serviceTestRig(t)
|
||||
eni := &stubENI{releaseErr: errors.New("nats timeout")}
|
||||
svc.eni = eni
|
||||
now := time.Now().UTC()
|
||||
|
||||
rec := TaskRecord{
|
||||
TaskID: "t-1", Cluster: "web", ARN: TaskARN(testRegion, testAccountID, "web", "t-1"),
|
||||
LastStatus: TaskStatusStopped, DesiredStatus: TaskStatusStopped,
|
||||
NetworkMode: NetworkModeAwsvpc, ENIID: "eni-1",
|
||||
StoppedAt: now.Add(-2 * time.Hour),
|
||||
}
|
||||
require.NoError(t, putJSON(t.Context(), kv, TaskKey("web", "t-1"), &rec))
|
||||
|
||||
pruned, due, err := svc.sweepStoppedBucket(t.Context(), kv, testAccountID, now, time.Hour)
|
||||
require.NoError(t, err)
|
||||
assert.Equal(t, 0, pruned)
|
||||
assert.Equal(t, eniReleaseBackoffBase, due)
|
||||
|
||||
var got TaskRecord
|
||||
found, gerr := getJSON(t.Context(), kv, TaskKey("web", "t-1"), &got)
|
||||
require.NoError(t, gerr)
|
||||
require.True(t, found)
|
||||
assert.Equal(t, "eni-1", got.ENIID, "a failed retry is still owed")
|
||||
assert.Equal(t, 1, got.ENIReleaseAttempts)
|
||||
assert.WithinDuration(t, now.Add(eniReleaseBackoffBase), got.ENIReleaseNextTry, time.Second)
|
||||
}
|
||||
|
||||
func TestSweepStoppedBucket_RetryNotDueYet_Skipped(t *testing.T) {
|
||||
svc, _, kv := serviceTestRig(t)
|
||||
eni := &stubENI{}
|
||||
svc.eni = eni
|
||||
now := time.Now().UTC()
|
||||
|
||||
rec := TaskRecord{
|
||||
TaskID: "t-1", Cluster: "web", ARN: TaskARN(testRegion, testAccountID, "web", "t-1"),
|
||||
LastStatus: TaskStatusStopped, DesiredStatus: TaskStatusStopped,
|
||||
NetworkMode: NetworkModeAwsvpc,
|
||||
ENIID: "eni-1",
|
||||
StoppedAt: now.Add(-2 * time.Hour),
|
||||
ENIReleaseNextTry: now.Add(10 * time.Minute),
|
||||
}
|
||||
require.NoError(t, putJSON(t.Context(), kv, TaskKey("web", "t-1"), &rec))
|
||||
|
||||
pruned, due, err := svc.sweepStoppedBucket(t.Context(), kv, testAccountID, now, time.Hour)
|
||||
require.NoError(t, err)
|
||||
assert.Equal(t, 0, pruned)
|
||||
assert.Equal(t, 0, eni.releaseCalls, "the next try has not arrived yet")
|
||||
assert.Equal(t, 10*time.Minute, due)
|
||||
}
|
||||
|
||||
func TestSweepStoppedBucket_HonoursPerPassCap(t *testing.T) {
|
||||
svc, _, kv := serviceTestRig(t)
|
||||
eni := &stubENI{releaseErr: errors.New("nats timeout")}
|
||||
svc.eni = eni
|
||||
now := time.Now().UTC()
|
||||
|
||||
total := eniReleaseRetriesPerPass + 2
|
||||
for i := range total {
|
||||
id := fmt.Sprintf("t-%d", i)
|
||||
rec := TaskRecord{
|
||||
TaskID: id, Cluster: "web", ARN: TaskARN(testRegion, testAccountID, "web", id),
|
||||
LastStatus: TaskStatusStopped, DesiredStatus: TaskStatusStopped,
|
||||
NetworkMode: NetworkModeAwsvpc, ENIID: "eni-" + id,
|
||||
StoppedAt: now.Add(-2 * time.Hour),
|
||||
}
|
||||
require.NoError(t, putJSON(t.Context(), kv, TaskKey("web", id), &rec))
|
||||
}
|
||||
|
||||
_, due, err := svc.sweepStoppedBucket(t.Context(), kv, testAccountID, now, time.Hour)
|
||||
require.NoError(t, err)
|
||||
assert.Equal(t, eniReleaseRetriesPerPass, eni.releaseCalls,
|
||||
"a bucket full of due retries must not stall the sweep past its per-pass cap")
|
||||
// The retried records back off to eniReleaseBackoffBase, which beats the
|
||||
// sweepInterval fallback the capped-out overflow records ask for; either
|
||||
// way the sweep must learn a deadline rather than falling through to none.
|
||||
assert.Equal(t, eniReleaseBackoffBase, due)
|
||||
}
|
||||
|
||||
// TestSweepStoppedBucket_CappedOverflow_AsksForSweepInterval isolates the
|
||||
// capped-out branch: every retry this pass succeeds (so none re-arms its own
|
||||
// backoff), leaving the overflow records as the only source of a deadline.
|
||||
func TestSweepStoppedBucket_CappedOverflow_AsksForSweepInterval(t *testing.T) {
|
||||
svc, _, kv := serviceTestRig(t)
|
||||
eni := &stubENI{}
|
||||
svc.eni = eni
|
||||
now := time.Now().UTC()
|
||||
|
||||
total := eniReleaseRetriesPerPass + 2
|
||||
for i := range total {
|
||||
id := fmt.Sprintf("t-%d", i)
|
||||
rec := TaskRecord{
|
||||
TaskID: id, Cluster: "web", ARN: TaskARN(testRegion, testAccountID, "web", id),
|
||||
LastStatus: TaskStatusStopped, DesiredStatus: TaskStatusStopped,
|
||||
NetworkMode: NetworkModeAwsvpc, ENIID: "eni-" + id,
|
||||
StoppedAt: now.Add(-2 * time.Hour),
|
||||
}
|
||||
require.NoError(t, putJSON(t.Context(), kv, TaskKey("web", id), &rec))
|
||||
}
|
||||
|
||||
_, due, err := svc.sweepStoppedBucket(t.Context(), kv, testAccountID, now, time.Hour)
|
||||
require.NoError(t, err)
|
||||
assert.Equal(t, eniReleaseRetriesPerPass, eni.releaseCalls)
|
||||
assert.Equal(t, sweepInterval, due,
|
||||
"the 2 records left over from the cap must still bring the sweep back soon")
|
||||
}
|
||||
|
||||
func TestEniReleaseBackoff_DoublesThenCaps(t *testing.T) {
|
||||
assert.Equal(t, eniReleaseBackoffBase, eniReleaseBackoff(1))
|
||||
assert.Equal(t, 2*eniReleaseBackoffBase, eniReleaseBackoff(2))
|
||||
assert.Equal(t, 4*eniReleaseBackoffBase, eniReleaseBackoff(3))
|
||||
assert.Equal(t, eniReleaseBackoffMax, eniReleaseBackoff(100), "backoff must never exceed the cap")
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user