fix: make target progress reflect fresh state
This commit is contained in:
+41
-16
@@ -76,12 +76,13 @@ type ProgressiveModule interface {
|
||||
}
|
||||
|
||||
type Application struct {
|
||||
store profile.Store
|
||||
state profile.State
|
||||
targets []target.Target
|
||||
inspections map[string]target.Inspection
|
||||
mutationLock MutationLock
|
||||
logger OperationLogger
|
||||
store profile.Store
|
||||
state profile.State
|
||||
targets []target.Target
|
||||
availabilities map[string]target.Availability
|
||||
inspections map[string]target.Inspection
|
||||
mutationLock MutationLock
|
||||
logger OperationLogger
|
||||
}
|
||||
|
||||
func New(store profile.Store) (*Application, error) {
|
||||
@@ -105,7 +106,15 @@ func NewWithTargetsAndRuntime(store profile.Store, targets []target.Target, muta
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
a := &Application{store: store, state: state, targets: append([]target.Target(nil), targets...), inspections: make(map[string]target.Inspection), mutationLock: mutationLock, logger: logger}
|
||||
a := &Application{
|
||||
store: store,
|
||||
state: state,
|
||||
targets: append([]target.Target(nil), targets...),
|
||||
availabilities: make(map[string]target.Availability),
|
||||
inspections: make(map[string]target.Inspection),
|
||||
mutationLock: mutationLock,
|
||||
logger: logger,
|
||||
}
|
||||
a.refreshTargets()
|
||||
return a, nil
|
||||
}
|
||||
@@ -114,7 +123,7 @@ func (a *Application) Snapshot() Snapshot {
|
||||
profiles := append([]profile.Profile(nil), a.state.Profiles...)
|
||||
snapshot := Snapshot{Profiles: profiles, ActiveProfile: a.state.ActiveProfile}
|
||||
for _, adapter := range a.targets {
|
||||
availability := adapter.Availability(context.Background())
|
||||
availability := a.availabilities[adapter.ID()]
|
||||
inspection := a.inspections[adapter.ID()]
|
||||
snapshot.Targets = append(snapshot.Targets, TargetSnapshot{ID: adapter.ID(), Description: adapter.Description(), Available: availability.Available, AvailabilityEvidence: availability.Evidence, State: inspection.State, Evidence: inspection.Evidence})
|
||||
}
|
||||
@@ -171,7 +180,7 @@ func (a *Application) dispatch(intent Intent, publish func(Result)) Result {
|
||||
})
|
||||
if err == nil {
|
||||
a.state = next
|
||||
a.refreshTargets()
|
||||
a.refreshInspections()
|
||||
} else {
|
||||
var committed *profile.CommitError
|
||||
if errors.As(err, &committed) {
|
||||
@@ -243,18 +252,22 @@ func (a *Application) activateProfile(intent Intent, publish func(Result)) Resul
|
||||
publish(Result{Snapshot: a.Snapshot(), Progress: []TargetProgress{{TargetID: adapter.ID(), Applying: true, State: a.inspections[adapter.ID()].State}}, Notice: "Updating " + adapter.Description().Name + " Target"})
|
||||
}
|
||||
operation := adapter.SetDesired(context.Background(), true, active)
|
||||
a.inspections[adapter.ID()] = operation.Inspection
|
||||
inspection := operation.Inspection
|
||||
var safeErr error
|
||||
if operation.Err != nil {
|
||||
safeErr = errors.New(profile.RedactText(operation.Err.Error()))
|
||||
// A failure-returned inspection may predate a partial write. Always
|
||||
// inspect explicitly against the new Active Profile before publishing.
|
||||
inspection = adapter.Inspect(context.Background(), active)
|
||||
safeErr = fmt.Errorf("%s (fresh inspection: %s — %s)", profile.RedactText(operation.Err.Error()), inspection.State, profile.RedactText(inspection.Evidence))
|
||||
}
|
||||
item := TargetProgress{TargetID: adapter.ID(), State: operation.Inspection.State, Evidence: operation.Inspection.Evidence, Err: safeErr}
|
||||
a.inspections[adapter.ID()] = inspection
|
||||
item := TargetProgress{TargetID: adapter.ID(), State: inspection.State, Evidence: inspection.Evidence, Err: safeErr}
|
||||
progress = append(progress, item)
|
||||
if publish != nil {
|
||||
publish(Result{Snapshot: a.Snapshot(), Progress: []TargetProgress{item}, Notice: adapter.Description().Name + " Target update completed", Err: safeErr})
|
||||
}
|
||||
if a.logger != nil {
|
||||
outcome, detail := "success", operation.Inspection.Evidence
|
||||
outcome, detail := "success", inspection.Evidence
|
||||
if safeErr != nil {
|
||||
outcome, detail = "failure", safeErr.Error()
|
||||
}
|
||||
@@ -294,6 +307,14 @@ func (a *Application) activeProfile() *profile.Profile {
|
||||
}
|
||||
|
||||
func (a *Application) refreshTargets() {
|
||||
active := a.activeProfile()
|
||||
for _, adapter := range a.targets {
|
||||
a.availabilities[adapter.ID()] = adapter.Availability(context.Background())
|
||||
a.inspections[adapter.ID()] = adapter.Inspect(context.Background(), active)
|
||||
}
|
||||
}
|
||||
|
||||
func (a *Application) refreshInspections() {
|
||||
active := a.activeProfile()
|
||||
for _, adapter := range a.targets {
|
||||
a.inspections[adapter.ID()] = adapter.Inspect(context.Background(), active)
|
||||
@@ -319,8 +340,12 @@ func (a *Application) setTargetDesired(intent Intent) Result {
|
||||
return Result{Snapshot: a.Snapshot(), Confirmation: warning}
|
||||
}
|
||||
result := selected.SetDesired(context.Background(), intent.Enabled, active)
|
||||
a.inspections[selected.ID()] = result.Inspection
|
||||
outcome, detail := "success", result.Inspection.Evidence
|
||||
inspection := result.Inspection
|
||||
if result.Err != nil {
|
||||
inspection = selected.Inspect(context.Background(), active)
|
||||
}
|
||||
a.inspections[selected.ID()] = inspection
|
||||
outcome, detail := "success", inspection.Evidence
|
||||
if result.Err != nil {
|
||||
outcome, detail = "failure", result.Err.Error()
|
||||
}
|
||||
@@ -333,7 +358,7 @@ func (a *Application) setTargetDesired(intent Intent) Result {
|
||||
}
|
||||
notice := ""
|
||||
if result.Err == nil {
|
||||
notice = fmt.Sprintf("%s Target is now %s", selected.Description().Name, result.Inspection.State)
|
||||
notice = fmt.Sprintf("%s Target is now %s", selected.Description().Name, inspection.State)
|
||||
}
|
||||
return Result{Snapshot: a.Snapshot(), Notice: notice, Err: result.Err}
|
||||
}
|
||||
|
||||
@@ -16,11 +16,13 @@ import (
|
||||
)
|
||||
|
||||
type coordinatedTarget struct {
|
||||
id string
|
||||
state target.State
|
||||
applied []string
|
||||
appliedName string
|
||||
failName string
|
||||
id string
|
||||
state target.State
|
||||
applied []string
|
||||
appliedName string
|
||||
failName string
|
||||
availabilityCalls int
|
||||
inspectCalls int
|
||||
}
|
||||
|
||||
func (t *coordinatedTarget) ID() string { return t.id }
|
||||
@@ -28,13 +30,15 @@ func (t *coordinatedTarget) Description() target.Description {
|
||||
return target.Description{Name: t.id, Summary: "summary", Details: "effects, non-effects, ownership, permissions, and side effects"}
|
||||
}
|
||||
func (t *coordinatedTarget) Availability(context.Context) target.Availability {
|
||||
t.availabilityCalls++
|
||||
return target.Availability{Available: true, Evidence: "detected"}
|
||||
}
|
||||
func (t *coordinatedTarget) Inspect(_ context.Context, active *profile.Profile) target.Inspection {
|
||||
t.inspectCalls++
|
||||
if t.state == target.Enabled && (active == nil || (t.appliedName != "" && active.Name != t.appliedName)) {
|
||||
return target.Inspection{State: target.Drifted, Evidence: "managed configuration does not match the Active Profile"}
|
||||
}
|
||||
return target.Inspection{State: t.state, Evidence: "actual configuration"}
|
||||
return target.Inspection{State: t.state, Evidence: "fresh actual configuration"}
|
||||
}
|
||||
func (t *coordinatedTarget) Confirmation(bool) string { return "" }
|
||||
func (t *coordinatedTarget) SetDesired(_ context.Context, enabled bool, active *profile.Profile) target.OperationResult {
|
||||
@@ -45,7 +49,7 @@ func (t *coordinatedTarget) SetDesired(_ context.Context, enabled bool, active *
|
||||
t.applied = append(t.applied, active.Name)
|
||||
if active.Name == t.failName {
|
||||
t.state = target.Drifted
|
||||
return target.OperationResult{Inspection: target.Inspection{State: t.state, Evidence: "re-inspected partial write"}, Err: errors.New("failed using http://alice:secret@proxy.example")}
|
||||
return target.OperationResult{Inspection: target.Inspection{State: target.Enabled, Evidence: "stale operation state"}, Err: errors.New("failed using http://alice:secret@proxy.example")}
|
||||
}
|
||||
t.state = target.Enabled
|
||||
t.appliedName = active.Name
|
||||
@@ -74,7 +78,15 @@ func TestActiveProfileSwitchCoordinatesEnabledTargetsWithoutRollback(t *testing.
|
||||
}
|
||||
}
|
||||
|
||||
result := application.Dispatch(app.Intent{Kind: app.ActivateProfile, Name: "new"})
|
||||
inspectCallsBeforeSwitch := second.inspectCalls
|
||||
var result app.Result
|
||||
var failedCompletion app.Result
|
||||
for update := range application.DispatchProgress(app.Intent{Kind: app.ActivateProfile, Name: "new"}) {
|
||||
result = update
|
||||
if len(update.Progress) == 1 && update.Progress[0].TargetID == "second" && !update.Progress[0].Applying {
|
||||
failedCompletion = update
|
||||
}
|
||||
}
|
||||
if result.Snapshot.ActiveProfile != "new" {
|
||||
t.Fatalf("Active Profile = %q", result.Snapshot.ActiveProfile)
|
||||
}
|
||||
@@ -84,9 +96,21 @@ func TestActiveProfileSwitchCoordinatesEnabledTargetsWithoutRollback(t *testing.
|
||||
if got := result.Snapshot.Targets; got[0].State != target.Enabled || got[1].State != target.Drifted {
|
||||
t.Fatalf("truthful partial states = %#v", got)
|
||||
}
|
||||
if len(result.Progress) != 2 || result.Progress[0].TargetID != "first" || result.Progress[0].State != target.Enabled || result.Progress[1].TargetID != "second" || result.Progress[1].State != target.Drifted {
|
||||
if len(result.Progress) != 2 || result.Progress[0].TargetID != "first" || result.Progress[0].State != target.Enabled || result.Progress[1].TargetID != "second" || result.Progress[1].State != target.Drifted || result.Progress[1].Evidence != "fresh actual configuration" {
|
||||
t.Fatalf("per-Target progress = %#v", result.Progress)
|
||||
}
|
||||
if len(failedCompletion.Progress) != 1 || failedCompletion.Progress[0].State != target.Drifted || failedCompletion.Progress[0].Evidence != "fresh actual configuration" || failedCompletion.Err == nil {
|
||||
t.Fatalf("failed completion event was not truthful: %#v", failedCompletion)
|
||||
}
|
||||
if failedCompletion.Snapshot.Targets[1].State != target.Drifted || failedCompletion.Snapshot.Targets[1].Evidence != "fresh actual configuration" {
|
||||
t.Fatalf("failed completion snapshot was not truthful: %#v", failedCompletion.Snapshot.Targets[1])
|
||||
}
|
||||
if second.inspectCalls < inspectCallsBeforeSwitch+2 {
|
||||
t.Fatalf("failed SetDesired was not explicitly re-inspected: Inspect calls before=%d after=%d", inspectCallsBeforeSwitch, second.inspectCalls)
|
||||
}
|
||||
if !strings.Contains(result.Err.Error(), "fresh actual configuration") || !strings.Contains(result.Err.Error(), "Drifted") {
|
||||
t.Fatalf("failure summary omitted fresh inspection: %v", result.Err)
|
||||
}
|
||||
if len(first.applied) != 2 || first.applied[1] != "new" {
|
||||
t.Fatalf("successful Target was not updated: %#v", first.applied)
|
||||
}
|
||||
@@ -140,6 +164,41 @@ func TestApplicationCoordinatesMultipleRealAndFakeHermeticAdaptersThroughPartial
|
||||
}
|
||||
}
|
||||
|
||||
func TestActiveProfileProgressReusesAvailabilityCache(t *testing.T) {
|
||||
adapter := &coordinatedTarget{id: "shell", state: target.Disabled}
|
||||
application, err := app.NewWithTargets(profile.NewFileStore(filepath.Join(t.TempDir(), "profiles.json")), []target.Target{adapter})
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
for _, name := range []string{"old", "new"} {
|
||||
if result := application.Dispatch(app.Intent{Kind: app.CreateProfile, Profile: profile.Profile{Name: name, SingleURL: "http://proxy.example:8080"}}); result.Err != nil {
|
||||
t.Fatal(result.Err)
|
||||
}
|
||||
}
|
||||
if result := application.Dispatch(app.Intent{Kind: app.ActivateProfile, Name: "old"}); result.Err != nil {
|
||||
t.Fatal(result.Err)
|
||||
}
|
||||
if result := application.Dispatch(app.Intent{Kind: app.SetTargetDesired, TargetID: "shell", Enabled: true}); result.Err != nil {
|
||||
t.Fatal(result.Err)
|
||||
}
|
||||
|
||||
callsBeforeSwitch := adapter.availabilityCalls
|
||||
var updates int
|
||||
for range application.DispatchProgress(app.Intent{Kind: app.ActivateProfile, Name: "new"}) {
|
||||
updates++
|
||||
}
|
||||
if updates < 3 {
|
||||
t.Fatalf("progress updates = %d", updates)
|
||||
}
|
||||
if adapter.availabilityCalls != callsBeforeSwitch {
|
||||
t.Fatalf("availability probes during progress = %d, want cached count %d", adapter.availabilityCalls, callsBeforeSwitch)
|
||||
}
|
||||
application.Dispatch(app.Intent{Kind: app.RefreshTargets})
|
||||
if adapter.availabilityCalls != callsBeforeSwitch+1 {
|
||||
t.Fatalf("explicit refresh probe count = %d, want %d", adapter.availabilityCalls, callsBeforeSwitch+1)
|
||||
}
|
||||
}
|
||||
|
||||
func TestActiveProfileSwitchStreamsApplyingAndCompletionForEachTarget(t *testing.T) {
|
||||
adapter := &coordinatedTarget{id: "shell", state: target.Disabled}
|
||||
application, err := app.NewWithTargets(profile.NewFileStore(filepath.Join(t.TempDir(), "profiles.json")), []target.Target{adapter})
|
||||
|
||||
@@ -68,7 +68,7 @@ func TestApplicationTargetSeamRequiresActiveProfileAndReturnsTruthfulResults(t *
|
||||
adapter.setErr = errors.New("permission denied")
|
||||
adapter.state = target.Drifted
|
||||
result = application.Dispatch(app.Intent{Kind: app.SetTargetDesired, TargetID: "shell", Enabled: false})
|
||||
if result.Err == nil || result.Snapshot.Targets[0].State != target.Drifted || result.Snapshot.Targets[0].Evidence != "re-inspected actual state" {
|
||||
if result.Err == nil || result.Snapshot.Targets[0].State != target.Drifted || result.Snapshot.Targets[0].Evidence != "actual state" {
|
||||
t.Fatalf("failed operation not truthful: %#v", result)
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user