mirror of
https://github.com/mulgadc/spinifex.git
synced 2026-10-15 09:01:31 +00:00
feat: add the resource envelope for spec/status records (#927)
Object[Spec, Status] separates what an operator asked for from what a node observed, with the identity and lifecycle bookkeeping common to both. Generation is bumped only through MutateSpec so NeedsSync means something, and ObserveGeneration records the generation the node acted on rather than the current one, so a spec change landing mid-reconcile is not marked done. No ARN or tag helpers: the arn package already builds ARNs per service, and the tag helpers in utils already cover the map every record carries.
This commit is contained in:
Josh Siviour
@@ -0,0 +1,167 @@
|
||||
// Package resource is the envelope a persisted control-plane record lives in:
|
||||
// what an operator asked for, what a node observed, and the identity and
|
||||
// lifecycle bookkeeping that is common to both. It is a leaf, importing nothing
|
||||
// from spinifex, so any layer can name a record without pulling in its service.
|
||||
package resource
|
||||
|
||||
import (
|
||||
"slices"
|
||||
"time"
|
||||
)
|
||||
|
||||
// Object separates the two halves of a record by write authority. Spec is
|
||||
// written only by the API layer, under CAS against the KV. Status is written
|
||||
// only by the node that owns the resource, to its local state file first and
|
||||
// then to the KV best-effort. Nothing in Go can enforce that, so the split is a
|
||||
// convention the type makes visible rather than a guarantee it provides.
|
||||
type Object[Spec, Status any] struct {
|
||||
Metadata Metadata `json:"metadata"`
|
||||
Spec Spec `json:"spec"`
|
||||
Status Status `json:"status"`
|
||||
}
|
||||
|
||||
// Metadata is the identity and lifecycle bookkeeping every record carries,
|
||||
// independent of what it describes.
|
||||
//
|
||||
// ARNs are deliberately absent: the arn package already builds them, and the
|
||||
// per-service formats are not uniform enough for one method over this struct.
|
||||
type Metadata struct {
|
||||
// Name is the AWS-visible identifier — an instance id, a cluster name.
|
||||
Name string `json:"name"`
|
||||
|
||||
// UID distinguishes this resource from an earlier one that carried the
|
||||
// same Name, so an OwnerRef cannot be satisfied by a replacement.
|
||||
UID string `json:"uid,omitempty"`
|
||||
|
||||
AccountID string `json:"account_id,omitempty"`
|
||||
Region string `json:"region,omitempty"`
|
||||
|
||||
// Generation counts spec changes; ObservedGeneration is the generation the
|
||||
// owning node has finished acting on. Equal means the node has caught up,
|
||||
// which is what a caller polls instead of the resource's own fields.
|
||||
Generation int64 `json:"generation,omitempty"`
|
||||
ObservedGeneration int64 `json:"observed_generation,omitempty"`
|
||||
|
||||
// OwnerRefs point at the resources this one is a dependent of. Deleting an
|
||||
// owner cascades to its dependents.
|
||||
OwnerRefs []OwnerRef `json:"owner_refs,omitempty"`
|
||||
|
||||
// Finalizers hold a delete open. A resource marked for deletion is not
|
||||
// removed until every finalizer has been cleared by whatever registered it.
|
||||
Finalizers []string `json:"finalizers,omitempty"`
|
||||
|
||||
Tags map[string]string `json:"tags,omitempty"`
|
||||
|
||||
// DeletionTimestamp marks the resource for deletion. It is set before the
|
||||
// resource's own state has caught up, so it is the signal to read during
|
||||
// the window where that state still says the resource is live.
|
||||
DeletionTimestamp *time.Time `json:"deletion_timestamp,omitempty"`
|
||||
}
|
||||
|
||||
// OwnerRef identifies the resource a dependent belongs to. Kind is the owner's
|
||||
// resource type as the service names it ("instance", "cluster").
|
||||
type OwnerRef struct {
|
||||
Kind string `json:"kind"`
|
||||
Name string `json:"name"`
|
||||
UID string `json:"uid,omitempty"`
|
||||
}
|
||||
|
||||
// MarkedForDeletion reports whether a delete has been accepted for this
|
||||
// resource, whether or not its status has caught up yet.
|
||||
func (m *Metadata) MarkedForDeletion() bool {
|
||||
return m.DeletionTimestamp != nil
|
||||
}
|
||||
|
||||
// MarkForDeletion stamps the deletion time, and reports false if one was
|
||||
// already stamped. Deleting twice must not move the timestamp: finalizer
|
||||
// timeouts are measured from it.
|
||||
func (m *Metadata) MarkForDeletion(now time.Time) bool {
|
||||
if m.DeletionTimestamp != nil {
|
||||
return false
|
||||
}
|
||||
t := now.UTC()
|
||||
m.DeletionTimestamp = &t
|
||||
return true
|
||||
}
|
||||
|
||||
func (m *Metadata) HasFinalizer(name string) bool {
|
||||
return slices.Contains(m.Finalizers, name)
|
||||
}
|
||||
|
||||
// AddFinalizer registers a finalizer, reporting false if it was already
|
||||
// present. Registering after the resource is marked for deletion is refused:
|
||||
// the delete has already been evaluated against the finalizers it had.
|
||||
func (m *Metadata) AddFinalizer(name string) bool {
|
||||
if m.MarkedForDeletion() || m.HasFinalizer(name) {
|
||||
return false
|
||||
}
|
||||
m.Finalizers = append(m.Finalizers, name)
|
||||
return true
|
||||
}
|
||||
|
||||
// RemoveFinalizer clears a finalizer, reporting false if it was not present.
|
||||
// The resource is collectable once the last one is gone.
|
||||
func (m *Metadata) RemoveFinalizer(name string) bool {
|
||||
idx := slices.Index(m.Finalizers, name)
|
||||
if idx < 0 {
|
||||
return false
|
||||
}
|
||||
m.Finalizers = slices.Delete(m.Finalizers, idx, idx+1)
|
||||
if len(m.Finalizers) == 0 {
|
||||
m.Finalizers = nil
|
||||
}
|
||||
return true
|
||||
}
|
||||
|
||||
// Collectable reports whether a marked resource can now be removed. A resource
|
||||
// nobody asked to delete is never collectable.
|
||||
func (m *Metadata) Collectable() bool {
|
||||
return m.MarkedForDeletion() && len(m.Finalizers) == 0
|
||||
}
|
||||
|
||||
// OwnerOfKind returns the owner reference of the given kind. A resource has at
|
||||
// most one owner per kind; a second is a bug in the caller, not a list.
|
||||
func (m *Metadata) OwnerOfKind(kind string) (OwnerRef, bool) {
|
||||
for _, ref := range m.OwnerRefs {
|
||||
if ref.Kind == kind {
|
||||
return ref, true
|
||||
}
|
||||
}
|
||||
return OwnerRef{}, false
|
||||
}
|
||||
|
||||
// AddOwnerRef records an owner, reporting false if one of that kind is already
|
||||
// recorded. Replacing an owner is a distinct operation from acquiring one, so
|
||||
// it is not silently allowed here.
|
||||
func (m *Metadata) AddOwnerRef(ref OwnerRef) bool {
|
||||
if _, ok := m.OwnerOfKind(ref.Kind); ok {
|
||||
return false
|
||||
}
|
||||
m.OwnerRefs = append(m.OwnerRefs, ref)
|
||||
return true
|
||||
}
|
||||
|
||||
// NeedsSync reports whether the owning node has yet to act on the current spec.
|
||||
func (m *Metadata) NeedsSync() bool {
|
||||
return m.Generation != m.ObservedGeneration
|
||||
}
|
||||
|
||||
// MutateSpec applies a spec change and bumps Generation. Going through this
|
||||
// rather than assigning to Spec is what keeps Generation honest, and therefore
|
||||
// what makes NeedsSync mean anything.
|
||||
func (o *Object[Spec, Status]) MutateSpec(fn func(*Spec)) {
|
||||
fn(&o.Spec)
|
||||
o.Metadata.Generation++
|
||||
}
|
||||
|
||||
// ObserveGeneration records that the owning node has finished acting on the
|
||||
// spec it read. Called by the node after a successful reconcile, never by the
|
||||
// API layer.
|
||||
func (o *Object[Spec, Status]) ObserveGeneration(generation int64) {
|
||||
o.Metadata.ObservedGeneration = generation
|
||||
}
|
||||
|
||||
// NeedsSync reports whether the owning node has yet to act on the current spec.
|
||||
func (o *Object[Spec, Status]) NeedsSync() bool {
|
||||
return o.Metadata.NeedsSync()
|
||||
}
|
||||
@@ -0,0 +1,211 @@
|
||||
package resource_test
|
||||
|
||||
import (
|
||||
"encoding/json"
|
||||
"slices"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"github.com/mulgadc/spinifex/spinifex/resource"
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
)
|
||||
|
||||
type instanceSpec struct {
|
||||
InstanceType string `json:"instance_type"`
|
||||
DesiredState string `json:"desired_state,omitempty"`
|
||||
}
|
||||
|
||||
type instanceStatus struct {
|
||||
State string `json:"state"`
|
||||
NodeID string `json:"node_id,omitempty"`
|
||||
}
|
||||
|
||||
type instance = resource.Object[instanceSpec, instanceStatus]
|
||||
|
||||
func TestMutateSpec_BumpsGeneration(t *testing.T) {
|
||||
var o instance
|
||||
require.False(t, o.NeedsSync(), "a record nobody has changed is already in sync")
|
||||
|
||||
o.MutateSpec(func(s *instanceSpec) { s.InstanceType = "t3.micro" })
|
||||
|
||||
assert.Equal(t, "t3.micro", o.Spec.InstanceType)
|
||||
assert.EqualValues(t, 1, o.Metadata.Generation)
|
||||
assert.True(t, o.NeedsSync(), "a spec change the node has not acted on is out of sync")
|
||||
}
|
||||
|
||||
// The node records the generation it acted on, not the current one: a spec
|
||||
// change that lands mid-reconcile must leave the record out of sync rather than
|
||||
// being marked done by a reconcile that never saw it.
|
||||
func TestObserveGeneration_RecordsWhatTheNodeActedOn(t *testing.T) {
|
||||
var o instance
|
||||
o.MutateSpec(func(s *instanceSpec) { s.InstanceType = "t3.micro" })
|
||||
acted := o.Metadata.Generation
|
||||
|
||||
o.MutateSpec(func(s *instanceSpec) { s.InstanceType = "t3.large" })
|
||||
o.ObserveGeneration(acted)
|
||||
|
||||
assert.True(t, o.NeedsSync(), "the node acted on generation 1, the spec is at 2")
|
||||
|
||||
o.ObserveGeneration(o.Metadata.Generation)
|
||||
assert.False(t, o.NeedsSync())
|
||||
}
|
||||
|
||||
// Status is the node's half, so writing it must not make the record look like
|
||||
// it has an unhandled spec change.
|
||||
func TestStatusWrite_DoesNotBumpGeneration(t *testing.T) {
|
||||
var o instance
|
||||
o.MutateSpec(func(s *instanceSpec) { s.InstanceType = "t3.micro" })
|
||||
o.ObserveGeneration(o.Metadata.Generation)
|
||||
|
||||
o.Status = instanceStatus{State: "running", NodeID: "node-1"}
|
||||
|
||||
assert.False(t, o.NeedsSync(), "an observation is not a request")
|
||||
}
|
||||
|
||||
func TestMarkForDeletion(t *testing.T) {
|
||||
var m resource.Metadata
|
||||
assert.False(t, m.MarkedForDeletion())
|
||||
|
||||
first := time.Date(2026, 8, 28, 10, 0, 0, 0, time.UTC)
|
||||
require.True(t, m.MarkForDeletion(first))
|
||||
assert.True(t, m.MarkedForDeletion())
|
||||
|
||||
assert.False(t, m.MarkForDeletion(first.Add(time.Hour)),
|
||||
"a second delete is accepted but changes nothing")
|
||||
assert.Equal(t, first, *m.DeletionTimestamp,
|
||||
"finalizer timeouts are measured from the first mark, so it must not move")
|
||||
}
|
||||
|
||||
func TestMarkForDeletion_NormalisesToUTC(t *testing.T) {
|
||||
var m resource.Metadata
|
||||
zone := time.FixedZone("AEST", 10*60*60)
|
||||
local := time.Date(2026, 8, 28, 20, 0, 0, 0, zone)
|
||||
|
||||
require.True(t, m.MarkForDeletion(local))
|
||||
assert.Equal(t, time.UTC, m.DeletionTimestamp.Location())
|
||||
assert.True(t, m.DeletionTimestamp.Equal(local))
|
||||
}
|
||||
|
||||
func TestFinalizers(t *testing.T) {
|
||||
var m resource.Metadata
|
||||
|
||||
assert.False(t, m.HasFinalizer("eni"))
|
||||
assert.False(t, m.RemoveFinalizer("eni"), "removing one that was never added reports nothing was done")
|
||||
|
||||
require.True(t, m.AddFinalizer("eni"))
|
||||
assert.False(t, m.AddFinalizer("eni"), "registering twice must not queue two clears")
|
||||
require.True(t, m.AddFinalizer("volume"))
|
||||
assert.True(t, m.HasFinalizer("eni"))
|
||||
|
||||
require.True(t, m.RemoveFinalizer("eni"))
|
||||
assert.False(t, m.HasFinalizer("eni"))
|
||||
assert.Equal(t, []string{"volume"}, m.Finalizers, "removing one must not disturb the others")
|
||||
|
||||
require.True(t, m.RemoveFinalizer("volume"))
|
||||
assert.Nil(t, m.Finalizers, "an empty list is dropped so it does not serialise")
|
||||
}
|
||||
|
||||
// A finalizer registered after the delete was accepted would hold open a
|
||||
// deletion that had already been evaluated without it.
|
||||
func TestAddFinalizer_RefusedAfterMarkedForDeletion(t *testing.T) {
|
||||
var m resource.Metadata
|
||||
require.True(t, m.MarkForDeletion(time.Now()))
|
||||
|
||||
assert.False(t, m.AddFinalizer("eni"))
|
||||
assert.Empty(t, m.Finalizers)
|
||||
}
|
||||
|
||||
func TestCollectable(t *testing.T) {
|
||||
var m resource.Metadata
|
||||
require.True(t, m.AddFinalizer("eni"))
|
||||
assert.False(t, m.Collectable(), "an unmarked resource is never collectable")
|
||||
|
||||
require.True(t, m.MarkForDeletion(time.Now()))
|
||||
assert.False(t, m.Collectable(), "a marked resource waits for its finalizers")
|
||||
|
||||
require.True(t, m.RemoveFinalizer("eni"))
|
||||
assert.True(t, m.Collectable())
|
||||
}
|
||||
|
||||
func TestOwnerRefs(t *testing.T) {
|
||||
var m resource.Metadata
|
||||
|
||||
_, ok := m.OwnerOfKind("instance")
|
||||
assert.False(t, ok)
|
||||
|
||||
owner := resource.OwnerRef{Kind: "instance", Name: "i-1", UID: "uid-1"}
|
||||
require.True(t, m.AddOwnerRef(owner))
|
||||
|
||||
got, ok := m.OwnerOfKind("instance")
|
||||
require.True(t, ok)
|
||||
assert.Equal(t, owner, got)
|
||||
|
||||
assert.False(t, m.AddOwnerRef(resource.OwnerRef{Kind: "instance", Name: "i-2"}),
|
||||
"reparenting is not something a second add should do quietly")
|
||||
assert.Len(t, m.OwnerRefs, 1)
|
||||
|
||||
require.True(t, m.AddOwnerRef(resource.OwnerRef{Kind: "cluster", Name: "c-1"}))
|
||||
assert.Len(t, m.OwnerRefs, 2, "a resource can be owned by one of each kind")
|
||||
}
|
||||
|
||||
// UID is what stops an OwnerRef being satisfied by a replacement that reused
|
||||
// the name.
|
||||
func TestOwnerRef_UIDDistinguishesARecreatedOwner(t *testing.T) {
|
||||
var m resource.Metadata
|
||||
require.True(t, m.AddOwnerRef(resource.OwnerRef{Kind: "instance", Name: "i-1", UID: "uid-1"}))
|
||||
|
||||
ref, ok := m.OwnerOfKind("instance")
|
||||
require.True(t, ok)
|
||||
live := resource.Metadata{Name: "i-1", UID: "uid-2"}
|
||||
|
||||
assert.Equal(t, live.Name, ref.Name)
|
||||
assert.NotEqual(t, live.UID, ref.UID, "same name, different resource — the ref is stale")
|
||||
}
|
||||
|
||||
// The envelope is persisted through kvstore, so an empty record must round-trip
|
||||
// without inventing keys a reader would then have to tolerate.
|
||||
func TestObjectJSON_OmitsEmptyBookkeeping(t *testing.T) {
|
||||
out, err := json.Marshal(instance{Metadata: resource.Metadata{Name: "i-1"}})
|
||||
require.NoError(t, err)
|
||||
|
||||
var raw map[string]json.RawMessage
|
||||
require.NoError(t, json.Unmarshal(out, &raw))
|
||||
assert.Equal(t, []string{"metadata", "spec", "status"}, sortedKeys(raw))
|
||||
|
||||
var meta map[string]json.RawMessage
|
||||
require.NoError(t, json.Unmarshal(raw["metadata"], &meta))
|
||||
assert.Equal(t, []string{"name"}, sortedKeys(meta))
|
||||
}
|
||||
|
||||
func TestObjectJSON_RoundTrips(t *testing.T) {
|
||||
now := time.Date(2026, 8, 28, 10, 0, 0, 0, time.UTC)
|
||||
want := instance{
|
||||
Metadata: resource.Metadata{
|
||||
Name: "i-1", UID: "uid-1", AccountID: "111122223333", Region: "ap-southeast-2",
|
||||
Generation: 3, ObservedGeneration: 2,
|
||||
OwnerRefs: []resource.OwnerRef{{Kind: "cluster", Name: "c-1", UID: "uid-c"}},
|
||||
Finalizers: []string{"eni"},
|
||||
Tags: map[string]string{"Name": "web"},
|
||||
DeletionTimestamp: &now,
|
||||
},
|
||||
Spec: instanceSpec{InstanceType: "t3.micro", DesiredState: "stopped"},
|
||||
Status: instanceStatus{State: "stopping", NodeID: "node-1"},
|
||||
}
|
||||
|
||||
out, err := json.Marshal(want)
|
||||
require.NoError(t, err)
|
||||
|
||||
var got instance
|
||||
require.NoError(t, json.Unmarshal(out, &got))
|
||||
assert.Equal(t, want, got)
|
||||
}
|
||||
|
||||
func sortedKeys(m map[string]json.RawMessage) []string {
|
||||
keys := make([]string, 0, len(m))
|
||||
for k := range m {
|
||||
keys = append(keys, k)
|
||||
}
|
||||
slices.Sort(keys)
|
||||
return keys
|
||||
}
|
||||
Reference in New Issue
Block a user