Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ import (
"github.com/platform-engineering-labs/formae/internal/metastructure/canonicalize"
"github.com/platform-engineering-labs/formae/internal/metastructure/changeset"
"github.com/platform-engineering-labs/formae/internal/metastructure/messages"
"github.com/platform-engineering-labs/formae/internal/metastructure/resource_update"
"github.com/platform-engineering-labs/formae/pkg/model"
"github.com/platform-engineering-labs/formae/pkg/plugin"
)
Expand Down Expand Up @@ -324,9 +323,8 @@ func (c *PluginCoordinator) spawnPluginOperator(req messages.SpawnPluginOperator
// ResourceUpdater sizes its watchdog window from — and the requesting process.
func pluginOperatorEnv(retryConfig model.RetryConfig, requestedBy gen.PID) map[gen.Env]any {
return map[gen.Env]any{
gen.Env("RetryConfig"): retryConfig,
gen.Env("PluginCallTimeout"): resource_update.PluginCallTimeout,
gen.Env("RequestedBy"): requestedBy,
gen.Env("RetryConfig"): retryConfig,
gen.Env("RequestedBy"): requestedBy,
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import (
"ergo.services/ergo/gen"
"ergo.services/ergo/testing/unit"
"github.com/platform-engineering-labs/formae/internal/metastructure/messages"
"github.com/platform-engineering-labs/formae/internal/metastructure/resource_update"
"github.com/platform-engineering-labs/formae/internal/testplugin/fakeaws"
"github.com/platform-engineering-labs/formae/pkg/model"
"github.com/platform-engineering-labs/formae/pkg/plugin"
Expand Down Expand Up @@ -235,8 +234,8 @@ func TestPluginCoordinator_SpawnReportsResolvedRetryConfig(t *testing.T) {
}

// TestPluginCoordinator_LocalSpawnEnvMatchesReportedConfig asserts a spawned
// operator is handed the same retry config the spawn result reports, plus the
// plugin call deadline the agent sizes its watchdog window from. It drives the
// operator is handed the same retry config the spawn result reports, which is
// the config the agent sizes its watchdog window from. It drives the
// local path, which is the observable one; the remote path spawns with the same
// environment.
func TestPluginCoordinator_LocalSpawnEnvMatchesReportedConfig(t *testing.T) {
Expand All @@ -261,6 +260,4 @@ func TestPluginCoordinator_LocalSpawnEnvMatchesReportedConfig(t *testing.T) {
require.NotNil(t, env, "the coordinator must have spawned a plugin operator")
assert.Equal(t, *spawned.RetryConfig, env[gen.Env("RetryConfig")],
"the operator must poll on the config the result reports")
assert.Equal(t, resource_update.PluginCallTimeout, env[gen.Env("PluginCallTimeout")],
"the operator must bound its plugin calls by the deadline the watchdog window is built from")
}
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ func TestMissingInActionTimeout_CoversLongestOperatorSleep(t *testing.T) {
strategy := resource.RetryStrategy{MaxRetries: cfg.MaxRetries, BaseDelay: cfg.RetryDelay}
longestSleep := max(cfg.StatusCheckInterval, cfg.RetryDelay, strategy.Backoff(cfg.MaxRetries+1))

assert.Equal(t, longestSleep+PluginCallTimeout+missingInActionMargin, missingInActionTimeout(cfg))
assert.Equal(t, longestSleep+PluginCallAllowance+missingInActionMargin, missingInActionTimeout(cfg))
assert.Equal(t, 100*time.Second, missingInActionTimeout(cfg),
"the shipped defaults must yield a 100s window")
assert.Greater(t, missingInActionTimeout(cfg), 2*cfg.StatusCheckInterval,
Expand All @@ -62,7 +62,7 @@ func TestMissingInActionTimeout_DrivenByFlatRetryDelay(t *testing.T) {
require.Greater(t, retryDelay, strategy.Backoff(cfg.MaxRetries+1),
"this case only bites when the flat delay outlasts the capped backoff")

assert.Equal(t, retryDelay+PluginCallTimeout+missingInActionMargin, missingInActionTimeout(cfg))
assert.Equal(t, retryDelay+PluginCallAllowance+missingInActionMargin, missingInActionTimeout(cfg))
}

// TestMissingInActionTimeout_DrivenByLastScheduledBackoff covers the throttling
Expand All @@ -80,9 +80,9 @@ func TestMissingInActionTimeout_DrivenByLastScheduledBackoff(t *testing.T) {
require.Greater(t, lastBackoff, strategy.Backoff(cfg.MaxRetries),
"the last scheduled backoff must outlast the one before it")

assert.Equal(t, lastBackoff+PluginCallTimeout+missingInActionMargin, missingInActionTimeout(cfg))
assert.Equal(t, lastBackoff+PluginCallAllowance+missingInActionMargin, missingInActionTimeout(cfg))
assert.Greater(t, missingInActionTimeout(cfg),
strategy.Backoff(cfg.MaxRetries)+PluginCallTimeout+missingInActionMargin,
strategy.Backoff(cfg.MaxRetries)+PluginCallAllowance+missingInActionMargin,
"a window built on Backoff(MaxRetries) would fire one backoff short")
}

Expand All @@ -93,37 +93,33 @@ func TestMissingInActionTimeout_SmallAndDegenerateConfigs(t *testing.T) {
t.Run("MaxRetriesZero", func(t *testing.T) {
cfg := pkgmodel.RetryConfig{StatusCheckInterval: 2 * time.Second, MaxRetries: 0, RetryDelay: 7 * time.Second}
// The single scheduled backoff is Backoff(1), which is the base delay.
assert.Equal(t, 7*time.Second+PluginCallTimeout+missingInActionMargin, missingInActionTimeout(cfg))
assert.Equal(t, 7*time.Second+PluginCallAllowance+missingInActionMargin, missingInActionTimeout(cfg))
})

t.Run("MaxRetriesOne", func(t *testing.T) {
cfg := pkgmodel.RetryConfig{StatusCheckInterval: 2 * time.Second, MaxRetries: 1, RetryDelay: 7 * time.Second}
// Backoff(2) doubles the base delay, still under DefaultMaxBackoff.
assert.Equal(t, 14*time.Second+PluginCallTimeout+missingInActionMargin, missingInActionTimeout(cfg))
assert.Equal(t, 14*time.Second+PluginCallAllowance+missingInActionMargin, missingInActionTimeout(cfg))
})

t.Run("ZeroConfig", func(t *testing.T) {
assert.Equal(t, PluginCallTimeout+missingInActionMargin, missingInActionTimeout(pkgmodel.RetryConfig{}))
assert.Equal(t, PluginCallAllowance+missingInActionMargin, missingInActionTimeout(pkgmodel.RetryConfig{}))
})

t.Run("NegativeDurations", func(t *testing.T) {
cfg := pkgmodel.RetryConfig{StatusCheckInterval: -5 * time.Second, MaxRetries: 0, RetryDelay: -5 * time.Second}
assert.Equal(t, PluginCallTimeout+missingInActionMargin, missingInActionTimeout(cfg),
assert.Equal(t, PluginCallAllowance+missingInActionMargin, missingInActionTimeout(cfg),
"a negative duration in config must not shrink the window")
})
}

// TestPluginCallTimeouts_OperatorDeadlineExpiresFirst pins both deadlines: the
// one the agent hands the operator for a single plugin call, and the one the
// agent puts on its own call to the operator. The operator's must expire first
// so its attributable failure progress wins the race, and it must equal the
// operator's compiled defaultPluginCallTimeout fallback (pkg/plugin), which it
// stands in for whenever the deadline is not supplied.
func TestPluginCallTimeouts_OperatorDeadlineExpiresFirst(t *testing.T) {
assert.Equal(t, 60*time.Second, PluginCallTimeout,
"must track the plugin operator's compiled defaultPluginCallTimeout")
assert.Equal(t, 70, PluginOperationCallTimeout,
"the agent's call timeout is the operator's deadline plus a margin, in seconds")
// TestPluginCallAllowance_MatchesTheUpdatersOwnCallTimeout pins the call
// allowance in the watchdog window to the longest the updater itself waits for
// a reply from an operator. Nothing enforces a bound inside the plugin, so the
// window has to assume the most the agent is willing to wait.
func TestPluginCallAllowance_MatchesTheUpdatersOwnCallTimeout(t *testing.T) {
assert.Equal(t, time.Duration(PluginOperationCallTimeout)*time.Second, PluginCallAllowance,
"the window's call allowance must match the updater's own call timeout")
}

// armingProcess is a gen.Process double for the two watchdog-arming handlers.
Expand Down
25 changes: 12 additions & 13 deletions internal/metastructure/resource_update/resource_updater.go
Original file line number Diff line number Diff line change
Expand Up @@ -183,19 +183,18 @@ const (
StateRejected = gen.Atom("rejected")
)

// PluginCallTimeout is the deadline the agent hands each plugin operator for a
// single watched plugin call. It matches the operator's own compiled fallback,
// so the watchdog window derived from it holds whether the operator runs on the
// supplied deadline or on its fallback. Exposed as a variable so tests can
// reduce it.
var PluginCallTimeout = 60 * time.Second

// PluginOperationCallTimeout is the maximum time (in seconds) to wait for a
// plugin operator to respond to a resource operation. It outlasts
// PluginCallTimeout so the operator's own deadline expires first and its
// attributable failure progress wins the race with this call. Exposed as a
// variable so tests can reduce it.
var PluginOperationCallTimeout = int((PluginCallTimeout + 10*time.Second) / time.Second)
// plugin operator to respond to a resource operation. Exposed as a variable so
// tests can reduce it.
var PluginOperationCallTimeout = 60

// PluginCallAllowance is how long a single plugin call may take before the
// watchdog stops treating an operator's silence as work in progress. Nothing
// enforces it: a plugin call runs to completion inside the plugin's own
// process, so this is the silence the agent tolerates, not a bound on the call.
// It matches the outer call timeout above, which is the longest the updater
// itself waits for a reply. Exposed as a variable so tests can reduce it.
var PluginCallAllowance = 60 * time.Second

type ResourceUpdateData struct {
resourceUpdate *ResourceUpdate
Expand Down Expand Up @@ -564,7 +563,7 @@ const missingInActionMargin = 10 * time.Second
func missingInActionTimeout(cfg pkgmodel.RetryConfig) time.Duration {
strategy := resource.RetryStrategy{MaxRetries: cfg.MaxRetries, BaseDelay: cfg.RetryDelay}
longestSleep := max(cfg.StatusCheckInterval, cfg.RetryDelay, strategy.Backoff(cfg.MaxRetries+1), 0)
return longestSleep + PluginCallTimeout + missingInActionMargin
return longestSleep + PluginCallAllowance + missingInActionMargin
}

// watchdogRetryConfig returns the config the watchdog window is derived from:
Expand Down
10 changes: 5 additions & 5 deletions internal/workflow_tests/local/missing_in_action_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ func TestSlowHeartbeatIsNotDeclaredMissingInAction(t *testing.T) {
testutil.RunTestFromProjectRoot(t, func(t *testing.T) {
const (
statusCheckInterval = 1 * time.Second
pluginCallTimeout = 5 * time.Second
pluginCallAllowance = 5 * time.Second
slowStatusCall = 2500 * time.Millisecond
)

Expand All @@ -80,7 +80,7 @@ func TestSlowHeartbeatIsNotDeclaredMissingInAction(t *testing.T) {
"the gap must outlast the flat twice-the-interval window, or the old rule would not have fired")
require.Less(t, heartbeatGap, watchdogMarginFloor,
"the gap must stay inside the derived window, of which the fixed margin is only one term")
require.LessOrEqual(t, slowStatusCall, pluginCallTimeout/2,
require.LessOrEqual(t, slowStatusCall, pluginCallAllowance/2,
"the slow call must leave as much headroom again inside the deadline the agent hands the "+
"operator, or a loaded runner turns this into a call that outran its deadline instead of "+
"a slow one that reported")
Expand Down Expand Up @@ -125,9 +125,9 @@ func TestSlowHeartbeatIsNotDeclaredMissingInAction(t *testing.T) {
},
}

origCallTimeout := resource_update.PluginCallTimeout
resource_update.PluginCallTimeout = pluginCallTimeout
t.Cleanup(func() { resource_update.PluginCallTimeout = origCallTimeout })
origCallAllowance := resource_update.PluginCallAllowance
resource_update.PluginCallAllowance = pluginCallAllowance
t.Cleanup(func() { resource_update.PluginCallAllowance = origCallAllowance })

cfg := test_helpers.NewTestMetastructureConfig()
cfg.Agent.Retry.StatusCheckInterval = statusCheckInterval
Expand Down
10 changes: 5 additions & 5 deletions internal/workflow_tests/local/supervision_cascade_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -634,11 +634,11 @@ func TestPluginOperatorCrashConvergesViaTimeout(t *testing.T) {
}

// Shorten every term the PluginOperatorMissingInAction window is derived
// fromthe operator's retry cadence and the plugin call deadline — so
// the watchdog fires quickly in the test.
origCallTimeout := resource_update.PluginCallTimeout
resource_update.PluginCallTimeout = 100 * time.Millisecond
t.Cleanup(func() { resource_update.PluginCallTimeout = origCallTimeout })
// from, the operator's retry cadence and the per-call allowance, so the
// watchdog fires quickly in the test.
origCallAllowance := resource_update.PluginCallAllowance
resource_update.PluginCallAllowance = 100 * time.Millisecond
t.Cleanup(func() { resource_update.PluginCallAllowance = origCallAllowance })

cfg := test_helpers.NewTestMetastructureConfig()
cfg.Agent.Retry.StatusCheckInterval = 1 * time.Second
Expand Down
Loading
Loading