diff --git a/rest-api/common/pkg/util/converter.go b/rest-api/common/pkg/util/converter.go index 2d4ed3d3c5..eb0234faea 100644 --- a/rest-api/common/pkg/util/converter.go +++ b/rest-api/common/pkg/util/converter.go @@ -46,6 +46,29 @@ func GetPtr[T any](v T) *T { return &v } +// GetPtrIfNotZero returns nil for a type's zero value or a pointer to a copy +// of v otherwise. +func GetPtrIfNotZero[T comparable](v T) *T { + var zero T + if v == zero { + return nil + } + + return &v +} + +// GetValueOrZero returns a type's zero value for nil or the dereferenced value +// otherwise. +func GetValueOrZero[T any](v *T) T { + if v == nil { + var zero T + + return zero + } + + return *v +} + // StrPtrToProtoTimePtr converts a string pointer to a protobuf timestamp pointer func StrPtrToProtoTimePtr(s *string) *timestamppb.Timestamp { if s == nil { diff --git a/rest-api/common/pkg/util/converter_test.go b/rest-api/common/pkg/util/converter_test.go index 8ea38294b4..6d46730e9e 100644 --- a/rest-api/common/pkg/util/converter_test.go +++ b/rest-api/common/pkg/util/converter_test.go @@ -97,3 +97,47 @@ func TestGetPtr(t *testing.T) { } }) } + +func TestGetPtrIfNotZero(t *testing.T) { + tests := []struct { + name string + value string + wantNil bool + }{ + {name: "zero value", wantNil: true}, + {name: "non-zero value", value: "test"}, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + got := GetPtrIfNotZero(test.value) + + if test.wantNil { + require.Nil(t, got) + + return + } + + require.NotNil(t, got) + require.Equal(t, test.value, *got) + }) + } +} + +func TestGetValueOrZero(t *testing.T) { + tests := []struct { + name string + value *string + want string + }{ + {name: "nil"}, + {name: "pointer to zero value", value: GetPtr("")}, + {name: "pointer to non-zero value", value: GetPtr("test"), want: "test"}, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + require.Equal(t, test.want, GetValueOrZero(test.value)) + }) + } +} diff --git a/rest-api/flow/internal/converter/dao/converter.go b/rest-api/flow/internal/converter/dao/converter.go index 2d6b3d4373..82c45f6ae1 100644 --- a/rest-api/flow/internal/converter/dao/converter.go +++ b/rest-api/flow/internal/converter/dao/converter.go @@ -12,6 +12,7 @@ import ( "github.com/google/uuid" "github.com/NVIDIA/infra-controller/rest-api/common/pkg/credential" + cutil "github.com/NVIDIA/infra-controller/rest-api/common/pkg/util" "github.com/NVIDIA/infra-controller/rest-api/flow/internal/db/model" "github.com/NVIDIA/infra-controller/rest-api/flow/internal/nicoapi" "github.com/NVIDIA/infra-controller/rest-api/flow/internal/operation" @@ -92,10 +93,6 @@ func ComponentFrom(dao model.Component) *component.Component { bmcsByType[t] = append(bmcsByType[t], BMCFrom(bd)) } - var componentID string - if dao.ComponentID != nil { - componentID = *dao.ComponentID - } var nvlDomainID uuid.UUID if dao.Rack != nil && dao.Rack.NVLDomainID != uuid.Nil { nvlDomainID = dao.Rack.NVLDomainID @@ -118,7 +115,7 @@ func ComponentFrom(dao model.Component) *component.Component { HostID: dao.HostID, }, BmcsByType: bmcsByType, - ComponentID: componentID, + ComponentID: cutil.GetValueOrZero(dao.ComponentID), RackID: dao.RackID, NVLDomainID: nvlDomainID, PowerState: powerStateFromDAO(dao.PowerState), @@ -294,10 +291,7 @@ func ComponentTo(c *component.Component, rackID uuid.UUID) *model.Component { TrayIndex: c.Position.TrayIndex, HostID: c.Position.HostID, RackID: rackID, - } - - if c.ComponentID != "" { - compDAO.ComponentID = &c.ComponentID + ComponentID: cutil.GetPtrIfNotZero(c.ComponentID), } for _, t := range devicetypes.BMCTypes() { diff --git a/rest-api/flow/internal/converter/dao/event.go b/rest-api/flow/internal/converter/dao/event.go index fee310bc35..44b011690a 100644 --- a/rest-api/flow/internal/converter/dao/event.go +++ b/rest-api/flow/internal/converter/dao/event.go @@ -35,7 +35,6 @@ func EventTo(event *eventrule.Event) (*dbmodel.Event, error) { Observations: event.Observations, CreatedAt: event.CreatedAt, LastObservedAt: event.LastObservedAt, - PlannedAt: event.PlannedAt, }, nil } @@ -64,7 +63,6 @@ func EventFrom(persisted *dbmodel.Event) (*eventrule.Event, error) { Observations: persisted.Observations, CreatedAt: persisted.CreatedAt, LastObservedAt: persisted.LastObservedAt, - PlannedAt: persisted.PlannedAt, } if err := event.Validate(); err != nil { diff --git a/rest-api/flow/internal/converter/dao/event_action_execution.go b/rest-api/flow/internal/converter/dao/event_action_execution.go index 0b852ff1fe..f08af9f6e2 100644 --- a/rest-api/flow/internal/converter/dao/event_action_execution.go +++ b/rest-api/flow/internal/converter/dao/event_action_execution.go @@ -5,8 +5,8 @@ package dao import ( "fmt" - "time" + cutil "github.com/NVIDIA/infra-controller/rest-api/common/pkg/util" dbmodel "github.com/NVIDIA/infra-controller/rest-api/flow/internal/db/model" "github.com/NVIDIA/infra-controller/rest-api/flow/internal/eventrule" eventrulecodec "github.com/NVIDIA/infra-controller/rest-api/flow/internal/eventrule/codec" @@ -19,26 +19,12 @@ func EventActionExecutionTo( if err := execution.Validate(); err != nil { return nil, err } + plan, err := eventrulecodec.MarshalExecutionPlan(execution.Plan) if err != nil { return nil, fmt.Errorf("encode execution plan: %w", err) } - var nextAttemptAt *time.Time - if !execution.NextAttemptAt.IsZero() { - next := execution.NextAttemptAt - nextAttemptAt = &next - } - var reason *string - if execution.Reason != eventrule.ExecutionReasonNone { - value := string(execution.Reason) - reason = &value - } - var statusMessage *string - if execution.StatusMessage != "" { - value := execution.StatusMessage - statusMessage = &value - } return &dbmodel.EventActionExecution{ ID: execution.ID, EventID: execution.EventID, @@ -46,12 +32,14 @@ func EventActionExecutionTo( ActionType: string(execution.Plan.Type()), Plan: plan, Status: string(execution.Status), - Reason: reason, + Reason: cutil.GetPtrIfNotZero(string(execution.Reason)), Attempts: execution.Attempts, - StatusMessage: statusMessage, + ClaimToken: cutil.GetPtrIfNotZero(execution.ClaimToken), + ClaimOwner: cutil.GetPtrIfNotZero(execution.ClaimOwner), + StatusMessage: cutil.GetPtrIfNotZero(execution.StatusMessage), CreatedAt: execution.CreatedAt, UpdatedAt: execution.UpdatedAt, - NextAttemptAt: nextAttemptAt, + NextAttemptAt: cutil.GetPtrIfNotZero(execution.NextAttemptAt), }, nil } @@ -63,18 +51,6 @@ func EventActionExecutionFrom( return nil, nil } - var nextAttemptAt time.Time - if persisted.NextAttemptAt != nil { - nextAttemptAt = *persisted.NextAttemptAt - } - var reason eventrule.ExecutionReason - if persisted.Reason != nil { - reason = eventrule.ExecutionReason(*persisted.Reason) - } - var statusMessage string - if persisted.StatusMessage != nil { - statusMessage = *persisted.StatusMessage - } plan, err := eventrulecodec.UnmarshalExecutionPlan(persisted.Plan) if err != nil { return nil, fmt.Errorf( @@ -83,6 +59,7 @@ func EventActionExecutionFrom( err, ) } + if string(plan.Type()) != persisted.ActionType { return nil, fmt.Errorf( "%w: action type %q does not match plan type %q", @@ -91,25 +68,30 @@ func EventActionExecutionFrom( plan.Type(), ) } + execution := &eventrule.Execution{ ExecutionState: eventrule.ExecutionState{ ExecutionStatusDetails: eventrule.ExecutionStatusDetails{ Status: eventrule.ExecutionStatus(persisted.Status), - Reason: reason, - StatusMessage: statusMessage, + Reason: eventrule.ExecutionReason(cutil.GetValueOrZero(persisted.Reason)), + StatusMessage: cutil.GetValueOrZero(persisted.StatusMessage), }, - NextAttemptAt: nextAttemptAt, + NextAttemptAt: cutil.GetValueOrZero(persisted.NextAttemptAt), }, ID: persisted.ID, EventID: persisted.EventID, ActionName: persisted.ActionName, Plan: plan, Attempts: persisted.Attempts, + ClaimToken: cutil.GetValueOrZero(persisted.ClaimToken), + ClaimOwner: cutil.GetValueOrZero(persisted.ClaimOwner), CreatedAt: persisted.CreatedAt, UpdatedAt: persisted.UpdatedAt, } + if err := execution.Validate(); err != nil { return nil, fmt.Errorf("%w: %w", eventrule.ErrInvalidPersistedExecution, err) } + return execution, nil } diff --git a/rest-api/flow/internal/converter/dao/event_action_execution_test.go b/rest-api/flow/internal/converter/dao/event_action_execution_test.go index 4fe88fe8eb..faf02683b4 100644 --- a/rest-api/flow/internal/converter/dao/event_action_execution_test.go +++ b/rest-api/flow/internal/converter/dao/event_action_execution_test.go @@ -19,29 +19,90 @@ func TestEventActionExecutionRoundTrip(t *testing.T) { base, err := eventrule.NewExecution(uuid.New(), "notify", &eventrule.NoopPlan{Reason: "test"}, now) require.NoError(t, err) - tests := map[string]eventrule.ExecutionResult{ - "completed": eventrule.CompletedExecutionResult(), - "skipped": eventrule.SkippedExecutionResult(eventrule.ExecutionReasonNoTargets), - "deferred": eventrule.DeferredExecutionResult(eventrule.ExecutionReasonAttemptFailed, "temporary", time.Minute), - "failed": eventrule.FailedExecutionResult("terminal"), + tests := map[string]struct { + claim bool + result *eventrule.ExecutionResult + activeClaim bool + }{ + "pending": {}, + "running": { + claim: true, + activeClaim: true, + }, + "completed": { + claim: true, + result: resultPointer(eventrule.CompletedExecutionResult()), + }, + "deferred": { + claim: true, + result: resultPointer(eventrule.DeferredExecutionResult( + eventrule.ExecutionReasonAttemptFailed, + "temporary", + time.Minute, + )), + }, + "interrupted": { + claim: true, + result: resultPointer(eventrule.DeferredExecutionResult( + eventrule.ExecutionReasonAttemptInterrupted, + "interrupted", + time.Minute, + )), + }, + "failed": { + claim: true, + result: resultPointer(eventrule.FailedExecutionResult("terminal")), + }, } - for name, result := range tests { + + for name, test := range tests { t.Run(name, func(t *testing.T) { execution := base.Clone() - require.NoError(t, execution.TransitionTo(result, now.Add(time.Second))) + token := uuid.New() + + if test.claim { + require.NoError(t, execution.Claim("scheduler-1", token, now.Add(time.Second))) + } + if test.result != nil { + require.NoError( + t, + execution.TransitionClaimedTo(token, *test.result, now.Add(2*time.Second)), + ) + } + persisted, err := EventActionExecutionTo(&execution) require.NoError(t, err) + if test.activeClaim { + require.NotNil(t, persisted.ClaimToken) + require.Equal(t, token, *persisted.ClaimToken) + require.NotNil(t, persisted.ClaimOwner) + require.Equal(t, "scheduler-1", *persisted.ClaimOwner) + } else { + require.Nil(t, persisted.ClaimToken) + require.Nil(t, persisted.ClaimOwner) + } + roundTripped, err := EventActionExecutionFrom(persisted) require.NoError(t, err) + require.Equal(t, &execution, roundTripped) + if !test.activeClaim { + require.Equal(t, uuid.Nil, roundTripped.ClaimToken) + require.Empty(t, roundTripped.ClaimOwner) + } }) } } +func resultPointer(result eventrule.ExecutionResult) *eventrule.ExecutionResult { + return &result +} + func TestEventActionExecutionFromRejectsInvalidPersistence(t *testing.T) { now := time.Date(2026, 8, 21, 12, 0, 0, 0, time.UTC) execution, err := eventrule.NewExecution(uuid.New(), "notify", &eventrule.NoopPlan{}, now) require.NoError(t, err) + valid, err := EventActionExecutionTo(execution) require.NoError(t, err) @@ -68,6 +129,7 @@ func TestEventActionExecutionFromRejectsInvalidPersistence(t *testing.T) { wantErr: "does not match plan type", }, } + for name, test := range tests { t.Run(name, func(t *testing.T) { var persisted *dbmodel.EventActionExecution @@ -76,12 +138,15 @@ func TestEventActionExecutionFromRejectsInvalidPersistence(t *testing.T) { persisted = © test.mutate(persisted) } + result, err := EventActionExecutionFrom(persisted) + if test.wantErr == "" { require.NoError(t, err) require.Equal(t, test.wantNil, result == nil) return } + require.ErrorIs(t, err, eventrule.ErrInvalidPersistedExecution) require.ErrorContains(t, err, test.wantErr) require.Nil(t, result) diff --git a/rest-api/flow/internal/converter/dao/event_rule.go b/rest-api/flow/internal/converter/dao/event_rule.go index 74d7abbd6c..72d0ad234f 100644 --- a/rest-api/flow/internal/converter/dao/event_rule.go +++ b/rest-api/flow/internal/converter/dao/event_rule.go @@ -7,10 +7,10 @@ import ( "fmt" "time" + cutil "github.com/NVIDIA/infra-controller/rest-api/common/pkg/util" dbmodel "github.com/NVIDIA/infra-controller/rest-api/flow/internal/db/model" "github.com/NVIDIA/infra-controller/rest-api/flow/internal/eventrule" eventrulecodec "github.com/NVIDIA/infra-controller/rest-api/flow/internal/eventrule/codec" - "github.com/google/uuid" ) // EventRuleTo converts a domain rule to a database model. @@ -87,18 +87,12 @@ func EventRuleBindingTo( return nil, err } - var scopeID *uuid.UUID - if binding.Scope.HasID() { - id := binding.Scope.ID - scopeID = &id - } - return &dbmodel.EventRuleBinding{ ID: binding.ID, RuleID: binding.RuleID, EventType: string(binding.EventType), ScopeType: string(binding.Scope.Type), - ScopeID: scopeID, + ScopeID: cutil.GetPtrIfNotZero(binding.Scope.ID), CreatedAt: createdAt, UpdatedAt: updatedAt, }, nil @@ -112,18 +106,13 @@ func EventRuleBindingFrom( return nil, nil } - var scopeID uuid.UUID - if dbBinding.ScopeID != nil { - scopeID = *dbBinding.ScopeID - } - binding := &eventrule.Binding{ ID: dbBinding.ID, RuleID: dbBinding.RuleID, EventType: eventrule.Type(dbBinding.EventType), Scope: eventrule.Scope{ Type: eventrule.ScopeType(dbBinding.ScopeType), - ID: scopeID, + ID: cutil.GetValueOrZero(dbBinding.ScopeID), }, } diff --git a/rest-api/flow/internal/converter/dao/event_test.go b/rest-api/flow/internal/converter/dao/event_test.go index 202521fefa..0db459b986 100644 --- a/rest-api/flow/internal/converter/dao/event_test.go +++ b/rest-api/flow/internal/converter/dao/event_test.go @@ -16,7 +16,6 @@ import ( func TestEventRoundTrip(t *testing.T) { now := time.Date(2026, 8, 21, 12, 0, 0, 0, time.UTC) - plannedAt := now.Add(time.Second) event := &eventrule.Event{ ID: uuid.New(), Key: eventrule.EventKey{SourceName: "collector", SourceKey: "event-1"}, @@ -30,7 +29,6 @@ func TestEventRoundTrip(t *testing.T) { Observations: 2, CreatedAt: now, LastObservedAt: now.Add(time.Second), - PlannedAt: &plannedAt, } persisted, err := EventTo(event) diff --git a/rest-api/flow/internal/db/model/event.go b/rest-api/flow/internal/db/model/event.go index ed587440b9..290d2fd44a 100644 --- a/rest-api/flow/internal/db/model/event.go +++ b/rest-api/flow/internal/db/model/event.go @@ -27,5 +27,4 @@ type Event struct { Observations int `bun:"observations,notnull"` CreatedAt time.Time `bun:"created_at,notnull"` LastObservedAt time.Time `bun:"last_observed_at,notnull"` - PlannedAt *time.Time `bun:"planned_at"` } diff --git a/rest-api/flow/internal/db/model/event_action_execution.go b/rest-api/flow/internal/db/model/event_action_execution.go index 073e117382..d305becca5 100644 --- a/rest-api/flow/internal/db/model/event_action_execution.go +++ b/rest-api/flow/internal/db/model/event_action_execution.go @@ -24,6 +24,8 @@ type EventActionExecution struct { Status string `bun:"status,notnull"` Reason *string `bun:"reason"` Attempts int `bun:"attempts,notnull"` + ClaimToken *uuid.UUID `bun:"claim_token,type:uuid"` + ClaimOwner *string `bun:"claim_owner"` StatusMessage *string `bun:"status_message"` CreatedAt time.Time `bun:"created_at,notnull"` UpdatedAt time.Time `bun:"updated_at,notnull"` diff --git a/rest-api/flow/internal/eventrule/event.go b/rest-api/flow/internal/eventrule/event.go index 465adca2e6..2c7d166638 100644 --- a/rest-api/flow/internal/eventrule/event.go +++ b/rest-api/flow/internal/eventrule/event.go @@ -26,6 +26,7 @@ func (k EventKey) Validate() error { if err := validateIdentifier("event source name", k.SourceName); err != nil { return err } + return validateRequiredString("event source key", k.SourceKey) } @@ -106,6 +107,7 @@ type Envelope struct { func (e Envelope) Clone() Envelope { cloned := e cloned.Payload = append(json.RawMessage(nil), e.Payload...) + return cloned } @@ -114,6 +116,7 @@ func (e *Envelope) Validate() error { if e == nil { return fmt.Errorf("event envelope is nil") } + if err := e.Key.Validate(); err != nil { return err } @@ -129,6 +132,7 @@ func (e *Envelope) Validate() error { if len(e.Payload) > 0 && !json.Valid(e.Payload) { return fmt.Errorf("event payload must be valid JSON") } + return nil } @@ -173,8 +177,8 @@ type ResolvedResource struct { } // ResourceIdentity is the durable canonical identity of the resource an event -// concerns. Topology attributes are deliberately reconstructed while planning -// missing executions rather than stored on the event. +// concerns. Planning captures required topology attributes in immutable +// execution plans rather than storing them on the event. type ResourceIdentity struct { Kind ResourceKind ID uuid.UUID @@ -188,14 +192,14 @@ func (r ResourceIdentity) Validate() error { if r.ID == uuid.Nil { return fmt.Errorf("event resource id is required") } + return nil } const maxEventSummaryRunes = 1024 // Event is one deduplicated, enriched, rule-matched observation. It owns the -// immutable information shared by all action executions and the durable -// planning checkpoint. +// immutable information shared by all action executions. type Event struct { ID uuid.UUID Key EventKey @@ -207,17 +211,13 @@ type Event struct { Observations int CreatedAt time.Time LastObservedAt time.Time - PlannedAt *time.Time } // Clone returns an independent event snapshot. func (e Event) Clone() Event { cloned := e cloned.EffectivePolicy = e.EffectivePolicy.Clone() - if e.PlannedAt != nil { - plannedAt := *e.PlannedAt - cloned.PlannedAt = &plannedAt - } + return cloned } @@ -246,6 +246,7 @@ func (e Event) ValidateDefinition() error { if utf8.RuneCountInString(e.Summary) > maxEventSummaryRunes { return fmt.Errorf("event summary exceeds %d characters", maxEventSummaryRunes) } + return nil } @@ -254,6 +255,7 @@ func (e *Event) Validate() error { if e == nil { return fmt.Errorf("event is nil") } + if e.ID == uuid.Nil { return fmt.Errorf("event id is required") } @@ -272,9 +274,7 @@ func (e *Event) Validate() error { if e.LastObservedAt.Before(e.CreatedAt) { return fmt.Errorf("event last-observed time cannot precede creation time") } - if e.PlannedAt != nil && e.PlannedAt.Before(e.CreatedAt) { - return fmt.Errorf("event planned time cannot precede creation time") - } + return nil } @@ -292,7 +292,7 @@ func NewEvent(definition Event, now time.Time) (*Event, error) { event.Observations = 1 event.CreatedAt = now event.LastObservedAt = now - event.PlannedAt = nil + return &event, nil } @@ -308,6 +308,7 @@ func (r ResolvedResource) Validate() error { if r.RackID == uuid.Nil { return fmt.Errorf("resolved resource rack id is required") } + if r.Kind == ResourceKindComponent { if err := r.ComponentType.Validate(); err != nil { return fmt.Errorf("resolved resource component type: %w", err) @@ -317,5 +318,6 @@ func (r ResolvedResource) Validate() error { return fmt.Errorf("resolved rack resource id must equal rack id") } } + return nil } diff --git a/rest-api/flow/internal/eventrule/event_test.go b/rest-api/flow/internal/eventrule/event_test.go index 9938dfbc07..b620bc86d7 100644 --- a/rest-api/flow/internal/eventrule/event_test.go +++ b/rest-api/flow/internal/eventrule/event_test.go @@ -25,7 +25,6 @@ func TestNewEvent(t *testing.T) { require.Equal(t, 1, event.Observations) require.Equal(t, now, event.CreatedAt) require.Equal(t, now, event.LastObservedAt) - require.Nil(t, event.PlannedAt) require.NoError(t, event.Validate()) } @@ -93,11 +92,14 @@ func TestParseSeverity(t *testing.T) { for name, test := range tests { t.Run(name, func(t *testing.T) { actual, err := ParseSeverity(test.value) + if test.wantErr { require.Error(t, err) require.Equal(t, SeverityUnspecified, actual) + return } + require.NoError(t, err) require.Equal(t, test.expected, actual) }) @@ -127,10 +129,13 @@ func TestEventKey_Validate(t *testing.T) { for name, test := range tests { t.Run(name, func(t *testing.T) { err := test.key.Validate() + if test.wantErr == "" { require.NoError(t, err) + return } + require.ErrorContains(t, err, test.wantErr) }) } @@ -160,6 +165,7 @@ func TestEnvelopeValidatePayload(t *testing.T) { } err := envelope.Validate() + if test.wantErr != "" { require.ErrorContains(t, err, test.wantErr) } else { @@ -176,28 +182,34 @@ func TestEnvelopeAllowsUnspecifiedSeverity(t *testing.T) { Severity: SeverityUnspecified, Resource: Resource{Kind: ResourceKindRack}, } + require.NoError(t, envelope.Validate()) } func TestEnvelope_Clone(t *testing.T) { original := Envelope{Payload: json.RawMessage(`{"value":42}`)} + cloned := original.Clone() cloned.Payload[2] = 'x' + require.NotEqual(t, original.Payload, cloned.Payload) } func TestResourceIDMayBeUnresolved(t *testing.T) { resource := Resource{Kind: ResourceKindRack} + require.Equal(t, uuid.Nil, resource.ID) require.NoError(t, resource.Validate()) resource.ID = uuid.New() + require.NoError(t, resource.Validate()) } func TestResolvedResource_Validate(t *testing.T) { componentID := uuid.New() rackID := uuid.New() + tests := []struct { name string resource ResolvedResource @@ -277,10 +289,13 @@ func TestResolvedResource_Validate(t *testing.T) { for _, test := range tests { t.Run(test.name, func(t *testing.T) { err := test.resource.Validate() + if test.wantErr != "" { require.ErrorContains(t, err, test.wantErr) + return } + require.NoError(t, err) }) } diff --git a/rest-api/flow/internal/eventrule/execution.go b/rest-api/flow/internal/eventrule/execution.go index 9d82753672..673a40f25a 100644 --- a/rest-api/flow/internal/eventrule/execution.go +++ b/rest-api/flow/internal/eventrule/execution.go @@ -21,6 +21,7 @@ func (k ExecutionKey) Validate() error { if k.EventID == uuid.Nil { return fmt.Errorf("execution event id is required") } + return validateIdentifier("event rule action name", k.ActionName) } @@ -32,7 +33,12 @@ type Execution struct { EventID uuid.UUID ActionName string Plan ExecutionPlan + // Attempts counts attempts that consume the retry budget. Claiming an + // execution allocates an attempt; an interrupted attempt is refunded when + // the execution is deferred. Attempts int + ClaimToken uuid.UUID + ClaimOwner string CreatedAt time.Time UpdatedAt time.Time } @@ -41,15 +47,66 @@ type Execution struct { func (e Execution) Clone() Execution { cloned := e cloned.Plan = CloneExecutionPlan(e.Plan) + return cloned } -// TransitionTo validates and applies a dispatch result at the given time. A -// non-skipped result records the attempt that produced it. -func (e *Execution) TransitionTo(result ExecutionResult, now time.Time) error { +// Claim allocates the next attempt and moves an eligible execution to running. +func (e *Execution) Claim(owner string, token uuid.UUID, now time.Time) error { if e == nil { return fmt.Errorf("execution is nil") } + + if !e.Status.CanBeClaimed() { + return fmt.Errorf("execution %s cannot be claimed from %q", e.ID, e.Status) + } + + if err := ValidateExecutionClaimOwner(owner); err != nil { + return err + } + if token == uuid.Nil { + return fmt.Errorf("execution claim token is required") + } + + if now.IsZero() { + return fmt.Errorf("execution claim time is required") + } + if now.Before(e.CreatedAt) { + return fmt.Errorf("execution claim time cannot precede creation time") + } + if now.Before(e.UpdatedAt) { + return fmt.Errorf("execution claim time cannot precede update time") + } + + e.ExecutionState = ExecutionState{ + ExecutionStatusDetails: ExecutionStatusDetails{Status: ExecutionStatusRunning}, + } + e.Attempts++ + e.ClaimToken = token + e.ClaimOwner = owner + e.UpdatedAt = now + + return nil +} + +// TransitionClaimedTo validates and applies the active attempt's result when +// token still owns the execution. +func (e *Execution) TransitionClaimedTo( + token uuid.UUID, + result ExecutionResult, + now time.Time, +) error { + if e == nil { + return fmt.Errorf("execution is nil") + } + + if token == uuid.Nil { + return fmt.Errorf("execution claim token is required") + } + if e.ClaimToken != token { + return fmt.Errorf("%w: execution %s", ErrExecutionClaimLost, e.ID) + } + if err := result.Validate(); err != nil { return err } @@ -61,20 +118,27 @@ func (e *Execution) TransitionTo(result ExecutionResult, now time.Time) error { result.Status, ) } + if now.IsZero() { return fmt.Errorf("execution transition time is required") } if now.Before(e.CreatedAt) { return fmt.Errorf("execution transition time cannot precede creation time") } + if now.Before(e.UpdatedAt) { + return fmt.Errorf("execution transition time cannot precede update time") + } - if result.Status != ExecutionStatusSkipped { - e.Attempts++ + if result.Status == ExecutionStatusDeferred && + result.Reason == ExecutionReasonAttemptInterrupted { + e.Attempts-- } + e.ExecutionState = result.stateAt(now) - if now.After(e.UpdatedAt) { - e.UpdatedAt = now - } + e.ClaimToken = uuid.Nil + e.ClaimOwner = "" + e.UpdatedAt = now + return nil } @@ -83,6 +147,7 @@ func (e *Execution) Validate() error { if e == nil { return fmt.Errorf("execution is nil") } + if e.ID == uuid.Nil { return fmt.Errorf("execution id is required") } @@ -95,17 +160,38 @@ func (e *Execution) Validate() error { if err := e.ExecutionState.Validate(); err != nil { return err } + if e.Attempts < 0 { return fmt.Errorf("execution attempts cannot be negative") } - if e.Status == ExecutionStatusPending && e.Attempts != 0 { - return fmt.Errorf("pending execution cannot have attempts") + if (e.Status == ExecutionStatusPending || e.Status == ExecutionStatusSkipped) && + e.Attempts != 0 { + return fmt.Errorf("%s execution cannot have attempts", e.Status) } if e.Status != ExecutionStatusPending && e.Status != ExecutionStatusSkipped && - e.Attempts == 0 { + e.Attempts == 0 && + !(e.Status == ExecutionStatusDeferred && + e.Reason == ExecutionReasonAttemptInterrupted) { return fmt.Errorf("%s execution requires an attempt", e.Status) } + + if e.Status == ExecutionStatusRunning { + if e.ClaimToken == uuid.Nil { + return fmt.Errorf("running execution requires claim token") + } + if err := ValidateExecutionClaimOwner(e.ClaimOwner); err != nil { + return err + } + } else { + if e.ClaimToken != uuid.Nil { + return fmt.Errorf("%s execution cannot have claim token", e.Status) + } + if e.ClaimOwner != "" { + return fmt.Errorf("%s execution cannot have claim owner", e.Status) + } + } + if e.CreatedAt.IsZero() { return fmt.Errorf("execution creation time is required") } @@ -131,6 +217,7 @@ func NewExecution( if err := key.Validate(); err != nil { return nil, err } + if err := ValidateExecutionPlan(plan); err != nil { return nil, err } diff --git a/rest-api/flow/internal/eventrule/execution_state.go b/rest-api/flow/internal/eventrule/execution_state.go index 73ec16876f..7c5bbf4530 100644 --- a/rest-api/flow/internal/eventrule/execution_state.go +++ b/rest-api/flow/internal/eventrule/execution_state.go @@ -14,26 +14,31 @@ type ExecutionStatus string const ( ExecutionStatusPending ExecutionStatus = "pending" + ExecutionStatusRunning ExecutionStatus = "running" ExecutionStatusSkipped ExecutionStatus = "skipped" ExecutionStatusDeferred ExecutionStatus = "deferred" ExecutionStatusCompleted ExecutionStatus = "completed" ExecutionStatusFailed ExecutionStatus = "failed" ) -// CanTransitionTo reports whether an execution with this status may accept an -// attempt result. Pending is used by the creator's first attempt; deferred is -// used by scheduler-owned retries. +// CanTransitionTo reports whether a running execution may accept an attempt +// result. func (s ExecutionStatus) CanTransitionTo(target ExecutionStatus) bool { - if s != ExecutionStatusPending && s != ExecutionStatusDeferred { + if s != ExecutionStatusRunning { return false } return target == ExecutionStatusCompleted || - target == ExecutionStatusSkipped || target == ExecutionStatusDeferred || target == ExecutionStatusFailed } +// CanBeClaimed reports whether the scheduler may allocate an attempt for the +// execution after applying lane-specific eligibility checks. +func (s ExecutionStatus) CanBeClaimed() bool { + return s == ExecutionStatusPending || s == ExecutionStatusDeferred +} + // RequiresRetryScheduling reports whether the status requires the store to // calculate a next-attempt time. func (s ExecutionStatus) RequiresRetryScheduling() bool { @@ -119,6 +124,7 @@ func (s ExecutionState) RetryDue(now time.Time) bool { var executionStatusReasons = map[ExecutionStatus][]ExecutionReason{ ExecutionStatusPending: nil, + ExecutionStatusRunning: nil, ExecutionStatusSkipped: { ExecutionReasonNoTargets, }, @@ -147,16 +153,6 @@ func CompletedExecutionResult() ExecutionResult { } } -// SkippedExecutionResult creates a skipped dispatch result. -func SkippedExecutionResult(reason ExecutionReason) ExecutionResult { - return ExecutionResult{ - ExecutionStatusDetails: ExecutionStatusDetails{ - Status: ExecutionStatusSkipped, - Reason: reason, - }, - } -} - // DeferredExecutionResult creates a deferred dispatch result. func DeferredExecutionResult( reason ExecutionReason, @@ -185,8 +181,10 @@ func FailedExecutionResult(statusMessage string) ExecutionResult { // Validate checks that the dispatch result is internally consistent. func (r ExecutionResult) Validate() error { - if r.Status == ExecutionStatusPending { - return fmt.Errorf("pending is not an execution result") + if r.Status == ExecutionStatusPending || + r.Status == ExecutionStatusRunning || + r.Status == ExecutionStatusSkipped { + return fmt.Errorf("%s is not an execution result", r.Status) } if err := r.ExecutionStatusDetails.Validate(); err != nil { diff --git a/rest-api/flow/internal/eventrule/execution_test.go b/rest-api/flow/internal/eventrule/execution_test.go index 2b7f374b35..5386b281e3 100644 --- a/rest-api/flow/internal/eventrule/execution_test.go +++ b/rest-api/flow/internal/eventrule/execution_test.go @@ -4,6 +4,7 @@ package eventrule import ( + "context" "testing" "time" @@ -16,8 +17,10 @@ import ( func TestNewExecution(t *testing.T) { now := time.Date(2026, 8, 21, 12, 0, 0, 0, time.UTC) + execution, err := NewExecution(uuid.New(), "notify", &NoopPlan{Reason: "test"}, now) require.NoError(t, err) + require.NotEqual(t, uuid.Nil, execution.ID) require.Equal(t, ExecutionStatusPending, execution.Status) require.Zero(t, execution.Attempts) @@ -43,13 +46,17 @@ func TestPlannedExecutionValidate(t *testing.T) { wantErr: "execution plan is required", }, } + for name, test := range tests { t.Run(name, func(t *testing.T) { err := test.planned.Validate() + if test.wantErr == "" { require.NoError(t, err) + return } + require.ErrorContains(t, err, test.wantErr) }) } @@ -59,8 +66,10 @@ func TestNewExecutionSkipsEmptySubmitTaskPlan(t *testing.T) { operationInfo := &operations.PowerControlTaskInfo{ Operation: operations.PowerOperationForcePowerOff, } + info, err := operationInfo.Marshal() require.NoError(t, err) + execution, err := NewExecution( uuid.New(), "power_off", @@ -75,42 +84,159 @@ func TestNewExecutionSkipsEmptySubmitTaskPlan(t *testing.T) { time.Date(2026, 8, 21, 12, 0, 0, 0, time.UTC), ) require.NoError(t, err) + require.Equal(t, ExecutionStatusSkipped, execution.Status) require.Equal(t, ExecutionReasonNoTargets, execution.Reason) require.Zero(t, execution.Attempts) require.NoError(t, execution.Validate()) } -func TestExecutionTransitionTo(t *testing.T) { +func TestExecution_Claim(t *testing.T) { now := time.Date(2026, 8, 21, 12, 0, 0, 0, time.UTC) - execution, err := NewExecution(uuid.New(), "notify", &NoopPlan{}, now) - require.NoError(t, err) + tests := map[string]struct { + updatedAt time.Time + claimAt time.Time + wantErr string + }{ + "claimed": { + claimAt: now.Add(time.Second), + }, + "claim time before latest update": { + updatedAt: now.Add(2 * time.Second), + claimAt: now.Add(time.Second), + wantErr: "execution claim time cannot precede update time", + }, + } - retryAt := now.Add(time.Second) - require.NoError(t, execution.TransitionTo( - DeferredExecutionResult(ExecutionReasonAttemptFailed, "temporary", time.Minute), - retryAt, - )) - require.Equal(t, ExecutionStatusDeferred, execution.Status) - require.Equal(t, 1, execution.Attempts) - require.Equal(t, retryAt.Add(time.Minute), execution.NextAttemptAt) - - completedAt := retryAt.Add(time.Second) - require.NoError(t, execution.TransitionTo(CompletedExecutionResult(), completedAt)) - require.Equal(t, ExecutionStatusCompleted, execution.Status) - require.Equal(t, 2, execution.Attempts) - require.True(t, execution.NextAttemptAt.IsZero()) - require.ErrorContains( - t, - execution.TransitionTo(FailedExecutionResult("late"), completedAt.Add(time.Second)), - "cannot transition", - ) + for name, test := range tests { + t.Run(name, func(t *testing.T) { + execution, err := NewExecution(uuid.New(), "notify", &NoopPlan{}, now) + require.NoError(t, err) + if !test.updatedAt.IsZero() { + execution.UpdatedAt = test.updatedAt + } + + before := execution.Clone() + token := uuid.New() + err = execution.Claim("scheduler-1", token, test.claimAt) + if test.wantErr != "" { + require.ErrorContains(t, err, test.wantErr) + require.Equal(t, before, *execution) + + return + } + + require.NoError(t, err) + require.Equal(t, ExecutionStatusRunning, execution.Status) + require.Equal(t, 1, execution.Attempts) + require.Equal(t, token, execution.ClaimToken) + require.Equal(t, "scheduler-1", execution.ClaimOwner) + require.True(t, execution.NextAttemptAt.IsZero()) + require.Equal(t, test.claimAt, execution.UpdatedAt) + require.NoError(t, execution.Validate()) + + require.ErrorContains( + t, + execution.Claim("scheduler-1", uuid.New(), now.Add(2*time.Second)), + "cannot be claimed", + ) + }) + } +} + +func TestExecution_TransitionClaimedTo(t *testing.T) { + now := time.Date(2026, 8, 21, 12, 0, 0, 0, time.UTC) + tests := map[string]struct { + result ExecutionResult + token func(uuid.UUID) uuid.UUID + claimAfter time.Duration + transitionAfter time.Duration + want ExecutionStatus + wantAttempts int + wantErr string + }{ + "completed": { + result: CompletedExecutionResult(), + transitionAfter: time.Second, + want: ExecutionStatusCompleted, + wantAttempts: 1, + }, + "deferred": { + result: DeferredExecutionResult(ExecutionReasonAttemptFailed, "temporary", time.Minute), + transitionAfter: time.Second, + want: ExecutionStatusDeferred, + wantAttempts: 1, + }, + "interrupted": { + result: DeferredExecutionResult( + ExecutionReasonAttemptInterrupted, + context.Canceled.Error(), + time.Minute, + ), + transitionAfter: time.Second, + want: ExecutionStatusDeferred, + }, + "failed": { + result: FailedExecutionResult("terminal"), + transitionAfter: time.Second, + want: ExecutionStatusFailed, + wantAttempts: 1, + }, + "stale token": { + result: CompletedExecutionResult(), + token: func(uuid.UUID) uuid.UUID { return uuid.New() }, + transitionAfter: time.Second, + wantErr: "execution claim lost", + }, + "transition time before latest update": { + result: CompletedExecutionResult(), + claimAfter: 2 * time.Second, + transitionAfter: time.Second, + wantErr: "execution transition time cannot precede update time", + }, + } + + for name, test := range tests { + t.Run(name, func(t *testing.T) { + execution, err := NewExecution(uuid.New(), "notify", &NoopPlan{}, now) + require.NoError(t, err) + + claimToken := uuid.New() + require.NoError(t, execution.Claim("scheduler-1", claimToken, now.Add(test.claimAfter))) + before := execution.Clone() + + transitionToken := claimToken + if test.token != nil { + transitionToken = test.token(claimToken) + } + + err = execution.TransitionClaimedTo( + transitionToken, + test.result, + now.Add(test.transitionAfter), + ) + if test.wantErr != "" { + require.ErrorContains(t, err, test.wantErr) + require.Equal(t, before, *execution) + + return + } + + require.NoError(t, err) + require.Equal(t, test.want, execution.Status) + require.Equal(t, test.wantAttempts, execution.Attempts) + require.Equal(t, uuid.Nil, execution.ClaimToken) + require.Empty(t, execution.ClaimOwner) + require.NoError(t, execution.Validate()) + }) + } } func TestExecutionValidate(t *testing.T) { now := time.Date(2026, 8, 21, 12, 0, 0, 0, time.UTC) valid, err := NewExecution(uuid.New(), "notify", &NoopPlan{}, now) require.NoError(t, err) + tests := map[string]struct { mutate func(*Execution) wantErr string @@ -136,18 +262,51 @@ func TestExecutionValidate(t *testing.T) { mutate: func(execution *Execution) { execution.Attempts = 1 }, wantErr: "pending execution cannot have attempts", }, + "running without claim token": { + mutate: func(execution *Execution) { + execution.ExecutionState = ExecutionState{ + ExecutionStatusDetails: ExecutionStatusDetails{Status: ExecutionStatusRunning}, + } + execution.Attempts = 1 + execution.ClaimOwner = "scheduler-1" + }, + wantErr: "running execution requires claim token", + }, + "running with invalid claim owner": { + mutate: func(execution *Execution) { + execution.ExecutionState = ExecutionState{ + ExecutionStatusDetails: ExecutionStatusDetails{Status: ExecutionStatusRunning}, + } + execution.Attempts = 1 + execution.ClaimToken = uuid.New() + }, + wantErr: "execution claim owner is empty", + }, + "non-running with claim token": { + mutate: func(execution *Execution) { execution.ClaimToken = uuid.New() }, + wantErr: "pending execution cannot have claim token", + }, + "non-running with claim owner": { + mutate: func(execution *Execution) { execution.ClaimOwner = "scheduler-1" }, + wantErr: "pending execution cannot have claim owner", + }, } + for name, test := range tests { t.Run(name, func(t *testing.T) { execution := valid.Clone() if test.mutate != nil { test.mutate(&execution) } + err := execution.Validate() + if test.wantErr == "" { require.NoError(t, err) + return } + require.ErrorContains(t, err, test.wantErr) }) } @@ -159,25 +318,39 @@ func TestExecutionResultValidate(t *testing.T) { wantErr string }{ "completed": {result: CompletedExecutionResult()}, - "skipped": {result: SkippedExecutionResult(ExecutionReasonNoTargets)}, "deferred": {result: DeferredExecutionResult(ExecutionReasonAttemptFailed, "temporary", time.Second)}, "failed": {result: FailedExecutionResult("terminal")}, "pending result": { result: ExecutionResult{ExecutionStatusDetails: ExecutionStatusDetails{Status: ExecutionStatusPending}}, wantErr: "pending is not an execution result", }, + "running result": { + result: ExecutionResult{ExecutionStatusDetails: ExecutionStatusDetails{Status: ExecutionStatusRunning}}, + wantErr: "running is not an execution result", + }, + "skipped result": { + result: ExecutionResult{ExecutionStatusDetails: ExecutionStatusDetails{ + Status: ExecutionStatusSkipped, + Reason: ExecutionReasonNoTargets, + }}, + wantErr: "skipped is not an execution result", + }, "negative retry": { result: DeferredExecutionResult(ExecutionReasonAttemptFailed, "temporary", -time.Second), wantErr: "cannot be negative", }, } + for name, test := range tests { t.Run(name, func(t *testing.T) { err := test.result.Validate() + if test.wantErr == "" { require.NoError(t, err) + return } + require.ErrorContains(t, err, test.wantErr) }) } diff --git a/rest-api/flow/internal/eventrule/executor/alert.go b/rest-api/flow/internal/eventrule/executor/alert.go index cdb9a1a448..fbba848ad5 100644 --- a/rest-api/flow/internal/eventrule/executor/alert.go +++ b/rest-api/flow/internal/eventrule/executor/alert.go @@ -36,13 +36,13 @@ func (e *AlertExecutor) Execute(ctx context.Context, request ExecutionRequest) e return terminalError(fmt.Errorf("alert sender is required")) } - plan, err := alertPlan(request.Execution.Plan) + plan, err := alertPlan(request.Plan) if err != nil { return terminalError(err) } req := AlertRequest{ - IdempotencyKey: alertIdempotencyKey(request.Execution.ID), + IdempotencyKey: alertIdempotencyKey(request.ExecutionID), Severity: plan.Severity, Message: plan.Message, } diff --git a/rest-api/flow/internal/eventrule/executor/alert_test.go b/rest-api/flow/internal/eventrule/executor/alert_test.go index 4b420a95cc..2177e75e12 100644 --- a/rest-api/flow/internal/eventrule/executor/alert_test.go +++ b/rest-api/flow/internal/eventrule/executor/alert_test.go @@ -14,7 +14,7 @@ import ( func TestAlertExecutor_Execute(t *testing.T) { request := newValidExecutionRequest(t) - request.Execution.Plan = &eventrule.SendAlertPlan{ + request.Plan = &eventrule.SendAlertPlan{ Severity: eventrule.SeverityCritical, Message: "leak detected", } @@ -60,7 +60,7 @@ func TestAlertExecutor_Execute(t *testing.T) { require.NoError(t, err) require.Equal(t, []AlertRequest{{ - IdempotencyKey: alertIdempotencyKey(request.Execution.ID), + IdempotencyKey: alertIdempotencyKey(request.ExecutionID), Severity: eventrule.SeverityCritical, Message: "leak detected", }}, test.sender.requests) @@ -70,7 +70,7 @@ func TestAlertExecutor_Execute(t *testing.T) { func TestAlertExecutor_ReusesIdempotencyKey(t *testing.T) { request := newValidExecutionRequest(t) - request.Execution.Plan = &eventrule.SendAlertPlan{Severity: eventrule.SeverityWarning} + request.Plan = &eventrule.SendAlertPlan{Severity: eventrule.SeverityWarning} sender := &recordingAlertSender{alertID: "stable-alert"} actionExecutor := &AlertExecutor{sender: sender} diff --git a/rest-api/flow/internal/eventrule/executor/errors.go b/rest-api/flow/internal/eventrule/executor/errors.go index c516c3ec6e..c5df9cb565 100644 --- a/rest-api/flow/internal/eventrule/executor/errors.go +++ b/rest-api/flow/internal/eventrule/executor/errors.go @@ -34,13 +34,19 @@ func retryableError(operation string, err error) error { return Retryable(fmt.Errorf("%s: %w", operation, err)) } +// IsInterrupted reports whether err represents context cancellation or a +// context deadline. +func IsInterrupted(err error) bool { + return errors.Is(err, context.Canceled) || + errors.Is(err, context.DeadlineExceeded) +} + // Retryable classifies err as an operational failure that may succeed on a // later attempt. Context cancellation and deadline errors remain interruption // errors rather than retryable failures. func Retryable(err error) error { if err == nil || - errors.Is(err, context.Canceled) || - errors.Is(err, context.DeadlineExceeded) || + IsInterrupted(err) || errors.Is(err, ErrRetryable) || errors.Is(err, ErrTerminal) { return err diff --git a/rest-api/flow/internal/eventrule/executor/errors_test.go b/rest-api/flow/internal/eventrule/executor/errors_test.go index 37491e90a1..649e845d87 100644 --- a/rest-api/flow/internal/eventrule/executor/errors_test.go +++ b/rest-api/flow/internal/eventrule/executor/errors_test.go @@ -6,6 +6,7 @@ package executor import ( "context" "errors" + "fmt" "testing" "github.com/stretchr/testify/require" @@ -53,3 +54,23 @@ func TestErrorClassification(t *testing.T) { }) } } + +func TestIsInterrupted(t *testing.T) { + tests := map[string]struct { + err error + want bool + }{ + "nil": {}, + "canceled": {err: context.Canceled, want: true}, + "wrapped canceled": {err: fmt.Errorf("execute: %w", context.Canceled), want: true}, + "deadline": {err: context.DeadlineExceeded, want: true}, + "wrapped deadline": {err: fmt.Errorf("execute: %w", context.DeadlineExceeded), want: true}, + "unrelated failure": {err: errors.New("downstream unavailable")}, + } + + for name, test := range tests { + t.Run(name, func(t *testing.T) { + require.Equal(t, test.want, IsInterrupted(test.err)) + }) + } +} diff --git a/rest-api/flow/internal/eventrule/executor/executor.go b/rest-api/flow/internal/eventrule/executor/executor.go index 4f68cd3973..d5229bfe5e 100644 --- a/rest-api/flow/internal/eventrule/executor/executor.go +++ b/rest-api/flow/internal/eventrule/executor/executor.go @@ -9,28 +9,36 @@ import ( "context" "fmt" + "github.com/google/uuid" + "github.com/NVIDIA/infra-controller/rest-api/flow/internal/eventrule" ) -// ExecutionRequest contains the persisted execution plan needed for one +// ExecutionRequest contains the immutable identity and plan needed for one // dispatch attempt. type ExecutionRequest struct { - Execution *eventrule.Execution + ExecutionID uuid.UUID + Plan eventrule.ExecutionPlan } // Validate checks the execution input. func (r ExecutionRequest) Validate() error { - if err := r.Execution.Validate(); err != nil { - return fmt.Errorf("execution: %w", err) + if r.ExecutionID == uuid.Nil { + return fmt.Errorf("execution id is required") + } + + if err := eventrule.ValidateExecutionPlan(r.Plan); err != nil { + return fmt.Errorf("execution plan: %w", err) } + return nil } // Executor performs the side effects for one action type. type Executor interface { - // Execute may be called multiple times for the same Execution.ID after a + // Execute may be called multiple times for the same ExecutionID after a // deferred result. Implementations that produce external side effects must - // use Execution.ID, or stable keys derived from it for partitioned work, to + // use ExecutionID, or stable keys derived from it for partitioned work, to // make repeated calls idempotent and reconcile an ambiguous prior result // before submitting again. Attempt numbers must not be used as downstream // idempotency identities. A nil error means the action completed successfully diff --git a/rest-api/flow/internal/eventrule/executor/executor_test.go b/rest-api/flow/internal/eventrule/executor/executor_test.go index 9f6a5403fa..bcf3ab737e 100644 --- a/rest-api/flow/internal/eventrule/executor/executor_test.go +++ b/rest-api/flow/internal/eventrule/executor/executor_test.go @@ -20,25 +20,20 @@ func TestExecutionRequestValidate(t *testing.T) { wantErr string }{ "valid": {}, - "nil execution": { - mutate: func(request *ExecutionRequest) { request.Execution = nil }, - wantErr: "execution: execution is nil", - }, "invalid execution id": { - mutate: func(request *ExecutionRequest) { request.Execution.ID = uuid.Nil }, - wantErr: "execution: execution id is required", + mutate: func(request *ExecutionRequest) { request.ExecutionID = uuid.Nil }, + wantErr: "execution id is required", }, "missing plan": { - mutate: func(request *ExecutionRequest) { request.Execution.Plan = nil }, - wantErr: "execution plan is required", + mutate: func(request *ExecutionRequest) { request.Plan = nil }, + wantErr: "execution plan: execution plan is required", }, } for name, test := range tests { t.Run(name, func(t *testing.T) { request := valid - execution := valid.Execution.Clone() - request.Execution = &execution + request.Plan = eventrule.CloneExecutionPlan(valid.Plan) if test.mutate != nil { test.mutate(&request) } @@ -61,5 +56,8 @@ func newValidExecutionRequest(t *testing.T) ExecutionRequest { time.Date(2026, 8, 10, 12, 0, 0, 0, time.UTC), ) require.NoError(t, err) - return ExecutionRequest{Execution: execution} + return ExecutionRequest{ + ExecutionID: execution.ID, + Plan: execution.Plan, + } } diff --git a/rest-api/flow/internal/eventrule/executor/noop.go b/rest-api/flow/internal/eventrule/executor/noop.go index 16de170b67..20fc5bde0e 100644 --- a/rest-api/flow/internal/eventrule/executor/noop.go +++ b/rest-api/flow/internal/eventrule/executor/noop.go @@ -15,11 +15,11 @@ type NoopExecutor struct{} // Execute completes a typed noop request. func (NoopExecutor) Execute(_ context.Context, request ExecutionRequest) error { - plan, ok := request.Execution.Plan.(*eventrule.NoopPlan) + plan, ok := request.Plan.(*eventrule.NoopPlan) if !ok || plan == nil { return terminalError(fmt.Errorf( "noop executor received plan %T", - request.Execution.Plan, + request.Plan, )) } diff --git a/rest-api/flow/internal/eventrule/executor/registry_test.go b/rest-api/flow/internal/eventrule/executor/registry_test.go index 08266a5d97..b5374a355a 100644 --- a/rest-api/flow/internal/eventrule/executor/registry_test.go +++ b/rest-api/flow/internal/eventrule/executor/registry_test.go @@ -129,7 +129,7 @@ func TestNoopExecutor_Execute(t *testing.T) { "completed": {}, "rejects wrong plan": { mutate: func(r *ExecutionRequest) { - r.Execution.Plan = &eventrule.SendAlertPlan{Severity: eventrule.SeverityWarning} + r.Plan = &eventrule.SendAlertPlan{Severity: eventrule.SeverityWarning} }, wantErr: "received plan", }, @@ -138,9 +138,7 @@ func TestNoopExecutor_Execute(t *testing.T) { for name, test := range tests { t.Run(name, func(t *testing.T) { input := request - execution := *input.Execution - execution.Plan = eventrule.CloneExecutionPlan(input.Execution.Plan) - input.Execution = &execution + input.Plan = eventrule.CloneExecutionPlan(request.Plan) if test.mutate != nil { test.mutate(&input) diff --git a/rest-api/flow/internal/eventrule/executor/task.go b/rest-api/flow/internal/eventrule/executor/task.go index e324b4b919..f8cb268c8e 100644 --- a/rest-api/flow/internal/eventrule/executor/task.go +++ b/rest-api/flow/internal/eventrule/executor/task.go @@ -34,16 +34,16 @@ func (e *TaskExecutor) Execute(ctx context.Context, request ExecutionRequest) er return terminalError(fmt.Errorf("task manager and execution task store are required")) } - plan, ok := request.Execution.Plan.(*eventrule.SubmitTaskPlan) + plan, ok := request.Plan.(*eventrule.SubmitTaskPlan) if !ok || plan == nil { return terminalError(fmt.Errorf( "task executor received plan %T", - request.Execution.Plan, + request.Plan, )) } for _, target := range plan.Targets { - if err := e.submitTarget(ctx, *request.Execution, plan, target); err != nil { + if err := e.submitTarget(ctx, request.ExecutionID, plan, target); err != nil { return err } } @@ -53,24 +53,24 @@ func (e *TaskExecutor) Execute(ctx context.Context, request ExecutionRequest) er func (e *TaskExecutor) submitTarget( ctx context.Context, - execution eventrule.Execution, + executionID uuid.UUID, plan *eventrule.SubmitTaskPlan, target operation.RackExecutionTarget, ) error { - associated, err := e.associations.GetExecutionTask(ctx, execution.ID, target.RackID) + associated, err := e.associations.GetExecutionTask(ctx, executionID, target.RackID) if err != nil { return retryableError("load execution task association", err) } if associated != nil { - if err := validateTaskAssociation(associated, execution.ID, target.RackID); err != nil { + if err := validateTaskAssociation(associated, executionID, target.RackID); err != nil { return terminalError(err) } return nil } - request, err := operationRequest(execution.ID, plan, target) + request, err := operationRequest(executionID, plan, target) if err != nil { return terminalError(err) } @@ -85,7 +85,7 @@ func (e *TaskExecutor) submitTarget( } requested := eventrule.ExecutionTask{ - ExecutionID: execution.ID, + ExecutionID: executionID, RackID: target.RackID, TaskID: taskIDs[0], } @@ -99,7 +99,7 @@ func (e *TaskExecutor) submitTarget( return terminalError(errors.New("execution task store returned a nil association")) } - if err := validateTaskAssociation(associated, execution.ID, target.RackID); err != nil { + if err := validateTaskAssociation(associated, executionID, target.RackID); err != nil { return terminalError(err) } diff --git a/rest-api/flow/internal/eventrule/executor/task_test.go b/rest-api/flow/internal/eventrule/executor/task_test.go index e28e18b0de..e74a30b48f 100644 --- a/rest-api/flow/internal/eventrule/executor/task_test.go +++ b/rest-api/flow/internal/eventrule/executor/task_test.go @@ -34,7 +34,7 @@ func TestTaskExecutorExecute(t *testing.T) { submitted := manager.requests[0] require.Equal(t, rackID, submitted.RequiredRackID) - require.Equal(t, taskIdempotencyKey(request.Execution.ID, rackID), submitted.IdempotencyKey) + require.Equal(t, taskIdempotencyKey(request.ExecutionID, rackID), submitted.IdempotencyKey) require.Equal(t, operation.TargetSpec{ Components: []operation.ComponentTarget{{UUID: componentID}}, }, submitted.TargetSpec) @@ -69,7 +69,7 @@ func TestTaskExecutorClassifiesFailures(t *testing.T) { manager: &recordingTaskManager{}, associations: newExecutionTaskStore(), mutate: func(request *ExecutionRequest) { - request.Execution.Plan = &eventrule.NoopPlan{} + request.Plan = &eventrule.NoopPlan{} }, wantErr: "received plan", classification: ErrTerminal, @@ -79,8 +79,7 @@ func TestTaskExecutorClassifiesFailures(t *testing.T) { for name, test := range tests { t.Run(name, func(t *testing.T) { input := request - execution := request.Execution.Clone() - input.Execution = &execution + input.Plan = eventrule.CloneExecutionPlan(request.Plan) if test.mutate != nil { test.mutate(&input) @@ -135,7 +134,10 @@ func submitTaskExecutionRequest( ) require.NoError(t, err) - return ExecutionRequest{Execution: execution} + return ExecutionRequest{ + ExecutionID: execution.ID, + Plan: execution.Plan, + } } var testTime = mustTestTime() diff --git a/rest-api/flow/internal/eventrule/manager/config.go b/rest-api/flow/internal/eventrule/manager/config.go index e92b50589d..1e70c1af84 100644 --- a/rest-api/flow/internal/eventrule/manager/config.go +++ b/rest-api/flow/internal/eventrule/manager/config.go @@ -6,14 +6,39 @@ package manager import ( "fmt" + "github.com/NVIDIA/infra-controller/rest-api/flow/internal/eventrule" eventexecutor "github.com/NVIDIA/infra-controller/rest-api/flow/internal/eventrule/executor" + eventscheduler "github.com/NVIDIA/infra-controller/rest-api/flow/internal/eventrule/scheduler" inventoryresolver "github.com/NVIDIA/infra-controller/rest-api/flow/internal/inventory/resolver" ) +// SchedulerConfig identifies the manager-owned scheduler and controls its +// runtime mechanics and execution policy. +type SchedulerConfig struct { + InstanceID string + Runtime eventscheduler.RuntimeConfig + Policy eventscheduler.PolicyConfig +} + +// Validate checks the scheduler identity, runtime mechanics, and policy. +func (c SchedulerConfig) Validate() error { + if err := eventrule.ValidateExecutionClaimOwner(c.InstanceID); err != nil { + return fmt.Errorf("scheduler instance ID: %w", err) + } + + if err := c.Runtime.Validate(); err != nil { + return err + } + + return c.Policy.Validate() +} + // Config contains the external capabilities used to assemble an event-rule -// manager. Internal registries and the processor are constructed by New. +// manager. Internal registries, the processor, and the scheduler are +// constructed by New. type Config struct { Store StoreConfig + Scheduler SchedulerConfig Inventory inventoryresolver.InventoryReader TaskManager eventexecutor.TaskManager AlertSender eventexecutor.AlertSender @@ -25,6 +50,10 @@ func (c Config) Validate() error { return err } + if err := c.Scheduler.Validate(); err != nil { + return err + } + if c.Inventory == nil { return fmt.Errorf("inventory reader is required") } diff --git a/rest-api/flow/internal/eventrule/manager/config_test.go b/rest-api/flow/internal/eventrule/manager/config_test.go index 2b5ea3abd2..f1063054a3 100644 --- a/rest-api/flow/internal/eventrule/manager/config_test.go +++ b/rest-api/flow/internal/eventrule/manager/config_test.go @@ -24,6 +24,24 @@ func TestConfig_Validate(t *testing.T) { }, wantErr: "unsupported event-rule store backend", }, + "missing scheduler instance id": { + mutate: func(config *Config) { + config.Scheduler.InstanceID = "" + }, + wantErr: "scheduler instance ID: execution claim owner is empty", + }, + "invalid scheduler runtime": { + mutate: func(config *Config) { + config.Scheduler.Runtime.PollInterval = 0 + }, + wantErr: "scheduler poll interval must be positive", + }, + "invalid scheduler policy": { + mutate: func(config *Config) { + config.Scheduler.Policy.MaxAttempts = 0 + }, + wantErr: "retry max attempts must be positive", + }, "missing inventory reader": { mutate: func(config *Config) { config.Inventory = nil diff --git a/rest-api/flow/internal/eventrule/manager/manager.go b/rest-api/flow/internal/eventrule/manager/manager.go index 63dd9c16a5..2e9cd0c781 100644 --- a/rest-api/flow/internal/eventrule/manager/manager.go +++ b/rest-api/flow/internal/eventrule/manager/manager.go @@ -14,6 +14,7 @@ import ( eventexecutor "github.com/NVIDIA/infra-controller/rest-api/flow/internal/eventrule/executor" "github.com/NVIDIA/infra-controller/rest-api/flow/internal/eventrule/leakage" eventprocessor "github.com/NVIDIA/infra-controller/rest-api/flow/internal/eventrule/processor" + eventscheduler "github.com/NVIDIA/infra-controller/rest-api/flow/internal/eventrule/scheduler" "github.com/NVIDIA/infra-controller/rest-api/flow/internal/eventrule/target" inventoryresolver "github.com/NVIDIA/infra-controller/rest-api/flow/internal/inventory/resolver" "github.com/google/uuid" @@ -27,6 +28,7 @@ type Manager struct { targets *target.Registry executors *eventexecutor.Registry processor *eventprocessor.Processor + scheduler *eventscheduler.Scheduler } // New constructs a fully assembled event-rule manager. @@ -57,7 +59,26 @@ func New(config Config) (*Manager, error) { return nil, err } - processor, err := newProcessor(config, store, builtIns, targets, executors) + executionScheduler, err := eventscheduler.New(eventscheduler.Config{ + InstanceID: config.Scheduler.InstanceID, + Dependencies: eventscheduler.Dependencies{ + Store: store, + Executors: executors, + }, + Runtime: config.Scheduler.Runtime, + Policy: config.Scheduler.Policy, + }) + if err != nil { + return nil, err + } + + processor, err := newProcessor( + config, + store, + builtIns, + targets, + executionScheduler, + ) if err != nil { return nil, err } @@ -68,6 +89,7 @@ func New(config Config) (*Manager, error) { targets: targets, executors: executors, processor: processor, + scheduler: executionScheduler, }, nil } @@ -98,6 +120,7 @@ func newBuiltInRulesRegistry( byID: make(map[uuid.UUID]eventrule.Rule, len(rules)), byEventType: make(map[eventrule.Type]uuid.UUID, len(rules)), } + for _, rule := range rules { if err := builtIns.addRule(rule); err != nil { return nil, err @@ -122,15 +145,14 @@ func newProcessor( store eventRuleStore, builtIns *builtInRegistry, targets *target.Registry, - executors *eventexecutor.Registry, + notifier eventprocessor.ExecutionNotifier, ) (*eventprocessor.Processor, error) { cfg := eventprocessor.Config{ - Inventory: config.Inventory, - Rules: &ruleResolver{builtIns: builtIns, store: store}, - Events: store, - Executions: store, - Targets: targets, - Executors: executors, + Inventory: config.Inventory, + Rules: &ruleResolver{builtIns: builtIns, store: store}, + Store: store, + Targets: targets, + Notifier: notifier, } return eventprocessor.New(cfg) @@ -149,6 +171,17 @@ func newExecutorRegistry( return eventexecutor.New(cfg) } +// Start launches the internally assembled execution scheduler in the +// background. +func (m *Manager) Start(ctx context.Context) error { + return m.scheduler.Start(ctx) +} + +// Stop stops the execution scheduler and waits for its workers to exit. +func (m *Manager) Stop() error { + return m.scheduler.Stop() +} + // Process delegates one collected event to the internally assembled processor. func (m *Manager) Process(ctx context.Context, envelope eventrule.Envelope) error { return m.processor.Process(ctx, envelope) @@ -206,6 +239,7 @@ func (m *Manager) Create( EventType: input.EventType, Policy: input.Policy.Clone(), } + if err := m.validateRuntimeRule(&rule); err != nil { return nil, err } @@ -259,6 +293,7 @@ func (m *Manager) ReplaceActions( candidate := rule.Clone() candidate.Actions = eventrule.CloneActions(actions) + if err := m.validateRuntimeRule(&candidate); err != nil { return err } diff --git a/rest-api/flow/internal/eventrule/manager/manager_test.go b/rest-api/flow/internal/eventrule/manager/manager_test.go index 8282cf69f4..61650f844d 100644 --- a/rest-api/flow/internal/eventrule/manager/manager_test.go +++ b/rest-api/flow/internal/eventrule/manager/manager_test.go @@ -10,6 +10,7 @@ import ( "github.com/NVIDIA/infra-controller/rest-api/flow/internal/eventrule" eventexecutor "github.com/NVIDIA/infra-controller/rest-api/flow/internal/eventrule/executor" "github.com/NVIDIA/infra-controller/rest-api/flow/internal/eventrule/leakage" + eventscheduler "github.com/NVIDIA/infra-controller/rest-api/flow/internal/eventrule/scheduler" identifier "github.com/NVIDIA/infra-controller/rest-api/flow/pkg/common/Identifier" "github.com/NVIDIA/infra-controller/rest-api/flow/pkg/inventoryobjects/component" "github.com/NVIDIA/infra-controller/rest-api/flow/pkg/inventoryobjects/rack" @@ -286,7 +287,12 @@ func TestManagerValidatesConfiguredActionExecutors(t *testing.T) { func testManagerConfig() Config { return Config{ - Store: StoreConfig{Backend: StoreBackendMemory}, + Store: StoreConfig{Backend: StoreBackendMemory}, + Scheduler: SchedulerConfig{ + InstanceID: "event-rule-manager-test", + Runtime: eventscheduler.DefaultRuntimeConfig(), + Policy: eventscheduler.DefaultPolicyConfig(), + }, Inventory: testInventory{}, TaskManager: configTaskManager{}, } diff --git a/rest-api/flow/internal/eventrule/manager/processing_integration_test.go b/rest-api/flow/internal/eventrule/manager/processing_integration_test.go index d285327af8..f7ddb1a355 100644 --- a/rest-api/flow/internal/eventrule/manager/processing_integration_test.go +++ b/rest-api/flow/internal/eventrule/manager/processing_integration_test.go @@ -6,6 +6,7 @@ package manager import ( "context" "testing" + "time" "github.com/NVIDIA/infra-controller/rest-api/flow/internal/eventrule" "github.com/NVIDIA/infra-controller/rest-api/flow/internal/eventrule/leakage" @@ -19,6 +20,99 @@ import ( "github.com/stretchr/testify/require" ) +func TestManager_Start(t *testing.T) { + ctx := context.Background() + eventType := eventrule.Type("test.event") + rackID := uuid.New() + config := testManagerConfig() + config.Scheduler.Runtime.PollInterval = time.Hour + config.Inventory = &processingInventory{ + rack: rack.New(deviceinfo.DeviceInfo{ID: rackID}, location.Location{}), + } + + manager, err := New(config) + require.NoError(t, err) + + store, ok := manager.store.(*memory.Store) + require.True(t, ok) + + rule, err := manager.Create(ctx, testRuleCreate(eventType, "scheduled")) + require.NoError(t, err) + _, err = manager.Bind(ctx, rule.ID, eventrule.Scope{ + Type: eventrule.ScopeTypeRack, + ID: rackID, + }) + require.NoError(t, err) + require.NoError(t, manager.SetEnabled(ctx, rule.ID, true)) + + runCtx, cancel := context.WithCancel(ctx) + defer cancel() + require.NoError(t, manager.Start(runCtx)) + + require.NoError(t, manager.Process(ctx, eventrule.Envelope{ + Key: eventrule.EventKey{SourceName: "test", SourceKey: "scheduled-event"}, + Type: eventType, + Resource: eventrule.Resource{Kind: eventrule.ResourceKindRack, ID: rackID}, + })) + + require.Eventually(t, func() bool { + executions, err := store.Executions() + return err == nil && + len(executions) == 1 && + executions[0].Status == eventrule.ExecutionStatusCompleted + }, time.Second, time.Millisecond) + + require.EqualError( + t, + manager.Start(context.Background()), + "scheduler can only be started once", + ) + require.NoError(t, manager.Stop()) +} + +func TestManager_Stop(t *testing.T) { + tests := map[string]struct { + prepare func(*testing.T, *Manager) + wantErr string + }{ + "before start": { + wantErr: "scheduler cannot be stopped before it is started", + }, + "after start": { + prepare: func(t *testing.T, manager *Manager) { + require.NoError(t, manager.Start(context.Background())) + }, + }, + "second stop": { + prepare: func(t *testing.T, manager *Manager) { + require.NoError(t, manager.Start(context.Background())) + require.NoError(t, manager.Stop()) + }, + wantErr: "scheduler can only be stopped once", + }, + } + + for name, test := range tests { + t.Run(name, func(t *testing.T) { + manager, err := New(testManagerConfig()) + require.NoError(t, err) + + if test.prepare != nil { + test.prepare(t, manager) + } + + err = manager.Stop() + if test.wantErr != "" { + require.EqualError(t, err, test.wantErr) + + return + } + + require.NoError(t, err) + }) + } +} + func TestManager_ProcessIntegration(t *testing.T) { ctx := context.Background() eventType := leakage.TypeHardwareLeakDetected diff --git a/rest-api/flow/internal/eventrule/manager/store.go b/rest-api/flow/internal/eventrule/manager/store.go index 270f396e33..b934227109 100644 --- a/rest-api/flow/internal/eventrule/manager/store.go +++ b/rest-api/flow/internal/eventrule/manager/store.go @@ -38,7 +38,7 @@ func (c StoreConfig) Validate() error { type eventRuleStore interface { eventrule.RuleStore eventrule.BindingStore - eventrule.EventStore + eventrule.EventPlanStore eventrule.ExecutionStore eventrule.ExecutionTaskStore } diff --git a/rest-api/flow/internal/eventrule/processor/config.go b/rest-api/flow/internal/eventrule/processor/config.go index f708857066..8c00052cee 100644 --- a/rest-api/flow/internal/eventrule/processor/config.go +++ b/rest-api/flow/internal/eventrule/processor/config.go @@ -7,24 +7,23 @@ import ( "fmt" "github.com/NVIDIA/infra-controller/rest-api/flow/internal/eventrule" - "github.com/NVIDIA/infra-controller/rest-api/flow/internal/eventrule/executor" "github.com/NVIDIA/infra-controller/rest-api/flow/internal/eventrule/target" inventoryresolver "github.com/NVIDIA/infra-controller/rest-api/flow/internal/inventory/resolver" ) -// ExecutorRegistry resolves the executor for an action type. -type ExecutorRegistry interface { - Executor(eventrule.ActionType) (executor.Executor, error) +// ExecutionNotifier hints that newly committed pending work is available. Notify +// must not block; periodic scheduler polling remains the reliability fallback. +type ExecutionNotifier interface { + Notify() } // Config contains the dependencies for a Processor. type Config struct { - Inventory inventoryresolver.InventoryReader - Rules RuleResolver - Events eventrule.EventStore - Executions eventrule.ExecutionStore - Targets *target.Registry - Executors ExecutorRegistry + Inventory inventoryresolver.InventoryReader + Rules RuleResolver + Store eventrule.EventPlanStore + Targets *target.Registry + Notifier ExecutionNotifier } // Validate checks that all required processor dependencies are present. @@ -35,17 +34,12 @@ func (c Config) Validate() error { if c.Rules == nil { return fmt.Errorf("rule resolver is required") } - if c.Executions == nil { - return fmt.Errorf("execution store is required") - } - if c.Events == nil { - return fmt.Errorf("event store is required") + if c.Store == nil { + return fmt.Errorf("event plan store is required") } if c.Targets == nil { return fmt.Errorf("target resolver registry is required") } - if c.Executors == nil { - return fmt.Errorf("executor registry is required") - } + return nil } diff --git a/rest-api/flow/internal/eventrule/processor/config_test.go b/rest-api/flow/internal/eventrule/processor/config_test.go index 19e5389dc9..1816c472b3 100644 --- a/rest-api/flow/internal/eventrule/processor/config_test.go +++ b/rest-api/flow/internal/eventrule/processor/config_test.go @@ -23,36 +23,32 @@ func TestConfigValidate(t *testing.T) { mutate: func(config *Config) { config.Rules = nil }, wantErr: "rule resolver is required", }, - "missing action execution store": { - mutate: func(config *Config) { config.Executions = nil }, - wantErr: "execution store is required", - }, - "missing event store": { - mutate: func(config *Config) { config.Events = nil }, - wantErr: "event store is required", + "missing event plan store": { + mutate: func(config *Config) { config.Store = nil }, + wantErr: "event plan store is required", }, "missing target resolver": { mutate: func(config *Config) { config.Targets = nil }, wantErr: "target resolver registry is required", }, - "missing executor registry": { - mutate: func(config *Config) { config.Executors = nil }, - wantErr: "executor registry is required", - }, } for name, test := range tests { t.Run(name, func(t *testing.T) { config := validProcessorConfig(t) + if test.mutate != nil { test.mutate(&config) } err := config.Validate() + if test.wantErr != "" { require.ErrorContains(t, err, test.wantErr) + return } + require.NoError(t, err) }) } @@ -61,12 +57,14 @@ func TestConfigValidate(t *testing.T) { func TestNew(t *testing.T) { t.Run("rejects invalid configuration", func(t *testing.T) { processor, err := New(Config{}) + require.Error(t, err) require.Nil(t, processor) }) t.Run("constructs processor", func(t *testing.T) { processor, err := New(validProcessorConfig(t)) + require.NoError(t, err) require.NotNil(t, processor) }) diff --git a/rest-api/flow/internal/eventrule/processor/execution.go b/rest-api/flow/internal/eventrule/processor/execution.go index 805d8a22b2..3cbc68e1db 100644 --- a/rest-api/flow/internal/eventrule/processor/execution.go +++ b/rest-api/flow/internal/eventrule/processor/execution.go @@ -6,27 +6,21 @@ package processor import ( "cmp" "context" - "errors" "fmt" "slices" - "time" "github.com/google/uuid" "github.com/NVIDIA/infra-controller/rest-api/flow/internal/eventrule" - "github.com/NVIDIA/infra-controller/rest-api/flow/internal/eventrule/executor" "github.com/NVIDIA/infra-controller/rest-api/flow/internal/eventrule/target" "github.com/NVIDIA/infra-controller/rest-api/flow/internal/operation" "github.com/NVIDIA/infra-controller/rest-api/flow/pkg/common/devicetypes" ) -const initialRetryDelay = 5 * time.Second -const executionPersistTimeout = 5 * time.Second - func (p *Processor) plan( ctx context.Context, prepared *preparedEvent, -) ([]eventrule.Execution, error) { +) (*eventrule.Event, error) { event := &prepared.Event planned := make([]eventrule.PlannedExecution, len(event.EffectivePolicy.Actions)) for i, action := range event.EffectivePolicy.Actions { @@ -41,28 +35,12 @@ func (p *Processor) plan( } } - executions, err := p.executions.CommitEventPlan(ctx, event.ID, planned) + committed, err := p.store.CommitEventPlan(ctx, *event, planned) if err != nil { return nil, fmt.Errorf("persist event plan: %w", err) } - return executions, nil -} - -func (p *Processor) dispatch( - ctx context.Context, - execution *eventrule.Execution, -) error { - actionExecutor, err := p.executors.Executor(execution.Plan.Type()) - if err == nil { - err = actionExecutor.Execute(ctx, executor.ExecutionRequest{Execution: execution}) - } - result := executionResult(ctx, err) - - persistCtx, cancel := context.WithTimeout(context.WithoutCancel(ctx), executionPersistTimeout) - defer cancel() - - return p.executions.TransitionExecution(persistCtx, execution.ID, result) + return committed, nil } func (p *Processor) planAction( @@ -100,22 +78,27 @@ func (p *Processor) planSubmitTask( } return nil, err } + targets, err := p.materializeTaskTargets(ctx, resolved) if err != nil { return nil, err } + info, err := spec.Operation.Marshal() if err != nil { return nil, fmt.Errorf("marshal task operation: %w", err) } + description := spec.Description if description == "" { description = spec.Operation.Description() } + conflictStrategy := operation.ConflictStrategyReject if spec.ConflictStrategy == eventrule.ConflictStrategyQueue { conflictStrategy = operation.ConflictStrategyQueue } + return &eventrule.SubmitTaskPlan{ Operation: operation.Wrapper{ Type: spec.Operation.Type(), @@ -136,15 +119,18 @@ func (p *Processor) resolveTargetRequest( if err != nil { return nil, err } + resolved, err := resolver.Resolve(ctx, request) if err != nil { return nil, err } + for i, candidate := range resolved { if err := candidate.Validate(); err != nil { return nil, fmt.Errorf("%w: resolver target %d: %v", target.ErrUnresolvable, i, err) } } + return resolved, nil } @@ -158,15 +144,18 @@ func (p *Processor) materializeTaskTargets( if err != nil { return nil, err } + if len(components) == 0 { continue } + if existing := byRack[candidate.RackID]; len(existing) > 0 { components, err = existing.Merge(components) if err != nil { return nil, fmt.Errorf("merge rack %s components: %w", candidate.RackID, err) } } + byRack[candidate.RackID] = components } @@ -177,9 +166,11 @@ func (p *Processor) materializeTaskTargets( ComponentsByType: components.Clone(), }) } + slices.SortFunc(targets, func(a, b operation.RackExecutionTarget) int { return cmp.Compare(a.RackID.String(), b.RackID.String()) }) + return targets, nil } @@ -193,6 +184,7 @@ func (p *Processor) componentsForTarget( if err != nil { return nil, classifyInventoryError(err) } + if component.RackID != candidate.RackID { return nil, terminalError(fmt.Errorf( "component %s belongs to rack %s, resolver selected rack %s", @@ -201,12 +193,14 @@ func (p *Processor) componentsForTarget( candidate.RackID, )) } + return operation.ComponentsByType{component.Type: []uuid.UUID{component.Info.ID}}, nil case eventrule.ResourceKindRack: rack, err := p.inventory.RackByID(ctx, candidate.RackID, true) if err != nil { return nil, classifyInventoryError(err) } + components := make(operation.ComponentsByType) for _, component := range rack.Components { if component.Type == devicetypes.ComponentTypeUnknown { @@ -216,37 +210,16 @@ func (p *Processor) componentsForTarget( component.Info.ID, )) } + components[component.Type] = append(components[component.Type], component.Info.ID) } + if len(components) == 0 { return nil, nil } + return components.Normalize() default: return nil, terminalError(fmt.Errorf("unsupported target kind %q", candidate.Kind)) } } - -func executionResult(ctx context.Context, err error) eventrule.ExecutionResult { - if err == nil { - return eventrule.CompletedExecutionResult() - } - if ctx.Err() != nil || errors.Is(err, context.Canceled) || errors.Is(err, context.DeadlineExceeded) { - return eventrule.DeferredExecutionResult( - eventrule.ExecutionReasonAttemptInterrupted, - fmt.Sprintf("executor execution interrupted: %v", err), - initialRetryDelay, - ) - } - if errors.Is(err, executor.ErrRetryable) { - return eventrule.DeferredExecutionResult( - eventrule.ExecutionReasonAttemptFailed, - err.Error(), - initialRetryDelay, - ) - } - if errors.Is(err, executor.ErrTerminal) { - return eventrule.FailedExecutionResult(err.Error()) - } - return eventrule.FailedExecutionResult(fmt.Sprintf("executor execution failed: %v", err)) -} diff --git a/rest-api/flow/internal/eventrule/processor/preparation.go b/rest-api/flow/internal/eventrule/processor/preparation.go index dc181658ef..af8bb485ba 100644 --- a/rest-api/flow/internal/eventrule/processor/preparation.go +++ b/rest-api/flow/internal/eventrule/processor/preparation.go @@ -17,16 +17,15 @@ type RuleResolver interface { GetEffective(context.Context, eventrule.Type, uuid.UUID) (*eventrule.Rule, error) } -// preparedEvent contains the durable event definition and the transient -// resolved resource needed by creator-owned planning. +// preparedEvent contains the event definition and transient resolved resource +// needed to construct its complete durable plan. type preparedEvent struct { Event eventrule.Event Resource eventrule.ResolvedResource } -// prepare admits a creator-owned event for planning. It deduplicates before -// performing preparation work, enriches the resource, resolves and evaluates -// the effective rule, and persists the event. A duplicate or absent rule is an +// prepare deduplicates before expensive work, enriches the resource, and +// resolves and evaluates the effective rule. A duplicate or absent rule is an // accepted no-op represented by (nil, nil). func (p *Processor) prepare( ctx context.Context, @@ -38,11 +37,11 @@ func (p *Processor) prepare( // ObserveEvent is the duplicate fast path before resource enrichment and // rule resolution. It records the duplicate observation while avoiding the - // preparation cost. Recovery of unplanned events is follow-on work. - observed, err := p.events.ObserveEvent(ctx, envelope.Key) + // preparation cost. + observed, err := p.store.ObserveEvent(ctx, envelope.Key) if err != nil || observed != nil { - // Propagate lookup errors; a successfully observed duplicate stops here - // because its creator retains responsibility for planning and dispatch. + // Propagate lookup errors; a successfully observed duplicate already has a + // complete durable plan and stops here. return nil, err } @@ -67,9 +66,9 @@ func (p *Processor) prepare( } } - // Persist an empty effective policy to record that the rule was evaluated - // and no action applied. This keeps duplicate handling stable and lets the - // planning checkpoint distinguish an intentional no-op from interrupted work. + // Persisting an empty effective policy records that the rule was evaluated + // and no action applied. The atomic commit creates the event with no + // executions. definition := eventrule.Event{ Key: envelope.Key, Type: envelope.Type, @@ -86,12 +85,5 @@ func (p *Processor) prepare( ), } - created, err := p.events.CreateEvent(ctx, definition) - if err != nil || created == nil { - // A nil event means another processor created it between ObserveEvent and - // CreateEvent. The store recorded this delivery as a duplicate, so stop here. - return nil, err - } - - return &preparedEvent{Event: *created, Resource: resource}, nil + return &preparedEvent{Event: definition, Resource: resource}, nil } diff --git a/rest-api/flow/internal/eventrule/processor/preparation_test.go b/rest-api/flow/internal/eventrule/processor/preparation_test.go index b45ee600b2..5fb9cbf914 100644 --- a/rest-api/flow/internal/eventrule/processor/preparation_test.go +++ b/rest-api/flow/internal/eventrule/processor/preparation_test.go @@ -78,23 +78,31 @@ func TestPrepare(t *testing.T) { resolvedRackID uuid.UUID, ) (*eventrule.Rule, error) { resolverCalled = true + require.Equal(t, eventrule.Type("test.event"), eventType) require.Equal(t, rackID, resolvedRackID) + return test.rule, test.ruleErr }) + processor := newRackProcessor(t, rackID, resolver) result, err := processor.prepare( context.Background(), envelope, ) + require.Equal(t, test.wantResolved, resolverCalled) + if test.wantErr == nil { require.NoError(t, err) + if test.rule == nil { require.Nil(t, result) + return } + require.NotNil(t, result) require.Equal(t, rackID, result.Resource.ID) require.Equal(t, rackID, result.Resource.RackID) @@ -102,14 +110,15 @@ func TestPrepare(t *testing.T) { require.Equal(t, rackID, result.Event.Resource.ID) require.Equal(t, eventrule.ResourceKindRack, result.Event.Resource.Kind) require.Equal(t, test.rule.ID, result.Event.AppliedRuleID) - require.NotEqual(t, uuid.Nil, result.Event.ID) - require.Equal(t, 1, result.Event.Observations) - require.False(t, result.Event.CreatedAt.IsZero()) + require.Equal(t, uuid.Nil, result.Event.ID) + require.Zero(t, result.Event.Observations) + require.True(t, result.Event.CreatedAt.IsZero()) return } require.Nil(t, result) require.ErrorIs(t, err, test.wantErr) + if test.wantMessage != "" { require.ErrorContains(t, err, test.wantMessage) } @@ -128,6 +137,7 @@ func newRackProcessor( rules RuleResolver, ) *Processor { t.Helper() + return newTestProcessor( t, &processorInventory{ diff --git a/rest-api/flow/internal/eventrule/processor/process_test.go b/rest-api/flow/internal/eventrule/processor/process_test.go index 0719d88348..0fe56ace7f 100644 --- a/rest-api/flow/internal/eventrule/processor/process_test.go +++ b/rest-api/flow/internal/eventrule/processor/process_test.go @@ -5,7 +5,6 @@ package processor import ( "context" - "fmt" "sync/atomic" "testing" "time" @@ -14,7 +13,6 @@ import ( "github.com/stretchr/testify/require" "github.com/NVIDIA/infra-controller/rest-api/flow/internal/eventrule" - eventexecutor "github.com/NVIDIA/infra-controller/rest-api/flow/internal/eventrule/executor" memorystore "github.com/NVIDIA/infra-controller/rest-api/flow/internal/eventrule/store/memory" eventtarget "github.com/NVIDIA/infra-controller/rest-api/flow/internal/eventrule/target" "github.com/NVIDIA/infra-controller/rest-api/flow/internal/task/operations" @@ -25,32 +23,27 @@ import ( "github.com/NVIDIA/infra-controller/rest-api/flow/pkg/inventoryobjects/rack" ) -func TestProcessorProcessPersistsEventBeforeDispatch(t *testing.T) { +func TestProcessorProcessPersistsAtomicPlan(t *testing.T) { rackID := uuid.New() store := memorystore.New() rule := processorRuntimeRule( noopAction("always"), conditionalNoopAction("critical", eventrule.SeverityCritical), ) - var received []eventrule.Execution + notifier := &countingNotifier{} processor := runtimeProcessor( t, processorInventoryWithRack(rackID), rule, store, defaultTargetResolver(rackID), - executorFunc(func(_ context.Context, request eventexecutor.ExecutionRequest) error { - received = append(received, request.Execution.Clone()) - events, err := store.Events() - require.NoError(t, err) - require.NotNil(t, events[0].PlannedAt) - return nil - }), + notifier, ) envelope := runtimeEnvelope(rackID) envelope.Severity = eventrule.SeverityInfo envelope.Payload = []byte(`{"secret":"must-not-be-persisted"}`) + require.NoError(t, processor.Process(context.Background(), envelope)) events, err := store.Events() @@ -60,26 +53,28 @@ func TestProcessorProcessPersistsEventBeforeDispatch(t *testing.T) { require.Equal(t, eventrule.ResourceIdentity{Kind: eventrule.ResourceKindRack, ID: rackID}, events[0].Resource) require.Equal(t, rule.ID, events[0].AppliedRuleID) require.Len(t, events[0].EffectivePolicy.Actions, 1) - require.NotNil(t, events[0].PlannedAt) + require.False(t, events[0].CreatedAt.IsZero()) require.NotContains(t, events[0].Summary, string(envelope.Payload)) executions, err := store.Executions() require.NoError(t, err) require.Len(t, executions, 1) + require.Equal(t, events[0].ID, executions[0].EventID) require.Equal(t, "always", executions[0].ActionName) require.IsType(t, &eventrule.NoopPlan{}, executions[0].Plan) - require.Equal(t, eventrule.ExecutionStatusCompleted, executions[0].Status) - require.Len(t, received, 1) + require.Equal(t, eventrule.ExecutionStatusPending, executions[0].Status) + require.Zero(t, executions[0].Attempts) + require.EqualValues(t, 1, notifier.calls.Load()) } func TestProcessorProcessDeduplicatesAtEventBoundary(t *testing.T) { - t.Run("planned duplicate records observation and stops", func(t *testing.T) { + t.Run("persisted duplicate records observation and stops", func(t *testing.T) { rackID := uuid.New() store := memorystore.New() - ruleCalls := 0 - executorCalls := 0 + var ruleCalls atomic.Int32 + notifier := &countingNotifier{} rules := ruleResolverFunc(func(context.Context, eventrule.Type, uuid.UUID) (*eventrule.Rule, error) { - ruleCalls++ + ruleCalls.Add(1) return processorRuntimeRule(noopAction("once")), nil }) processor := runtimeProcessorWithRules( @@ -88,149 +83,107 @@ func TestProcessorProcessDeduplicatesAtEventBoundary(t *testing.T) { rules, store, defaultTargetResolver(rackID), - executorFunc(func(context.Context, eventexecutor.ExecutionRequest) error { - executorCalls++ - return nil - }), + notifier, ) + envelope := runtimeEnvelope(rackID) + require.NoError(t, processor.Process(context.Background(), envelope)) require.NoError(t, processor.Process(context.Background(), envelope)) - require.Equal(t, 1, ruleCalls) - require.Equal(t, 1, executorCalls) + require.EqualValues(t, 1, ruleCalls.Load()) + require.EqualValues(t, 1, notifier.calls.Load()) + events, err := store.Events() require.NoError(t, err) require.Equal(t, 2, events[0].Observations) }) - t.Run("duplicate does not take over active creator planning", func(t *testing.T) { + t.Run("concurrent planner loser stops after atomic commit", func(t *testing.T) { rackID := uuid.New() store := memorystore.New() - enteredPlanning := make(chan struct{}) - releasePlanning := make(chan struct{}) - executions := &blockingExecutionStore{ - ExecutionStore: store, - entered: enteredPlanning, - release: releasePlanning, + enteredCommit := make(chan struct{}) + releaseCommit := make(chan struct{}) + planStore := &blockingEventPlanStore{ + Store: store, + entered: enteredCommit, + release: releaseCommit, } - executorCalls := 0 + notifier := &countingNotifier{} processor, err := New(Config{ Inventory: processorInventoryWithRack(rackID), Rules: ruleResolverFunc(func(context.Context, eventrule.Type, uuid.UUID) (*eventrule.Rule, error) { return processorRuntimeRule(noopAction("once")), nil }), - Events: store, - Executions: executions, - Targets: targetRegistry(t, defaultTargetResolver(rackID)), - Executors: executorRegistry(t, executorFunc(func(context.Context, eventexecutor.ExecutionRequest) error { - executorCalls++ - return nil - })), + Store: planStore, + Targets: targetRegistry(t, defaultTargetResolver(rackID)), + Notifier: notifier, }) require.NoError(t, err) envelope := runtimeEnvelope(rackID) - creatorResult := make(chan error, 1) + firstResult := make(chan error, 1) + go func() { - creatorResult <- processor.Process(context.Background(), envelope) + firstResult <- processor.Process(context.Background(), envelope) }() select { - case <-enteredPlanning: + case <-enteredCommit: case <-time.After(5 * time.Second): - close(releasePlanning) - t.Fatal("creator did not reach execution planning") + close(releaseCommit) + t.Fatal("first processor did not reach the event-plan commit") } - duplicateErr := processor.Process(context.Background(), envelope) - eventsBeforeRelease, eventsErr := store.Events() - executionsBeforeRelease, executionsErr := store.Executions() - executorCallsBeforeRelease := executorCalls - close(releasePlanning) + require.NoError(t, processor.Process(context.Background(), envelope)) + close(releaseCommit) - var creatorErr error select { - case creatorErr = <-creatorResult: + case err := <-firstResult: + require.NoError(t, err) case <-time.After(5 * time.Second): - t.Fatal("creator did not finish execution planning") + t.Fatal("first processor did not finish the event-plan commit") } - require.NoError(t, duplicateErr) - require.NoError(t, creatorErr) - require.NoError(t, eventsErr) - require.Len(t, eventsBeforeRelease, 1) - require.Nil(t, eventsBeforeRelease[0].PlannedAt) - require.Equal(t, 2, eventsBeforeRelease[0].Observations) - require.NoError(t, executionsErr) - require.Empty(t, executionsBeforeRelease) - require.Zero(t, executorCallsBeforeRelease) - - storedExecutions, err := store.Executions() - require.NoError(t, err) - require.Len(t, storedExecutions, 1) - require.Equal(t, 1, executorCalls) - }) - - t.Run("concurrent event creation loser stops", func(t *testing.T) { - rackID := uuid.New() - store := memorystore.New() - events := &concurrentWinnerEventStore{EventStore: store} - executorCalls := 0 - processor, err := New(Config{ - Inventory: processorInventoryWithRack(rackID), - Rules: ruleResolverFunc(func(context.Context, eventrule.Type, uuid.UUID) (*eventrule.Rule, error) { - return processorRuntimeRule(noopAction("once")), nil - }), - Events: events, - Executions: store, - Targets: targetRegistry(t, defaultTargetResolver(rackID)), - Executors: executorRegistry(t, executorFunc(func(context.Context, eventexecutor.ExecutionRequest) error { - executorCalls++ - return nil - })), - }) + events, err := store.Events() require.NoError(t, err) + require.Len(t, events, 1) + require.Equal(t, 2, events[0].Observations) - require.NoError(t, processor.Process(context.Background(), runtimeEnvelope(rackID))) - - storedEvents, err := store.Events() - require.NoError(t, err) - require.Len(t, storedEvents, 1) - require.Equal(t, 2, storedEvents[0].Observations) - require.Nil(t, storedEvents[0].PlannedAt) storedExecutions, err := store.Executions() require.NoError(t, err) - require.Empty(t, storedExecutions) - require.Zero(t, executorCalls) + require.Len(t, storedExecutions, 1) + require.Equal(t, eventrule.ExecutionStatusPending, storedExecutions[0].Status) + require.EqualValues(t, 1, notifier.calls.Load()) }) } func TestProcessorProcessPersistsEmptyEffectivePolicy(t *testing.T) { rackID := uuid.New() store := memorystore.New() + notifier := &countingNotifier{} processor := runtimeProcessor( t, processorInventoryWithRack(rackID), processorRuntimeRule(conditionalNoopAction("critical", eventrule.SeverityCritical)), store, defaultTargetResolver(rackID), - executorFunc(func(context.Context, eventexecutor.ExecutionRequest) error { - t.Fatal("empty effective policy must not dispatch") - return nil - }), + notifier, ) + envelope := runtimeEnvelope(rackID) envelope.Severity = eventrule.SeverityInfo + require.NoError(t, processor.Process(context.Background(), envelope)) events, err := store.Events() require.NoError(t, err) require.Empty(t, events[0].EffectivePolicy.Actions) - require.NotNil(t, events[0].PlannedAt) + executions, err := store.Executions() require.NoError(t, err) require.Empty(t, executions) + require.EqualValues(t, 1, notifier.calls.Load()) } func TestProcessorPlansConcreteSubmitTaskTargets(t *testing.T) { @@ -242,6 +195,7 @@ func TestProcessorPlansConcreteSubmitTaskTargets(t *testing.T) { component.New(devicetypes.ComponentTypeNVSwitch, &deviceinfo.DeviceInfo{ID: nvSwitchID}, "", nil), component.New(devicetypes.ComponentTypeCompute, &deviceinfo.DeviceInfo{ID: computeID}, "", nil), } + store := memorystore.New() processor := runtimeProcessor( t, @@ -249,12 +203,14 @@ func TestProcessorPlansConcreteSubmitTaskTargets(t *testing.T) { processorRuntimeRule(submitAction("power_off")), store, defaultTargetResolver(rackID), - executorFunc(func(context.Context, eventexecutor.ExecutionRequest) error { return nil }), + nil, ) + require.NoError(t, processor.Process(context.Background(), runtimeEnvelope(rackID))) executions, err := store.Executions() require.NoError(t, err) + plan := executions[0].Plan.(*eventrule.SubmitTaskPlan) require.Equal(t, operations.PowerOperationForcePowerOff.CodeString(), plan.Operation.Code) require.Equal(t, "ForcePowerOff, forced false", plan.Description) @@ -273,12 +229,11 @@ func TestProcessorPersistsNoTargetExecutionAsSkipped(t *testing.T) { processorRuntimeRule(submitAction("power_off")), store, &testTargetResolver{}, - executorFunc(func(context.Context, eventexecutor.ExecutionRequest) error { - t.Fatal("no-target execution must not reach executor") - return nil - }), + nil, ) + require.NoError(t, processor.Process(context.Background(), runtimeEnvelope(rackID))) + executions, err := store.Executions() require.NoError(t, err) require.Equal(t, eventrule.ExecutionStatusSkipped, executions[0].Status) @@ -291,16 +246,17 @@ func runtimeProcessor( rule *eventrule.Rule, store *memorystore.Store, targets eventtarget.Resolver, - execute eventexecutor.Executor, + notifier ExecutionNotifier, ) *Processor { t.Helper() + return runtimeProcessorWithRules(t, inventory, ruleResolverFunc(func( context.Context, eventrule.Type, uuid.UUID, ) (*eventrule.Rule, error) { return rule, nil - }), store, targets, execute) + }), store, targets, notifier) } func runtimeProcessorWithRules( @@ -309,36 +265,40 @@ func runtimeProcessorWithRules( rules RuleResolver, store *memorystore.Store, targets eventtarget.Resolver, - execute eventexecutor.Executor, + notifier ExecutionNotifier, ) *Processor { t.Helper() + processor, err := New(Config{ - Inventory: inventory, - Rules: rules, - Events: store, - Executions: store, - Targets: targetRegistry(t, targets), - Executors: executorRegistry(t, execute), + Inventory: inventory, + Rules: rules, + Store: store, + Targets: targetRegistry(t, targets), + Notifier: notifier, }) require.NoError(t, err) + return processor } func newTestProcessor(t *testing.T, inventory *processorInventory, rules RuleResolver) *Processor { t.Helper() + if rules == nil { rules = ruleResolverFunc(func(context.Context, eventrule.Type, uuid.UUID) (*eventrule.Rule, error) { return nil, nil }) } + store := memorystore.New() + return runtimeProcessorWithRules( t, inventory, rules, store, defaultTargetResolver(uuid.New()), - executorFunc(func(context.Context, eventexecutor.ExecutionRequest) error { return nil }), + nil, ) } @@ -376,8 +336,10 @@ func submitAction(name string) eventrule.Action { func targetRegistry(t *testing.T, resolver eventtarget.Resolver) *eventtarget.Registry { t.Helper() + registry := eventtarget.New() require.NoError(t, registry.Register("test.event", eventrule.TargetStrategyRack, resolver)) + return registry } @@ -398,97 +360,51 @@ func (r *testTargetResolver) Resolve(context.Context, eventtarget.ResolveRequest return r.targets, r.err } -type executorFunc func(context.Context, eventexecutor.ExecutionRequest) error - -func (f executorFunc) Execute(ctx context.Context, request eventexecutor.ExecutionRequest) error { - return f(ctx, request) -} - -type blockingExecutionStore struct { - eventrule.ExecutionStore +type blockingEventPlanStore struct { + *memorystore.Store blocked atomic.Bool entered chan struct{} release chan struct{} } -func (s *blockingExecutionStore) CommitEventPlan( +func (s *blockingEventPlanStore) CommitEventPlan( ctx context.Context, - eventID uuid.UUID, + event eventrule.Event, planned []eventrule.PlannedExecution, -) ([]eventrule.Execution, error) { +) (*eventrule.Event, error) { if s.blocked.CompareAndSwap(false, true) { close(s.entered) + select { case <-s.release: case <-ctx.Done(): return nil, ctx.Err() } } - return s.ExecutionStore.CommitEventPlan(ctx, eventID, planned) -} -type concurrentWinnerEventStore struct { - eventrule.EventStore + return s.Store.CommitEventPlan(ctx, event, planned) } -func (*concurrentWinnerEventStore) ObserveEvent( - context.Context, - eventrule.EventKey, -) (*eventrule.Event, error) { - return nil, nil +type countingNotifier struct { + calls atomic.Int32 } -func (s *concurrentWinnerEventStore) CreateEvent( - ctx context.Context, - definition eventrule.Event, -) (*eventrule.Event, error) { - winner, err := s.EventStore.CreateEvent(ctx, definition) - if err != nil || winner == nil { - return nil, err - } - return s.EventStore.CreateEvent(ctx, definition) -} - -type executorLookup map[eventrule.ActionType]eventexecutor.Executor - -func (r executorLookup) Executor( - actionType eventrule.ActionType, -) (eventexecutor.Executor, error) { - actionExecutor, ok := r[actionType] - if !ok { - return nil, fmt.Errorf("no executor registered for action type %q", actionType) - } - - return actionExecutor, nil -} - -func executorRegistry(t *testing.T, actionExecutor eventexecutor.Executor) ExecutorRegistry { - t.Helper() - registry := executorLookup{} - for _, actionType := range []eventrule.ActionType{ - eventrule.ActionTypeSubmitTask, - eventrule.ActionTypeSendAlert, - eventrule.ActionTypeNoop, - } { - registry[actionType] = actionExecutor - } - return registry +func (n *countingNotifier) Notify() { + n.calls.Add(1) } func validProcessorConfig(t *testing.T) Config { t.Helper() + store := memorystore.New() + return Config{ Inventory: &processorInventory{}, Rules: ruleResolverFunc(func(context.Context, eventrule.Type, uuid.UUID) (*eventrule.Rule, error) { return nil, nil }), - Events: store, - Executions: store, - Targets: targetRegistry(t, defaultTargetResolver(uuid.New())), - Executors: executorRegistry(t, executorFunc(func(context.Context, eventexecutor.ExecutionRequest) error { - return nil - })), + Store: store, + Targets: targetRegistry(t, defaultTargetResolver(uuid.New())), } } diff --git a/rest-api/flow/internal/eventrule/processor/processor.go b/rest-api/flow/internal/eventrule/processor/processor.go index a94e68bd95..182fbc7fb2 100644 --- a/rest-api/flow/internal/eventrule/processor/processor.go +++ b/rest-api/flow/internal/eventrule/processor/processor.go @@ -5,8 +5,6 @@ package processor import ( "context" - "errors" - "fmt" "github.com/NVIDIA/infra-controller/rest-api/flow/internal/eventrule" "github.com/NVIDIA/infra-controller/rest-api/flow/internal/eventrule/target" @@ -15,12 +13,11 @@ import ( // Processor orchestrates event enrichment, rule selection, and processing. type Processor struct { - inventory *inventoryresolver.Resolver - rules RuleResolver - events eventrule.EventStore - executions eventrule.ExecutionStore - targets *target.Registry - executors ExecutorRegistry + inventory *inventoryresolver.Resolver + rules RuleResolver + store eventrule.EventPlanStore + targets *target.Registry + notifier ExecutionNotifier } // New constructs an event processor. @@ -30,41 +27,30 @@ func New(config Config) (*Processor, error) { } return &Processor{ - inventory: inventoryresolver.New(config.Inventory), - rules: config.Rules, - events: config.Events, - executions: config.Executions, - targets: config.Targets, - executors: config.Executors, + inventory: inventoryresolver.New(config.Inventory), + rules: config.Rules, + store: config.Store, + targets: config.Targets, + notifier: config.Notifier, }, nil } -// Process deduplicates an envelope into a durable event. Only the caller that -// creates the event plans and dispatches it; duplicates record an observation -// and stop. +// Process deduplicates an envelope and atomically persists its complete event +// plan. The scheduler owns all execution attempts. func (p *Processor) Process(ctx context.Context, envelope eventrule.Envelope) error { prepared, err := p.prepare(ctx, envelope) if err != nil || prepared == nil { return err } - executions, err := p.plan(ctx, prepared) - if err != nil { + committed, err := p.plan(ctx, prepared) + if err != nil || committed == nil { return err } - var executionErrors []error - for i := range executions { - if executions[i].Status != eventrule.ExecutionStatusPending { - continue - } - - if err := p.dispatch(ctx, &executions[i]); err != nil { - executionErrors = append( - executionErrors, - fmt.Errorf("action %q: %w", executions[i].ActionName, err), - ) - } + if p.notifier != nil { + p.notifier.Notify() } - return errors.Join(executionErrors...) + + return nil } diff --git a/rest-api/flow/internal/eventrule/scheduler/config.go b/rest-api/flow/internal/eventrule/scheduler/config.go new file mode 100644 index 0000000000..b173166368 --- /dev/null +++ b/rest-api/flow/internal/eventrule/scheduler/config.go @@ -0,0 +1,171 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +package scheduler + +import ( + "fmt" + "sort" + "time" + + "github.com/NVIDIA/infra-controller/rest-api/flow/internal/eventrule" + "github.com/NVIDIA/infra-controller/rest-api/flow/internal/eventrule/executor" +) + +// ExecutorRegistry resolves the executor for a persisted plan type. +type ExecutorRegistry interface { + Executor(eventrule.ActionType) (executor.Executor, error) +} + +// Dependencies contains the services used by a Scheduler. +type Dependencies struct { + Store eventrule.ExecutionStore + Executors ExecutorRegistry +} + +// Validate checks the scheduler dependencies. +func (d Dependencies) Validate() error { + if d.Store == nil { + return fmt.Errorf("scheduler store is required") + } + + if d.Executors == nil { + return fmt.Errorf("executor registry is required") + } + + return nil +} + +// LaneConfig bounds one scheduler lane's worker and claim capacity. +type LaneConfig struct { + Workers int + ScanLimit int +} + +func (c LaneConfig) validate(name string) error { + if c.Workers <= 0 { + return fmt.Errorf("%s worker count must be positive", name) + } + + if c.ScanLimit <= 0 { + return fmt.Errorf("%s scan limit must be positive", name) + } + + return nil +} + +// RuntimeConfig bounds the scheduler's runtime mechanics. +type RuntimeConfig struct { + PollInterval time.Duration + PersistTimeout time.Duration + // Lanes overrides capacity by supported lane name. Omitted lanes use their + // defaults; lane priority remains defined by the scheduler. + Lanes map[string]LaneConfig +} + +// DefaultRuntimeConfig returns the default scheduler runtime configuration. +func DefaultRuntimeConfig() RuntimeConfig { + lanes := make(map[string]LaneConfig, len(laneDefinitions)) + for _, definition := range laneDefinitions { + lanes[definition.name] = definition.defaultConfig + } + + return RuntimeConfig{ + PollInterval: time.Minute, + PersistTimeout: time.Second, + Lanes: lanes, + } +} + +func (c RuntimeConfig) laneConfig(definition laneDefinition) LaneConfig { + if configured, ok := c.Lanes[definition.name]; ok { + return configured + } + + return definition.defaultConfig +} + +// Validate checks scheduler timing and explicit lane-capacity overrides. +func (c RuntimeConfig) Validate() error { + if c.PollInterval <= 0 { + return fmt.Errorf("scheduler poll interval must be positive") + } + + if c.PersistTimeout <= 0 { + return fmt.Errorf("execution persist timeout must be positive") + } + + supportedLanes := make(map[string]struct{}, len(laneDefinitions)) + for _, definition := range laneDefinitions { + supportedLanes[definition.name] = struct{}{} + } + + configuredLanes := make([]string, 0, len(c.Lanes)) + for name := range c.Lanes { + configuredLanes = append(configuredLanes, name) + } + sort.Strings(configuredLanes) + + var unknownLanes []string + for _, name := range configuredLanes { + if _, ok := supportedLanes[name]; !ok { + unknownLanes = append(unknownLanes, name) + continue + } + + if err := c.Lanes[name].validate(name); err != nil { + return err + } + } + + if len(unknownLanes) > 0 { + return fmt.Errorf("scheduler lane %q is not supported", unknownLanes[0]) + } + + return nil +} + +// Config identifies a scheduler and contains its dependencies and bounded +// scheduling behavior. +type Config struct { + // InstanceID uniquely identifies this scheduler among concurrently running + // instances and remains stable for its lifetime. + InstanceID string + Dependencies Dependencies + Runtime RuntimeConfig + Policy PolicyConfig +} + +// Validate checks all scheduler dependencies and limits. +func (c Config) Validate() error { + if err := c.Dependencies.Validate(); err != nil { + return err + } + + if err := eventrule.ValidateExecutionClaimOwner(c.InstanceID); err != nil { + return err + } + + if err := c.Runtime.Validate(); err != nil { + return err + } + + return c.Policy.Validate() +} + +func (c Config) runtime() runtime { + workerCount := 0 + for _, definition := range laneDefinitions { + workerCount += c.Runtime.laneConfig(definition).Workers + } + + return runtime{ + store: c.Dependencies.Store, + executors: c.Dependencies.Executors, + policy: c.Policy, + pollInterval: c.Runtime.PollInterval, + persistTimeout: c.Runtime.PersistTimeout, + wakeCh: make(chan struct{}, 1), + fatalWorkerErrors: make(chan error, workerCount), + } +} diff --git a/rest-api/flow/internal/eventrule/scheduler/config_test.go b/rest-api/flow/internal/eventrule/scheduler/config_test.go new file mode 100644 index 0000000000..8e3af1f9d6 --- /dev/null +++ b/rest-api/flow/internal/eventrule/scheduler/config_test.go @@ -0,0 +1,201 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +package scheduler + +import ( + "testing" + "time" + + "github.com/stretchr/testify/require" +) + +func TestConfigValidate(t *testing.T) { + tests := map[string]struct { + mutate func(*Config) + wantErr string + }{ + "valid": {}, + "missing store": { + mutate: func(config *Config) { config.Dependencies.Store = nil }, + wantErr: "scheduler store is required", + }, + "missing executor registry": { + mutate: func(config *Config) { config.Dependencies.Executors = nil }, + wantErr: "executor registry is required", + }, + "invalid poll interval": { + mutate: func(config *Config) { config.Runtime.PollInterval = 0 }, + wantErr: "scheduler poll interval must be positive", + }, + "missing instance id": { + mutate: func(config *Config) { config.InstanceID = "" }, + wantErr: "execution claim owner is empty", + }, + "invalid pending worker count": { + mutate: func(config *Config) { + config.Runtime.Lanes["pending"] = LaneConfig{ScanLimit: 1} + }, + wantErr: "pending worker count must be positive", + }, + "invalid deferred worker count": { + mutate: func(config *Config) { + config.Runtime.Lanes["deferred"] = LaneConfig{ScanLimit: 1} + }, + wantErr: "deferred worker count must be positive", + }, + "invalid pending scan limit": { + mutate: func(config *Config) { + config.Runtime.Lanes["pending"] = LaneConfig{Workers: 1} + }, + wantErr: "pending scan limit must be positive", + }, + "invalid deferred scan limit": { + mutate: func(config *Config) { + config.Runtime.Lanes["deferred"] = LaneConfig{Workers: 1} + }, + wantErr: "deferred scan limit must be positive", + }, + "missing pending lane uses default": { + mutate: func(config *Config) { delete(config.Runtime.Lanes, "pending") }, + }, + "nil lane map uses defaults": { + mutate: func(config *Config) { config.Runtime.Lanes = nil }, + }, + "unsupported lane": { + mutate: func(config *Config) { + config.Runtime.Lanes["unsupported-z"] = LaneConfig{Workers: 1, ScanLimit: 1} + config.Runtime.Lanes["unsupported-a"] = LaneConfig{Workers: 1, ScanLimit: 1} + }, + wantErr: `scheduler lane "unsupported-a" is not supported`, + }, + "invalid persist timeout": { + mutate: func(config *Config) { config.Runtime.PersistTimeout = 0 }, + wantErr: "execution persist timeout must be positive", + }, + "invalid max attempts": { + mutate: func(config *Config) { config.Policy.MaxAttempts = 0 }, + wantErr: "retry max attempts must be positive", + }, + "invalid initial delay": { + mutate: func(config *Config) { config.Policy.InitialDelay = 0 }, + wantErr: "retry initial delay must be positive", + }, + "max delay below initial delay": { + mutate: func(config *Config) { config.Policy.MaxDelay = time.Second }, + wantErr: "retry max delay must be at least the initial delay", + }, + } + + for name, test := range tests { + t.Run(name, func(t *testing.T) { + config := validConfig() + + if test.mutate != nil { + test.mutate(&config) + } + + err := config.Validate() + + if test.wantErr == "" { + require.NoError(t, err) + return + } + + require.EqualError(t, err, test.wantErr) + }) + } +} + +func TestDefaultRuntimeConfig(t *testing.T) { + expected := RuntimeConfig{ + PollInterval: time.Minute, + PersistTimeout: time.Second, + Lanes: map[string]LaneConfig{ + "pending": {Workers: 1, ScanLimit: 1}, + "deferred": {Workers: 1, ScanLimit: 1}, + }, + } + + actual := DefaultRuntimeConfig() + + require.Equal(t, expected, actual) + require.NoError(t, actual.Validate()) + + actual.Lanes["pending"] = LaneConfig{} + require.Equal(t, expected, DefaultRuntimeConfig()) +} + +func TestRuntimeConfig_laneConfig(t *testing.T) { + definition := laneDefinition{ + name: "test", + defaultConfig: LaneConfig{Workers: 1, ScanLimit: 2}, + } + tests := map[string]struct { + lanes map[string]LaneConfig + want LaneConfig + }{ + "nil map uses default": { + want: definition.defaultConfig, + }, + "missing lane uses default": { + lanes: map[string]LaneConfig{ + "another": {Workers: 2, ScanLimit: 3}, + }, + want: definition.defaultConfig, + }, + "configured lane overrides default": { + lanes: map[string]LaneConfig{ + definition.name: {Workers: 4, ScanLimit: 5}, + }, + want: LaneConfig{Workers: 4, ScanLimit: 5}, + }, + } + + for name, test := range tests { + t.Run(name, func(t *testing.T) { + configured := RuntimeConfig{Lanes: test.lanes} + + require.Equal(t, test.want, configured.laneConfig(definition)) + }) + } +} + +func TestConfig_runtime(t *testing.T) { + config := validConfig() + config.Runtime.Lanes["pending"] = LaneConfig{Workers: 2, ScanLimit: 1} + delete(config.Runtime.Lanes, "deferred") + + actual := config.runtime() + + require.Equal(t, config.Dependencies.Store, actual.store) + require.NotNil(t, actual.executors) + require.Equal(t, config.Policy, actual.policy) + require.Equal(t, config.Runtime.PollInterval, actual.pollInterval) + require.Equal(t, config.Runtime.PersistTimeout, actual.persistTimeout) + require.Equal(t, 1, cap(actual.wakeCh)) + require.Equal(t, 3, cap(actual.fatalWorkerErrors)) +} + +func TestNew(t *testing.T) { + configured, err := New(validConfig()) + + require.NoError(t, err) + require.NotNil(t, configured) + require.Len(t, configured.lanes, len(laneDefinitions)) + for index, definition := range laneDefinitions { + require.Equal(t, definition.name, configured.lanes[index].name) + } + + config := validConfig() + config.Runtime.Lanes = nil + configured, err = New(config) + + require.NoError(t, err) + require.Len(t, configured.lanes, len(laneDefinitions)) + require.Equal(t, 2, cap(configured.runtime.fatalWorkerErrors)) + + _, err = New(Config{}) + + require.EqualError(t, err, "scheduler store is required") +} diff --git a/rest-api/flow/internal/eventrule/scheduler/lane.go b/rest-api/flow/internal/eventrule/scheduler/lane.go new file mode 100644 index 0000000000..45b7ac5068 --- /dev/null +++ b/rest-api/flow/internal/eventrule/scheduler/lane.go @@ -0,0 +1,282 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +package scheduler + +import ( + "context" + "errors" + "fmt" + "sync" + + "github.com/rs/zerolog/log" + + "github.com/NVIDIA/infra-controller/rest-api/flow/internal/eventrule" + "github.com/NVIDIA/infra-controller/rest-api/flow/internal/eventrule/executor" +) + +type claimExecutionsFunc func( + context.Context, + eventrule.ExecutionClaimRequest, +) ([]eventrule.ClaimedExecution, error) + +type laneDefinition struct { + name string + defaultConfig LaneConfig + claimFunc func(eventrule.ExecutionStore) claimExecutionsFunc +} + +var laneDefinitions = []laneDefinition{ + { + name: "pending", + defaultConfig: LaneConfig{ + Workers: 1, + ScanLimit: 1, + }, + claimFunc: func(store eventrule.ExecutionStore) claimExecutionsFunc { + return store.ClaimPendingExecutions + }, + }, + { + name: "deferred", + defaultConfig: LaneConfig{ + Workers: 1, + ScanLimit: 1, + }, + claimFunc: func(store eventrule.ExecutionStore) claimExecutionsFunc { + return store.ClaimRetryExecutions + }, + }, +} + +var errLaneCapacityAccounting = errors.New("lane capacity accounting mismatch") + +type lane struct { + name string + claimFunc claimExecutionsFunc + scanLimit int + jobs chan eventrule.ClaimedExecution + slots chan struct{} +} + +func newLane( + name string, + config LaneConfig, + claimFunc claimExecutionsFunc, +) *lane { + slots := make(chan struct{}, config.Workers) + for range config.Workers { + slots <- struct{}{} + } + + return &lane{ + name: name, + claimFunc: claimFunc, + scanLimit: config.ScanLimit, + jobs: make(chan eventrule.ClaimedExecution, config.Workers), + slots: slots, + } +} + +func (l *lane) reserveAvailableSlots() int { + reserved := 0 + for reserved < l.scanLimit { + select { + case <-l.slots: + reserved++ + default: + return reserved + } + } + + return reserved +} + +func (l *lane) returnSlots(count int) error { + for range count { + if err := l.returnSlot(); err != nil { + return err + } + } + + return nil +} + +func (l *lane) returnSlot() error { + select { + case l.slots <- struct{}{}: + return nil + default: + return fmt.Errorf( + "cannot return %s worker slot: %w", + l.name, + errLaneCapacityAccounting, + ) + } +} + +func (l *lane) claim( + ctx context.Context, + owner string, +) ([]eventrule.ClaimedExecution, error) { + reserved := l.reserveAvailableSlots() + if reserved == 0 { + return nil, nil + } + + claims, err := l.claimFunc( + ctx, + eventrule.ExecutionClaimRequest{ + Owner: owner, + Limit: reserved, + }, + ) + if err != nil { + slotErr := l.returnSlots(reserved) + if ctxErr := ctx.Err(); ctxErr != nil && errors.Is(err, ctxErr) { + return nil, slotErr + } + + return nil, errors.Join( + fmt.Errorf("claim %s executions: %w", l.name, err), + slotErr, + ) + } + + if err := l.returnSlots(reserved - len(claims)); err != nil { + return nil, err + } + + return claims, nil +} + +func (l *lane) startWorkers( + ctx context.Context, + workers *sync.WaitGroup, + runtime *runtime, +) { + for range cap(l.slots) { + workers.Go(func() { + l.runWorker(ctx, runtime) + }) + } +} + +func (l *lane) runWorker(ctx context.Context, runtime *runtime) { + for { + select { + case <-ctx.Done(): + return + case claim := <-l.jobs: + if err := l.dispatch(ctx, claim, runtime); err != nil { + // Outcome persistence is claim-scoped. Report the failure but + // keep this worker available for unrelated executions. + l.logExecutionPersistenceError(claim, err) + } + + if err := l.returnSlot(); err != nil { + // A failed slot return means lane-capacity accounting is no + // longer trustworthy, so stop the scheduler through its fatal + // worker-error path. + l.reportFatalWorkerError(ctx, err, runtime) + return + } + + l.notifyScheduler(runtime) + } + } +} + +func (l *lane) dispatch( + ctx context.Context, + claim eventrule.ClaimedExecution, + runtime *runtime, +) error { + execution := claim.Execution + actionExecutor, err := runtime.executors.Executor(execution.Plan.Type()) + if err != nil { + return l.persistResult( + ctx, + claim, + runtime.policy.resultForExecutionError(execution.Attempts, err), + runtime, + ) + } + + if err := actionExecutor.Execute( + ctx, + executor.ExecutionRequest{ + ExecutionID: execution.ID, + Plan: execution.Plan, + }, + ); err != nil { + return l.persistResult( + ctx, + claim, + runtime.policy.resultForExecutionError(execution.Attempts, err), + runtime, + ) + } + + return l.persistResult( + ctx, + claim, + eventrule.CompletedExecutionResult(), + runtime, + ) +} + +func (l *lane) persistResult( + ctx context.Context, + claim eventrule.ClaimedExecution, + result eventrule.ExecutionResult, + runtime *runtime, +) error { + persistCtx, cancel := context.WithTimeout( + context.WithoutCancel(ctx), + runtime.persistTimeout, + ) + defer cancel() + + return runtime.store.TransitionClaimedExecution( + persistCtx, + claim.Execution.ID, + claim.Token, + result, + ) +} + +func (l *lane) logExecutionPersistenceError( + claim eventrule.ClaimedExecution, + err error, +) { + event := log.Error() + if errors.Is(err, eventrule.ErrExecutionClaimLost) { + event = log.Warn() + } + + event. + Err(err). + Str("lane", l.name). + Str("action_name", claim.Execution.ActionName). + Stringer("execution_id", claim.Execution.ID). + Msg("failed to persist event-rule execution outcome") +} + +func (l *lane) reportFatalWorkerError( + ctx context.Context, + err error, + runtime *runtime, +) { + select { + case runtime.fatalWorkerErrors <- err: + case <-ctx.Done(): + } +} + +func (l *lane) notifyScheduler(runtime *runtime) { + select { + case runtime.wakeCh <- struct{}{}: + default: + } +} diff --git a/rest-api/flow/internal/eventrule/scheduler/lane_test.go b/rest-api/flow/internal/eventrule/scheduler/lane_test.go new file mode 100644 index 0000000000..47c340bdfe --- /dev/null +++ b/rest-api/flow/internal/eventrule/scheduler/lane_test.go @@ -0,0 +1,70 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +package scheduler + +import ( + "context" + "errors" + "testing" + + "github.com/stretchr/testify/require" + + "github.com/NVIDIA/infra-controller/rest-api/flow/internal/eventrule" +) + +func TestLane_claim(t *testing.T) { + t.Run("bounds claims by available capacity", func(t *testing.T) { + store := newFakeStore() + workLane := newLane( + "pending", + LaneConfig{Workers: 2, ScanLimit: 10}, + store.ClaimPendingExecutions, + ) + + _, err := workLane.claim(context.Background(), testInstanceID) + require.NoError(t, err) + require.Len(t, store.requests, 1) + require.Equal(t, 2, store.requests[0].Limit) + require.Len(t, workLane.slots, 2) + + <-workLane.slots + + _, err = workLane.claim(context.Background(), testInstanceID) + require.NoError(t, err) + require.Len(t, store.requests, 2) + require.Equal(t, 1, store.requests[1].Limit) + }) + + t.Run("normalizes claim cancellation", func(t *testing.T) { + store := newFakeStore() + workLane := newLane( + "pending", + LaneConfig{Workers: 1, ScanLimit: 1}, + store.ClaimPendingExecutions, + ) + ctx, cancel := context.WithCancel(context.Background()) + cancel() + + _, err := workLane.claim(ctx, testInstanceID) + require.NoError(t, err) + require.Len(t, workLane.slots, 1) + }) + + t.Run("preserves a store error during cancellation", func(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + workLane := newLane( + "pending", + LaneConfig{Workers: 1, ScanLimit: 1}, + func(context.Context, eventrule.ExecutionClaimRequest) ([]eventrule.ClaimedExecution, error) { + cancel() + + return nil, errors.New("store unavailable") + }, + ) + _, err := workLane.claim(ctx, testInstanceID) + + require.EqualError(t, err, "claim pending executions: store unavailable") + require.Len(t, workLane.slots, 1) + }) +} diff --git a/rest-api/flow/internal/eventrule/scheduler/policy.go b/rest-api/flow/internal/eventrule/scheduler/policy.go new file mode 100644 index 0000000000..6d278cfcb4 --- /dev/null +++ b/rest-api/flow/internal/eventrule/scheduler/policy.go @@ -0,0 +1,107 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +package scheduler + +import ( + "errors" + "fmt" + "time" + + "github.com/NVIDIA/infra-controller/rest-api/flow/internal/eventrule" + "github.com/NVIDIA/infra-controller/rest-api/flow/internal/eventrule/executor" +) + +// PolicyConfig contains scheduler-owned execution policies. +type PolicyConfig struct { + // MaxAttempts limits attempts that return retryable execution failures. + // Interrupted attempts are refunded when their result is persisted. + MaxAttempts int + InitialDelay time.Duration + MaxDelay time.Duration +} + +// DefaultPolicyConfig returns the default scheduler policy configuration. +func DefaultPolicyConfig() PolicyConfig { + return PolicyConfig{ + MaxAttempts: 4, + InitialDelay: 10 * time.Second, + MaxDelay: time.Minute, + } +} + +// Validate checks every scheduler policy. +func (p PolicyConfig) Validate() error { + if p.MaxAttempts <= 0 { + return fmt.Errorf("retry max attempts must be positive") + } + + if p.InitialDelay <= 0 { + return fmt.Errorf("retry initial delay must be positive") + } + + if p.MaxDelay < p.InitialDelay { + return fmt.Errorf("retry max delay must be at least the initial delay") + } + + return nil +} + +func (p PolicyConfig) resultForExecutionError( + attempts int, + err error, +) eventrule.ExecutionResult { + if executor.IsInterrupted(err) { + return eventrule.DeferredExecutionResult( + eventrule.ExecutionReasonAttemptInterrupted, + err.Error(), + p.retryDelay(attempts), + ) + } + + if errors.Is(err, executor.ErrRetryable) { + return p.resultForRetryableError( + attempts, + eventrule.ExecutionReasonAttemptFailed, + err, + ) + } + + return eventrule.FailedExecutionResult(err.Error()) +} + +func (p PolicyConfig) resultForRetryableError( + attempts int, + reason eventrule.ExecutionReason, + err error, +) eventrule.ExecutionResult { + if attempts < p.MaxAttempts { + return eventrule.DeferredExecutionResult( + reason, + err.Error(), + p.retryDelay(attempts), + ) + } + + err = fmt.Errorf("execution failed after %d attempts: %w", attempts, err) + + return eventrule.FailedExecutionResult(err.Error()) +} + +// retryDelay returns exponential backoff for a failed allocated attempt. +func (p PolicyConfig) retryDelay(attempts int) time.Duration { + delay := p.InitialDelay + for attempt := 1; attempt < attempts; attempt++ { + if delay >= p.MaxDelay/2 { + return p.MaxDelay + } + + delay *= 2 + } + + if delay > p.MaxDelay { + return p.MaxDelay + } + + return delay +} diff --git a/rest-api/flow/internal/eventrule/scheduler/policy_test.go b/rest-api/flow/internal/eventrule/scheduler/policy_test.go new file mode 100644 index 0000000000..4d7757bc48 --- /dev/null +++ b/rest-api/flow/internal/eventrule/scheduler/policy_test.go @@ -0,0 +1,130 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +package scheduler + +import ( + "context" + "errors" + "testing" + "time" + + "github.com/stretchr/testify/require" + + "github.com/NVIDIA/infra-controller/rest-api/flow/internal/eventrule" + "github.com/NVIDIA/infra-controller/rest-api/flow/internal/eventrule/executor" +) + +func TestDefaultPolicyConfig(t *testing.T) { + expected := PolicyConfig{ + MaxAttempts: 4, + InitialDelay: 10 * time.Second, + MaxDelay: time.Minute, + } + + actual := DefaultPolicyConfig() + + require.Equal(t, expected, actual) + require.NoError(t, actual.Validate()) +} + +func TestPolicyConfig_resultForExecutionError(t *testing.T) { + policy := PolicyConfig{ + MaxAttempts: 4, + InitialDelay: 10 * time.Second, + MaxDelay: time.Minute, + } + tests := map[string]struct { + attempts int + executionErr error + wantStatus eventrule.ExecutionStatus + wantReason eventrule.ExecutionReason + wantMessage string + wantRetryAfter time.Duration + }{ + "retryable": { + attempts: 1, + executionErr: executor.Retryable(errors.New("temporarily unavailable")), + wantStatus: eventrule.ExecutionStatusDeferred, + wantReason: eventrule.ExecutionReasonAttemptFailed, + wantMessage: "temporarily unavailable", + wantRetryAfter: 10 * time.Second, + }, + "canceled": { + attempts: 1, + executionErr: context.Canceled, + wantStatus: eventrule.ExecutionStatusDeferred, + wantReason: eventrule.ExecutionReasonAttemptInterrupted, + wantMessage: context.Canceled.Error(), + wantRetryAfter: 10 * time.Second, + }, + "deadline exceeded": { + attempts: 2, + executionErr: context.DeadlineExceeded, + wantStatus: eventrule.ExecutionStatusDeferred, + wantReason: eventrule.ExecutionReasonAttemptInterrupted, + wantMessage: context.DeadlineExceeded.Error(), + wantRetryAfter: 20 * time.Second, + }, + "interruption does not exhaust retry limit": { + attempts: 4, + executionErr: context.Canceled, + wantStatus: eventrule.ExecutionStatusDeferred, + wantReason: eventrule.ExecutionReasonAttemptInterrupted, + wantMessage: context.Canceled.Error(), + wantRetryAfter: time.Minute, + }, + "terminal": { + attempts: 1, + executionErr: executor.Terminal(errors.New("invalid request")), + wantStatus: eventrule.ExecutionStatusFailed, + wantMessage: "invalid request", + }, + "unclassified": { + attempts: 1, + executionErr: errors.New("unexpected response"), + wantStatus: eventrule.ExecutionStatusFailed, + wantMessage: "unexpected response", + }, + "retry limit reached": { + attempts: 4, + executionErr: executor.Retryable(errors.New("still unavailable")), + wantStatus: eventrule.ExecutionStatusFailed, + wantMessage: "execution failed after 4 attempts: still unavailable", + }, + } + + for name, test := range tests { + t.Run(name, func(t *testing.T) { + result := policy.resultForExecutionError(test.attempts, test.executionErr) + + require.Equal(t, test.wantStatus, result.Status) + require.Equal(t, test.wantReason, result.Reason) + require.Equal(t, test.wantMessage, result.StatusMessage) + require.Equal(t, test.wantRetryAfter, result.RetryAfter) + }) + } +} + +func TestPolicyConfig_retryDelay(t *testing.T) { + policy := PolicyConfig{ + InitialDelay: 10 * time.Second, + MaxDelay: 45 * time.Second, + } + tests := map[string]struct { + attempts int + want time.Duration + }{ + "first attempt": {attempts: 1, want: 10 * time.Second}, + "second attempt": {attempts: 2, want: 20 * time.Second}, + "third attempt": {attempts: 3, want: 40 * time.Second}, + "bounded": {attempts: 4, want: 45 * time.Second}, + "large attempt count": {attempts: 100, want: 45 * time.Second}, + } + + for name, test := range tests { + t.Run(name, func(t *testing.T) { + require.Equal(t, test.want, policy.retryDelay(test.attempts)) + }) + } +} diff --git a/rest-api/flow/internal/eventrule/scheduler/scheduler.go b/rest-api/flow/internal/eventrule/scheduler/scheduler.go new file mode 100644 index 0000000000..c692c36e43 --- /dev/null +++ b/rest-api/flow/internal/eventrule/scheduler/scheduler.go @@ -0,0 +1,235 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +// Package scheduler owns initial and deferred event-rule execution attempts. +package scheduler + +import ( + "context" + "errors" + "fmt" + "sync" + "time" + + "github.com/rs/zerolog/log" + + "github.com/NVIDIA/infra-controller/rest-api/flow/internal/eventrule" +) + +// TODO(pending-admission-control): Backpressure collectors when durable pending +// work exceeds the capacity established for the event-rule execution path. + +type runtime struct { + store eventrule.ExecutionStore + executors ExecutorRegistry + policy PolicyConfig + pollInterval time.Duration + persistTimeout time.Duration + wakeCh chan struct{} + fatalWorkerErrors chan error +} + +// Scheduler claims and dispatches pending and due deferred executions. +type Scheduler struct { + instanceID string + runtime runtime + lanes []*lane + lifecycleMu sync.Mutex + started bool + stopped bool + cancel context.CancelFunc + done chan struct{} + runErr error +} + +// New constructs an execution scheduler without starting it. +func New(config Config) (*Scheduler, error) { + if err := config.Validate(); err != nil { + return nil, err + } + + lanes := make([]*lane, 0, len(laneDefinitions)) + for _, definition := range laneDefinitions { + lanes = append(lanes, newLane( + definition.name, + config.Runtime.laneConfig(definition), + definition.claimFunc(config.Dependencies.Store), + )) + } + + return &Scheduler{ + instanceID: config.InstanceID, + runtime: config.runtime(), + lanes: lanes, + }, nil +} + +// Notify non-blockingly hints that eligible work is available. +func (s *Scheduler) Notify() { + s.wake() +} + +// wake non-blockingly hints that eligible work or worker capacity is +// available. Signals coalesce; periodic polling remains the reliability path. +func (s *Scheduler) wake() { + select { + case s.runtime.wakeCh <- struct{}{}: + default: + } +} + +// Start launches both worker pools and the scheduling loop in the background. +// One Scheduler value may be started only once. Runtime failures are retained +// and returned by Stop. +func (s *Scheduler) Start(ctx context.Context) error { + s.lifecycleMu.Lock() + defer s.lifecycleMu.Unlock() + + if s.started { + return fmt.Errorf("scheduler can only be started once") + } + + runCtx, cancel := context.WithCancel(ctx) + s.started = true + s.cancel = cancel + s.done = make(chan struct{}) + + go func() { + err := s.run(runCtx) + if err != nil { + log.Error(). + Err(err). + Str("instance_id", s.instanceID). + Msg("event-rule scheduler stopped unexpectedly") + } + + s.lifecycleMu.Lock() + s.runErr = err + s.lifecycleMu.Unlock() + + close(s.done) + }() + + return nil +} + +// Stop cancels the scheduling loop and waits for all workers to exit. It +// returns any runtime failure that stopped the scheduler. +func (s *Scheduler) Stop() error { + s.lifecycleMu.Lock() + if !s.started { + s.lifecycleMu.Unlock() + return fmt.Errorf("scheduler cannot be stopped before it is started") + } + if s.stopped { + s.lifecycleMu.Unlock() + return fmt.Errorf("scheduler can only be stopped once") + } + + s.stopped = true + cancel := s.cancel + done := s.done + s.lifecycleMu.Unlock() + + cancel() + <-done + + s.lifecycleMu.Lock() + defer s.lifecycleMu.Unlock() + + return s.runErr +} + +func (s *Scheduler) run(ctx context.Context) error { + runCtx, cancel := context.WithCancel(ctx) + var workers sync.WaitGroup + for _, workLane := range s.lanes { + workLane.startWorkers(runCtx, &workers, &s.runtime) + } + defer func() { + cancel() + workers.Wait() + }() + + ticker := time.NewTicker(s.runtime.pollInterval) + defer ticker.Stop() + s.wake() + + for { + select { + case <-ctx.Done(): + return nil + case err := <-s.runtime.fatalWorkerErrors: + return err + case <-ticker.C: + s.wake() + case <-s.runtime.wakeCh: + // A select may choose this ready wake signal even when + // cancellation is also ready, so avoid starting new store work. + if ctx.Err() != nil { + return nil + } + + if err := s.refill(runCtx); err != nil { + return err + } + } + } +} + +func (s *Scheduler) refill(ctx context.Context) error { + for _, workLane := range s.lanes { + // Do not claim new work after shutdown begins, including when it begins + // between two lane refills. + if ctx.Err() != nil { + return nil + } + + claims, err := workLane.claim(ctx, s.instanceID) + if err != nil { + if errors.Is(err, errLaneCapacityAccounting) { + return err + } + + // Store availability is independent from scheduler correctness. Report + // the failed cycle and keep all lanes running; a notification or the + // periodic poll will retry the claim. + log.Error(). + Err(err). + Str("lane", workLane.name). + Msg("failed to claim event-rule executions") + continue + } + + channelCapacityMismatch := false + for _, claim := range claims { + select { + case workLane.jobs <- claim: + default: + // A reserved slot guarantees worker-channel capacity. If the + // channel is nevertheless full, its accounting is untrustworthy: + // continue through the already-claimed batch, then stop the + // scheduler. Do not add special-case claim recovery or repair slot + // state here: abandoned claims belong to the same stuck-running + // recovery required for other scheduler failures, while shutdown + // discards this lane's capacity state. + channelCapacityMismatch = true + } + } + + if channelCapacityMismatch { + return fmt.Errorf( + "%s work channel has no capacity despite reserved worker slots", + workLane.name, + ) + } + + // A full scan may have truncated additional eligible work, so schedule + // another pass. Partial and empty scans avoid a redundant store read. + if len(claims) == workLane.scanLimit { + s.wake() + } + } + + return nil +} diff --git a/rest-api/flow/internal/eventrule/scheduler/scheduler_test.go b/rest-api/flow/internal/eventrule/scheduler/scheduler_test.go new file mode 100644 index 0000000000..9c068b2350 --- /dev/null +++ b/rest-api/flow/internal/eventrule/scheduler/scheduler_test.go @@ -0,0 +1,531 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +package scheduler + +import ( + "context" + "errors" + "sync" + "sync/atomic" + "testing" + "time" + + "github.com/google/uuid" + "github.com/stretchr/testify/require" + + "github.com/NVIDIA/infra-controller/rest-api/flow/internal/eventrule" + "github.com/NVIDIA/infra-controller/rest-api/flow/internal/eventrule/executor" +) + +func TestScheduler_Start(t *testing.T) { + t.Run("dispatches a pending execution", func(t *testing.T) { + store := newFakeStore() + store.claims["pending"] = []eventrule.ClaimedExecution{ + newClaimedExecution(t, "pending_action", 1, testInstanceID), + } + var executorCalls atomic.Int32 + configured := newTestScheduler(t, store, executorRegistryFunc( + func(eventrule.ActionType) (executor.Executor, error) { + return executorFunc(func(context.Context, executor.ExecutionRequest) error { + executorCalls.Add(1) + + return nil + }), nil + }, + )) + + require.NoError(t, configured.Start(context.Background())) + + transition := receiveTransition(t, store.transitions) + require.Equal(t, eventrule.ExecutionStatusCompleted, transition.result.Status) + require.EqualValues(t, 1, executorCalls.Load()) + + require.NoError(t, configured.Stop()) + }) + + t.Run("processor signal wakes a sleeping scheduler", func(t *testing.T) { + store := newFakeStore() + configured := newTestScheduler(t, store, successfulExecutorRegistry()) + require.NoError(t, configured.Start(context.Background())) + + require.Eventually(t, func() bool { return store.requestCount() >= 2 }, time.Second, time.Millisecond) + + store.enqueue( + "pending", + newClaimedExecution(t, "signaled_action", 1, testInstanceID), + ) + configured.Notify() + + transition := receiveTransition(t, store.transitions) + require.Equal(t, "signaled_action", transition.actionName) + + require.NoError(t, configured.Stop()) + }) + + t.Run("polling finds work after a dropped signal", func(t *testing.T) { + store := newFakeStore() + config := validConfig() + config.Dependencies.Store = store + config.Runtime.PollInterval = 10 * time.Millisecond + configured, err := New(config) + require.NoError(t, err) + + require.NoError(t, configured.Start(context.Background())) + + require.Eventually(t, func() bool { return store.requestCount() >= 2 }, time.Second, time.Millisecond) + + store.enqueue( + "pending", + newClaimedExecution(t, "polled_action", 1, testInstanceID), + ) + + transition := receiveTransition(t, store.transitions) + require.Equal(t, "polled_action", transition.actionName) + + require.NoError(t, configured.Stop()) + }) + + t.Run("worker completion schedules the next execution", func(t *testing.T) { + store := newFakeStore() + store.claims["pending"] = []eventrule.ClaimedExecution{ + newClaimedExecution(t, "first_action", 1, testInstanceID), + newClaimedExecution(t, "second_action", 1, testInstanceID), + } + + configured := newTestScheduler(t, store, successfulExecutorRegistry()) + require.NoError(t, configured.Start(context.Background())) + + first := receiveTransition(t, store.transitions) + second := receiveTransition(t, store.transitions) + require.ElementsMatch(t, []string{"first_action", "second_action"}, []string{ + first.actionName, + second.actionName, + }) + + require.NoError(t, configured.Stop()) + }) + + t.Run("keeps deferred capacity independent from blocked pending work", func(t *testing.T) { + store := newFakeStore() + store.claims["pending"] = []eventrule.ClaimedExecution{ + newClaimedExecution(t, "pending_action", 1, testInstanceID), + } + store.claims["deferred"] = []eventrule.ClaimedExecution{ + newClaimedExecution(t, "deferred_action", 2, testInstanceID), + } + pendingStarted := make(chan struct{}) + releasePending := make(chan struct{}) + configured := newTestScheduler(t, store, executorRegistryFunc( + func(eventrule.ActionType) (executor.Executor, error) { + return executorFunc(func(_ context.Context, request executor.ExecutionRequest) error { + plan := request.Plan.(*eventrule.NoopPlan) + if plan.Reason == "pending_action" { + close(pendingStarted) + <-releasePending + } + + return nil + }), nil + }, + )) + + require.NoError(t, configured.Start(context.Background())) + + receiveSignal(t, pendingStarted, "pending worker did not start") + transition := receiveTransition(t, store.transitions) + require.Equal(t, "deferred_action", transition.actionName) + close(releasePending) + transition = receiveTransition(t, store.transitions) + require.Equal(t, "pending_action", transition.actionName) + + require.NoError(t, configured.Stop()) + }) + + t.Run("continues after a claim failure", func(t *testing.T) { + store := newFakeStore() + store.enqueue( + "pending", + newClaimedExecution(t, "pending_action", 1, testInstanceID), + ) + store.setClaimError("pending", errors.New("store unavailable")) + configured := newTestScheduler(t, store, successfulExecutorRegistry()) + + require.NoError(t, configured.Start(context.Background())) + require.Eventually( + t, + func() bool { return store.requestCount() >= 1 }, + time.Second, + time.Millisecond, + ) + store.setClaimError("pending", nil) + configured.Notify() + + transition := receiveTransition(t, store.transitions) + + require.Equal(t, "pending_action", transition.actionName) + require.GreaterOrEqual(t, store.requestCount(), 2) + require.NoError(t, configured.Stop()) + }) + + t.Run("continues after a transition failure", func(t *testing.T) { + tests := map[string]error{ + "store failure": errors.New("store unavailable"), + "claim lost": eventrule.ErrExecutionClaimLost, + } + + for name, transitionErr := range tests { + t.Run(name, func(t *testing.T) { + store := newFakeStore() + store.claims["pending"] = []eventrule.ClaimedExecution{ + newClaimedExecution(t, "first_action", 1, testInstanceID), + newClaimedExecution(t, "second_action", 1, testInstanceID), + } + store.transitionErr = transitionErr + configured := newTestScheduler(t, store, successfulExecutorRegistry()) + require.NoError(t, configured.Start(context.Background())) + + first := receiveTransition(t, store.transitions) + second := receiveTransition(t, store.transitions) + require.Equal(t, "first_action", first.actionName) + require.Equal(t, "second_action", second.actionName) + + require.NoError(t, configured.Stop()) + }) + } + }) + + t.Run("rejects a second start", func(t *testing.T) { + configured := newTestScheduler(t, newFakeStore(), successfulExecutorRegistry()) + + require.NoError(t, configured.Start(context.Background())) + require.EqualError( + t, + configured.Start(context.Background()), + "scheduler can only be started once", + ) + require.NoError(t, configured.Stop()) + }) +} + +func TestScheduler_Stop(t *testing.T) { + t.Run("waits for active workers", func(t *testing.T) { + store := newFakeStore() + store.claims["pending"] = []eventrule.ClaimedExecution{ + newClaimedExecution(t, "pending_action", 4, testInstanceID), + } + executionStarted := make(chan struct{}) + executionStopped := make(chan struct{}) + configured := newTestScheduler(t, store, executorRegistryFunc( + func(eventrule.ActionType) (executor.Executor, error) { + return executorFunc(func(ctx context.Context, _ executor.ExecutionRequest) error { + close(executionStarted) + <-ctx.Done() + close(executionStopped) + + return ctx.Err() + }), nil + }, + )) + + require.NoError(t, configured.Start(context.Background())) + receiveSignal(t, executionStarted, "execution did not start") + require.NoError(t, configured.Stop()) + receiveSignal(t, executionStopped, "execution did not stop") + + transition := receiveTransition(t, store.transitions) + require.Equal(t, eventrule.ExecutionStatusDeferred, transition.result.Status) + require.Equal(t, eventrule.ExecutionReasonAttemptInterrupted, transition.result.Reason) + }) + + t.Run("rejects stop before start", func(t *testing.T) { + configured := newTestScheduler(t, newFakeStore(), successfulExecutorRegistry()) + + require.EqualError( + t, + configured.Stop(), + "scheduler cannot be stopped before it is started", + ) + }) + + t.Run("rejects a second stop", func(t *testing.T) { + configured := newTestScheduler(t, newFakeStore(), successfulExecutorRegistry()) + + require.NoError(t, configured.Start(context.Background())) + require.NoError(t, configured.Stop()) + require.EqualError( + t, + configured.Stop(), + "scheduler can only be stopped once", + ) + }) +} + +func TestScheduler_Notify(t *testing.T) { + configured := newTestScheduler(t, newFakeStore(), successfulExecutorRegistry()) + + for range 20 { + configured.Notify() + } + + require.Len(t, configured.runtime.wakeCh, 1) +} + +func TestScheduler_refill(t *testing.T) { + t.Run("refills lanes in priority order", func(t *testing.T) { + store := newFakeStore() + configured := newTestScheduler(t, store, successfulExecutorRegistry()) + + require.NoError(t, configured.refill(context.Background())) + require.Equal(t, []string{"pending", "deferred"}, store.requestLanes) + }) + + t.Run("stops after reserved capacity cannot be handed off", func(t *testing.T) { + store := newFakeStore() + store.claims["pending"] = []eventrule.ClaimedExecution{ + newClaimedExecution(t, "claimed_action", 1, testInstanceID), + } + configured := newTestScheduler(t, store, successfulExecutorRegistry()) + pending := configured.lanes[0] + pending.jobs <- newClaimedExecution(t, "occupying_action", 1, testInstanceID) + + err := configured.refill(context.Background()) + + require.EqualError( + t, + err, + "pending work channel has no capacity despite reserved worker slots", + ) + require.Empty(t, pending.slots) + require.Empty(t, store.transitions) + }) +} + +const testInstanceID = "scheduler-test" + +type transitionRecord struct { + executionID uuid.UUID + actionName string + token uuid.UUID + result eventrule.ExecutionResult + contextErr error +} + +type fakeStore struct { + mu sync.Mutex + claims map[string][]eventrule.ClaimedExecution + claimErrors map[string]error + requests []eventrule.ExecutionClaimRequest + requestLanes []string + actionNames map[uuid.UUID]string + transitionErr error + transitions chan transitionRecord +} + +func newFakeStore() *fakeStore { + return &fakeStore{ + claims: make(map[string][]eventrule.ClaimedExecution), + claimErrors: make(map[string]error), + actionNames: make(map[uuid.UUID]string), + transitions: make(chan transitionRecord, 10), + } +} + +func (s *fakeStore) enqueue( + lane string, + claims ...eventrule.ClaimedExecution, +) { + s.mu.Lock() + defer s.mu.Unlock() + + s.claims[lane] = append(s.claims[lane], claims...) +} + +func (s *fakeStore) setClaimError(lane string, err error) { + s.mu.Lock() + defer s.mu.Unlock() + + s.claimErrors[lane] = err +} + +func (s *fakeStore) requestCount() int { + s.mu.Lock() + defer s.mu.Unlock() + + return len(s.requests) +} + +func (s *fakeStore) ClaimPendingExecutions( + ctx context.Context, + request eventrule.ExecutionClaimRequest, +) ([]eventrule.ClaimedExecution, error) { + return s.claim(ctx, request, "pending") +} + +func (s *fakeStore) ClaimRetryExecutions( + ctx context.Context, + request eventrule.ExecutionClaimRequest, +) ([]eventrule.ClaimedExecution, error) { + return s.claim(ctx, request, "deferred") +} + +func (s *fakeStore) claim( + ctx context.Context, + request eventrule.ExecutionClaimRequest, + lane string, +) ([]eventrule.ClaimedExecution, error) { + s.mu.Lock() + defer s.mu.Unlock() + + if err := ctx.Err(); err != nil { + return nil, err + } + + s.requests = append(s.requests, request) + s.requestLanes = append(s.requestLanes, lane) + + if err := s.claimErrors[lane]; err != nil { + return nil, err + } + + available := s.claims[lane] + count := min(request.Limit, len(available)) + claimed := make([]eventrule.ClaimedExecution, count) + copy(claimed, available[:count]) + for i := range claimed { + s.actionNames[claimed[i].Execution.ID] = claimed[i].Execution.ActionName + } + + s.claims[lane] = available[count:] + + return claimed, nil +} + +func (s *fakeStore) TransitionClaimedExecution( + ctx context.Context, + executionID uuid.UUID, + token uuid.UUID, + result eventrule.ExecutionResult, +) error { + s.mu.Lock() + transitionErr := s.transitionErr + actionName := s.actionNames[executionID] + s.mu.Unlock() + + record := transitionRecord{ + executionID: executionID, + actionName: actionName, + token: token, + result: result, + contextErr: ctx.Err(), + } + + select { + case s.transitions <- record: + case <-ctx.Done(): + return ctx.Err() + } + + return transitionErr +} + +func newClaimedExecution( + t *testing.T, + actionName string, + attempts int, + owner string, +) eventrule.ClaimedExecution { + t.Helper() + + createdAt := time.Date(2026, 8, 24, 12, 0, 0, 0, time.UTC) + execution, err := eventrule.NewExecution( + uuid.New(), + actionName, + &eventrule.NoopPlan{Reason: actionName}, + createdAt, + ) + require.NoError(t, err) + + token := uuid.New() + require.NoError(t, execution.Claim(owner, token, createdAt.Add(time.Second))) + + execution.Attempts = attempts + require.NoError(t, execution.Validate()) + + return eventrule.ClaimedExecution{Execution: *execution, Token: token} +} + +type executorRegistryFunc func(eventrule.ActionType) (executor.Executor, error) + +func (f executorRegistryFunc) Executor(actionType eventrule.ActionType) (executor.Executor, error) { + return f(actionType) +} + +type executorFunc func(context.Context, executor.ExecutionRequest) error + +func (f executorFunc) Execute(ctx context.Context, request executor.ExecutionRequest) error { + return f(ctx, request) +} + +func validConfig() Config { + return Config{ + InstanceID: testInstanceID, + Dependencies: Dependencies{ + Store: newFakeStore(), + Executors: successfulExecutorRegistry(), + }, + Runtime: DefaultRuntimeConfig(), + Policy: DefaultPolicyConfig(), + } +} + +func newTestScheduler( + t *testing.T, + store eventrule.ExecutionStore, + executors ExecutorRegistry, +) *Scheduler { + t.Helper() + + config := validConfig() + config.Dependencies.Store = store + config.Dependencies.Executors = executors + + configured, err := New(config) + require.NoError(t, err) + + return configured +} + +func successfulExecutorRegistry() ExecutorRegistry { + return executorRegistryFunc(func(eventrule.ActionType) (executor.Executor, error) { + return executorFunc(func(context.Context, executor.ExecutionRequest) error { + return nil + }), nil + }) +} + +func receiveTransition(t *testing.T, transitions <-chan transitionRecord) transitionRecord { + t.Helper() + + select { + case transition := <-transitions: + return transition + case <-time.After(5 * time.Second): + t.Fatal("scheduler did not persist an execution transition") + return transitionRecord{} + } +} + +func receiveSignal(t *testing.T, signal <-chan struct{}, message string) { + t.Helper() + + select { + case <-signal: + case <-time.After(5 * time.Second): + t.Fatal(message) + } +} + +var _ eventrule.ExecutionStore = (*fakeStore)(nil) +var _ ExecutorRegistry = executorRegistryFunc(nil) +var _ executor.Executor = executorFunc(nil) diff --git a/rest-api/flow/internal/eventrule/store.go b/rest-api/flow/internal/eventrule/store.go index 2331e15ff0..63e2ba6fa3 100644 --- a/rest-api/flow/internal/eventrule/store.go +++ b/rest-api/flow/internal/eventrule/store.go @@ -6,6 +6,8 @@ package eventrule import ( "context" "errors" + "fmt" + "unicode/utf8" "github.com/google/uuid" ) @@ -32,14 +34,81 @@ var ErrExecutionNotFound = errors.New("execution not found") // ErrEventNotFound identifies an unsuccessful durable event lookup. var ErrEventNotFound = errors.New("event not found") -// ErrEventAlreadyPlanned identifies an attempt to replace a committed event -// plan. -var ErrEventAlreadyPlanned = errors.New("event already planned") - // ErrExecutionAlreadyExists identifies an execution identity that existed // before its event plan was committed. var ErrExecutionAlreadyExists = errors.New("execution already exists") +// ErrExecutionClaimLost identifies a fenced update from a worker that no +// longer owns the execution attempt. +var ErrExecutionClaimLost = errors.New("execution claim lost") + +const maxExecutionClaimOwnerRunes = 128 + +// ValidateExecutionClaimOwner checks that a claim owner is a bounded, nonempty +// identity. +func ValidateExecutionClaimOwner(owner string) error { + if err := validateRequiredString("execution claim owner", owner); err != nil { + return err + } + if utf8.RuneCountInString(owner) > maxExecutionClaimOwnerRunes { + return fmt.Errorf( + "execution claim owner exceeds %d characters", + maxExecutionClaimOwnerRunes, + ) + } + + return nil +} + +// ExecutionClaimRequest bounds one atomic scheduler-store selection. +type ExecutionClaimRequest struct { + Owner string + Limit int +} + +// Validate checks the owner and claim limit. +func (r ExecutionClaimRequest) Validate() error { + if err := ValidateExecutionClaimOwner(r.Owner); err != nil { + return err + } + if r.Limit <= 0 { + return fmt.Errorf("execution claim limit must be positive") + } + + return nil +} + +// ClaimedExecution contains one running execution and its ownership fence. +// Token is not a downstream idempotency key. +type ClaimedExecution struct { + Execution Execution + Token uuid.UUID +} + +// Validate checks the running execution and ownership fence. +func (c ClaimedExecution) Validate() error { + if err := c.Execution.Validate(); err != nil { + return fmt.Errorf("execution: %w", err) + } + + if c.Execution.Status != ExecutionStatusRunning { + return fmt.Errorf( + "claimed execution %s has status %q, want %q", + c.Execution.ID, + c.Execution.Status, + ExecutionStatusRunning, + ) + } + if c.Token == uuid.Nil { + return fmt.Errorf("execution claim token is required") + } + if c.Execution.ClaimToken != c.Token { + return fmt.Errorf("execution claim token does not match running execution") + } + + return nil +} + // RuleFilter limits rules returned by a store. type RuleFilter struct { EventType *Type @@ -52,6 +121,7 @@ func (f RuleFilter) Matches(rule *Rule) bool { if rule == nil { return false } + if f.EventType != nil && rule.EventType != *f.EventType { return false } @@ -61,6 +131,7 @@ func (f RuleFilter) Matches(rule *Rule) bool { if f.Enabled != nil && rule.Enabled != *f.Enabled { return false } + return true } @@ -103,35 +174,45 @@ type BindingStore interface { GetForScope(context.Context, Type, Scope) (*Binding, error) } -// EventStore owns source-event deduplication and observation accounting. -// ObserveEvent returns (nil, nil) when the source event has not been persisted. -// CreateEvent returns the newly inserted event. A concurrent duplicate is -// recorded as another observation and returns (nil, nil). -type EventStore interface { +// EventPlanStore owns the source-event duplicate fast path and atomic event-plan +// commit. Implementations own all persistence timestamps. +type EventPlanStore interface { + // ObserveEvent records an existing event observation and returns (nil, nil) + // when no event is persisted. ObserveEvent(context.Context, EventKey) (*Event, error) - CreateEvent(context.Context, Event) (*Event, error) + // CommitEventPlan is all-or-nothing: it persists the event and one execution + // per planned action in the same transaction. A concurrent duplicate records + // another observation and returns (nil, nil). Any error persists nothing. + CommitEventPlan( + ctx context.Context, + event Event, + planned []PlannedExecution, + ) (*Event, error) } -// ExecutionStore atomically commits an event's complete ordered plan and -// persists attempt results. CommitEventPlan creates every execution and marks -// the event planned in one transaction. It returns executions in the same -// order as planned and rejects replacement of a committed or partial plan. -// TransitionExecution returns ErrExecutionNotFound for an unknown -// execution ID. Implementations own planning, transition, and retry-scheduling -// timestamps. +// ExecutionStore owns scheduler claims and fenced attempt outcomes. +// Implementations own all persistence and retry timestamps. type ExecutionStore interface { - // CommitEventPlan is all-or-nothing: on success it persists one execution - // per planned action, in the supplied order, and marks the event planned in - // the same transaction. It returns those executions in the same order. On - // error it persists no execution and leaves the event unplanned. - CommitEventPlan( + // ClaimPendingExecutions atomically selects at most request.Limit pending + // rows, moves them to running, allocates attempts, and assigns request.Owner + // and fencing tokens. + ClaimPendingExecutions( ctx context.Context, - eventID uuid.UUID, - planned []PlannedExecution, - ) ([]Execution, error) - TransitionExecution( + request ExecutionClaimRequest, + ) ([]ClaimedExecution, error) + // ClaimRetryExecutions atomically selects at most request.Limit due retry + // rows, moves them to running, allocates attempts, and assigns request.Owner + // and fencing tokens. + ClaimRetryExecutions( + ctx context.Context, + request ExecutionClaimRequest, + ) ([]ClaimedExecution, error) + // TransitionClaimedExecution persists an attempt outcome only while token + // owns the running execution. + TransitionClaimedExecution( ctx context.Context, executionID uuid.UUID, + token uuid.UUID, result ExecutionResult, ) error } diff --git a/rest-api/flow/internal/eventrule/store/memory/event.go b/rest-api/flow/internal/eventrule/store/memory/event.go index 2e15a01a68..b1b2e30a81 100644 --- a/rest-api/flow/internal/eventrule/store/memory/event.go +++ b/rest-api/flow/internal/eventrule/store/memory/event.go @@ -8,6 +8,7 @@ import ( "context" "fmt" "slices" + "time" converterdao "github.com/NVIDIA/infra-controller/rest-api/flow/internal/converter/dao" "github.com/NVIDIA/infra-controller/rest-api/flow/internal/eventrule" @@ -40,9 +41,12 @@ func (s *Store) Events() ([]eventrule.Event, error) { // ObserveEvent returns and records an existing source event. A missing event // is represented by (nil, nil). func (s *Store) ObserveEvent( - _ context.Context, + ctx context.Context, key eventrule.EventKey, ) (*eventrule.Event, error) { + if err := ctx.Err(); err != nil { + return nil, err + } if err := key.Validate(); err != nil { return nil, err } @@ -50,6 +54,13 @@ func (s *Store) ObserveEvent( s.mu.Lock() defer s.mu.Unlock() + return s.observeEvent(key, s.now().UTC()) +} + +func (s *Store) observeEvent( + key eventrule.EventKey, + now time.Time, +) (*eventrule.Event, error) { id, exists := s.eventsByKey[key] if !exists { return nil, nil @@ -61,8 +72,6 @@ func (s *Store) ObserveEvent( } event.Observations++ - now := s.now().UTC() - if now.After(event.LastObservedAt) { event.LastObservedAt = now } @@ -74,56 +83,6 @@ func (s *Store) ObserveEvent( return s.event(id) } -// CreateEvent inserts and returns a durable event. A concurrent duplicate -// records another observation and returns (nil, nil). -func (s *Store) CreateEvent( - _ context.Context, - definition eventrule.Event, -) (*eventrule.Event, error) { - if err := definition.ValidateDefinition(); err != nil { - return nil, err - } - - s.mu.Lock() - defer s.mu.Unlock() - - now := s.now().UTC() - - if id, exists := s.eventsByKey[definition.Key]; exists { - event, err := s.event(id) - if err != nil { - return nil, err - } - - event.Observations++ - - if now.After(event.LastObservedAt) { - event.LastObservedAt = now - } - - if err := s.setEvent(event); err != nil { - return nil, err - } - - return nil, nil - } - - event, err := eventrule.NewEvent(definition, now) - if err != nil { - return nil, err - } - - persisted, err := converterdao.EventTo(event) - if err != nil { - return nil, err - } - - s.events[event.ID] = *persisted - s.eventsByKey[event.Key] = event.ID - - return s.event(event.ID) -} - func (s *Store) event(id uuid.UUID) (*eventrule.Event, error) { persisted, exists := s.events[id] if !exists { diff --git a/rest-api/flow/internal/eventrule/store/memory/execution.go b/rest-api/flow/internal/eventrule/store/memory/execution.go index ed09628d81..9839438285 100644 --- a/rest-api/flow/internal/eventrule/store/memory/execution.go +++ b/rest-api/flow/internal/eventrule/store/memory/execution.go @@ -8,6 +8,7 @@ import ( "context" "fmt" "slices" + "time" converterdao "github.com/NVIDIA/infra-controller/rest-api/flow/internal/converter/dao" dbmodel "github.com/NVIDIA/infra-controller/rest-api/flow/internal/db/model" @@ -19,21 +20,33 @@ type memoryExecution struct { persisted dbmodel.EventActionExecution } -// CommitEventPlan atomically inserts every immutable action plan and marks the -// event planned. +type executionClaimKind uint8 + +const ( + pendingExecutionClaim executionClaimKind = iota + retryExecutionClaim +) + +// CommitEventPlan atomically inserts the event and every immutable action plan. +// A concurrent duplicate records another observation and returns (nil, nil). func (s *Store) CommitEventPlan( - _ context.Context, - eventID uuid.UUID, + ctx context.Context, + definition eventrule.Event, planned []eventrule.PlannedExecution, -) ([]eventrule.Execution, error) { - if eventID == uuid.Nil { - return nil, fmt.Errorf("execution event id is required") +) (*eventrule.Event, error) { + if err := ctx.Err(); err != nil { + return nil, err } + if err := definition.ValidateDefinition(); err != nil { + return nil, err + } + seen := make(map[string]struct{}, len(planned)) for i, item := range planned { if err := item.Validate(); err != nil { return nil, fmt.Errorf("planned executions[%d]: %w", i, err) } + if _, exists := seen[item.ActionName]; exists { return nil, fmt.Errorf( "planned executions[%d]: duplicate action name %q", @@ -41,62 +54,45 @@ func (s *Store) CommitEventPlan( item.ActionName, ) } + seen[item.ActionName] = struct{}{} } - s.mu.Lock() - defer s.mu.Unlock() - event, err := s.event(eventID) - if err != nil { + if err := validateEventPlan(definition, planned); err != nil { return nil, err } - if event.PlannedAt != nil { - return nil, fmt.Errorf("%w: %s", eventrule.ErrEventAlreadyPlanned, eventID) - } - if len(planned) != len(event.EffectivePolicy.Actions) { - return nil, fmt.Errorf( - "event plan has %d executions for %d applicable actions", - len(planned), - len(event.EffectivePolicy.Actions), - ) - } - for i, action := range event.EffectivePolicy.Actions { - if planned[i].ActionName != action.Name { - return nil, fmt.Errorf( - "planned executions[%d] has action name %q, want %q", - i, - planned[i].ActionName, - action.Name, - ) - } - if planned[i].ExecutionPlan.Type() != action.Spec.Type() { - return nil, fmt.Errorf( - "planned executions[%d] has type %q, want %q", - i, - planned[i].ExecutionPlan.Type(), - action.Spec.Type(), - ) + + s.mu.Lock() + defer s.mu.Unlock() + + now := s.now().UTC() + if _, exists := s.eventsByKey[definition.Key]; exists { + if _, err := s.observeEvent(definition.Key, now); err != nil { + return nil, err } + + return nil, nil } - now := s.now().UTC() - if now.Before(event.CreatedAt) { - return nil, fmt.Errorf("event planned time cannot precede creation time") + event, err := eventrule.NewEvent(definition, now) + if err != nil { + return nil, err } records := make([]dbmodel.EventActionExecution, len(planned)) for i, item := range planned { - key := eventrule.ExecutionKey{EventID: eventID, ActionName: item.ActionName} + key := eventrule.ExecutionKey{EventID: event.ID, ActionName: item.ActionName} if _, exists := s.executionsByKey[key]; exists { return nil, fmt.Errorf( "%w: event %s action %q", eventrule.ErrExecutionAlreadyExists, - eventID, + event.ID, item.ActionName, ) } + execution, err := eventrule.NewExecution( - eventID, + event.ID, item.ActionName, item.ExecutionPlan, now, @@ -104,29 +100,24 @@ func (s *Store) CommitEventPlan( if err != nil { return nil, fmt.Errorf("create action %q execution: %w", item.ActionName, err) } + persisted, err := converterdao.EventActionExecutionTo(execution) if err != nil { return nil, fmt.Errorf("convert action %q execution: %w", item.ActionName, err) } + records[i] = *persisted } - event.PlannedAt = &now persistedEvent, err := converterdao.EventTo(event) if err != nil { return nil, err } - executions := make([]eventrule.Execution, len(records)) - for i := range records { - execution, err := converterdao.EventActionExecutionFrom(&records[i]) - if err != nil { - return nil, err - } - executions[i] = *execution - } // All validation and conversion completes before changing store state so // the following writes model one database transaction. + s.events[event.ID] = *persistedEvent + s.eventsByKey[event.Key] = event.ID for i := range records { record := records[i] s.executions[record.ID] = &memoryExecution{persisted: record} @@ -135,28 +126,181 @@ func (s *Store) CommitEventPlan( ActionName: record.ActionName, }] = record.ID } - s.events[eventID] = *persistedEvent - return executions, nil + return s.event(event.ID) +} + +func validateEventPlan( + event eventrule.Event, + planned []eventrule.PlannedExecution, +) error { + if len(planned) != len(event.EffectivePolicy.Actions) { + return fmt.Errorf( + "event plan has %d executions for %d applicable actions", + len(planned), + len(event.EffectivePolicy.Actions), + ) + } + + for i, action := range event.EffectivePolicy.Actions { + if planned[i].ActionName != action.Name { + return fmt.Errorf( + "planned executions[%d] has action name %q, want %q", + i, + planned[i].ActionName, + action.Name, + ) + } + + if planned[i].ExecutionPlan.Type() != action.Spec.Type() { + return fmt.Errorf( + "planned executions[%d] has type %q, want %q", + i, + planned[i].ExecutionPlan.Type(), + action.Spec.Type(), + ) + } + } + + return nil +} + +// ClaimPendingExecutions atomically allocates pending attempts. +func (s *Store) ClaimPendingExecutions( + ctx context.Context, + request eventrule.ExecutionClaimRequest, +) ([]eventrule.ClaimedExecution, error) { + return s.claimExecutions(ctx, request, pendingExecutionClaim) +} + +// ClaimRetryExecutions atomically allocates due deferred attempts. +func (s *Store) ClaimRetryExecutions( + ctx context.Context, + request eventrule.ExecutionClaimRequest, +) ([]eventrule.ClaimedExecution, error) { + return s.claimExecutions(ctx, request, retryExecutionClaim) +} + +func (s *Store) claimExecutions( + ctx context.Context, + request eventrule.ExecutionClaimRequest, + kind executionClaimKind, +) ([]eventrule.ClaimedExecution, error) { + if err := ctx.Err(); err != nil { + return nil, err + } + if err := request.Validate(); err != nil { + return nil, err + } + + s.mu.Lock() + defer s.mu.Unlock() + + now := s.now().UTC() + eligible := make([]eventrule.Execution, 0, len(s.executions)) + for id := range s.executions { + execution, err := s.execution(id) + if err != nil { + return nil, err + } + + if executionEligible(*execution, kind, now) { + eligible = append(eligible, *execution) + } + } + + slices.SortFunc(eligible, func(a, b eventrule.Execution) int { + if kind == retryExecutionClaim { + if order := a.NextAttemptAt.Compare(b.NextAttemptAt); order != 0 { + return order + } + } else if order := a.CreatedAt.Compare(b.CreatedAt); order != 0 { + return order + } + + return cmp.Compare(a.ID.String(), b.ID.String()) + }) + + if len(eligible) > request.Limit { + eligible = eligible[:request.Limit] + } + + type update struct { + id uuid.UUID + persisted dbmodel.EventActionExecution + claim eventrule.ClaimedExecution + } + updates := make([]update, len(eligible)) + for i := range eligible { + execution := eligible[i].Clone() + token := uuid.New() + + if err := execution.Claim(request.Owner, token, now); err != nil { + return nil, err + } + + persisted, err := converterdao.EventActionExecutionTo(&execution) + if err != nil { + return nil, err + } + + updates[i] = update{ + id: execution.ID, + persisted: *persisted, + claim: eventrule.ClaimedExecution{ + Execution: execution, + Token: token, + }, + } + } + + claims := make([]eventrule.ClaimedExecution, len(updates)) + for i, update := range updates { + s.executions[update.id].persisted = update.persisted + claims[i] = update.claim + } + + return claims, nil +} + +func executionEligible( + execution eventrule.Execution, + kind executionClaimKind, + now time.Time, +) bool { + switch kind { + case pendingExecutionClaim: + return execution.Status == eventrule.ExecutionStatusPending + case retryExecutionClaim: + return execution.RetryDue(now) + default: + return false + } } -// TransitionExecution atomically persists an attempt result. -func (s *Store) TransitionExecution( - _ context.Context, +// TransitionClaimedExecution atomically persists an owned attempt result. +func (s *Store) TransitionClaimedExecution( + ctx context.Context, id uuid.UUID, + token uuid.UUID, result eventrule.ExecutionResult, ) error { + if err := ctx.Err(); err != nil { + return err + } + s.mu.Lock() defer s.mu.Unlock() - now := s.now().UTC() execution, err := s.execution(id) if err != nil { return err } - if err := execution.TransitionTo(result, now); err != nil { + + if err := execution.TransitionClaimedTo(token, result, s.now().UTC()); err != nil { return err } + if err := s.setExecution(execution); err != nil { return err } @@ -176,6 +320,7 @@ func (s *Store) Executions() ([]eventrule.Execution, error) { if err != nil { return nil, err } + executions = append(executions, *execution) } @@ -205,6 +350,7 @@ func (s *Store) setExecution(execution *eventrule.Execution) error { if err != nil { return err } + record.persisted = *persisted return nil diff --git a/rest-api/flow/internal/eventrule/store/memory/execution_task_test.go b/rest-api/flow/internal/eventrule/store/memory/execution_task_test.go index 7b542e7bc4..b460387838 100644 --- a/rest-api/flow/internal/eventrule/store/memory/execution_task_test.go +++ b/rest-api/flow/internal/eventrule/store/memory/execution_task_test.go @@ -55,6 +55,7 @@ func TestStore_CreateExecutionTask(t *testing.T) { require.Equal(t, requested, *created) created.TaskID = uuid.New() + loaded, err := store.GetExecutionTask( context.Background(), requested.ExecutionID, @@ -65,6 +66,7 @@ func TestStore_CreateExecutionTask(t *testing.T) { duplicate := requested duplicate.TaskID = uuid.New() + existing, err := store.CreateExecutionTask(context.Background(), duplicate) require.NoError(t, err) require.Equal(t, requested, *existing) @@ -98,6 +100,7 @@ func TestStore_GetExecutionTask(t *testing.T) { test.executionID, test.rackID, ) + if test.wantErr != "" { require.ErrorContains(t, err, test.wantErr) require.Nil(t, association) @@ -113,7 +116,7 @@ func TestStore_GetExecutionTask(t *testing.T) { func createExecution(t *testing.T, store *Store) eventrule.Execution { t.Helper() - event, err := store.CreateEvent(context.Background(), eventrule.Event{ + definition := eventrule.Event{ Key: eventrule.EventKey{ SourceName: "test", SourceKey: uuid.NewString(), @@ -128,12 +131,11 @@ func createExecution(t *testing.T, store *Store) eventrule.Execution { {Name: "noop", Spec: &eventrule.Noop{}}, }}, Summary: "test event", - }) - require.NoError(t, err) + } - executions, err := store.CommitEventPlan( + event, err := store.CommitEventPlan( context.Background(), - event.ID, + definition, []eventrule.PlannedExecution{ { ActionName: "noop", @@ -142,6 +144,10 @@ func createExecution(t *testing.T, store *Store) eventrule.Execution { }, ) require.NoError(t, err) + require.NotNil(t, event) + + executions, err := store.Executions() + require.NoError(t, err) require.Len(t, executions, 1) return executions[0] diff --git a/rest-api/flow/internal/eventrule/store/memory/manager_integration_test.go b/rest-api/flow/internal/eventrule/store/memory/manager_integration_test.go index d33ae3838f..5e0128a0ff 100644 --- a/rest-api/flow/internal/eventrule/store/memory/manager_integration_test.go +++ b/rest-api/flow/internal/eventrule/store/memory/manager_integration_test.go @@ -10,6 +10,7 @@ import ( "github.com/NVIDIA/infra-controller/rest-api/flow/internal/eventrule" "github.com/NVIDIA/infra-controller/rest-api/flow/internal/eventrule/leakage" "github.com/NVIDIA/infra-controller/rest-api/flow/internal/eventrule/manager" + eventscheduler "github.com/NVIDIA/infra-controller/rest-api/flow/internal/eventrule/scheduler" "github.com/NVIDIA/infra-controller/rest-api/flow/internal/operation" identifier "github.com/NVIDIA/infra-controller/rest-api/flow/pkg/common/Identifier" "github.com/NVIDIA/infra-controller/rest-api/flow/pkg/inventoryobjects/component" @@ -26,6 +27,11 @@ func TestManagerIntegration(t *testing.T) { Store: manager.StoreConfig{ Backend: manager.StoreBackendMemory, }, + Scheduler: manager.SchedulerConfig{ + InstanceID: "memory-manager-integration-test", + Runtime: eventscheduler.DefaultRuntimeConfig(), + Policy: eventscheduler.DefaultPolicyConfig(), + }, Inventory: integrationInventory{}, TaskManager: integrationTaskManager{}, }) diff --git a/rest-api/flow/internal/eventrule/store/memory/store.go b/rest-api/flow/internal/eventrule/store/memory/store.go index 0f4a3c7eff..61dfb1013a 100644 --- a/rest-api/flow/internal/eventrule/store/memory/store.go +++ b/rest-api/flow/internal/eventrule/store/memory/store.go @@ -52,7 +52,7 @@ func NewWithClock(now func() time.Time) *Store { var ( _ eventrule.RuleStore = (*Store)(nil) _ eventrule.BindingStore = (*Store)(nil) - _ eventrule.EventStore = (*Store)(nil) + _ eventrule.EventPlanStore = (*Store)(nil) _ eventrule.ExecutionStore = (*Store)(nil) _ eventrule.ExecutionTaskStore = (*Store)(nil) ) diff --git a/rest-api/flow/internal/eventrule/store/memory/store_test.go b/rest-api/flow/internal/eventrule/store/memory/store_test.go index 5d3224cd4c..4b32b30a4c 100644 --- a/rest-api/flow/internal/eventrule/store/memory/store_test.go +++ b/rest-api/flow/internal/eventrule/store/memory/store_test.go @@ -18,8 +18,10 @@ import ( func TestStoreContract(t *testing.T) { storetest.RunRuleBindingContract(t, func() (eventrule.RuleStore, eventrule.BindingStore) { store := New() + return store, store }) + storetest.RunExecutionContract(t, func(now *time.Time) storetest.EventExecutionStore { return NewWithClock(func() time.Time { return *now }) }) @@ -62,7 +64,7 @@ func TestBindingScansIgnoreUnrelatedInvalidRecords(t *testing.T) { require.NoError(t, store.Delete(ctx, rule.ID)) } -func TestStore_CreateEventRejectsDanglingIndexes(t *testing.T) { +func TestStore_CommitEventPlanRejectsDanglingIndexes(t *testing.T) { now := time.Date(2026, 8, 7, 12, 0, 0, 0, time.UTC) store := NewWithClock(func() time.Time { return now }) definition := eventrule.Event{ @@ -75,9 +77,14 @@ func TestStore_CreateEventRejectsDanglingIndexes(t *testing.T) { }}, Summary: "Test event", } + store.eventsByKey[definition.Key] = uuid.New() - event, err := store.CreateEvent(context.Background(), definition) + event, err := store.CommitEventPlan(context.Background(), definition, []eventrule.PlannedExecution{{ + ActionName: "action", + ExecutionPlan: &eventrule.NoopPlan{}, + }}) + require.ErrorIs(t, err, eventrule.ErrEventNotFound) require.Nil(t, event) } diff --git a/rest-api/flow/internal/eventrule/store/storetest/execution_contract.go b/rest-api/flow/internal/eventrule/store/storetest/execution_contract.go index 73941a4ab8..214a92ed1b 100644 --- a/rest-api/flow/internal/eventrule/store/storetest/execution_contract.go +++ b/rest-api/flow/internal/eventrule/store/storetest/execution_contract.go @@ -18,7 +18,7 @@ import ( // EventExecutionStore is the combined persistence boundary exercised by this // contract. type EventExecutionStore interface { - eventrule.EventStore + eventrule.EventPlanStore eventrule.ExecutionStore } @@ -26,65 +26,170 @@ type EventExecutionStore interface { // the supplied time. type ExecutionFactory func(*time.Time) EventExecutionStore -// RunExecutionContract executes the shared durable event and execution-plan +// RunExecutionContract executes the shared durable event, plan, and claim // contract. func RunExecutionContract(t *testing.T, factory ExecutionFactory) { t.Helper() - t.Run("event lifecycle", func(t *testing.T) { testEventLifecycle(t, factory) }) + + t.Run("atomic event lifecycle", func(t *testing.T) { testAtomicEventLifecycle(t, factory) }) t.Run("concurrent event deduplication", func(t *testing.T) { testConcurrentEventDeduplication(t, factory) }) - t.Run("execution planning and transition", func(t *testing.T) { - testExecutionLifecycle(t, factory) + t.Run("execution claim lifecycle", func(t *testing.T) { + testExecutionClaimLifecycle(t, factory) + }) + t.Run("interrupted attempt refund", func(t *testing.T) { + testInterruptedAttemptRefund(t, factory) + }) + t.Run("execution claim limit", func(t *testing.T) { + testExecutionClaimLimit(t, factory) }) t.Run("ordered atomic plan commit", func(t *testing.T) { testOrderedAtomicPlanCommit(t, factory) }) + t.Run("concurrent claim fencing", func(t *testing.T) { + testConcurrentClaimFencing(t, factory) + }) +} + +func testInterruptedAttemptRefund(t *testing.T, factory ExecutionFactory) { + t.Helper() + + ctx := context.Background() + now := time.Date(2026, 8, 21, 12, 0, 0, 0, time.UTC) + store := factory(&now) + _, err := store.CommitEventPlan(ctx, newEventDefinition(), newEventPlan()) + require.NoError(t, err) + + claims, err := store.ClaimPendingExecutions(ctx, eventrule.ExecutionClaimRequest{ + Owner: "scheduler-1", + Limit: 1, + }) + require.NoError(t, err) + require.Len(t, claims, 1) + + first := claims[0] + require.Equal(t, 1, first.Execution.Attempts) + + require.NoError(t, store.TransitionClaimedExecution( + ctx, + first.Execution.ID, + first.Token, + eventrule.DeferredExecutionResult( + eventrule.ExecutionReasonAttemptInterrupted, + context.Canceled.Error(), + time.Minute, + ), + )) + + now = now.Add(time.Minute) + claims, err = store.ClaimRetryExecutions(ctx, eventrule.ExecutionClaimRequest{ + Owner: "scheduler-2", + Limit: 1, + }) + require.NoError(t, err) + require.Len(t, claims, 1) + + second := claims[0] + // The interrupted first claim was refunded before this claim allocated the + // next attempt. + require.Equal(t, 1, second.Execution.Attempts) +} + +func testExecutionClaimLimit(t *testing.T, factory ExecutionFactory) { + t.Helper() + + ctx := context.Background() + now := time.Date(2026, 8, 21, 12, 0, 0, 0, time.UTC) + store := factory(&now) + definition := newEventDefinition() + definition.EffectivePolicy.Actions = append( + definition.EffectivePolicy.Actions, + eventrule.Action{Name: "archive", Spec: &eventrule.Noop{Reason: "test"}}, + ) + plan := append( + newEventPlan(), + eventrule.PlannedExecution{ + ActionName: "archive", + ExecutionPlan: &eventrule.NoopPlan{Reason: "test"}, + }, + ) + + created, err := store.CommitEventPlan(ctx, definition, plan) + require.NoError(t, err) + require.NotNil(t, created) + + first, err := store.ClaimPendingExecutions(ctx, eventrule.ExecutionClaimRequest{ + Owner: "scheduler-1", + Limit: 1, + }) + require.NoError(t, err) + require.Len(t, first, 1) + requireValidClaims(t, first, "scheduler-1") + + second, err := store.ClaimPendingExecutions(ctx, eventrule.ExecutionClaimRequest{ + Owner: "scheduler-1", + Limit: 1, + }) + require.NoError(t, err) + require.Len(t, second, 1) + requireValidClaims(t, second, "scheduler-1") + require.NotEqual(t, first[0].Execution.ID, second[0].Execution.ID) } -func testEventLifecycle(t *testing.T, factory ExecutionFactory) { +func testAtomicEventLifecycle(t *testing.T, factory ExecutionFactory) { t.Helper() + ctx := context.Background() now := time.Date(2026, 8, 21, 12, 0, 0, 0, time.UTC) store := factory(&now) definition := newEventDefinition() + plan := newEventPlan() missing, err := store.ObserveEvent(ctx, definition.Key) require.NoError(t, err) require.Nil(t, missing) - created, err := store.CreateEvent(ctx, definition) + created, err := store.CommitEventPlan(ctx, definition, plan) require.NoError(t, err) require.NotNil(t, created) require.Equal(t, 1, created.Observations) - require.Nil(t, created.PlannedAt) + require.Equal(t, now, created.CreatedAt) now = now.Add(time.Second) + duplicate, err := store.CommitEventPlan(ctx, definition, plan) + require.NoError(t, err) + require.Nil(t, duplicate) + observed, err := store.ObserveEvent(ctx, definition.Key) require.NoError(t, err) require.Equal(t, created.ID, observed.ID) - require.Equal(t, 2, observed.Observations) + require.Equal(t, 3, observed.Observations) require.Equal(t, now, observed.LastObservedAt) - plan := []eventrule.PlannedExecution{{ - ActionName: "notify", - ExecutionPlan: &eventrule.NoopPlan{Reason: "test"}, - }} - committed, err := store.CommitEventPlan(ctx, created.ID, plan) + empty := newEventDefinition() + empty.EffectivePolicy = eventrule.Policy{} + created, err = store.CommitEventPlan(ctx, empty, nil) require.NoError(t, err) - require.Len(t, committed, 1) - planned, err := store.ObserveEvent(ctx, definition.Key) + require.NotNil(t, created) + + claims, err := store.ClaimPendingExecutions(ctx, eventrule.ExecutionClaimRequest{ + Owner: "scheduler-1", + Limit: 10, + }) require.NoError(t, err) - require.Equal(t, now, *planned.PlannedAt) - _, err = store.CommitEventPlan(ctx, created.ID, plan) - require.ErrorIs(t, err, eventrule.ErrEventAlreadyPlanned) + require.Len(t, claims, 1, "only the first event has a dispatchable execution") + requireValidClaims(t, claims, "scheduler-1") } func testConcurrentEventDeduplication(t *testing.T, factory ExecutionFactory) { t.Helper() + now := time.Date(2026, 8, 21, 12, 0, 0, 0, time.UTC) store := factory(&now) definition := newEventDefinition() + plan := newEventPlan() + const deliveries = 20 var wg sync.WaitGroup @@ -94,7 +199,9 @@ func testConcurrentEventDeduplication(t *testing.T, factory ExecutionFactory) { wg.Add(1) go func() { defer wg.Done() - created, err := store.CreateEvent(context.Background(), definition) + + created, err := store.CommitEventPlan(context.Background(), definition, plan) + results <- created errs <- err }() @@ -102,6 +209,7 @@ func testConcurrentEventDeduplication(t *testing.T, factory ExecutionFactory) { wg.Wait() close(results) close(errs) + insertions := 0 for created := range results { if created != nil { @@ -111,49 +219,56 @@ func testConcurrentEventDeduplication(t *testing.T, factory ExecutionFactory) { for err := range errs { require.NoError(t, err) } + require.Equal(t, 1, insertions) + stored, err := store.ObserveEvent(context.Background(), definition.Key) require.NoError(t, err) require.Equal(t, deliveries+1, stored.Observations) + + claims, err := store.ClaimPendingExecutions(context.Background(), eventrule.ExecutionClaimRequest{ + Owner: "scheduler-1", + Limit: deliveries, + }) + require.NoError(t, err) + require.Len(t, claims, 1) + requireValidClaims(t, claims, "scheduler-1") } -func testExecutionLifecycle(t *testing.T, factory ExecutionFactory) { +func testExecutionClaimLifecycle(t *testing.T, factory ExecutionFactory) { t.Helper() + ctx := context.Background() now := time.Date(2026, 8, 21, 12, 0, 0, 0, time.UTC) store := factory(&now) - event, err := store.CreateEvent(ctx, newEventDefinition()) + _, err := store.CommitEventPlan(ctx, newEventDefinition(), newEventPlan()) require.NoError(t, err) - require.NotNil(t, event) - committed, err := store.CommitEventPlan( - ctx, - event.ID, - []eventrule.PlannedExecution{{ - ActionName: "notify", - ExecutionPlan: &eventrule.NoopPlan{Reason: "test"}, - }}, - ) + pending, err := store.ClaimPendingExecutions(ctx, eventrule.ExecutionClaimRequest{ + Owner: "scheduler-1", + Limit: 1, + }) require.NoError(t, err) - require.Len(t, committed, 1) - created := committed[0] - require.Equal(t, eventrule.ExecutionStatusPending, created.Status) - require.Zero(t, created.Attempts) + require.Len(t, pending, 1) + requireValidClaims(t, pending, "scheduler-1") - _, err = store.CommitEventPlan( - ctx, - event.ID, - []eventrule.PlannedExecution{{ - ActionName: "notify", - ExecutionPlan: &eventrule.NoopPlan{Reason: "different plan"}, - }}, - ) - require.ErrorIs(t, err, eventrule.ErrEventAlreadyPlanned) + first := pending[0] + require.Equal(t, eventrule.ExecutionStatusRunning, first.Execution.Status) + require.Equal(t, 1, first.Execution.Attempts) + require.NoError(t, first.Validate()) - now = now.Add(time.Second) - err = store.TransitionExecution( + none, err := store.ClaimPendingExecutions(ctx, eventrule.ExecutionClaimRequest{ + Owner: "scheduler-2", + Limit: 1, + }) + require.NoError(t, err) + require.Empty(t, none) + requireValidClaims(t, none, "scheduler-2") + + err = store.TransitionClaimedExecution( ctx, - created.ID, + first.Execution.ID, + first.Token, eventrule.DeferredExecutionResult( eventrule.ExecutionReasonAttemptFailed, "temporarily unavailable", @@ -162,23 +277,46 @@ func testExecutionLifecycle(t *testing.T, factory ExecutionFactory) { ) require.NoError(t, err) - now = now.Add(time.Second) - err = store.TransitionExecution( + none, err = store.ClaimRetryExecutions(ctx, eventrule.ExecutionClaimRequest{ + Owner: "scheduler-2", + Limit: 1, + }) + require.NoError(t, err) + require.Empty(t, none) + requireValidClaims(t, none, "scheduler-2") + + now = now.Add(time.Minute) + deferred, err := store.ClaimRetryExecutions(ctx, eventrule.ExecutionClaimRequest{ + Owner: "scheduler-2", + Limit: 1, + }) + require.NoError(t, err) + require.Len(t, deferred, 1) + requireValidClaims(t, deferred, "scheduler-2") + + second := deferred[0] + require.Equal(t, 2, second.Execution.Attempts) + require.NotEqual(t, first.Token, second.Token) + + err = store.TransitionClaimedExecution( ctx, - created.ID, + second.Execution.ID, + first.Token, eventrule.CompletedExecutionResult(), ) - require.NoError(t, err) - err = store.TransitionExecution( + require.ErrorIs(t, err, eventrule.ErrExecutionClaimLost) + + require.NoError(t, store.TransitionClaimedExecution( ctx, - created.ID, - eventrule.FailedExecutionResult("late failure"), - ) - require.Error(t, err) + second.Execution.ID, + second.Token, + eventrule.CompletedExecutionResult(), + )) } func testOrderedAtomicPlanCommit(t *testing.T, factory ExecutionFactory) { t.Helper() + ctx := context.Background() now := time.Date(2026, 8, 21, 12, 0, 0, 0, time.UTC) store := factory(&now) @@ -187,28 +325,111 @@ func testOrderedAtomicPlanCommit(t *testing.T, factory ExecutionFactory) { definition.EffectivePolicy.Actions, eventrule.Action{Name: "archive", Spec: &eventrule.Noop{Reason: "test"}}, ) - event, err := store.CreateEvent(ctx, definition) - require.NoError(t, err) reversed := []eventrule.PlannedExecution{ {ActionName: "archive", ExecutionPlan: &eventrule.NoopPlan{Reason: "test"}}, {ActionName: "notify", ExecutionPlan: &eventrule.NoopPlan{Reason: "test"}}, } - _, err = store.CommitEventPlan(ctx, event.ID, reversed) + created, err := store.CommitEventPlan(ctx, definition, reversed) require.ErrorContains(t, err, `action name "archive", want "notify"`) + require.Nil(t, created) + + missing, err := store.ObserveEvent(ctx, definition.Key) + require.NoError(t, err) + require.Nil(t, missing) planned := []eventrule.PlannedExecution{ {ActionName: "notify", ExecutionPlan: &eventrule.NoopPlan{Reason: "test"}}, {ActionName: "archive", ExecutionPlan: &eventrule.NoopPlan{Reason: "test"}}, } - executions, err := store.CommitEventPlan(ctx, event.ID, planned) + created, err = store.CommitEventPlan(ctx, definition, planned) + require.NoError(t, err) + require.NotNil(t, created) + + claims, err := store.ClaimPendingExecutions(ctx, eventrule.ExecutionClaimRequest{ + Owner: "scheduler-1", + Limit: 2, + }) require.NoError(t, err) - require.Equal(t, []string{"notify", "archive"}, []string{ - executions[0].ActionName, - executions[1].ActionName, + require.Len(t, claims, 2) + requireValidClaims(t, claims, "scheduler-1") + require.ElementsMatch(t, []string{"notify", "archive"}, []string{ + claims[0].Execution.ActionName, + claims[1].Execution.ActionName, }) } +func testConcurrentClaimFencing(t *testing.T, factory ExecutionFactory) { + t.Helper() + + now := time.Date(2026, 8, 21, 12, 0, 0, 0, time.UTC) + store := factory(&now) + _, err := store.CommitEventPlan(context.Background(), newEventDefinition(), newEventPlan()) + require.NoError(t, err) + + const schedulers = 20 + + var wg sync.WaitGroup + type result struct { + owner string + claims []eventrule.ClaimedExecution + } + results := make(chan result, schedulers) + errs := make(chan error, schedulers) + for range schedulers { + wg.Add(1) + go func() { + defer wg.Done() + + owner := "scheduler-" + uuid.NewString() + claims, err := store.ClaimPendingExecutions( + context.Background(), + eventrule.ExecutionClaimRequest{ + Owner: owner, + Limit: 1, + }, + ) + + results <- result{owner: owner, claims: claims} + errs <- err + }() + } + wg.Wait() + close(results) + close(errs) + + claimed := 0 + for result := range results { + requireValidClaims(t, result.claims, result.owner) + claimed += len(result.claims) + } + for err := range errs { + require.NoError(t, err) + } + + require.Equal(t, 1, claimed) +} + +func requireValidClaims( + t *testing.T, + claims []eventrule.ClaimedExecution, + owner string, +) { + t.Helper() + + for i := range claims { + require.NoError(t, claims[i].Validate(), "claims[%d]", i) + require.Equal(t, owner, claims[i].Execution.ClaimOwner, "claims[%d]", i) + } +} + +func newEventPlan() []eventrule.PlannedExecution { + return []eventrule.PlannedExecution{{ + ActionName: "notify", + ExecutionPlan: &eventrule.NoopPlan{Reason: "test"}, + }} +} + func newEventDefinition() eventrule.Event { return eventrule.Event{ Key: eventrule.EventKey{SourceName: "test", SourceKey: uuid.NewString()}, diff --git a/rest-api/flow/internal/eventrule/store_test.go b/rest-api/flow/internal/eventrule/store_test.go index cb822a732b..dde0ea6dfc 100644 --- a/rest-api/flow/internal/eventrule/store_test.go +++ b/rest-api/flow/internal/eventrule/store_test.go @@ -4,11 +4,102 @@ package eventrule import ( + "strings" "testing" + "time" + "github.com/google/uuid" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) +func TestExecutionClaimRequest_Validate(t *testing.T) { + tests := map[string]struct { + request ExecutionClaimRequest + wantErr string + }{ + "valid": { + request: ExecutionClaimRequest{Owner: "scheduler-1", Limit: 1}, + }, + "missing owner": { + request: ExecutionClaimRequest{Limit: 1}, + wantErr: "execution claim owner is empty", + }, + "owner too long": { + request: ExecutionClaimRequest{ + Owner: strings.Repeat("x", maxExecutionClaimOwnerRunes+1), + Limit: 1, + }, + wantErr: "execution claim owner exceeds 128 characters", + }, + "invalid limit": { + request: ExecutionClaimRequest{Owner: "scheduler-1"}, + wantErr: "execution claim limit must be positive", + }, + } + + for name, test := range tests { + t.Run(name, func(t *testing.T) { + err := test.request.Validate() + if test.wantErr == "" { + require.NoError(t, err) + return + } + + require.EqualError(t, err, test.wantErr) + }) + } +} + +func TestClaimedExecution_Validate(t *testing.T) { + createdAt := time.Date(2026, 8, 24, 12, 0, 0, 0, time.UTC) + execution, err := NewExecution(uuid.New(), "action", &NoopPlan{}, createdAt) + require.NoError(t, err) + + token := uuid.New() + require.NoError(t, execution.Claim("scheduler-1", token, createdAt)) + + valid := ClaimedExecution{Execution: *execution, Token: token} + + tests := map[string]struct { + mutate func(*ClaimedExecution) + wantErr string + }{ + "valid": {}, + "invalid execution": { + mutate: func(claim *ClaimedExecution) { claim.Execution.ID = uuid.Nil }, + wantErr: "execution: execution id is required", + }, + "missing token": { + mutate: func(claim *ClaimedExecution) { claim.Token = uuid.Nil }, + wantErr: "execution claim token is required", + }, + "mismatched token": { + mutate: func(claim *ClaimedExecution) { claim.Token = uuid.New() }, + wantErr: "execution claim token does not match running execution", + }, + } + + for name, test := range tests { + t.Run(name, func(t *testing.T) { + claim := valid + claim.Execution = valid.Execution.Clone() + + if test.mutate != nil { + test.mutate(&claim) + } + + err := claim.Validate() + if test.wantErr == "" { + require.NoError(t, err) + return + } + + require.ErrorContains(t, err, test.wantErr) + }) + } +} + func TestRuleFilterMatches(t *testing.T) { eventType := Type("test.event") otherEventType := Type("other.event")