Add parameter provisioningTimeout to Kubernetes driver options. #2457
Conversation
| @@ -113,7 +114,7 @@ func (d *Driver) wait(ctx context.Context) error { | |||
| select { | |||
| case <-ctx.Done(): | |||
| return ctx.Err() | |||
| case <-time.After(time.Duration(100+try*20) * time.Millisecond): | |||
There was a problem hiding this comment.
I think we should keep the backoff logic
There was a problem hiding this comment.
It will be ok for you if I will just change 100 → 300?
There was a problem hiding this comment.
@crazy-max I've hit this issue too and would appreciate a timeout option. I'd be interested to know why the backoff is better when the nodes don't spin up in time.
There was a problem hiding this comment.
I'd be interested to know why the backoff is better when the nodes don't spin up in time.
@SimonJPegg Don't think you would want to hammer the API every Xms but instead backoff on each retry. Although looking at the retry logic it seems fair to not keep it.
It will be ok for you if I will just change
100→300?
I think it's fine to keep 100 as default but might be better to use a duration, see #2457 (comment)
driver/kubernetes/factory.go
Outdated
| @@ -229,6 +232,8 @@ func (f *factory) processDriverOpts(deploymentName string, namespace string, cfg | |||
| if err != nil { | |||
| return nil, "", "", false, err | |||
| } | |||
| case "provisioningTimeout": | |||
| deploymentOpt.ProvisioningTimeout, err = strconv.Atoi(v) | |||
There was a problem hiding this comment.
Can we use a time.Duration type instead? Would need to do a time.ParseDuration here.
driver/kubernetes/driver.go
Outdated
| configMapClient clientcorev1.ConfigMapInterface | ||
| podChooser podchooser.PodChooser | ||
| defaultLoad bool | ||
| provisioningTimeout int |
There was a problem hiding this comment.
I'm not sure on naming it provisioningTimeout as this is applied in a retry logic. Maybe retryTimeout?
|
PTAL #2472 |
Arsobbiak
force-pushed
the
extend_k8s_provisioning_timeout
branch
from
6a18032
to
fe4788b
Compare
|
Sorry that it took me so long to come back to this PR. Thanks, @crazy-max, for pointing out that I can use With the chatbot's help and your comments, I improved it slightly. |
Signed-off-by: Arnold Sobanski <arnold@l4g.dev>
Arsobbiak
force-pushed
the
extend_k8s_provisioning_timeout
branch
from
fe4788b
to
6677e0d
Compare
| tryCounter++ | ||
| } | ||
| ticker.Stop() | ||
| ticker = time.NewTicker(time.Duration(100+tryCounter*20) * time.Millisecond) |
There was a problem hiding this comment.
Im not sure if that logic is correct and if there is no better way of doing that.
Or maybe I should keep it static as 100*time.Millisecond
During running builds on the Kubernetes, we hit timeout on pod provisioning.
Our k8s cluster is running on EKS with Karpenter.
From time to time, node provisioning takes around one and a half minutes till it is ready to use.
Error:
This PR will let to specify
provisioningTimeoutatdriver-optparameter.If that looks good, I will add a parameter to the docker documentation.
As a second option, I'm happy to change only 100 → 200 at line: