Skip to content
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

etcdctl: fix exit status of endpoint command #8316

Closed
wants to merge 1 commit into from
Closed

etcdctl: fix exit status of endpoint command #8316

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
25 changes: 21 additions & 4 deletions etcdctl/ctlv3/command/ep_command.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,15 +81,28 @@ func epHealthCommandFunc(cmd *cobra.Command, args []string) {
}

var wg sync.WaitGroup
errc := make(chan error, len(cfgs))
errCountc := make(chan int, 1)
defer close(errCountc)

// drain errors channel until it is closed.
go func() {
Copy link
Contributor

Choose a reason for hiding this comment

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

this extra concurrency isn't necessary since errc is already buffered-- keep it simple by doing the error pass after the other goroutine complete:

wg.Wait()
close(errc)
errs := false
for err := range errc {
    if err != nil {
        errs = true
        fmt.Fprintln(os.Stderr, err)
    }   
}
if errs {
    ExitWithError(ExitError, fmt.Errorf("unhealthy cluster"))
}

var count int
for err := range errc {
count++
fmt.Fprintln(os.Stderr, err)
}
errCountc <- count
}()

for _, cfg := range cfgs {
wg.Add(1)
go func(cfg *v3.Config) {
go func(cfg *v3.Config, errc chan<- error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

this line can stay the same-- errc is already captured by the closure

defer wg.Done()
ep := cfg.Endpoints[0]
cli, err := v3.New(*cfg)
if err != nil {
fmt.Printf("%s is unhealthy: failed to connect: %v\n", ep, err)
errc <- fmt.Errorf("%s is unhealthy: failed to connect: %v", ep, err)
return
}
st := time.Now()
Expand All @@ -102,12 +115,16 @@ func epHealthCommandFunc(cmd *cobra.Command, args []string) {
if err == nil || err == rpctypes.ErrPermissionDenied {
fmt.Printf("%s is healthy: successfully committed proposal: took = %v\n", ep, time.Since(st))
} else {
fmt.Printf("%s is unhealthy: failed to commit proposal: %v\n", ep, err)
errc <- fmt.Errorf("%s is unhealthy: failed to commit proposal: %v", ep, err)
}
}(cfg)
}(cfg, errc)
}

wg.Wait()
close(errc)

// exit with non-zero exit status in case of errors.
os.Exit(<-errCountc)
Copy link
Contributor

Choose a reason for hiding this comment

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

the exit code shouldn't change based on the number of errors; ExitWithError(ExitError, fmt.Errorf("unhealthy cluster")) would be more consistent with the rest of etcdctl

}

type epStatus struct {
Expand Down