Bug fixes for timeout #1654
Bug fixes for timeout #1654
Conversation
asn1809
commented
Nov 15, 2024
asn1809
commented
- TImeout checks to include the check hook execution as well.
- Incorporating review comments pending from Check hook implementation #1649
Signed-off-by: Annaraya Narasagond <[email protected]> Signed-off-by: Annaraya Narasagond <[email protected]> Annaraya Narasagond <[email protected]>
Signed-off-by: Annaraya Narasagond <[email protected]> Signed-off-by: Annaraya Narasagond <[email protected]> Annaraya Narasagond <[email protected]>
| // err := WaitUntilResourceExists(client, resource, nsScopedName, time.Duration(timeout)*time.Second) | ||
| // if err != nil { | ||
| // return false, err | ||
| // } |
There was a problem hiding this comment.
Why do you keep this as commented code?
|
|
||
| return EvaluateCheckHookExp(hook.Chk.Condition, resource) | ||
| //return EvaluateCheckHookExp(hook.Chk.Condition, resource) |
There was a problem hiding this comment.
Why do you keep this commented code?
| ctx, cancel := context.WithTimeout(context.Background(), timeout) | ||
| defer cancel() | ||
|
|
||
| ticker := time.NewTicker(pollInterval * time.Millisecond) // Poll every 100 milliseconds |
There was a problem hiding this comment.
This comment will quickly become stale when we change pollInternal. It is better to remove it.
However polling every 100 millisecond is too much. If we need to check a resource until the resource reach some state, the right way is to watch the resource. Each time the resources changes, you check the desired state.
Until we get a proper watch we can make this more efficient by using a backoff. We can start with short wait internal to be able to detect very short waits quickly (e.g. 10 ms), and double the interval on each retry, until we reach maximum internal (e.g. 10 seconds).
| @@ -232,27 +266,27 @@ func compareValues(val1, val2 interface{}, operator string) (bool, error) { | |||
| case float64: | |||
| v2, ok := val2.(float64) | |||
| if !ok { | |||
| return false, fmt.Errorf("mismatched types") | |||
| return false, fmt.Errorf("mismatched types %T and %T", val1, val2) | |||
There was a problem hiding this comment.
This error is not clear - which is the expected value? we can make it clear using:
fmt.Errorf("type mismatch: expected: %T, actual: %T", expected, actual)
Same for other errors bellow.
Signed-off-by: Annaraya Narasagond <[email protected]> Signed-off-by: Annaraya Narasagond <[email protected]> Annaraya Narasagond <[email protected]>
Signed-off-by: Annaraya Narasagond <[email protected]> Signed-off-by: Annaraya Narasagond <[email protected]> Annaraya Narasagond <[email protected]>
