rackspace / gophercloud

A Go SDK for OpenStack. IN FEATURE FREEZE. See Issue #592
http://gophercloud.io
Other
456 stars 185 forks source link

util function WaitFor (and depending function servers.WaitForStatus) do not properly / never timeout #625

Open HeikoOnnebrink opened 7 years ago

HeikoOnnebrink commented 7 years ago

I had issues that servers.WaitForStatus does not timeout when I pass timeouts > 60 seconds I debugged and found that at the end the timeout is implemented in gophercloud package func WaitFor. As it looks like the code has bug as var start is of type int and can have a range 1..60 If I pass timeout > 60 it will never terminate, if I pass values <= 60 wait time is random

Go lang spec: func (t Time) Second() int Second returns the second offset within the minute specified by t, in the range [0, 59].

package gophercloud

func WaitFor(timeout int, predicate func() (bool, error)) error { start := time.Now().Second() for { // Force a 1s sleep time.Sleep(1 * time.Second) // If a timeout is set, and that's been exceeded, shut it down if timeout >= 0 && time.Now().Second()-start >= timeout { return errors.New("A timeout occurred") } // Execute the function satisfied, err := predicate() if err != nil { return err } if satisfied { return nil } } }

HeikoOnnebrink commented 7 years ago

here the fix ...

// WaitFor polls a predicate function, once per second, up to a timeout limit. // It usually does this to wait for a resource to transition to a certain state. // Resource packages will wrap this in a more convenient function that's // specific to a certain resource, but it can also be useful on its own. func WaitFor(timeout int, predicate func() (bool, error)) error { start := time.Now() for { // Force a 1s sleep time.Sleep(1 * time.Second)

    // If a timeout is set, and that's been exceeded, shut it down
    if timeout >= 0 && time.Since(start) >= time.Duration(timeout) * time.Millisecond {
        return errors.New("A timeout occurred")
    }

    // Execute the function
    satisfied, err := predicate()
    if err != nil {
        return err
    }
    if satisfied {
        return nil
    }
}

}

HeikoOnnebrink commented 7 years ago

timeout in Seconds > if timeout >= 0 && time.Since(start) >= time.Duration(timeout) * time.Second {

spurti-chopra commented 7 years ago

I am also facing the same issue while using this utility function. Would like to know when would this case be handled ? Is it planned for any new release ?