feat: Add provisioner force-cancel flag #4947
Conversation
cli/deployment/config.go
Outdated
| @@ -359,6 +359,14 @@ func newConfig() *codersdk.DeploymentConfig { | |||
| Flag: "user-workspace-quota", | |||
| Enterprise: true, | |||
| }, | |||
| Provisionerd: &codersdk.ProvisionerdConfig{ | |||
There was a problem hiding this comment.
We have another config called ProvisionerDaemons. I think it would be sensible to unify these under Provisioner.Daemons and Provisioner.ForceCancelInterval?
Should be a non-breaking change as well since the flag/env translate to the same.
There was a problem hiding this comment.
I assumed that this would be a breaking change if there is a customer who uses the ProvisionerDaemons. I'm for unification, but a bit concerned about disturbing somebody's environment.
There was a problem hiding this comment.
I think it should be fine. Neither flag or env should break (was PROVISIONER_DAEMONS and will be the same in the new structure, please correct me if I’m wrong @f0ssel).
The only breakage I can think if is when used as part of server.yaml. But it’s a recent addition and not widely used yet AFAIK.
There may be even more breaking changes to this in the future as we split out provisionerd servers from the coderd server.
| @@ -65,7 +65,7 @@ func New(clientDialer Dialer, opts *Options) *Server { | |||
| opts.UpdateInterval = 5 * time.Second | |||
| } | |||
| if opts.ForceCancelInterval == 0 { | |||
There was a problem hiding this comment.
A thought/musing: Do we want to support disabling this by setting it to 0? Or do we just ask users to raise it to a really high number ™️?
(No change necessary here, the latter is fine.)
There was a problem hiding this comment.
Good question, Mathias. I wouldn't recommend setting it to infinite as it can be problematic for some cases (stuck forever). I guess that it implicates the latter option.
There was a problem hiding this comment.
That’s a very good point. So yeah, let’s not allow our users to create a deadlock with all provisioners waiting forever 😄
mtojek
changed the title
Fixes: #4586
This PR adds a new flag to configure the force-cancel interval for provisionerd. It also increases the default value.