chore: Adds CoudEvents when a ScaledObject scales a workload from/to zero or one #5632
Conversation
… zero or one replicas Signed-off-by: neelanjan00 <neelanjan.manna@harness.io>
Signed-off-by: neelanjan00 <neelanjan.manna@harness.io>
Signed-off-by: neelanjan00 <neelanjan.manna@harness.io>
|
Can you open a PR for our docs as well please? |
| @@ -28,4 +28,8 @@ const ( | |||
| ScaleTargetNotFoundMsg = "Target resource doesn't exist" | |||
|
|
|||
| ScaleTargetNoSubresourceMsg = "Target resource doesn't expose /scale subresource" | |||
|
|
|||
| ScaleTargetFromZero = "Target resource is scaling up from zero number of replicas" | |||
There was a problem hiding this comment.
| ScaleTargetFromZero = "Target resource is scaling up from zero number of replicas" | |
| ScaleTargetFromZero = "Target resource is scaling out from zero" |
|
|
||
| ScaleTargetFromZero = "Target resource is scaling up from zero number of replicas" | ||
|
|
||
| ScaleTargetToZero = "Target resource is scaling down to zero number of replicas" |
There was a problem hiding this comment.
| ScaleTargetToZero = "Target resource is scaling down to zero number of replicas" | |
| ScaleTargetToZero = "Target resource is scaling in to zero replicas" |
|
@neelanjan00 any update on this please? |
|
eventEmitter.Emit() will emit a k8s event at the same time. Replacing the recorder with eventEmitter and r.Recorder.Event() with eventEmitter.Emit() is better than passing an eventEmitter as a new parameter. Alternatively, we could pass an eventEmitter as a new parameter and delete the recorder once all r.Recorder.Event() calls have been replaced. |
| @@ -272,6 +275,13 @@ func (e *scaleExecutor) scaleToZeroOrIdle(ctx context.Context, logger logr.Logge | |||
|
|
|||
| e.recorder.Eventf(scaledObject, corev1.EventTypeNormal, eventreason.KEDAScaleTargetDeactivated, | |||
There was a problem hiding this comment.
This one and the new one are duplicated. We should merge these two into one event. eventEmitter.Emit() includes emitting k8s event.
| corev1.EventTypeNormal, eventemitter.ScaleFromZeroType, eventreason.KEDAScaleTargetActivated, | ||
| message.ScaleTargetFromZero) | ||
| } | ||
|
|
||
| e.recorder.Eventf(scaledObject, corev1.EventTypeNormal, eventreason.KEDAScaleTargetActivated, "Scaled %s %s/%s from %d to %d", scaledObject.Status.ScaleTargetKind, scaledObject.Namespace, scaledObject.Spec.ScaleTargetRef.Name, currentReplicas, replicas) |
There was a problem hiding this comment.
This one and the new one are duplicated. We should merge these two into one event.
| @@ -80,7 +80,7 @@ func init() { | |||
|
|
|||
| // SetupWithManager initializes the ScaledJobReconciler instance and starts a new controller managed by the passed Manager instance. | |||
| func (r *ScaledJobReconciler) SetupWithManager(mgr ctrl.Manager, options controller.Options) error { | |||
| r.scaleHandler = scaling.NewScaleHandler(mgr.GetClient(), nil, mgr.GetScheme(), r.GlobalHTTPTimeout, mgr.GetEventRecorderFor("scale-handler"), r.SecretsLister) | |||
| r.scaleHandler = scaling.NewScaleHandler(mgr.GetClient(), nil, mgr.GetScheme(), r.GlobalHTTPTimeout, mgr.GetEventRecorderFor("scale-handler"), r.SecretsLister, nil) | |||
There was a problem hiding this comment.
create and pass a new event emitter as well so that it doesn't need to check nil later.
|
We are planning release on Thursday, any chance we can incorporate the feedback? |
|
Hey @tomkerkhove, @zroubalik sorry for not being active on this! I am presently occupied with some office work so not able to put time away for this, not sure if I will be able to add the E2E tests on time for the release. |
|
No worries! |
|
This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions. |
zroubalik
removed
the
stale
|
Any update on this @neelanjan00? If you prefer us to work on this then that is fine as well |
|
Hi @tomkerkhove sorry for being inactive on this, I don't think it will be feasible for me to work on the remaining tasks, it will be helpful if someone else can take it up. Thanks! |
|
Closing as discussed on #3527 (comment) @SpiritZhou is taking over and will use this one as inspiration - Thanks for working on this anyway @neelanjan00! |
Checklist
Relates to #3527