Skip to content
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

Rename ClusterRing to ControllerRing #429

Closed
5 tasks done
timebertt opened this issue Jan 19, 2025 · 0 comments · Fixed by #438
Closed
5 tasks done

Rename ClusterRing to ControllerRing #429

timebertt opened this issue Jan 19, 2025 · 0 comments · Fixed by #438
Assignees
Labels
enhancement New feature or request

Comments

@timebertt
Copy link
Owner

timebertt commented Jan 19, 2025

What would you like to be added:

  • rename the ClusterRing resource to ControllerRing
  • drop the clusterring- part of shard and drain label keys
  • drop kind and namespace params from all shardingv1alpha1.* functions
  • drop the kind-related and namespace labels from metrics
  • drop the shardring.Ring interface

Why is this needed:

Originally, I envisioned that there could be two implementations of a "controller ring" resource: a namespaced one (Ring) and a cluster-scoped one (ClusterRing). The first one would only shard objects in the same namespace, the second one in all namespaces.
With the existing namespaceSelector field of the ClusterRing resource, one can simply implement a namespaced or even multi-namespaced controller ring.
Also, the resulting MutatingWebhookConfiguration is a cluster-scoped object and, thus, can only be owned by other cluster-scoped objects but not by namespaced ones.
Furthermore, implementing both versions of the resource would require one controller each.

In the end, there doesn't seem to be a good reason for implementing a namespaced version of the ClusterRing resource.
Hence, I suggest renaming the resource from ClusterRing to ControllerRing – which is more specific and less confusing.

With this, all places that kept options open for handling namespaced rings can be simplified.

@timebertt timebertt added the enhancement New feature or request label Jan 19, 2025
@timebertt timebertt added this to the v0.9.0 milestone Jan 19, 2025
@timebertt timebertt removed this from the v0.9.0 milestone Jan 22, 2025
@timebertt timebertt moved this from Backlog to In progress in kubernetes-controller-sharding Jan 22, 2025
@timebertt timebertt self-assigned this Jan 22, 2025
@timebertt timebertt linked a pull request Jan 24, 2025 that will close this issue
@github-project-automation github-project-automation bot moved this from In progress to Done in kubernetes-controller-sharding Jan 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Development

Successfully merging a pull request may close this issue.

1 participant