Skip to content

[Feature][RayCluster]: Generate GCS FT Redis Cleanup Job creation events #2382

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged

Conversation

rueian
Copy link
Contributor

@rueian rueian commented Sep 15, 2024

Why are these changes needed?

Following the KubeRay CRD Observability Improvement Workstream, we add the two missing events for Redis Cleanup Job Creation:

  • CreatedRedisCleanupJob
  • FailedToCreateRedisCleanupJob

Related issue number

Checks

  • I've made sure the tests are passing.
  • Testing Strategy
    • Unit tests
    • Manual tests

@rueian
Copy link
Contributor Author

rueian commented Sep 15, 2024

Hi @kevin85421, please take a look. Thanks!

@@ -302,9 +302,13 @@ func (r *RayClusterReconciler) rayClusterReconcile(ctx context.Context, request
logger.Info("Redis cleanup Job already exists. Requeue the RayCluster CR.")
return ctrl.Result{RequeueAfter: DefaultRequeueDuration}, nil
}
r.Recorder.Eventf(instance, corev1.EventTypeWarning, string(utils.FailedToCreateRedisCleanupJob),
"Failed to create Redis cleanup Job %s/%s, %v", redisCleanupJob.Namespace, redisCleanupJob.Name, err)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: make the wording consistent. Either "Failed to create" or "Failed creating" is fine.

r.Recorder.Eventf(instance, corev1.EventTypeWarning, string(utils.FailedToCreateService), "Failed creating service %s/%s, %v", svc.Namespace, svc.Name, err)

@kevin85421 kevin85421 merged commit d025792 into ray-project:master Sep 16, 2024
27 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants