Compare commits
1
Commits
next
...
afc07c5ac6
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
afc07c5ac6 |
@@ -22,6 +22,15 @@ the tag exists and is exercised; what is left is merging `next` to
|
|||||||
|
|
||||||
# Completed Steps
|
# Completed Steps
|
||||||
|
|
||||||
|
- 2026-10-07: Made an interrupted command exit 130 and say so
|
||||||
|
([issue #267](https://git.eeqj.de/sneak/vaultik/issues/267)). Ctrl-C
|
||||||
|
or SIGTERM during `snapshot create`, `snapshot restore` or `snapshot
|
||||||
|
verify` exited 0 with no error line (`snapshot verify --json` exited
|
||||||
|
1, also without one), so a `--cron` run that never finished looked
|
||||||
|
like a success. A command stopped by either signal now exits 130 and
|
||||||
|
prints `interrupted before the command finished` on stderr, under
|
||||||
|
`--cron` and `--json` too.
|
||||||
|
|
||||||
- 2026-10-07: Corrected documentation, help text and comments that were
|
- 2026-10-07: Corrected documentation, help text and comments that were
|
||||||
false about the code
|
false about the code
|
||||||
([issue #233](https://git.eeqj.de/sneak/vaultik/issues/233)). A blob
|
([issue #233](https://git.eeqj.de/sneak/vaultik/issues/233)). A blob
|
||||||
|
|||||||
+40
-12
@@ -206,6 +206,10 @@ func RunApp(ctx context.Context, app *fx.App) error {
|
|||||||
// RunOperation through cobra to Entry.
|
// RunOperation through cobra to Entry.
|
||||||
var errReported = errors.New("operation failed")
|
var errReported = errors.New("operation failed")
|
||||||
|
|
||||||
|
// errInterrupted marks an operation that SIGINT or SIGTERM stopped
|
||||||
|
// before it finished. Entry shows it and returns exitCodeInterrupted.
|
||||||
|
var errInterrupted = errors.New("interrupted before the command finished")
|
||||||
|
|
||||||
// RunOperation runs op against the Vaultik instance inside the fx app
|
// RunOperation runs op against the Vaultik instance inside the fx app
|
||||||
// and turns a failure into a returned error rather than an os.Exit from
|
// and turns a failure into a returned error rather than an os.Exit from
|
||||||
// within the goroutine. An os.Exit there skipped main's deferred
|
// within the goroutine. An os.Exit there skipped main's deferred
|
||||||
@@ -220,17 +224,17 @@ var errReported = errors.New("operation failed")
|
|||||||
// interrupt OnStop cancels op and waits for the goroutine to return, so
|
// interrupt OnStop cancels op and waits for the goroutine to return, so
|
||||||
// op's cleanup (removing decrypted scratch files) runs before the
|
// op's cleanup (removing decrypted scratch files) runs before the
|
||||||
// process exits; the wait is bounded by shutdownTimeout. report is
|
// process exits; the wait is bounded by shutdownTimeout. report is
|
||||||
// called with a non-canceled failure so the caller can show it to the
|
// called with a failure so the caller can show it to the user before
|
||||||
// user before it becomes errReported. A context cancellation is the
|
// it becomes errReported. An interrupted op is not reported, whatever
|
||||||
// interrupt path, not a failure: it is neither reported nor counted as
|
// it returned; RunOperation returns errInterrupted instead.
|
||||||
// one.
|
|
||||||
func RunOperation(
|
func RunOperation(
|
||||||
ctx context.Context, opts AppOptions,
|
ctx context.Context, opts AppOptions,
|
||||||
op func(v *vaultik.Vaultik) error, report func(err error),
|
op func(v *vaultik.Vaultik) error, report func(err error),
|
||||||
) error {
|
) error {
|
||||||
var (
|
var (
|
||||||
mu sync.Mutex
|
mu sync.Mutex
|
||||||
failed bool
|
failed bool
|
||||||
|
interrupted bool
|
||||||
)
|
)
|
||||||
|
|
||||||
opts.Invokes = append(opts.Invokes,
|
opts.Invokes = append(opts.Invokes,
|
||||||
@@ -241,7 +245,19 @@ func RunOperation(
|
|||||||
OnStart: func(_ context.Context) error {
|
OnStart: func(_ context.Context) error {
|
||||||
stop = v.StartOperation(func() {
|
stop = v.StartOperation(func() {
|
||||||
err := op(v)
|
err := op(v)
|
||||||
if err != nil && !errors.Is(err, context.Canceled) {
|
|
||||||
|
// Only stop, called from OnStop below, cancels the
|
||||||
|
// Vaultik context; before op has returned, that
|
||||||
|
// happens only on an interrupt. Check the context,
|
||||||
|
// not err: an interrupted op need not return
|
||||||
|
// context.Canceled (`snapshot verify --json`
|
||||||
|
// returns a verification failure).
|
||||||
|
switch {
|
||||||
|
case v.Context().Err() != nil:
|
||||||
|
mu.Lock()
|
||||||
|
interrupted = true
|
||||||
|
mu.Unlock()
|
||||||
|
case err != nil:
|
||||||
report(err)
|
report(err)
|
||||||
|
|
||||||
mu.Lock()
|
mu.Lock()
|
||||||
@@ -266,6 +282,12 @@ func RunOperation(
|
|||||||
if !stop(ctx) {
|
if !stop(ctx) {
|
||||||
log.Warn("Shutdown timed out before the operation " +
|
log.Warn("Shutdown timed out before the operation " +
|
||||||
"finished; decrypted temporary files may remain")
|
"finished; decrypted temporary files may remain")
|
||||||
|
|
||||||
|
// op has not returned, so the app is stopping on
|
||||||
|
// an interrupt.
|
||||||
|
mu.Lock()
|
||||||
|
interrupted = true
|
||||||
|
mu.Unlock()
|
||||||
}
|
}
|
||||||
|
|
||||||
return nil
|
return nil
|
||||||
@@ -278,16 +300,22 @@ func RunOperation(
|
|||||||
return err
|
return err
|
||||||
}
|
}
|
||||||
|
|
||||||
// The goroutine sets failed before triggering the shutdown that lets
|
// RunWithApp returns only after OnStop has waited for the goroutine
|
||||||
// RunWithApp return, so the write is in place by the time we read it.
|
// to return, whether the shutdown came from op finishing or from an
|
||||||
|
// interrupt, so the goroutine's write to failed or interrupted is in
|
||||||
|
// place by the time we read it. If OnStop timed out, the goroutine
|
||||||
|
// may still be running, and OnStop has set interrupted itself.
|
||||||
mu.Lock()
|
mu.Lock()
|
||||||
defer mu.Unlock()
|
defer mu.Unlock()
|
||||||
|
|
||||||
if failed {
|
switch {
|
||||||
|
case interrupted:
|
||||||
|
return errInterrupted
|
||||||
|
case failed:
|
||||||
return errReported
|
return errReported
|
||||||
|
default:
|
||||||
|
return nil
|
||||||
}
|
}
|
||||||
|
|
||||||
return nil
|
|
||||||
}
|
}
|
||||||
|
|
||||||
// runVaultikApp runs the standard single-operation command lifecycle
|
// runVaultikApp runs the standard single-operation command lifecycle
|
||||||
|
|||||||
+15
-4
@@ -15,6 +15,11 @@ import (
|
|||||||
// the startup banner.
|
// the startup banner.
|
||||||
const shortCommitLen = 12
|
const shortCommitLen = 12
|
||||||
|
|
||||||
|
// exitCodeInterrupted is the exit status of a command that SIGINT or
|
||||||
|
// SIGTERM stopped. It is 128 plus SIGINT's number, 2, which is what a
|
||||||
|
// shell reports for a command stopped by Ctrl-C.
|
||||||
|
const exitCodeInterrupted = 130
|
||||||
|
|
||||||
// Entry is the main entry point for the CLI application.
|
// Entry is the main entry point for the CLI application.
|
||||||
// It prints the startup banner to stderr (unless a banner-suppressing
|
// It prints the startup banner to stderr (unless a banner-suppressing
|
||||||
// flag is present in os.Args — see bannerSuppressedInArgs), executes the
|
// flag is present in os.Args — see bannerSuppressedInArgs), executes the
|
||||||
@@ -23,9 +28,10 @@ const shortCommitLen = 12
|
|||||||
// The banner goes to stderr because stdout carries only the output the
|
// The banner goes to stderr because stdout carries only the output the
|
||||||
// user asked for, such as a completion script or a `config get` value.
|
// user asked for, such as a completion script or a `config get` value.
|
||||||
//
|
//
|
||||||
// It returns the process exit code (0 on success, 1 on error) rather
|
// It returns the process exit code (0 on success, 130 when interrupted,
|
||||||
// than calling os.Exit, so that main's deferred profile writers run
|
// 1 on any other error) rather than calling os.Exit, so that main's
|
||||||
// before the process ends. See run in cmd/vaultik/main.go.
|
// deferred profile writers run before the process ends. See run in
|
||||||
|
// cmd/vaultik/main.go.
|
||||||
func Entry() int {
|
func Entry() int {
|
||||||
emitStartupBanner(os.Args[1:], os.Stderr)
|
emitStartupBanner(os.Args[1:], os.Stderr)
|
||||||
|
|
||||||
@@ -39,11 +45,16 @@ func Entry() int {
|
|||||||
// document instead); errReported says so. Printing it again
|
// document instead); errReported says so. Printing it again
|
||||||
// here would double the error line.
|
// here would double the error line.
|
||||||
// Every other error — bad arguments, a config that would not
|
// Every other error — bad arguments, a config that would not
|
||||||
// load — reaches Entry unreported, so it is shown here.
|
// load, an interrupt — reaches Entry unreported, so it is shown
|
||||||
|
// here.
|
||||||
if !errors.Is(err, errReported) {
|
if !errors.Is(err, errReported) {
|
||||||
ReportErrorf("%s", err.Error())
|
ReportErrorf("%s", err.Error())
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if errors.Is(err, errInterrupted) {
|
||||||
|
return exitCodeInterrupted
|
||||||
|
}
|
||||||
|
|
||||||
return 1
|
return 1
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -0,0 +1,203 @@
|
|||||||
|
package cli //nolint:testpackage // shares runEntry and the argument constants
|
||||||
|
|
||||||
|
import (
|
||||||
|
"fmt"
|
||||||
|
"net/http"
|
||||||
|
"net/http/httptest"
|
||||||
|
"os"
|
||||||
|
"os/signal"
|
||||||
|
"path/filepath"
|
||||||
|
"strings"
|
||||||
|
"testing"
|
||||||
|
"time"
|
||||||
|
|
||||||
|
"github.com/adrg/xdg"
|
||||||
|
"github.com/stretchr/testify/assert"
|
||||||
|
"github.com/stretchr/testify/require"
|
||||||
|
)
|
||||||
|
|
||||||
|
// stalledStoreConfig is hermeticConfig with an s3:// destination store
|
||||||
|
// in place of the file:// one. The server behind it accepts any
|
||||||
|
// credentials.
|
||||||
|
const stalledStoreConfig = `age_recipients:
|
||||||
|
- age1278m9q7dp3chsh2dcy82qk27v047zywyvtxwnj4cvt0z65jw6a7q5dqhfj
|
||||||
|
snapshots:
|
||||||
|
test:
|
||||||
|
paths:
|
||||||
|
- %s
|
||||||
|
storage_url: s3://bucket?endpoint=%s&ssl=false
|
||||||
|
s3:
|
||||||
|
access_key_id: key
|
||||||
|
secret_access_key: secret
|
||||||
|
index_path: %s
|
||||||
|
hostname: test-host
|
||||||
|
`
|
||||||
|
|
||||||
|
// interruptRepeat is how often interruptOnFirstRequest sends SIGINT.
|
||||||
|
const interruptRepeat = 50 * time.Millisecond
|
||||||
|
|
||||||
|
// TestEntryInterruptedRun sends SIGINT to the test process while a
|
||||||
|
// command waits on the destination store, and checks that Entry returns
|
||||||
|
// 130 and prints one line on stderr saying the run was interrupted. The
|
||||||
|
// store is a local HTTP server that holds every request open, so the
|
||||||
|
// command is always mid-operation when the signal arrives. The two
|
||||||
|
// cases cover --cron and --json, which silence other output.
|
||||||
|
//
|
||||||
|
// Not parallel: it signals the process and replaces os.Args, os.Stdout,
|
||||||
|
// os.Stderr and the xdg globals.
|
||||||
|
//
|
||||||
|
//nolint:paralleltest // signals the process and replaces process globals
|
||||||
|
func TestEntryInterruptedRun(t *testing.T) {
|
||||||
|
for _, testCase := range []struct {
|
||||||
|
name string
|
||||||
|
args []string
|
||||||
|
}{
|
||||||
|
{
|
||||||
|
name: "snapshot create --cron",
|
||||||
|
args: []string{cmdSnapshot, cmdCreate, "--cron"},
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "snapshot verify --json",
|
||||||
|
args: []string{cmdSnapshot, cmdVerify, someSnapshotID, flagJSON},
|
||||||
|
},
|
||||||
|
} {
|
||||||
|
t.Run(testCase.name, func(t *testing.T) {
|
||||||
|
endpoint, requestArrived := startStalledStore(t)
|
||||||
|
configPath := writeStalledStoreConfig(t, endpoint)
|
||||||
|
interruptOnFirstRequest(t, requestArrived)
|
||||||
|
|
||||||
|
code, _, stderr := runEntry(t,
|
||||||
|
append([]string{flagConfig, configPath}, testCase.args...)...)
|
||||||
|
|
||||||
|
assert.Equal(t, 130, code)
|
||||||
|
assert.Equal(t, 1,
|
||||||
|
strings.Count(stderr, errInterrupted.Error()), stderr)
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// interruptOnFirstRequest sends SIGINT to the test process every
|
||||||
|
// interruptRepeat, from the first request to the destination store until
|
||||||
|
// the test ends. One signal is not enough: the command can reach the
|
||||||
|
// store before fx has started catching signals. The test catches SIGINT
|
||||||
|
// too, so that a signal fx is not catching does not kill the test
|
||||||
|
// binary.
|
||||||
|
func interruptOnFirstRequest(t *testing.T, requestArrived <-chan struct{}) {
|
||||||
|
t.Helper()
|
||||||
|
|
||||||
|
self, err := os.FindProcess(os.Getpid())
|
||||||
|
require.NoError(t, err)
|
||||||
|
|
||||||
|
caught := make(chan os.Signal, 1)
|
||||||
|
signal.Notify(caught, os.Interrupt)
|
||||||
|
|
||||||
|
testEnded := make(chan struct{})
|
||||||
|
senderDone := make(chan struct{})
|
||||||
|
|
||||||
|
// Stop catching SIGINT only after the sender has returned. The sender
|
||||||
|
// waits for each SIGINT it sends to arrive on caught; one still on
|
||||||
|
// its way after signal.Stop would kill the test binary.
|
||||||
|
t.Cleanup(func() {
|
||||||
|
close(testEnded)
|
||||||
|
<-senderDone
|
||||||
|
signal.Stop(caught)
|
||||||
|
})
|
||||||
|
|
||||||
|
go func() {
|
||||||
|
defer close(senderDone)
|
||||||
|
|
||||||
|
select {
|
||||||
|
case <-requestArrived:
|
||||||
|
case <-testEnded:
|
||||||
|
return
|
||||||
|
}
|
||||||
|
|
||||||
|
ticker := time.NewTicker(interruptRepeat)
|
||||||
|
defer ticker.Stop()
|
||||||
|
|
||||||
|
for {
|
||||||
|
// Empty caught, so that the receive below waits for this
|
||||||
|
// SIGINT rather than an earlier one.
|
||||||
|
select {
|
||||||
|
case <-caught:
|
||||||
|
default:
|
||||||
|
}
|
||||||
|
|
||||||
|
sendErr := self.Signal(os.Interrupt)
|
||||||
|
if sendErr != nil {
|
||||||
|
t.Errorf("sending SIGINT: %v", sendErr)
|
||||||
|
|
||||||
|
return
|
||||||
|
}
|
||||||
|
|
||||||
|
<-caught
|
||||||
|
|
||||||
|
select {
|
||||||
|
case <-testEnded:
|
||||||
|
return
|
||||||
|
case <-ticker.C:
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}()
|
||||||
|
}
|
||||||
|
|
||||||
|
// startStalledStore starts an HTTP server that never answers: each
|
||||||
|
// request is held until the client gives up on it or the test ends.
|
||||||
|
// It returns the server's host:port and a channel that receives a value
|
||||||
|
// when the first request arrives.
|
||||||
|
func startStalledStore(t *testing.T) (string, <-chan struct{}) {
|
||||||
|
t.Helper()
|
||||||
|
|
||||||
|
requestArrived := make(chan struct{}, 1)
|
||||||
|
release := make(chan struct{})
|
||||||
|
|
||||||
|
server := httptest.NewServer(http.HandlerFunc(
|
||||||
|
func(_ http.ResponseWriter, r *http.Request) {
|
||||||
|
select {
|
||||||
|
case requestArrived <- struct{}{}:
|
||||||
|
default:
|
||||||
|
}
|
||||||
|
|
||||||
|
select {
|
||||||
|
case <-r.Context().Done():
|
||||||
|
case <-release:
|
||||||
|
}
|
||||||
|
}))
|
||||||
|
|
||||||
|
// Cleanups run last-registered first, so release lets any held
|
||||||
|
// request return before Close waits for it.
|
||||||
|
t.Cleanup(server.Close)
|
||||||
|
t.Cleanup(func() { close(release) })
|
||||||
|
|
||||||
|
return server.Listener.Addr().String(), requestArrived
|
||||||
|
}
|
||||||
|
|
||||||
|
// writeStalledStoreConfig writes a config whose destination store is the
|
||||||
|
// server at endpoint and whose snapshot source holds one small file, so
|
||||||
|
// that `snapshot create` has a blob to upload. Returns the config path.
|
||||||
|
func writeStalledStoreConfig(t *testing.T, endpoint string) string {
|
||||||
|
t.Helper()
|
||||||
|
|
||||||
|
dir := t.TempDir()
|
||||||
|
configPath := filepath.Join(dir, "config.yml")
|
||||||
|
sourceDir := filepath.Join(dir, "source")
|
||||||
|
|
||||||
|
require.NoError(t, os.Mkdir(sourceDir, 0o750))
|
||||||
|
require.NoError(t, os.WriteFile(filepath.Join(sourceDir, "file.txt"),
|
||||||
|
[]byte("contents"), 0o600))
|
||||||
|
|
||||||
|
contents := fmt.Sprintf(stalledStoreConfig,
|
||||||
|
sourceDir, endpoint, filepath.Join(dir, "index.sqlite"))
|
||||||
|
|
||||||
|
require.NoError(t,
|
||||||
|
os.WriteFile(configPath, []byte(contents), configFileMode))
|
||||||
|
|
||||||
|
// The PID lock lives under xdg.DataHome, which xdg resolves at
|
||||||
|
// package init; point it at the temp dir so the test neither
|
||||||
|
// touches nor collides with the real one.
|
||||||
|
t.Setenv("XDG_DATA_HOME", filepath.Join(dir, "data"))
|
||||||
|
xdg.Reload()
|
||||||
|
t.Cleanup(xdg.Reload)
|
||||||
|
|
||||||
|
return configPath
|
||||||
|
}
|
||||||
Reference in New Issue
Block a user