From cd5eb8485ed51d0ca4b1a82086505d135b6663bb Mon Sep 17 00:00:00 2001 From: Tim Ebert Date: Mon, 27 Jan 2025 22:48:28 +0100 Subject: [PATCH] Limit `ControllerRing` name length, simplify shard/drain label keys, simplify webhook name (#441) * Limit `ControllerRing` name length * Simplify webhook name * Simplify shard and drain labels * Update docs --- README.md | 2 +- ...harding.timebertt.dev_controllerrings.yaml | 3 ++ docs/development.md | 7 ++--- docs/evaluation.md | 10 +++---- docs/getting-started.md | 30 +++++++++---------- docs/implement-sharding.md | 11 +++---- pkg/apis/sharding/v1alpha1/constants.go | 22 ++------------ .../sharding/v1alpha1/types_controllerring.go | 1 + pkg/controller/controllerring/reconciler.go | 2 +- 9 files changed, 37 insertions(+), 51 deletions(-) diff --git a/README.md b/README.md index 69164270..5e69b115 100644 --- a/README.md +++ b/README.md @@ -76,4 +76,4 @@ See [Design](docs/design.md) for more details on the sharding architecture and d Feel free to contact me on the [Kubernetes Slack](https://kubernetes.slack.com/) ([get an invitation](https://slack.k8s.io/)): [@timebertt](https://kubernetes.slack.com/team/UF8C35Z0D). Also check out my talk at KubeCon EU 2025 about this project (see the [schedule](https://sched.co/1txFG)). -Reach out on Slack or try to grab me for a chat at the STACKIT booth! +Reach out on Slack or try to grab me for a chat at the STACKIT or Gardener booth! diff --git a/config/crds/sharding.timebertt.dev_controllerrings.yaml b/config/crds/sharding.timebertt.dev_controllerrings.yaml index 705728b9..9696c972 100644 --- a/config/crds/sharding.timebertt.dev_controllerrings.yaml +++ b/config/crds/sharding.timebertt.dev_controllerrings.yaml @@ -237,6 +237,9 @@ spec: - shards type: object type: object + x-kubernetes-validations: + - message: ClusterRing name must not be longer than 63 characters + rule: size(self.metadata.name) <= 63 served: true storage: true subresources: diff --git a/docs/development.md b/docs/development.md index d9b05811..ce79ba8b 100644 --- a/docs/development.md +++ b/docs/development.md @@ -120,13 +120,12 @@ The `Secrets` created by the example shard controller should be assigned to the $ kubectl create cm foo configmap/foo created -$ kubectl get cm,secret -L shard.alpha.sharding.timebertt.dev/50d858e0-example -NAME DATA AGE 50D858E0-EXAMPLE +$ kubectl get cm,secret -L shard.alpha.sharding.timebertt.dev/example +NAME DATA AGE EXAMPLE configmap/foo 0 1s shard-656d588475-5746d -NAME TYPE DATA AGE 50D858E0-EXAMPLE +NAME TYPE DATA AGE EXAMPLE secret/dummy-foo Opaque 0 1s shard-656d588475-5746d -secret/dummy-kube-root-ca.crt Opaque 0 2m14s ``` ## Monitoring diff --git a/docs/evaluation.md b/docs/evaluation.md index 2834688e..1af7a211 100644 --- a/docs/evaluation.md +++ b/docs/evaluation.md @@ -25,20 +25,20 @@ To perform a quick test of the webhosting-operator, create some example `Website $ kubectl apply -k webhosting-operator/config/samples ... -$ kubectl -n project-foo get website,deploy,svc,ing -L shard.alpha.sharding.timebertt.dev/ef3d63cd-webhosting-operator -NAME THEME PHASE SINCE AGE EF3D63CD-WEBHOSTING-OPERATOR +$ kubectl -n project-foo get website,deploy,svc,ing -L shard.alpha.sharding.timebertt.dev/webhosting-operator +NAME THEME PHASE SINCE AGE WEBHOSTING-OPERATOR website.webhosting.timebertt.dev/homepage exciting Ready 6s 16s webhosting-operator-98ff76b66-tdrtc website.webhosting.timebertt.dev/official lame Ready 5s 16s webhosting-operator-98ff76b66-tdrtc -NAME READY UP-TO-DATE AVAILABLE AGE EF3D63CD-WEBHOSTING-OPERATOR +NAME READY UP-TO-DATE AVAILABLE AGE WEBHOSTING-OPERATOR deployment.apps/homepage-98bad4 1/1 1 1 15s webhosting-operator-98ff76b66-tdrtc deployment.apps/official-10ff22 1/1 1 1 15s webhosting-operator-98ff76b66-tdrtc -NAME TYPE CLUSTER-IP EXTERNAL-IP PORT(S) AGE EF3D63CD-WEBHOSTING-OPERATOR +NAME TYPE CLUSTER-IP EXTERNAL-IP PORT(S) AGE WEBHOSTING-OPERATOR service/homepage-98bad4 ClusterIP 100.82.128.107 8080/TCP 16s webhosting-operator-98ff76b66-tdrtc service/official-10ff22 ClusterIP 100.82.194.21 8080/TCP 16s webhosting-operator-98ff76b66-tdrtc -NAME CLASS HOSTS ADDRESS PORTS AGE EF3D63CD-WEBHOSTING-OPERATOR +NAME CLASS HOSTS ADDRESS PORTS AGE WEBHOSTING-OPERATOR ingress.networking.k8s.io/homepage-98bad4 nginx webhosting.timebertt.dev 80, 443 16s webhosting-operator-98ff76b66-tdrtc ingress.networking.k8s.io/official-10ff22 nginx webhosting.timebertt.dev 80, 443 15s webhosting-operator-98ff76b66-tdrtc ``` diff --git a/docs/getting-started.md b/docs/getting-started.md index e206b7fa..ac1f3893 100644 --- a/docs/getting-started.md +++ b/docs/getting-started.md @@ -79,8 +79,8 @@ The sharder created a `MutatingWebhookConfiguration` for the resources listed in ```bash $ kubectl get mutatingwebhookconfiguration -l alpha.sharding.timebertt.dev/controllerring=example -NAME WEBHOOKS AGE -sharding-50d858e0-example 1 2m50s +NAME WEBHOOKS AGE +controllerring-example 1 2m50s ``` Let's examine the webhook configuration for more details. @@ -92,7 +92,7 @@ I.e., it gets called for unassigned objects and adds the shard assignment label apiVersion: admissionregistration.k8s.io/v1 kind: MutatingWebhookConfiguration metadata: - name: sharding-50d858e0-example + name: controllerring-example webhooks: - clientConfig: service: @@ -106,7 +106,7 @@ webhooks: kubernetes.io/metadata.name: default objectSelector: matchExpressions: - - key: shard.alpha.sharding.timebertt.dev/50d858e0-example + - key: shard.alpha.sharding.timebertt.dev/example operator: DoesNotExist rules: - apiGroups: @@ -144,7 +144,7 @@ apiVersion: v1 kind: ConfigMap metadata: labels: - shard.alpha.sharding.timebertt.dev/50d858e0-example: shard-9c6678c9f-8jc5b + shard.alpha.sharding.timebertt.dev/example: shard-9c6678c9f-8jc5b name: foo namespace: default ``` @@ -162,7 +162,7 @@ apiVersion: v1 kind: Secret metadata: labels: - shard.alpha.sharding.timebertt.dev/50d858e0-example: shard-9c6678c9f-8jc5b + shard.alpha.sharding.timebertt.dev/example: shard-9c6678c9f-8jc5b name: dummy-foo namespace: default ownerReferences: @@ -176,8 +176,8 @@ Let's create a few more `ConfigMaps` and observe the distribution of objects acr ```bash $ for i in $(seq 1 9); do k create cm foo$i ; done -$ kubectl get cm,secret -L shard.alpha.sharding.timebertt.dev/50d858e0-example -NAME DATA AGE 50D858E0-EXAMPLE +$ kubectl get cm,secret -L shard.alpha.sharding.timebertt.dev/example +NAME DATA AGE EXAMPLE configmap/foo 0 52s shard-9c6678c9f-8jc5b configmap/foo1 0 7s shard-9c6678c9f-v4bw2 configmap/foo2 0 6s shard-9c6678c9f-8jc5b @@ -189,7 +189,7 @@ configmap/foo7 0 6s shard-9c6678c9f-xntqc configmap/foo8 0 6s shard-9c6678c9f-xntqc configmap/foo9 0 6s shard-9c6678c9f-8jc5b -NAME TYPE DATA AGE 50D858E0-EXAMPLE +NAME TYPE DATA AGE EXAMPLE secret/dummy-foo Opaque 0 52s shard-9c6678c9f-8jc5b secret/dummy-foo1 Opaque 0 7s shard-9c6678c9f-v4bw2 secret/dummy-foo2 Opaque 0 6s shard-9c6678c9f-8jc5b @@ -234,8 +234,8 @@ As the original shard is not available anymore, moving the objects doesn't need ```bash $ kubectl get cm --show-labels -w --output-watch-events --watch-only EVENT NAME DATA AGE LABELS -MODIFIED foo4 0 7m52s shard.alpha.sharding.timebertt.dev/50d858e0-example=shard-9c6678c9f-ppzf7 -MODIFIED foo6 0 7m52s shard.alpha.sharding.timebertt.dev/50d858e0-example=shard-9c6678c9f-ppzf7 +MODIFIED foo4 0 7m52s shard.alpha.sharding.timebertt.dev/example=shard-9c6678c9f-ppzf7 +MODIFIED foo6 0 7m52s shard.alpha.sharding.timebertt.dev/example=shard-9c6678c9f-ppzf7 ``` ## Adding Shards to the Ring @@ -270,10 +270,10 @@ This triggers the sharder webhook which immediately assigns the object to the de ```bash $ kubectl get cm --show-labels -w --output-watch-events --watch-only EVENT NAME DATA AGE LABELS -MODIFIED foo4 0 9m2s drain.alpha.sharding.timebertt.dev/50d858e0-example=true,shard.alpha.sharding.timebertt.dev/50d858e0-example=shard-9c6678c9f-ppzf7 -MODIFIED foo7 0 9m2s drain.alpha.sharding.timebertt.dev/50d858e0-example=true,shard.alpha.sharding.timebertt.dev/50d858e0-example=shard-9c6678c9f-ppzf7 -MODIFIED foo4 0 9m2s shard.alpha.sharding.timebertt.dev/50d858e0-example=shard-9c6678c9f-jkgj6 -MODIFIED foo7 0 9m2s shard.alpha.sharding.timebertt.dev/50d858e0-example=shard-9c6678c9f-jkgj6 +MODIFIED foo4 0 9m2s drain.alpha.sharding.timebertt.dev/example=true,shard.alpha.sharding.timebertt.dev/example=shard-9c6678c9f-ppzf7 +MODIFIED foo7 0 9m2s drain.alpha.sharding.timebertt.dev/example=true,shard.alpha.sharding.timebertt.dev/example=shard-9c6678c9f-ppzf7 +MODIFIED foo4 0 9m2s shard.alpha.sharding.timebertt.dev/example=shard-9c6678c9f-jkgj6 +MODIFIED foo7 0 9m2s shard.alpha.sharding.timebertt.dev/example=shard-9c6678c9f-jkgj6 ``` ## Clean Up diff --git a/docs/implement-sharding.md b/docs/implement-sharding.md index 58e6d63f..35ea71f4 100644 --- a/docs/implement-sharding.md +++ b/docs/implement-sharding.md @@ -29,6 +29,8 @@ spec: resource: replicasets ``` +Note that the `ControllerRing` name must not be longer than 63 characters because it is used as part of the shard and drain label key (see below). + To allow the sharder to reassign the sharded objects during rebalancing, we need to grant the corresponding permissions. We need to grant these permissions explicitly depending on what is configured in the `ControllerRing`. Otherwise, the sharder would basically require `cluster-admin` access. @@ -166,13 +168,12 @@ func run() error { In short: use the following label selector on watches for all sharded resources listed in the `ControllerRing`. ```text -shard.alpha.sharding.timebertt.dev/50d858e0-example: my-operator-565df55f4b-5vwpj +shard.alpha.sharding.timebertt.dev/example: my-operator-565df55f4b-5vwpj ``` The sharder assigns all sharded objects by adding a shard label that is specific to the `ControllerRing` (resources could be part of multiple `ControllerRings`). -The shard label's key consists of the `shard.alpha.sharding.timebertt.dev/` prefix followed by the first 8 hex characters of the SHA256 checksum of the `ControllerRing` name followed by a `-` followed by the `ControllerRing` name itself. -The key part after the `/` is shortened to 63 characters so that it is a valid label key. -The checksum is added to the label key to derive unique label keys even for `ControllerRings` with long names that would cause the pattern to exceed the 63 characters limit after the `/`. +The shard label's key consists of the `shard.alpha.sharding.timebertt.dev/` prefix followed by the `ControllerRing` name. +As the key part after the `/` must not exceed 63 characters, the `ControllerRing` name must not be longer than 63 characters. The shard label's value is the name of the shard, i.e., the name of the shard lease and the shard lease's `holderIdentity`. Once you have determined the shard label key for your `ControllerRing`, use it as a selector on all watches that your controller starts for any of the sharded resources. @@ -221,7 +222,7 @@ In short: ensure your sharded controller acknowledges drain operations. When the drain label like this is added by the sharder, the controller needs to remove both the shard and the drain label and stop reconciling the object. ```text -drain.alpha.sharding.timebertt.dev/50d858e0-example +drain.alpha.sharding.timebertt.dev/example ``` When the sharder needs to move an object from an available shard to another shard for rebalancing, it first adds the drain label to instruct the currently responsible shard to stop reconciling the object. diff --git a/pkg/apis/sharding/v1alpha1/constants.go b/pkg/apis/sharding/v1alpha1/constants.go index ca5b0108..8872b03c 100644 --- a/pkg/apis/sharding/v1alpha1/constants.go +++ b/pkg/apis/sharding/v1alpha1/constants.go @@ -16,13 +16,6 @@ limitations under the License. package v1alpha1 -import ( - "crypto/sha256" - "encoding/hex" - - "k8s.io/utils/strings" -) - // This file contains API-related constants for the sharding implementation, e.g. well-known annotations and labels. const ( @@ -51,26 +44,15 @@ const ( // IdentityShardLeaseController is the identity that the shardlease controller uses to acquire leases of unavailable // shards. IdentityShardLeaseController = "shardlease-controller" - - delimiter = "-" ) // LabelShard returns the label on sharded objects that holds the name of the responsible shard within a ring. func LabelShard(ringName string) string { - return LabelShardPrefix + RingSuffix(ringName) + return LabelShardPrefix + ringName } // LabelDrain returns the label on sharded objects that instructs the responsible shard within a ring to stop reconciling // the object and remove both the shard and drain label. func LabelDrain(ringName string) string { - return LabelDrainPrefix + RingSuffix(ringName) -} - -// RingSuffix returns the label key for a given ring name that is appended to a qualified prefix. -func RingSuffix(ringName string) string { - keyHash := sha256.Sum256([]byte(ringName)) - hexHash := hex.EncodeToString(keyHash[:]) - - // the label part after the "/" must not exceed 63 characters, cut off at 63 characters - return strings.ShortenString(hexHash[:8]+delimiter+ringName, 63) + return LabelDrainPrefix + ringName } diff --git a/pkg/apis/sharding/v1alpha1/types_controllerring.go b/pkg/apis/sharding/v1alpha1/types_controllerring.go index 71a1e21e..58bb14d8 100644 --- a/pkg/apis/sharding/v1alpha1/types_controllerring.go +++ b/pkg/apis/sharding/v1alpha1/types_controllerring.go @@ -28,6 +28,7 @@ import ( //+kubebuilder:printcolumn:name="Available",type=string,JSONPath=`.status.availableShards` //+kubebuilder:printcolumn:name="Shards",type=string,JSONPath=`.status.shards` //+kubebuilder:printcolumn:name="Age",type="date",JSONPath=`.metadata.creationTimestamp` +//+kubebuilder:validation:XValidation:rule="size(self.metadata.name) <= 63",message="ClusterRing name must not be longer than 63 characters" // ControllerRing declares a virtual ring of sharded controller instances. Objects of the specified resources are // distributed across shards of this ring. Objects in all namespaces are considered unless a namespaceSelector is diff --git a/pkg/controller/controllerring/reconciler.go b/pkg/controller/controllerring/reconciler.go index 332f2954..dbcdcc79 100644 --- a/pkg/controller/controllerring/reconciler.go +++ b/pkg/controller/controllerring/reconciler.go @@ -147,7 +147,7 @@ func (r *Reconciler) reconcileWebhooks(ctx context.Context, controllerRing *shar Kind: "MutatingWebhookConfiguration", }, ObjectMeta: metav1.ObjectMeta{ - Name: "sharding-" + shardingv1alpha1.RingSuffix(controllerRing.Name), + Name: "controllerring-" + controllerRing.Name, Labels: map[string]string{ "app.kubernetes.io/name": shardingv1alpha1.AppControllerSharding, shardingv1alpha1.LabelControllerRing: controllerRing.Name,