Change Horizontal Pod Autoscaler to scale on OpenTelemetry Collector … #984
Conversation
…object Signed-off-by: Kevin Earls <[email protected]>
Signed-off-by: Kevin Earls <[email protected]>
Signed-off-by: Kevin Earls <[email protected]>
|
@pavolloffay Can you please review this again and then merge it if it looks ok? |
Signed-off-by: Kevin Earls <[email protected]>
| } | ||
| upgradedInstance, err := versionUpgrade.ManagedInstance(context.Background(), collectorInstance) | ||
| assert.NoError(t, err) | ||
| assert.Equal(t, one, *upgradedInstance.Spec.MinReplicas) |
There was a problem hiding this comment.
The test should as well assert that the HPA object was changed accordingly.
There was a problem hiding this comment.
I'm not sure if this is possible as upgrade get the HPA by calling the k8sclient. It's not in the object that is passed in:
https://jenkins-cpaas-distributed-tracing.apps.cpaas-poc.r6c9.p1.openshiftapps.com/
There was a problem hiding this comment.
It should be possible the test can access the k8sClient and create the objects before running the upgrade procedure.
pkg/collector/upgrade/v0_56_0.go
Outdated
| patch := client.MergeFrom(&existing) | ||
| err := u.Client.Patch(ctx, updated, patch) | ||
| if err != nil { | ||
| return nil, err |
There was a problem hiding this comment.
We could continue on an error and then return a multierror at the end wrapped with a message that the upgrade of HPA was not successful.
Either way the upgrade will have to be restarted or done manually.
| } | ||
| } | ||
|
|
||
| u.Log.Info("in upgrade0_56_0", "Otel Instance", otelcol.Name, "Upgrade version", u.Version.String()) |
There was a problem hiding this comment.
IIRC the other upgrade procedures use event API to record the "log" messages for the user.
pkg/collector/upgrade/v0_56_0.go
Outdated
| hpaList := &autoscalingv1.HorizontalPodAutoscalerList{} | ||
| ctx := context.Background() | ||
| if err := u.Client.List(ctx, hpaList, listOptions...); err != nil { | ||
| return nil, err |
There was a problem hiding this comment.
We should wrap the error to make it clear what operation failed
Signed-off-by: Kevin Earls <[email protected]>
| } | ||
| upgradedInstance, err := versionUpgrade.ManagedInstance(context.Background(), collectorInstance) | ||
| assert.NoError(t, err) | ||
| assert.Equal(t, one, *upgradedInstance.Spec.MinReplicas) |
There was a problem hiding this comment.
It should be possible the test can access the k8sClient and create the objects before running the upgrade procedure.
open-telemetry#984) * Change Horizontal Pod Autoscaler to scale on OpenTelemetry Collector object Signed-off-by: Kevin Earls <[email protected]> * Update doc; move upgrade code out of reconciler Signed-off-by: Kevin Earls <[email protected]> * Fixed doc Signed-off-by: Kevin Earls <[email protected]> * Respond to comments, implement test Signed-off-by: Kevin Earls <[email protected]> * respond to comments Signed-off-by: Kevin Earls <[email protected]>
…object
Signed-off-by: Kevin Earls [email protected]