Skip to content

Commit

Permalink
fix: restic cli commands through 'run command' are cancelled when clo…
Browse files Browse the repository at this point in the history
…sing dialogue
  • Loading branch information
garethgeorge committed Aug 26, 2024
1 parent 79cae5b commit bb00afa
Show file tree
Hide file tree
Showing 4 changed files with 48 additions and 22 deletions.
6 changes: 5 additions & 1 deletion internal/api/backresthandler.go
Original file line number Diff line number Diff line change
Expand Up @@ -439,10 +439,14 @@ func (s *BackrestHandler) RunCommand(ctx context.Context, req *connect.Request[v
errChan := make(chan error, 1)
go func() {
start := time.Now()
zap.S().Infof("running command for webui: %v", req.Msg.Command)
if err := repo.RunCommand(ctx, req.Msg.Command, func(output []byte) {
outputs <- bytes.Clone(output)
}); err != nil {
}); err != nil && ctx.Err() == nil {
zap.S().Errorf("error running command for webui: %v", err)
errChan <- err
} else {
zap.S().Infof("command completed for webui: %v", time.Since(start))
}
outputs <- []byte("took " + time.Since(start).String())
cancel()
Expand Down
36 changes: 20 additions & 16 deletions internal/orchestrator/repo/repo.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
"time"

v1 "github.com/garethgeorge/backrest/gen/go/v1"
"github.com/garethgeorge/backrest/internal/orchestrator/logging"
"github.com/garethgeorge/backrest/internal/protoutil"
"github.com/garethgeorge/backrest/pkg/restic"
"github.com/google/shlex"
Expand All @@ -23,7 +24,6 @@ import (
type RepoOrchestrator struct {
mu sync.Mutex

l *zap.Logger
config *v1.Config
repoConfig *v1.Repo
repo *restic.Repo
Expand Down Expand Up @@ -76,10 +76,13 @@ func NewRepoOrchestrator(config *v1.Config, repoConfig *v1.Repo, resticPath stri
config: config,
repoConfig: repoConfig,
repo: repo,
l: zap.L().With(zap.String("repo", repoConfig.Id)),
}, nil
}

func (r *RepoOrchestrator) logger(ctx context.Context) *zap.Logger {
return logging.Logger(ctx).With(zap.String("repo", r.repoConfig.Id))
}

func (r *RepoOrchestrator) Init(ctx context.Context) error {
ctx, flush := forwardResticLogs(ctx)
defer flush()
Expand Down Expand Up @@ -117,7 +120,8 @@ func (r *RepoOrchestrator) SnapshotsForPlan(ctx context.Context, plan *v1.Plan)
}

func (r *RepoOrchestrator) Backup(ctx context.Context, plan *v1.Plan, progressCallback func(event *restic.BackupProgressEntry)) (*restic.BackupProgressEntry, error) {
zap.L().Debug("repo orchestrator starting backup", zap.String("repo", r.repoConfig.Id))
l := r.logger(ctx)
l.Debug("repo orchestrator starting backup", zap.String("repo", r.repoConfig.Id))

r.mu.Lock()
defer r.mu.Unlock()
Expand All @@ -127,7 +131,7 @@ func (r *RepoOrchestrator) Backup(ctx context.Context, plan *v1.Plan, progressCa
return nil, fmt.Errorf("failed to get snapshots for plan: %w", err)
}

r.l.Debug("got snapshots for plan", zap.String("repo", r.repoConfig.Id), zap.Int("count", len(snapshots)), zap.String("plan", plan.Id), zap.String("tag", TagForPlan(plan.Id)))
l.Debug("got snapshots for plan", zap.String("repo", r.repoConfig.Id), zap.Int("count", len(snapshots)), zap.String("plan", plan.Id), zap.String("tag", TagForPlan(plan.Id)))

startTime := time.Now()

Expand Down Expand Up @@ -163,13 +167,13 @@ func (r *RepoOrchestrator) Backup(ctx context.Context, plan *v1.Plan, progressCa

ctx, flush := forwardResticLogs(ctx)
defer flush()
r.l.Debug("starting backup", zap.String("repo", r.repoConfig.Id), zap.String("plan", plan.Id))
l.Debug("starting backup", zap.String("repo", r.repoConfig.Id), zap.String("plan", plan.Id))
summary, err := r.repo.Backup(ctx, plan.Paths, progressCallback, opts...)
if err != nil {
return summary, fmt.Errorf("failed to backup: %w", err)
}

r.l.Debug("backup completed", zap.String("repo", r.repoConfig.Id), zap.Duration("duration", time.Since(startTime)))
l.Debug("backup completed", zap.String("repo", r.repoConfig.Id), zap.Duration("duration", time.Since(startTime)))
return summary, nil
}

Expand Down Expand Up @@ -219,7 +223,7 @@ func (r *RepoOrchestrator) Forget(ctx context.Context, plan *v1.Plan, tags []str
forgotten = append(forgotten, snapshotProto)
}

zap.L().Debug("forget snapshots", zap.String("plan", plan.Id), zap.Int("count", len(forgotten)), zap.Any("policy", policy))
r.logger(ctx).Debug("forget snapshots", zap.String("plan", plan.Id), zap.Int("count", len(forgotten)), zap.Any("policy", policy))

return forgotten, nil
}
Expand All @@ -230,7 +234,7 @@ func (r *RepoOrchestrator) ForgetSnapshot(ctx context.Context, snapshotId string
ctx, flush := forwardResticLogs(ctx)
defer flush()

r.l.Debug("forget snapshot with ID", zap.String("snapshot", snapshotId), zap.String("repo", r.repoConfig.Id))
r.logger(ctx).Debug("forget snapshot with ID", zap.String("snapshot", snapshotId), zap.String("repo", r.repoConfig.Id))
return r.repo.ForgetSnapshot(ctx, snapshotId)
}

Expand All @@ -254,7 +258,7 @@ func (r *RepoOrchestrator) Prune(ctx context.Context, output io.Writer) error {
opts = append(opts, restic.WithFlags("--max-unused", fmt.Sprintf("%v%%", policy.MaxUnusedPercent)))
}

r.l.Debug("prune snapshots")
r.logger(ctx).Debug("prune snapshots")
err := r.repo.Prune(ctx, output, opts...)
if err != nil {
return fmt.Errorf("prune snapshots for repo %v: %w", r.repoConfig.Id, err)
Expand All @@ -280,7 +284,7 @@ func (r *RepoOrchestrator) Check(ctx context.Context, output io.Writer) error {
}
}

r.l.Debug("checking repo")
r.logger(ctx).Debug("checking repo")
err := r.repo.Check(ctx, output, opts...)
if err != nil {
return fmt.Errorf("check repo %v: %w", r.repoConfig.Id, err)
Expand All @@ -294,7 +298,7 @@ func (r *RepoOrchestrator) Restore(ctx context.Context, snapshotId string, path
ctx, flush := forwardResticLogs(ctx)
defer flush()

r.l.Debug("restore snapshot", zap.String("snapshot", snapshotId), zap.String("target", target))
r.logger(ctx).Debug("restore snapshot", zap.String("snapshot", snapshotId), zap.String("target", target))

var opts []restic.GenericOption
opts = append(opts, restic.WithFlags("--target", target))
Expand Down Expand Up @@ -324,7 +328,7 @@ func (r *RepoOrchestrator) UnlockIfAutoEnabled(ctx context.Context) error {
ctx, flush := forwardResticLogs(ctx)
defer flush()

zap.L().Debug("auto-unlocking repo", zap.String("repo", r.repoConfig.Id))
r.logger(ctx).Debug("auto-unlocking repo", zap.String("repo", r.repoConfig.Id))

return r.repo.Unlock(ctx)
}
Expand All @@ -333,7 +337,7 @@ func (r *RepoOrchestrator) Unlock(ctx context.Context) error {
r.mu.Lock()
defer r.mu.Unlock()

r.l.Debug("unlocking repo", zap.String("repo", r.repoConfig.Id))
r.logger(ctx).Debug("unlocking repo", zap.String("repo", r.repoConfig.Id))
r.repo.Unlock(ctx)

return nil
Expand All @@ -345,7 +349,7 @@ func (r *RepoOrchestrator) Stats(ctx context.Context) (*v1.RepoStats, error) {
ctx, flush := forwardResticLogs(ctx)
defer flush()

r.l.Debug("getting repo stats", zap.String("repo", r.repoConfig.Id))
r.logger(ctx).Debug("getting repo stats", zap.String("repo", r.repoConfig.Id))
stats, err := r.repo.Stats(ctx)
if err != nil {
return nil, fmt.Errorf("stats for repo %v: %w", r.repoConfig.Id, err)
Expand All @@ -361,7 +365,7 @@ func (r *RepoOrchestrator) AddTags(ctx context.Context, snapshotIDs []string, ta
defer flush()

for idx, snapshotIDs := range chunkBy(snapshotIDs, 20) {
r.l.Debug("adding tag to snapshots", zap.Strings("snapshots", snapshotIDs), zap.Strings("tags", tags))
r.logger(ctx).Debug("adding tag to snapshots", zap.Strings("snapshots", snapshotIDs), zap.Strings("tags", tags))
if err := r.repo.AddTags(ctx, snapshotIDs, tags); err != nil {
return fmt.Errorf("batch %v: %w", idx, err)
}
Expand All @@ -377,7 +381,7 @@ func (r *RepoOrchestrator) RunCommand(ctx context.Context, command string, onPro
ctx, flush := forwardResticLogs(ctx)
defer flush()

r.l.Debug("running command", zap.String("command", command))
r.logger(ctx).Debug("running command", zap.String("command", command))
args, err := shlex.Split(command)
if err != nil {
return fmt.Errorf("parse command: %w", err)
Expand Down
2 changes: 1 addition & 1 deletion webui/src/components/ActivityBar.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ export const ActivityBar = () => {
return (
<span key={idx}>
{displayName} in progress for plan {op.planId} to {op.repoId} for{" "}
{formatDuration(Number(op.unixTimeStartMs - op.unixTimeEndMs))}
{formatDuration(Number(op.unixTimeEndMs - op.unixTimeStartMs))}
</span>
);
})}
Expand Down
26 changes: 22 additions & 4 deletions webui/src/views/RunCommandModal.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { useShowModal } from "../components/ModalManager";
import { backrestService } from "../api";
import { SpinButton } from "../components/SpinButton";
import { ConnectError } from "@connectrpc/connect";
import { useAlertApi } from "../components/Alerts";

interface Invocation {
command: string;
Expand All @@ -13,11 +14,19 @@ interface Invocation {

export const RunCommandModal = ({ repoId }: { repoId: string }) => {
const showModal = useShowModal();
const alertApi = useAlertApi()!;
const [command, setCommand] = React.useState("");
const [running, setRunning] = React.useState(false);
const [invocations, setInvocations] = React.useState<Invocation[]>([]);
const [abortController, setAbortController] = React.useState<
AbortController | undefined
>();

const handleCancel = () => {
if (abortController) {
alertApi.warning("In-progress restic command was aborted");
abortController.abort();
}
showModal(null);
};

Expand All @@ -31,10 +40,18 @@ export const RunCommandModal = ({ repoId }: { repoId: string }) => {
let segments: string[] = [];

try {
for await (const bytes of backrestService.runCommand({
repoId,
command,
})) {
const abortController = new AbortController();
setAbortController(abortController);

for await (const bytes of backrestService.runCommand(
{
repoId,
command,
},
{
signal: abortController.signal,
}
)) {
const output = new TextDecoder("utf-8").decode(bytes.value);
segments.push(output);
setInvocations((invocations) => {
Expand All @@ -57,6 +74,7 @@ export const RunCommandModal = ({ repoId }: { repoId: string }) => {
});
} finally {
setRunning(false);
setAbortController(undefined);
}
};

Expand Down

0 comments on commit bb00afa

Please sign in to comment.