fix(app): starting with past units should not increase the process units #2614
Conversation
Sure... lemme improve this test case. |
provision/provision.go
Outdated
| @@ -451,6 +451,12 @@ type InterAppProvisioner interface { | |||
| InternalAddresses(ctx context.Context, a App) ([]AppInternalAddress, error) | |||
| } | |||
|
|
|||
| // CurrentReplicasGetter implements how to get the current (desired) | |||
| // number of replicas (units) of an app. | |||
| type CurrentReplicasGetter interface { | |||
There was a problem hiding this comment.
@nettoclaudio could you rename to CurrentReplicasProvisioner to be like other interfaces ?
app/app.go
Outdated
|
|
||
| fmt.Fprintf(w, "Before being stopped, the %q process had %d replicas... re-adding them.\n", p, replicas) | ||
|
|
||
| err := prov.AddUnits(ctx, app, uint(replicas), p, version, w) |
There was a problem hiding this comment.
@nettoclaudio to perfectly achieve the number of replicas without risky of race-condition, we should implement the SetUnits method instead of AddUnits, but it is a nice to have.
There was a problem hiding this comment.
Although the name, the implementation on provisioner side is set-like already.
There was a problem hiding this comment.
Wait... it isn't. So you found a bug. 🎉
|
lgtm 👍🏻 |
|
Please don't merge this PR yet. I found a problem when user doesn't pass a specific version - in this case, the PR doesn't do anything at all. |
Prior to this PR, running consecutive app-stop and app-start could increase the number of replicas since we didn't check the status of the pods. This implementation delegates to the provisioner a new method to get the desired replicas number.