mirror of
https://github.com/rcourtman/Pulse.git
synced 2026-07-09 16:00:59 +00:00
fix(docker): stop manual update check from looping on ack failure
getDockerCommandPayload returned dispatched commands on every report fetch, causing the agent to re-execute check-updates on every poll cycle. When the ack also failed, the report was buffered and retried, creating an infinite loop. - Only return command payload on the queued->dispatched transition; subsequent fetches return nil (agent already received it). - Don't propagate ack errors from handleCheckUpdatesCommand; the report was delivered and check-updates is fire-and-forget. Command expires if ack never succeeds. Refs #1504
This commit is contained in:
parent
b5ddbd5606
commit
d6aed650b1
4 changed files with 34 additions and 36 deletions
|
|
@ -939,9 +939,16 @@ func (a *Agent) handleCheckUpdatesCommand(ctx context.Context, target TargetConf
|
|||
a.registryChecker.ForceCheck()
|
||||
}
|
||||
|
||||
// Send intermediate completion ack
|
||||
// Send intermediate completion ack. Don't propagate the error — the
|
||||
// report was already delivered successfully. Propagating causes the
|
||||
// report to be buffered and retried, which re-fetches the same command
|
||||
// and creates a loop (issue #1504).
|
||||
if err := a.sendCommandAck(ctx, target, command.ID, agentsdocker.CommandStatusCompleted, "Registry cache cleared; checking for updates on next report cycle"); err != nil {
|
||||
return fmt.Errorf("send check updates acknowledgement: %w", err)
|
||||
a.logger.Warn().
|
||||
Err(err).
|
||||
Str("commandID", command.ID).
|
||||
Str("target", target.URL).
|
||||
Msg("Failed to send check updates acknowledgement")
|
||||
}
|
||||
|
||||
// Trigger an immediate collection cycle to report updates.
|
||||
|
|
|
|||
|
|
@ -600,7 +600,7 @@ func TestHandleCommand(t *testing.T) {
|
|||
}
|
||||
})
|
||||
|
||||
t.Run("check updates command ack error", func(t *testing.T) {
|
||||
t.Run("check updates command ack error does not propagate", func(t *testing.T) {
|
||||
registryChecker := NewRegistryChecker(zerolog.Nop())
|
||||
registryChecker.MarkChecked()
|
||||
registryChecker.cacheDigest("cached-key", "sha256:cached")
|
||||
|
|
@ -610,13 +610,14 @@ func TestHandleCommand(t *testing.T) {
|
|||
hostID: "host1",
|
||||
registryChecker: registryChecker,
|
||||
}
|
||||
t.Cleanup(func() { _ = agent.Close() })
|
||||
|
||||
err := agent.handleCheckUpdatesCommand(context.Background(), TargetConfig{URL: "http://example.com/\x7f", Token: "token"}, agentsdocker.Command{
|
||||
ID: "cmd4",
|
||||
Type: agentsdocker.CommandTypeCheckUpdates,
|
||||
})
|
||||
if err == nil || !strings.Contains(err.Error(), "send check updates acknowledgement") {
|
||||
t.Fatalf("expected check-updates ack error, got %v", err)
|
||||
if err != nil {
|
||||
t.Fatalf("expected nil error on ack failure, got: %v", err)
|
||||
}
|
||||
|
||||
registryChecker.mu.RLock()
|
||||
|
|
|
|||
|
|
@ -400,12 +400,17 @@ func (m *Monitor) getDockerCommandPayload(hostID string) (map[string]any, *model
|
|||
return nil, nil
|
||||
}
|
||||
|
||||
// Update dispatch metadata
|
||||
if cmd.status.Status == DockerCommandStatusQueued {
|
||||
cmd.markDispatched()
|
||||
m.state.SetDockerHostCommand(hostID, &cmd.status)
|
||||
// Only return the command on the first dispatch (queued → dispatched).
|
||||
// Re-sending an already-dispatched command causes the agent to re-execute
|
||||
// it on every report cycle until the ack succeeds, creating an infinite
|
||||
// loop when the ack HTTP call fails (issue #1504).
|
||||
if cmd.status.Status != DockerCommandStatusQueued {
|
||||
return nil, nil
|
||||
}
|
||||
|
||||
cmd.markDispatched()
|
||||
m.state.SetDockerHostCommand(hostID, &cmd.status)
|
||||
|
||||
statusCopy := cmd.status
|
||||
return cmd.payload, &statusCopy
|
||||
}
|
||||
|
|
|
|||
|
|
@ -891,7 +891,7 @@ func TestGetDockerCommandPayload(t *testing.T) {
|
|||
}
|
||||
})
|
||||
|
||||
t.Run("already dispatched command is not re-marked", func(t *testing.T) {
|
||||
t.Run("already dispatched command is not re-sent", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
monitor := newTestMonitorForCommands(t)
|
||||
|
|
@ -910,39 +910,24 @@ func TestGetDockerCommandPayload(t *testing.T) {
|
|||
t.Fatalf("queue stop command: %v", err)
|
||||
}
|
||||
|
||||
// First fetch - marks as dispatched
|
||||
// First fetch - marks as dispatched and returns status
|
||||
_, status1 := monitor.getDockerCommandPayload(host.ID)
|
||||
if status1 == nil {
|
||||
t.Fatal("expected status from first fetch")
|
||||
}
|
||||
firstDispatchedAt := status1.DispatchedAt
|
||||
firstUpdatedAt := status1.UpdatedAt
|
||||
|
||||
// Small delay to ensure time would change if re-marked
|
||||
time.Sleep(time.Millisecond)
|
||||
|
||||
// Second fetch - should NOT re-mark
|
||||
_, status2 := monitor.getDockerCommandPayload(host.ID)
|
||||
if status2 == nil {
|
||||
t.Fatal("expected status from second fetch")
|
||||
if status1.Status != DockerCommandStatusDispatched {
|
||||
t.Fatalf("expected dispatched status, got %q", status1.Status)
|
||||
}
|
||||
|
||||
// DispatchedAt should be the same
|
||||
if status2.DispatchedAt == nil {
|
||||
t.Fatal("expected DispatchedAt to still be set")
|
||||
// Second fetch - should return nil. The command was already delivered
|
||||
// in the first dispatch response; re-sending it causes the agent to
|
||||
// re-execute the command on every report cycle (issue #1504).
|
||||
payload2, status2 := monitor.getDockerCommandPayload(host.ID)
|
||||
if payload2 != nil {
|
||||
t.Fatalf("expected nil payload from second fetch, got %v", payload2)
|
||||
}
|
||||
if !status2.DispatchedAt.Equal(*firstDispatchedAt) {
|
||||
t.Fatalf("DispatchedAt changed: first=%v, second=%v", *firstDispatchedAt, *status2.DispatchedAt)
|
||||
}
|
||||
|
||||
// UpdatedAt should be the same (not re-updated)
|
||||
if !status2.UpdatedAt.Equal(firstUpdatedAt) {
|
||||
t.Fatalf("UpdatedAt changed: first=%v, second=%v", firstUpdatedAt, status2.UpdatedAt)
|
||||
}
|
||||
|
||||
// Status should still be dispatched
|
||||
if status2.Status != DockerCommandStatusDispatched {
|
||||
t.Fatalf("expected dispatched status, got %q", status2.Status)
|
||||
if status2 != nil {
|
||||
t.Fatalf("expected nil status from second fetch, got %+v", status2)
|
||||
}
|
||||
})
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue