Skip to content
Merged
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
50 changes: 40 additions & 10 deletions commands/play_executor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -133,18 +133,15 @@ func TestMultiPlayPlayFilterComposesWithTags(t *testing.T) {
tasks:
- name: deploy-api
tags: [deploy]
dokku_app:
app: docket-test-deploy-api
dokku_stub: { key: deploy-api }
- name: configure-api
tags: [configure]
dokku_app:
app: docket-test-configure-api
dokku_stub: { key: configure-api }
- name: worker
tasks:
- name: deploy-worker
tags: [deploy]
dokku_app:
app: docket-test-deploy-worker
dokku_stub: { key: deploy-worker }
`)
stdout, _, _ := runPlan(t, path, "--play", "api", "--tags", "deploy")
if !strings.Contains(stdout, "deploy-api") {
Expand All @@ -168,14 +165,12 @@ func TestMultiPlayPlayLevelTagsPropagateToTasks(t *testing.T) {
tags: [api]
tasks:
- name: deploy-api
dokku_app:
app: docket-test-deploy-api
dokku_stub: { key: deploy-api }
- name: worker
tags: [worker]
tasks:
- name: deploy-worker
dokku_app:
app: docket-test-deploy-worker
dokku_stub: { key: deploy-worker }
`)
stdout, _, _ := runPlan(t, path, "--tags", "api")
if !strings.Contains(stdout, "deploy-api") {
Expand Down Expand Up @@ -238,6 +233,41 @@ func TestPlanFailedWhenClearsError(t *testing.T) {
}
}

// TestPlanProbeErrorRendersMarkerAndExits pins the documented contract
// that an uncleared probe error renders [!] on stderr and makes plan
// exit 1, and that errors win over drift under --detailed-exitcode (exit
// 1, not 2). This is the end-to-end guard for #328: a probe that could
// not run must not be reported as absent with an optimistic [+] create.
func TestPlanProbeErrorRendersMarkerAndExits(t *testing.T) {
defer stubReset()
stubSet("a", StubFixture{
PlanError: errors.New(`exec: "dokku": executable file not found in $PATH`),
})

path := writeMultiPlayTasks(t, `---
- tasks:
- name: needs-dokku
dokku_stub: { key: a }
`)

stdout, stderr, exit := runPlan(t, path)
if exit != 1 {
t.Fatalf("exit = %d, want 1; stdout=%s stderr=%s", exit, stdout, stderr)
}
if !strings.Contains(stderr, "[!]") {
t.Errorf("expected [!] marker on stderr; got stdout=%s stderr=%s", stdout, stderr)
}
if strings.Contains(stdout, "[+]") {
t.Errorf("plan must not predict [+] create for state it never read; got:\n%s", stdout)
}

// --detailed-exitcode: errors win over drift, so exit stays 1, not 2.
_, _, exit = runPlan(t, path, "--detailed-exitcode")
if exit != 1 {
t.Errorf("detailed-exitcode exit = %d, want 1 (errors win over drift)", exit)
}
}

// TestPlanChangedWhenTrueRecomputesStatus pins #313: changed_when: 'true'
// on an in-sync task must flip the marker and the JSON status to a
// would-change verdict, not leave the stale [ok] / "status":"ok" the
Expand Down
4 changes: 4 additions & 0 deletions docs/command-reference.md
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,10 @@ decide whether to mutate. A few tasks (notably `dokku_git_auth`, `dokku_registry
`dokku_storage_ensure`) cannot read their state without running the underlying command, so they
always report drift with a `(... not probed)` reason.

When the probe command cannot run at all - for example the local `dokku` CLI is not installed, or
the configured SSH host is unreachable - the task renders `[!]` and `plan` exits `1`, rather than
optimistically predicting `[+] create` for state it never actually read.

| Flag | Effect |
|------|--------|
| `--tasks <path>` | Use a specific recipe. Accepts a local path or an `http(s)://` URL (fetched over HTTP). |
Expand Down
15 changes: 14 additions & 1 deletion subprocess/exec.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,15 @@ type ExecCommandInput struct {
type ExecError struct {
Response ExecCommandResponse
Err error

// Ran is true only when the command executed to completion and
// Response.ExitCode is its real exit status. It is false when the
// command could not be started (binary not found, permission denied)
// or was cancelled, in which cases Response.ExitCode is not
// meaningful. Probe() uses this to tell a dokku-level "state absent"
// (Ran, non-zero exit) apart from a real execution failure that must
// be propagated.
Ran bool
}

// Error returns the wrapped error's message so existing string-based
Expand Down Expand Up @@ -312,6 +321,10 @@ func defaultExecRunner(ctx context.Context, input ExecCommandInput) (ExecCommand

res, err := cmd.Execute(ctx)
if err != nil {
// The command could not be run to completion: the binary was not
// found, was not executable, or the context was cancelled. The
// exit code is not meaningful, so Ran stays false and callers such
// as Probe surface the failure instead of reading it as absence.
response := ExecCommandResponse{
Command: resolved,
Stdout: res.Stdout,
Expand All @@ -330,7 +343,7 @@ func defaultExecRunner(ctx context.Context, input ExecCommandInput) (ExecCommand
ExitCode: res.ExitCode,
Cancelled: res.Cancelled,
}
return response, &ExecError{Response: response, Err: errors.New(res.Stderr)}
return response, &ExecError{Response: response, Err: errors.New(res.Stderr), Ran: true}
}

return ExecCommandResponse{
Expand Down
20 changes: 20 additions & 0 deletions subprocess/exec_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package subprocess
import (
"bytes"
"context"
"errors"
"log"
"strings"
"testing"
Expand Down Expand Up @@ -170,6 +171,15 @@ func TestCallExecCommandFailure(t *testing.T) {
if resp.ExitCode == 0 {
t.Error("expected non-zero exit code")
}
// The command ran and exited non-zero, so the exit code is real:
// ExecError.Ran must be true so Probe reads it as "state absent".
var execErr *ExecError
if !errors.As(err, &execErr) {
t.Fatalf("expected *ExecError, got %T", err)
}
if !execErr.Ran {
t.Error("ExecError.Ran should be true when the command ran and exited non-zero")
}
}

func TestCallExecCommandNotFound(t *testing.T) {
Expand All @@ -179,6 +189,16 @@ func TestCallExecCommandNotFound(t *testing.T) {
if err == nil {
t.Fatal("expected error for nonexistent command")
}
// The command could not be started, so there is no real exit code:
// ExecError.Ran must be false so Probe propagates the failure instead
// of reporting the probed state as absent.
var execErr *ExecError
if !errors.As(err, &execErr) {
t.Fatalf("expected *ExecError, got %T", err)
}
if execErr.Ran {
t.Error("ExecError.Ran should be false when the binary is not found")
}
}

func TestCallExecCommandWithEnv(t *testing.T) {
Expand Down
36 changes: 28 additions & 8 deletions subprocess/ssh.go
Original file line number Diff line number Diff line change
Expand Up @@ -217,12 +217,18 @@ var (
)

// Probe runs input as a state probe and reports whether it matched
// (exit 0). A dokku-level non-zero exit is reported as `(false, nil)`,
// i.e. "the probed state is absent," so callers can write idempotent
// probes without unwrapping errors themselves. A transport-level
// failure (`*SSHError`) is propagated as `(false, err)` so the caller
// can short-circuit `Plan()` with `PlanResult{Error: err}` and let the
// formatter render `! ssh: ...`.
// (exit 0). A non-zero exit from a command that actually ran is reported
// as `(false, nil)`, i.e. "the probed state is absent," so callers can
// write idempotent probes without unwrapping errors themselves.
//
// Any failure that means the probe never produced a real answer is
// propagated as `(false, err)` so the caller can short-circuit `Plan()`
// with `PlanResult{Error: err}` and let the formatter render `[!]`. That
// covers a transport-level failure (`*SSHError`), a command that could
// not be executed at all (the dokku binary is missing or not
// executable), and a cancelled probe. Distinguishing "ran and said no"
// from "could not run" relies on `ExecError.Ran`, since binary-not-found
// reports `ExitCode 0` and so cannot be told apart by exit code.
//
// Use this for any plan-time probe that today reads exit code only
// (`apps:exists`, `network:exists`, `<service>:linked`, etc.). Probes
Expand All @@ -231,11 +237,22 @@ var (
func Probe(input ExecCommandInput) (bool, error) {
result, err := CallExecCommand(input)
if err != nil {
// Transport-level failure (ssh connect/auth/host-key): propagate
// so the caller can render `! ssh: ...`.
var sshErr *SSHError
if errors.As(err, &sshErr) {
return false, err
}
return false, nil
// The command executed and exited non-zero: the probed state is
// absent. Report (false, nil) so idempotent probes need not unwrap.
var execErr *ExecError
if errors.As(err, &execErr) && execErr.Ran {
return false, nil
}
// Anything else - the command could not be executed (binary not
// found, permission denied) or was cancelled - is a real failure
// the caller must surface, not "state absent".
return false, err
}
return result.ExitCode == 0, nil
}
Expand Down Expand Up @@ -390,7 +407,10 @@ func classifySshResult(target sshTarget, remote []string, resp ExecCommandRespon
}
}
if resp.ExitCode != 0 {
return resp, &ExecError{Response: resp, Err: errors.New(resp.Stderr)}
// The remote dokku command ran and exited non-zero (not a
// transport failure). Ran marks the exit code as authoritative so
// Probe treats it as "state absent" rather than an execution error.
return resp, &ExecError{Response: resp, Err: errors.New(resp.Stderr), Ran: true}
}
return resp, nil
}
Expand Down
57 changes: 57 additions & 0 deletions subprocess/ssh_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package subprocess

import (
"context"
"errors"
"strings"
"testing"
Expand Down Expand Up @@ -430,6 +431,62 @@ func TestProbeSshTransportErrorPropagates(t *testing.T) {
}
}

func TestProbeLocalExecErrorPropagates(t *testing.T) {
// A local probe whose binary is not on PATH must surface the failure
// rather than reporting the probed state as absent. This is the
// no-dokku-installed scenario: without propagation, plan would print a
// confident [+] create for every resource instead of [!].
matched, err := Probe(ExecCommandInput{
Command: "nonexistent-binary-docket-test-12345",
Args: []string{"--quiet", "apps:exists", "anything"},
})
if matched {
t.Error("Probe should report matched=false when the binary is missing")
}
if err == nil {
t.Fatal("Probe should propagate a binary-not-found error, not swallow it as absent")
}
}

func TestProbeExecRanFlagControlsAbsence(t *testing.T) {
// The Ran flag is the sole signal that separates "the command ran and
// exited non-zero" (state absent) from "the command could not run"
// (real failure). Inject each case via the swappable runner so the
// behaviour is pinned without spawning a process.
t.Run("ran non-zero reports absent", func(t *testing.T) {
defer SetExecRunner(func(_ context.Context, _ ExecCommandInput) (ExecCommandResponse, error) {
resp := ExecCommandResponse{ExitCode: 1}
return resp, &ExecError{Response: resp, Err: errors.New("absent"), Ran: true}
})()

matched, err := Probe(ExecCommandInput{Command: "dokku"})
if err != nil {
t.Fatalf("Probe should treat a ran non-zero exit as absent, got err %v", err)
}
if matched {
t.Error("Probe should report matched=false for a ran non-zero exit")
}
})

t.Run("cancelled probe propagates", func(t *testing.T) {
defer SetExecRunner(func(_ context.Context, _ ExecCommandInput) (ExecCommandResponse, error) {
resp := ExecCommandResponse{ExitCode: -1, Cancelled: true}
return resp, &ExecError{Response: resp, Err: context.Canceled}
})()

matched, err := Probe(ExecCommandInput{Command: "dokku"})
if matched {
t.Error("Probe should report matched=false on a cancelled probe")
}
if err == nil {
t.Fatal("Probe should propagate a cancelled probe, not swallow it as absent")
}
if !errors.Is(err, context.Canceled) {
t.Errorf("propagated error should wrap context.Canceled, got %v", err)
}
})
}

func TestSetAndGetDefaultHost(t *testing.T) {
t.Cleanup(func() { SetDefaultHost("") })
SetDefaultHost("alice@host:2222")
Expand Down
7 changes: 4 additions & 3 deletions tasks/app_lock_task.go
Original file line number Diff line number Diff line change
Expand Up @@ -131,9 +131,10 @@ func (t AppLockTask) ExportApp(app string) ([]interface{}, error) {
return []interface{}{AppLockTask{App: app, State: StatePresent}}, nil
}

// appLocked checks if a dokku app is locked. Returns (false,
// *subprocess.SSHError) on transport failure; (false, nil) when dokku
// reports unlocked; (true, nil) when locked.
// appLocked checks if a dokku app is locked. Returns (false, err) when
// the probe could not run - a transport failure, a missing dokku binary,
// or a cancellation; (false, nil) when dokku reports unlocked; (true,
// nil) when locked.
func appLocked(app string) (bool, error) {
return subprocess.Probe(subprocess.ExecCommandInput{
Command: "dokku",
Expand Down
5 changes: 3 additions & 2 deletions tasks/app_task.go
Original file line number Diff line number Diff line change
Expand Up @@ -115,8 +115,9 @@ func (t AppTask) ExportApp(app string) ([]interface{}, error) {

// appExists checks if an app exists. Returns (true, nil) when the app
// is present, (false, nil) when dokku reports it absent, and
// (false, *subprocess.SSHError) when the probe could not reach the
// server. Plan() callers must short-circuit on the error.
// (false, err) when the probe could not run - a transport failure, a
// missing dokku binary, or a cancellation. Plan() callers must
// short-circuit on the error.
func appExists(appName string) (bool, error) {
return subprocess.Probe(subprocess.ExecCommandInput{
Command: "dokku",
Expand Down
7 changes: 4 additions & 3 deletions tasks/network_task.go
Original file line number Diff line number Diff line change
Expand Up @@ -164,9 +164,10 @@ func (t NetworkTask) ExportGlobal() ([]interface{}, error) {
return out, nil
}

// networkExists checks if a Docker network exists. Returns (false,
// *subprocess.SSHError) on transport failure; (false, nil) when dokku
// reports the network absent; (true, nil) when present.
// networkExists checks if a Docker network exists. Returns (false, err)
// when the probe could not run - a transport failure, a missing dokku
// binary, or a cancellation; (false, nil) when dokku reports the network
// absent; (true, nil) when present.
func networkExists(name string) (bool, error) {
return subprocess.Probe(subprocess.ExecCommandInput{
Command: "dokku",
Expand Down
7 changes: 4 additions & 3 deletions tasks/service_create_task.go
Original file line number Diff line number Diff line change
Expand Up @@ -147,9 +147,10 @@ func (t ServiceCreateTask) Plan() PlanResult {
})
}

// serviceExists checks if a dokku service exists. Returns (false,
// *subprocess.SSHError) on transport failure, (false, nil) when dokku
// reports the service absent, (true, nil) when present.
// serviceExists checks if a dokku service exists. Returns (false, err)
// when the probe could not run - a transport failure, a missing dokku
// binary, or a cancellation, (false, nil) when dokku reports the service
// absent, (true, nil) when present.
func serviceExists(service, name string) (bool, error) {
return subprocess.Probe(subprocess.ExecCommandInput{
Command: "dokku",
Expand Down
5 changes: 3 additions & 2 deletions tasks/service_link_task.go
Original file line number Diff line number Diff line change
Expand Up @@ -190,8 +190,9 @@ func (t ServiceLinkTask) Plan() PlanResult {
}

// serviceLinked checks if a dokku service is linked to an app. Returns
// (false, *subprocess.SSHError) on transport failure; (false, nil) when
// dokku reports no link; (true, nil) when linked.
// (false, err) when the probe could not run - a transport failure, a
// missing dokku binary, or a cancellation; (false, nil) when dokku
// reports no link; (true, nil) when linked.
func serviceLinked(service, name, app string) (bool, error) {
return subprocess.Probe(subprocess.ExecCommandInput{
Command: "dokku",
Expand Down
Loading