helper/resource: restore retval of resource.Retry on timeout

In #4700 while fixing a data race I made an incorrect assumption about
the return value of StateChangeConf, and ended up changing the behavior
in the timeout case to always return:

> timeout while waiting for state to become '[success]'

When it used to capture the "most recent error" from the function
itself.

It's much more useful to see that error bubbling up, so here we revert
to pulling it out of the function as we did before, and we protect
against the data race with a good old fashioned mutex.
This commit is contained in:
Paul Hinze 2016-03-04 11:20:48 -06:00
parent f11448c692
commit 5160578e18
1 changed files with 18 additions and 4 deletions

View File

@ -1,6 +1,7 @@
package resource
import (
"sync"
"time"
)
@ -10,6 +11,11 @@ type RetryFunc func() error
// Retry is a basic wrapper around StateChangeConf that will just retry
// a function until it no longer returns an error.
func Retry(timeout time.Duration, f RetryFunc) error {
// These are used to pull the error out of the function; need a mutex to
// avoid a data race.
var resultErr error
var resultErrMu sync.Mutex
c := &StateChangeConf{
Pending: []string{"error"},
Target: []string{"success"},
@ -21,17 +27,25 @@ func Retry(timeout time.Duration, f RetryFunc) error {
return 42, "success", nil
}
resultErrMu.Lock()
defer resultErrMu.Unlock()
resultErr = err
if rerr, ok := err.(RetryError); ok {
err = rerr.Err
return nil, "quit", err
resultErr = rerr.Err
return nil, "quit", rerr.Err
}
return 42, "error", nil
},
}
_, err := c.WaitForState()
return err
c.WaitForState()
// Need to acquire the lock here to be able to avoid race using resultErr as
// the return value
resultErrMu.Lock()
defer resultErrMu.Unlock()
return resultErr
}
// RetryError, if returned, will quit the retry immediately with the