Skip to content

Signal failure status from fsck in headless mode #878

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
roger2hk wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Signal failure status from fsck in headless mode #878

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
13 changes: 11 additions & 2 deletions cmd/fsck/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,11 +80,17 @@ func main() {
}
f := fsck.New(*origin, v, src, defaultMerkleLeafHasher, fsck.Opts{N: *N})

checkErr := make(chan error, 1)
Copy link
Collaborator

@AlCutter AlCutter Mar 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe rename this checkResult? I spend some time trying to reason about the replacement of ctx.Done with <-checkErr in the loop below before I noticed that you just send whatever err is, even if it's nil.

Probably worth a comment to that effect on this channel, too as it's a little unusual.

go func() {
if err := f.Check(ctx); err != nil {
err := f.Check(ctx)

// Only log the error if we're using the TUI, otherwise it will be double logged in headless mode.
if err != nil && *ui {
klog.Errorf("fsck failed: %v", err)
}
klog.V(1).Infof("Completed ranges:\n%s", f.Status())

checkErr <- err
Copy link
Collaborator

@AlCutter AlCutter Mar 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: strictly, you should probably close(checkErr) here.

cancel()
Copy link
Collaborator

@AlCutter AlCutter Mar 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should this cancel() now live in the loop below just before the return in the <-checkErr case? (I think it's +/- equivalent, but mirrors the cancellation in the if *ui {} side.

}()

Expand All @@ -97,7 +103,10 @@ func main() {
} else {
for {
select {
case <-ctx.Done():
case err := <-checkErr:
if err != nil {
klog.Exitf("fsck failed: %v", err)
}
return
case <-time.After(time.Second):
klog.V(1).Infof("Ranges:\n%s", f.Status())
Expand Down
Loading