Skip to content

Signal failure status from fsck in headless mode#878

Open
roger2hk wants to merge 1 commit intotransparency-dev:mainfrom
roger2hk:fsck-headless-signal-failure
Open

Signal failure status from fsck in headless mode#878
roger2hk wants to merge 1 commit intotransparency-dev:mainfrom
roger2hk:fsck-headless-signal-failure

Conversation

@roger2hk
Copy link
Contributor

@roger2hk roger2hk commented Mar 4, 2026

No description provided.

@roger2hk roger2hk requested a review from AlCutter March 4, 2026 11:42
@roger2hk roger2hk requested a review from a team as a code owner March 4, 2026 11:42
@roger2hk roger2hk force-pushed the fsck-headless-signal-failure branch from 709ad2a to ffb92a0 Compare March 4, 2026 15:35
}
klog.V(1).Infof("Completed ranges:\n%s", f.Status())

checkErr <- err
Copy link
Collaborator

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.

}
f := fsck.New(*origin, v, src, defaultMerkleLeafHasher, fsck.Opts{N: *N})

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

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.

klog.V(1).Infof("Completed ranges:\n%s", f.Status())

checkErr <- err
cancel()
Copy link
Collaborator

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants