amadan
boxvolumes: a shutting-down record's detaching DeleteOnTermination volumes are evidence, not a refusal
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2 files changed,
+53
−6
internal/boxvolumes/volumes.go+15 −6internal/boxvolumes/volumes_test.go+38 −0
diff --git a/internal/boxvolumes/volumes.go b/internal/boxvolumes/volumes.go| index 5941e53ce..02c4b1d8b 100644 |
| --- a/internal/boxvolumes/volumes.go |
| +++ b/internal/boxvolumes/volumes.go |
| @@ -275,9 +275,17 @@ func List(ctx context.Context, s release.Store, enrollment string) ([]Record, er |
| } |
| // incomplete says why a stored record is not evidence of a running |
| -// instance's mappings, or "" when it is. Write never produces one of |
| -// these; a record that is one anyway (an older writer, a hand edit, a |
| -// bug) keeps the chores open rather than being read as "nothing attached". |
| +// instance's mappings, or "" when it is. Write can store such a record |
| +// (Observe records every state in recordable, and a volume's status as |
| +// EC2 reports it), and a stored one may also come from an older writer, a |
| +// hand edit or a bug; any of them keeps the chores open rather than being |
| +// read as "nothing attached". |
| +// |
| +// A volume must be attached or attaching, with one exception: while the |
| +// instance is shutting down EC2 reports its DeleteOnTermination volumes |
| +// as detaching on their way to deletion, so in a shutting-down record a |
| +// detaching volume whose flag is true is still evidence. A detaching |
| +// volume with a false flag, or in any other state, may outlive the box. |
| func incomplete(r Record) string { |
| if !recordable[r.InstanceState] { |
| return fmt.Sprintf("has state %q, which carries no mapping", r.InstanceState) |
| @@ -289,8 +297,8 @@ func incomplete(r Record) string { |
| if d.VolumeID == "" || d.Device == "" { |
| return "lists a volume with no id or device" |
| } |
| - // A volume that is detaching or detached may outlive the box. |
| - if d.Status != "attached" && d.Status != "attaching" { |
| + dying := r.InstanceState == "shutting-down" && d.DeleteOnTermination && d.Status == "detaching" |
| + if d.Status != "attached" && d.Status != "attaching" && !dying { |
| return fmt.Sprintf("lists volume %s with status %q", d.VolumeID, d.Status) |
| } |
| } |
| @@ -311,7 +319,8 @@ type Verdict struct { |
| // none ever removed (spec §5.9, H19). Every record is checked, none |
| // skipped: one with another version, enrollment or cloud identity, one |
| // that is incomplete or carries no mapping, a volume not attached or |
| -// attaching, a record outside enrolledAt to terminatedAt, or a gap between |
| +// attaching (bar a DeleteOnTermination one detaching as the instance |
| +// shuts down), a record outside enrolledAt to terminatedAt, or a gap between |
| // records over LastRecordWindow keeps the chores open, as do missing or |
| // misordered times. Anything short of that keeps the chores open for an |
| // operator, which costs cleanup and never recovery. |
diff --git a/internal/boxvolumes/volumes_test.go b/internal/boxvolumes/volumes_test.go| index d3e264ab3..a46706090 100644 |
| --- a/internal/boxvolumes/volumes_test.go |
| +++ b/internal/boxvolumes/volumes_test.go |
| @@ -321,3 +321,41 @@ func TestDischargeRefusesTimesThatDoNotFit(t *testing.T) { |
| }) |
| } |
| } |
| + |
| +// Final review 1: as EC2 shuts an instance down, its DeleteOnTermination |
| +// volumes show "detaching" on the way to deletion. A shutting-down record |
| +// listing them is the normal last record of a box and must discharge; a |
| +// detaching volume with a false flag, or one in any other state, must not. |
| +func TestDischargeAcceptsDetachingDeleteOnTerminationVolumesWhileShuttingDown(t *testing.T) { |
| + running, _ := Next(nil, entry, writer, observe(t, reply("111122223333", types.InstanceStateNameRunning, root, data)), t0) |
| + shutting := func(dot bool) Record { |
| + out := reply("111122223333", types.InstanceStateNameShuttingDown, root, vol{data.dev, data.id, dot}) |
| + for _, m := range out.Reservations[0].Instances[0].BlockDeviceMappings { |
| + m.Ebs.Status = types.AttachmentStatusDetaching |
| + } |
| + rec, ok := Next(&running, entry, writer, observe(t, out), t0.Add(10*time.Minute)) |
| + if !ok { |
| + t.Fatal("the shutting-down read was not a new record") |
| + } |
| + return rec |
| + } |
| + if v := Discharge([]Record{running, shutting(true)}, entry, t0, t0.Add(15*time.Minute)); !v.Discharge { |
| + t.Fatalf("shutting down, every volume detaching and DeleteOnTermination: %+v", v) |
| + } |
| + if v := Discharge([]Record{running, shutting(false)}, entry, t0, t0.Add(15*time.Minute)); v.Discharge || v.Reason == "" { |
| + t.Fatalf("shutting down with a detaching volume that is not DeleteOnTermination: %+v", v) |
| + } |
| + if reason := incomplete(shutting(false)); reason == "" { |
| + t.Fatal("incomplete accepted a detaching volume with a false flag") |
| + } |
| + stopping := shutting(true) |
| + stopping.InstanceState = "stopping" |
| + if v := Discharge([]Record{running, stopping}, entry, t0, t0.Add(15*time.Minute)); v.Discharge || v.Reason == "" { |
| + t.Fatalf("a stopping record with detaching volumes: %+v", v) |
| + } |
| + detached := shutting(true) |
| + detached.BlockDevices[0].Status = "detached" |
| + if v := Discharge([]Record{running, detached}, entry, t0, t0.Add(15*time.Minute)); v.Discharge || v.Reason == "" { |
| + t.Fatalf("a shutting-down record with a detached volume: %+v", v) |
| + } |
| +} |