fixed autoscaling bug and updated hpa sample #270
Conversation
Signed-off-by: Andrew Steurer <[email protected]>
Signed-off-by: Andrew Steurer <[email protected]>
asteurer
requested review from
calebschoepp,
endocrimes,
bacongobbler and
michelleN
as code owners
Signed-off-by: Andrew Steurer <[email protected]>
|
@bacongobbler @calebschoepp I removed the requirements for a minimum pod count, but I'm running into an issue with |
| @@ -5,7 +5,7 @@ metadata: | |||
| spec: | |||
| image: ghcr.io/spinkube/spin-operator/cpu-load-gen:20240311-163328-g1121986 | |||
| executor: containerd-shim-spin | |||
| enableAutoscaling: true | |||
| replicas: 1 | |||
There was a problem hiding this comment.
@bacongobbler says:
Shouldn't this be removed if both
minReplicasandmaxReplicasare set?
| } else { | ||
| replicas = ptr(app.Spec.Replicas) | ||
| } | ||
| replicas := app.Spec.Replicas |
There was a problem hiding this comment.
@bacongobbler says:
related to the above comment RE: min/maxReplicas, shouldn't this be nil if either of those are set?
If only minReplicas is set (is that valid?), should this be set to minReplicas as a starting value?
If only maxReplicas is set, should this be set to 1 (or possibly nil, assuming we can scale to zero)?
| @@ -99,6 +91,9 @@ type SpinAppStatus struct { | |||
|
|
|||
| // Represents the current number of active replicas on the application deployment. | |||
| ReadyReplicas int32 `json:"readyReplicas"` | |||
|
|
|||
| // Selector for the underlying pods | |||
| Selector string `json:"selector"` | |||
There was a problem hiding this comment.
@endocrimes would we have to make this optional to support runtimes where this selector is meaningless? Or is setting it to the empty string sufficient.
Maybe you can't port forward the service because the replicas defaults to zero, so there are no pods, so there is nothing to port forward. It's a bit circular haha. You could try playing with KEDA scaling on some external factor like messages in a queue. That way to generate load you just put a bunch of messages in a queue and then it would scale it up from zero. |
Shouldn't there be at least 1 pod since we set a minReplicas? If so, I'm not clear on how we would reference the HPA in the SpinApp deployment. Also, |
This looks relevant. |
There was a problem hiding this comment.
No new comments from me. I just want you to address @bacongobbler's comments
| @@ -5,7 +5,7 @@ metadata: | |||
| spec: | |||
| image: ghcr.io/spinkube/spin-operator/cpu-load-gen:20240311-163328-g1121986 | |||
| executor: containerd-shim-spin | |||
| enableAutoscaling: true | |||
| replicas: 1 | |||
| } else { | ||
| replicas = ptr(app.Spec.Replicas) | ||
| } | ||
| replicas := app.Spec.Replicas |
|
There's a bug in SpinKube that is preventing metrics from being properly emitted from Spin applications, which is preventing autoscaling, and is making it impossible for me to test. Kate Goldenring is working with jsturtevant (slack handle) from MS to get this fixed. |
No description provided.