From 19da5f03c1a604b7783826581ddf60361b195cea Mon Sep 17 00:00:00 2001 From: Ertan Deniz Date: Wed, 22 Apr 2026 16:10:56 +0200 Subject: [PATCH 1/2] feat: support keeper on a different namespace --- api/v1alpha1/clickhousecluster_types.go | 35 ++++- api/v1alpha1/types_test.go | 42 ++++++ api/v1alpha1/zz_generated.deepcopy.go | 17 ++- .../clickhouse.com_clickhouseclusters.yaml | 19 +-- .../clickhouseclusters.clickhouse.com.yaml | 19 +-- docs/api_reference.md | 15 +- docs/configuration.md | 5 +- docs/introduction.md | 4 +- internal/controller/clickhouse/controller.go | 40 +++-- .../controller/clickhouse/controller_test.go | 138 +++++++++++++++++- internal/controller/clickhouse/sync.go | 10 +- .../v1alpha1/clickhousecluster_webhook.go | 12 ++ .../clickhousecluster_webhook_test.go | 23 ++- test/deploy/deploy_test.go | 3 +- test/e2e/clickhouse_e2e_test.go | 22 +-- 15 files changed, 344 insertions(+), 60 deletions(-) diff --git a/api/v1alpha1/clickhousecluster_types.go b/api/v1alpha1/clickhousecluster_types.go index 9cabf663..2ccc20ef 100644 --- a/api/v1alpha1/clickhousecluster_types.go +++ b/api/v1alpha1/clickhousecluster_types.go @@ -32,8 +32,9 @@ type ClickHouseClusterSpec struct { Shards *int32 `json:"shards"` // Reference to the KeeperCluster that is used for ClickHouse coordination. + // When namespace is omitted, the ClickHouseCluster namespace is used. // +operator-sdk:csv:customresourcedefinitions:type=spec,displayName="Keeper Cluster Reference" - KeeperClusterRef *corev1.LocalObjectReference `json:"keeperClusterRef"` + KeeperClusterRef *KeeperClusterReference `json:"keeperClusterRef"` // Parameters passed to the ClickHouse pod spec. // +optional @@ -206,6 +207,33 @@ type ClickHouseClusterStatus struct { VersionProbeRevision string `json:"versionProbeRevision,omitempty"` } +// KeeperClusterReference identifies the KeeperCluster used by a ClickHouseCluster. +type KeeperClusterReference struct { + // Name of the KeeperCluster resource. + Name string `json:"name"` + // Namespace of the KeeperCluster resource. + // When omitted, the ClickHouseCluster namespace is used. + // +optional + Namespace string `json:"namespace,omitempty"` +} + +// NamespacedName resolves the reference to a fully-qualified Kubernetes object key. +func (r *KeeperClusterReference) NamespacedName(defaultNamespace string) types.NamespacedName { + if r == nil { + return types.NamespacedName{} + } + + namespace := r.Namespace + if namespace == "" { + namespace = defaultNamespace + } + + return types.NamespacedName{ + Namespace: namespace, + Name: r.Name, + } +} + // ClickHouseCluster is the Schema for the `clickhouseclusters` API. // +kubebuilder:object:root=true // +kubebuilder:subresource:status @@ -374,6 +402,11 @@ func (v *ClickHouseCluster) StatefulSetNameByReplicaID(id ClickHouseReplicaID) s return fmt.Sprintf("%s-%d-%d", v.SpecificName(), id.ShardID, id.Index) } +// KeeperClusterNamespacedName returns the fully-qualified KeeperCluster reference. +func (v *ClickHouseCluster) KeeperClusterNamespacedName() types.NamespacedName { + return v.Spec.KeeperClusterRef.NamespacedName(v.Namespace) +} + // HostnameByID returns domain name for the specific replica to access within Kubernetes cluster. func (v *ClickHouseCluster) HostnameByID(id ClickHouseReplicaID) string { return formatPodHostname(v.StatefulSetNameByReplicaID(id), v.HeadlessServiceName(), v.Namespace, v.Spec.ClusterDomain) diff --git a/api/v1alpha1/types_test.go b/api/v1alpha1/types_test.go index 1ea78f73..a9b21a94 100644 --- a/api/v1alpha1/types_test.go +++ b/api/v1alpha1/types_test.go @@ -6,6 +6,7 @@ import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" ) const testDefaultClusterDomain = "cluster.local" @@ -57,6 +58,47 @@ var _ = Describe("KeeperCluster", func() { }) var _ = Describe("ClickHouseCluster", func() { + Describe("KeeperClusterNamespacedName", func() { + It("should default the keeper namespace to the ClickHouseCluster namespace", func() { + cluster := &ClickHouseCluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + Namespace: "clickhouse-ns", + }, + Spec: ClickHouseClusterSpec{ + KeeperClusterRef: &KeeperClusterReference{ + Name: "keeper", + }, + }, + } + + Expect(cluster.KeeperClusterNamespacedName()).To(Equal(types.NamespacedName{ + Namespace: "clickhouse-ns", + Name: "keeper", + })) + }) + + It("should use the explicit keeper namespace when provided", func() { + cluster := &ClickHouseCluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + Namespace: "clickhouse-ns", + }, + Spec: ClickHouseClusterSpec{ + KeeperClusterRef: &KeeperClusterReference{ + Name: "keeper", + Namespace: "keeper-ns", + }, + }, + } + + Expect(cluster.KeeperClusterNamespacedName()).To(Equal(types.NamespacedName{ + Namespace: "keeper-ns", + Name: "keeper", + })) + }) + }) + Describe("HostnameByID", func() { var cluster *ClickHouseCluster diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index 4d91f037..66f90fbe 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -102,7 +102,7 @@ func (in *ClickHouseClusterSpec) DeepCopyInto(out *ClickHouseClusterSpec) { } if in.KeeperClusterRef != nil { in, out := &in.KeeperClusterRef, &out.KeeperClusterRef - *out = new(v1.LocalObjectReference) + *out = new(KeeperClusterReference) **out = **in } in.PodTemplate.DeepCopyInto(&out.PodTemplate) @@ -400,6 +400,21 @@ func (in *KeeperClusterList) DeepCopyObject() runtime.Object { return nil } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *KeeperClusterReference) DeepCopyInto(out *KeeperClusterReference) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new KeeperClusterReference. +func (in *KeeperClusterReference) DeepCopy() *KeeperClusterReference { + if in == nil { + return nil + } + out := new(KeeperClusterReference) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *KeeperClusterSpec) DeepCopyInto(out *KeeperClusterSpec) { *out = *in diff --git a/config/crd/bases/clickhouse.com_clickhouseclusters.yaml b/config/crd/bases/clickhouse.com_clickhouseclusters.yaml index befc2e87..cde155c1 100644 --- a/config/crd/bases/clickhouse.com_clickhouseclusters.yaml +++ b/config/crd/bases/clickhouse.com_clickhouseclusters.yaml @@ -1113,20 +1113,21 @@ spec: - name type: object keeperClusterRef: - description: Reference to the KeeperCluster that is used for ClickHouse - coordination. + description: |- + Reference to the KeeperCluster that is used for ClickHouse coordination. + When namespace is omitted, the ClickHouseCluster namespace is used. properties: name: - default: "" + description: Name of the KeeperCluster resource. + type: string + namespace: description: |- - Name of the referent. - This field is effectively required, but due to backwards compatibility is - allowed to be empty. Instances of this type with an empty value here are - almost certainly wrong. - More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names + Namespace of the KeeperCluster resource. + When omitted, the ClickHouseCluster namespace is used. type: string + required: + - name type: object - x-kubernetes-map-type: atomic labels: additionalProperties: type: string diff --git a/dist/chart/templates/crd/clickhouseclusters.clickhouse.com.yaml b/dist/chart/templates/crd/clickhouseclusters.clickhouse.com.yaml index 3e92b3cf..842cd1ab 100644 --- a/dist/chart/templates/crd/clickhouseclusters.clickhouse.com.yaml +++ b/dist/chart/templates/crd/clickhouseclusters.clickhouse.com.yaml @@ -1116,20 +1116,21 @@ spec: - name type: object keeperClusterRef: - description: Reference to the KeeperCluster that is used for ClickHouse - coordination. + description: |- + Reference to the KeeperCluster that is used for ClickHouse coordination. + When namespace is omitted, the ClickHouseCluster namespace is used. properties: name: - default: "" + description: Name of the KeeperCluster resource. + type: string + namespace: description: |- - Name of the referent. - This field is effectively required, but due to backwards compatibility is - allowed to be empty. Instances of this type with an empty value here are - almost certainly wrong. - More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names + Namespace of the KeeperCluster resource. + When omitted, the ClickHouseCluster namespace is used. type: string + required: + - name type: object - x-kubernetes-map-type: atomic labels: additionalProperties: type: string diff --git a/docs/api_reference.md b/docs/api_reference.md index ae28df20..7108678c 100644 --- a/docs/api_reference.md +++ b/docs/api_reference.md @@ -47,7 +47,7 @@ ClickHouseClusterSpec defines the desired state of ClickHouseCluster. |-------|------|-------------|----------|---------| | `replicas` | integer | Number of replicas in the single shard. | false | 3 | | `shards` | integer | Number of shards in the cluster. | false | 1 | -| `keeperClusterRef` | [LocalObjectReference](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.28/#localobjectreference-v1-core) | Reference to the KeeperCluster that is used for ClickHouse coordination. | true | | +| `keeperClusterRef` | [KeeperClusterReference](#keeperclusterreference) | Reference to the KeeperCluster that is used for ClickHouse coordination.
When namespace is omitted, the ClickHouseCluster namespace is used. | true | | | `podTemplate` | [PodTemplateSpec](#podtemplatespec) | Parameters passed to the ClickHouse pod spec. | false | | | `containerTemplate` | [ContainerTemplateSpec](#containertemplatespec) | Parameters passed to the ClickHouse container spec. | false | | | `dataVolumeClaimSpec` | [PersistentVolumeClaimSpec](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.28/#persistentvolumeclaimspec-v1-core) | Specification of persistent storage for ClickHouse data. | false | | @@ -243,6 +243,19 @@ kind: KeeperClusterList | `items` | [KeeperCluster](#keepercluster) array | | true | | +## KeeperClusterReference + +KeeperClusterReference identifies the KeeperCluster used by a ClickHouseCluster. + +| Field | Type | Description | Required | Default | +|-------|------|-------------|----------|---------| +| `name` | string | Name of the KeeperCluster resource. | true | | +| `namespace` | string | Namespace of the KeeperCluster resource.
When omitted, the ClickHouseCluster namespace is used. | false | | + +Appears in: +- [ClickHouseClusterSpec](#clickhouseclusterspec) + + ## KeeperClusterSpec KeeperClusterSpec defines the desired state of KeeperCluster. diff --git a/docs/configuration.md b/docs/configuration.md index fc2c2475..c123ce55 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -56,9 +56,12 @@ Every ClickHouse cluster must reference a KeeperCluster for coordination: ```yaml spec: keeperClusterRef: - name: my-keeper # Name of the KeeperCluster in the same namespace + name: my-keeper + # namespace: keeper-system # Optional, defaults to the ClickHouseCluster namespace ``` +When `keeperClusterRef.namespace` is set, the operator must watch both namespaces. If `WATCH_NAMESPACE` is configured, include the ClickHouse and Keeper namespaces in that list. + ## KeeperCluster Configuration ```yaml diff --git a/docs/introduction.md b/docs/introduction.md index 4ff15108..b4108a09 100644 --- a/docs/introduction.md +++ b/docs/introduction.md @@ -68,8 +68,8 @@ spec: ### ClickHouse Keeper is Required -Every ClickHouseCluster requires a ClickHouse Keeper cluster for distributed coordination. -The Keeper cluster must be referenced in the ClickHouseCluster spec using `keeperClusterRef`. +Every ClickHouseCluster requires a ClickHouse Keeper cluster for distributed coordination. +The Keeper cluster must be referenced in the ClickHouseCluster spec using `keeperClusterRef`. By default the operator looks in the ClickHouseCluster namespace, but you can also set `keeperClusterRef.namespace` to point at a KeeperCluster in another watched namespace. ### One-to-One Keeper Relationship diff --git a/internal/controller/clickhouse/controller.go b/internal/controller/clickhouse/controller.go index dbb16dac..aebdd3c8 100644 --- a/internal/controller/clickhouse/controller.go +++ b/internal/controller/clickhouse/controller.go @@ -40,6 +40,17 @@ type ClusterController struct { EnablePDB bool } +const keeperClusterReferenceField = "clickhouse.com/keeperClusterReference" + +func keeperReferenceFieldValue(cluster *v1.ClickHouseCluster) []string { + keeperKey := cluster.KeeperClusterNamespacedName() + if keeperKey.Name == "" { + return nil + } + + return []string{keeperKey.String()} +} + // +kubebuilder:rbac:groups=clickhouse.com,resources=clickhouseclusters,verbs=get;list;watch;create;update;patch;delete // +kubebuilder:rbac:groups=clickhouse.com,resources=clickhouseclusters/status,verbs=get;update;patch // +kubebuilder:rbac:groups=clickhouse.com,resources=clickhouseclusters/finalizers,verbs=update @@ -156,6 +167,17 @@ func SetupWithManager(mgr ctrl.Manager, log controllerutil.Logger, checker *upgr EnablePDB: enablePDB, } + if err := mgr.GetFieldIndexer().IndexField(context.Background(), &v1.ClickHouseCluster{}, keeperClusterReferenceField, func(obj client.Object) []string { + cluster, ok := obj.(*v1.ClickHouseCluster) + if !ok { + return nil + } + + return keeperReferenceFieldValue(cluster) + }); err != nil { + return fmt.Errorf("index ClickHouseCluster keeper reference: %w", err) + } + controllerBuilder := ctrl.NewControllerManagedBy(mgr). For(&v1.ClickHouseCluster{}, builder.WithPredicates(predicate.Or(predicate.GenerationChangedPredicate{}, predicate.LabelChangedPredicate{}, predicate.AnnotationChangedPredicate{}))). Watches( @@ -191,20 +213,20 @@ func (cc *ClusterController) clickHouseClustersForKeeper(ctx context.Context, ob // List all ClickHouseClusters that reference this KeeperCluster var chList v1.ClickHouseClusterList - if err := cc.List(ctx, &chList, client.InNamespace(zk.Namespace)); err != nil { + if err := cc.List(ctx, &chList, client.MatchingFields{ + keeperClusterReferenceField: zk.NamespacedName().String(), + }); err != nil { return nil } var requests []reconcile.Request for _, ch := range chList.Items { - if ch.Spec.KeeperClusterRef.Name == zk.Name { - requests = append(requests, reconcile.Request{ - NamespacedName: types.NamespacedName{ - Name: ch.Name, - Namespace: ch.Namespace, - }, - }) - } + requests = append(requests, reconcile.Request{ + NamespacedName: types.NamespacedName{ + Name: ch.Name, + Namespace: ch.Namespace, + }, + }) } return requests diff --git a/internal/controller/clickhouse/controller_test.go b/internal/controller/clickhouse/controller_test.go index 4b3a19b6..a6e82fc3 100644 --- a/internal/controller/clickhouse/controller_test.go +++ b/internal/controller/clickhouse/controller_test.go @@ -3,7 +3,10 @@ package clickhouse import ( "context" "errors" + "maps" "net" + "slices" + "strings" "testing" . "github.com/onsi/ginkgo/v2" @@ -16,11 +19,14 @@ import ( "k8s.io/apimachinery/pkg/api/meta" "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + k8sruntime "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" - "k8s.io/client-go/kubernetes/scheme" + clientgoscheme "k8s.io/client-go/kubernetes/scheme" "k8s.io/client-go/tools/events" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + "sigs.k8s.io/controller-runtime/pkg/reconcile" v1 "github.com/ClickHouse/clickhouse-operator/api/v1alpha1" "github.com/ClickHouse/clickhouse-operator/internal/controller/testutil" @@ -34,6 +40,65 @@ func TestControllers(t *testing.T) { RunSpecs(t, "ClickHouse Controller Suite") } +var _ = Describe("keeper watch mapping", func() { + It("should enqueue ClickHouse clusters that explicitly reference a keeper in another namespace", func(ctx context.Context) { + testScheme := k8sruntime.NewScheme() + Expect(clientgoscheme.AddToScheme(testScheme)).To(Succeed()) + Expect(v1.AddToScheme(testScheme)).To(Succeed()) + + referencedCluster := &v1.ClickHouseCluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cross-namespace-cluster", + Namespace: "clickhouse-ns", + }, + Spec: v1.ClickHouseClusterSpec{ + KeeperClusterRef: &v1.KeeperClusterReference{ + Name: "keeper", + Namespace: "keeper-ns", + }, + }, + } + sameNameDifferentNamespace := &v1.ClickHouseCluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "same-name-different-namespace", + Namespace: "other-ns", + }, + Spec: v1.ClickHouseClusterSpec{ + KeeperClusterRef: &v1.KeeperClusterReference{ + Name: "keeper", + }, + }, + } + + controller := &ClusterController{ + Client: fake.NewClientBuilder(). + WithScheme(testScheme). + WithObjects(referencedCluster, sameNameDifferentNamespace). + WithIndex(&v1.ClickHouseCluster{}, keeperClusterReferenceField, func(obj client.Object) []string { + cluster, ok := obj.(*v1.ClickHouseCluster) + if !ok { + return nil + } + + return keeperReferenceFieldValue(cluster) + }). + Build(), + } + + Expect(controller.clickHouseClustersForKeeper(ctx, &v1.KeeperCluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "keeper", + Namespace: "keeper-ns", + }, + })).To(ConsistOf(reconcile.Request{ + NamespacedName: types.NamespacedName{ + Name: referencedCluster.Name, + Namespace: referencedCluster.Namespace, + }, + })) + }) +}) + var _ = When("reconciling ClickHouseCluster", Ordered, func() { var ( suite testutil.TestSuit @@ -54,7 +119,7 @@ var _ = When("reconciling ClickHouseCluster", Ordered, func() { Spec: v1.ClickHouseClusterSpec{ Replicas: new(int32(2)), Shards: new(int32(2)), - KeeperClusterRef: &corev1.LocalObjectReference{Name: keeperName}, + KeeperClusterRef: &v1.KeeperClusterReference{Name: keeperName}, Labels: map[string]string{ "test-label": "test-val", }, @@ -74,7 +139,7 @@ var _ = When("reconciling ClickHouseCluster", Ordered, func() { recorder = events.NewFakeRecorder(128) controller = &ClusterController{ Client: suite.Client, - Scheme: scheme.Scheme, + Scheme: clientgoscheme.Scheme, Logger: suite.Log.Named("clickhouse"), Recorder: recorder, Webhook: webhookv1.ClickHouseClusterWebhook{ @@ -158,6 +223,69 @@ var _ = When("reconciling ClickHouseCluster", Ordered, func() { Expect(statefulsets.Items).To(HaveLen(4)) }) + It("should reconcile a cluster that references Keeper in another namespace", func(ctx context.Context) { + keeperNamespace := &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: "keeper-remote", + }, + } + Expect(suite.Client.Create(ctx, keeperNamespace)).To(Succeed()) + + keeper := &v1.KeeperCluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "remote-keeper", + Namespace: keeperNamespace.Name, + }, + } + Expect(suite.Client.Create(ctx, keeper)).To(Succeed()) + Expect(suite.Client.Get(ctx, keeper.NamespacedName(), keeper)).To(Succeed()) + meta.SetStatusCondition(&keeper.Status.Conditions, metav1.Condition{ + Type: v1.ConditionTypeReady, + Status: metav1.ConditionTrue, + Reason: v1.KeeperConditionReasonStandaloneReady, + }) + Expect(suite.Client.Status().Update(ctx, keeper)).To(Succeed()) + + crossNamespaceCluster := &v1.ClickHouseCluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cross-namespace", + Namespace: "default", + }, + Spec: v1.ClickHouseClusterSpec{ + Replicas: new(int32(1)), + Shards: new(int32(1)), + KeeperClusterRef: &v1.KeeperClusterReference{ + Name: keeper.Name, + Namespace: keeper.Namespace, + }, + }, + Status: v1.ClickHouseClusterStatus{ + Version: "26.1.1.1", + }, + } + Expect(suite.Client.Create(ctx, crossNamespaceCluster)).To(Succeed()) + + _, err := controller.Reconcile(ctx, ctrl.Request{NamespacedName: crossNamespaceCluster.NamespacedName()}) + Expect(err).NotTo(HaveOccurred()) + testutil.AssertEvents(recorder.Events, map[string]int{ + "ClusterNotReady": 1, + }) + + testutil.CompleteVersionProbeJob(ctx, suite, crossNamespaceCluster.Namespace, crossNamespaceCluster.SpecificName(), "26.1.1.1") + + _, err = controller.Reconcile(ctx, ctrl.Request{NamespacedName: crossNamespaceCluster.NamespacedName()}) + Expect(err).NotTo(HaveOccurred()) + + var config corev1.ConfigMap + Expect(suite.Client.Get(ctx, types.NamespacedName{ + Namespace: crossNamespaceCluster.Namespace, + Name: crossNamespaceCluster.ConfigMapNameByReplicaID(v1.ClickHouseReplicaID{ShardID: 0, Index: 0}), + }, &config)).To(Succeed()) + + renderedConfig := strings.Join(slices.Collect(maps.Values(config.Data)), "\n") + Expect(renderedConfig).To(ContainSubstring(".keeper-remote.svc.")) + }) + It("should propagate version probe overrides to the job", func(ctx context.Context) { By("updating the CR with version probe overrides") @@ -437,7 +565,7 @@ var _ = When("reconciling ClickHouseCluster", Ordered, func() { Spec: v1.ClickHouseClusterSpec{ Replicas: new(int32(2)), Shards: new(int32(1)), - KeeperClusterRef: &corev1.LocalObjectReference{Name: keeperName}, + KeeperClusterRef: &v1.KeeperClusterReference{Name: keeperName}, DataVolumeClaimSpec: &corev1.PersistentVolumeClaimSpec{ AccessModes: []corev1.PersistentVolumeAccessMode{corev1.ReadWriteOnce}, Resources: corev1.VolumeResourceRequirements{ @@ -541,7 +669,7 @@ var _ = When("reconciling ClickHouseCluster", Ordered, func() { Namespace: "default", }, Spec: v1.ClickHouseClusterSpec{ - KeeperClusterRef: &corev1.LocalObjectReference{Name: keeperName}, + KeeperClusterRef: &v1.KeeperClusterReference{Name: keeperName}, ExternalSecret: &v1.ExternalSecret{ Name: secret.Name, }, diff --git a/internal/controller/clickhouse/sync.go b/internal/controller/clickhouse/sync.go index 1ff56f0d..8bab65e4 100644 --- a/internal/controller/clickhouse/sync.go +++ b/internal/controller/clickhouse/sync.go @@ -521,17 +521,15 @@ func (r *clickhouseReconciler) reconcileClusterRevisions(ctx context.Context, lo log.Debug(fmt.Sprintf("observed new CR revision %q", updateRevision)) } - if err := r.GetClient().Get(ctx, types.NamespacedName{ - Namespace: r.Cluster.Namespace, - Name: r.Cluster.Spec.KeeperClusterRef.Name, - }, &r.keeper); err != nil { + keeperNamespacedName := r.Cluster.KeeperClusterNamespacedName() + if err := r.GetClient().Get(ctx, keeperNamespacedName, &r.keeper); err != nil { if k8serrors.IsNotFound(err) { - log.Debug("keeper cluster not found, waiting") + log.Debug("keeper cluster not found, waiting", "keeper", keeperNamespacedName.String()) return chctrl.StepBlocked(chctrl.RequeueOnRefreshTimeout), nil } - return chctrl.StepResult{}, fmt.Errorf("get keeper cluster: %w", err) + return chctrl.StepResult{}, fmt.Errorf("get keeper cluster %q: %w", keeperNamespacedName.String(), err) } if cond := meta.FindStatusCondition(r.keeper.Status.Conditions, v1.ConditionTypeReady); cond == nil || cond.Status != metav1.ConditionTrue { diff --git a/internal/webhook/v1alpha1/clickhousecluster_webhook.go b/internal/webhook/v1alpha1/clickhousecluster_webhook.go index 8cc73011..ad373fe5 100644 --- a/internal/webhook/v1alpha1/clickhousecluster_webhook.go +++ b/internal/webhook/v1alpha1/clickhousecluster_webhook.go @@ -4,7 +4,9 @@ import ( "context" "errors" "fmt" + "strings" + "k8s.io/apimachinery/pkg/util/validation" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/webhook/admission" @@ -93,6 +95,16 @@ func (w *ClickHouseClusterWebhook) validateImpl(obj *chv1.ClickHouseCluster) (ad errs = append(errs, errors.New("keeperClusterRef name must not be empty")) } + if obj.Spec.KeeperClusterRef != nil && obj.Spec.KeeperClusterRef.Namespace != "" { + if validationErrs := validation.IsDNS1123Label(obj.Spec.KeeperClusterRef.Namespace); len(validationErrs) > 0 { + errs = append(errs, fmt.Errorf( + "keeperClusterRef namespace %q is invalid: %s", + obj.Spec.KeeperClusterRef.Namespace, + strings.Join(validationErrs, ", "), + )) + } + } + if err := obj.Spec.Settings.TLS.Validate(); err != nil { errs = append(errs, err) } diff --git a/internal/webhook/v1alpha1/clickhousecluster_webhook_test.go b/internal/webhook/v1alpha1/clickhousecluster_webhook_test.go index ff28b570..69396bf7 100644 --- a/internal/webhook/v1alpha1/clickhousecluster_webhook_test.go +++ b/internal/webhook/v1alpha1/clickhousecluster_webhook_test.go @@ -24,7 +24,7 @@ var _ = Describe("ClickHouseCluster Webhook", func() { Name: "test-default", }, Spec: chv1.ClickHouseClusterSpec{ - KeeperClusterRef: &corev1.LocalObjectReference{ + KeeperClusterRef: &chv1.KeeperClusterReference{ Name: "some-keeper-cluster", }, }, @@ -47,7 +47,7 @@ var _ = Describe("ClickHouseCluster Webhook", func() { Name: "test-default", }, Spec: chv1.ClickHouseClusterSpec{ - KeeperClusterRef: &corev1.LocalObjectReference{ + KeeperClusterRef: &chv1.KeeperClusterReference{ Name: "some-keeper-cluster", }, DataVolumeClaimSpec: &corev1.PersistentVolumeClaimSpec{Resources: corev1.VolumeResourceRequirements{ @@ -73,7 +73,7 @@ var _ = Describe("ClickHouseCluster Webhook", func() { Name: "test-validate", }, Spec: chv1.ClickHouseClusterSpec{ - KeeperClusterRef: &corev1.LocalObjectReference{ + KeeperClusterRef: &chv1.KeeperClusterReference{ Name: "some-keeper-cluster", }, }, @@ -93,6 +93,23 @@ var _ = Describe("ClickHouseCluster Webhook", func() { Expect(err.Error()).To(ContainSubstring("TLS cannot be required")) }) + It("Should allow explicit keeper namespace", func(ctx context.Context) { + cluster := chCluster.DeepCopy() + cluster.Spec.KeeperClusterRef.Namespace = "keeper-ns" + + Expect(k8sClient.Create(ctx, cluster)).To(Succeed()) + deferCleanup(cluster) + }) + + It("Should reject invalid keeper namespace", func(ctx context.Context) { + cluster := chCluster.DeepCopy() + cluster.Spec.KeeperClusterRef.Namespace = "Keeper_NS" + + err := k8sClient.Create(ctx, cluster) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("keeperClusterRef namespace")) + }) + It("Should check certificate passed if TLS enabled", func(ctx context.Context) { By("Rejecting wrong settings") diff --git a/test/deploy/deploy_test.go b/test/deploy/deploy_test.go index b6379e95..14fdec95 100644 --- a/test/deploy/deploy_test.go +++ b/test/deploy/deploy_test.go @@ -15,7 +15,6 @@ import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" "gopkg.in/yaml.v2" - corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/discovery" "k8s.io/client-go/kubernetes/scheme" @@ -377,7 +376,7 @@ func testDeployment(namespace string) { }, Spec: v1.ClickHouseClusterSpec{ Replicas: new(int32(1)), - KeeperClusterRef: &corev1.LocalObjectReference{ + KeeperClusterRef: &v1.KeeperClusterReference{ Name: keeper.Name, }, ContainerTemplate: v1.ContainerTemplateSpec{ diff --git a/test/e2e/clickhouse_e2e_test.go b/test/e2e/clickhouse_e2e_test.go index df2756ad..97d66985 100644 --- a/test/e2e/clickhouse_e2e_test.go +++ b/test/e2e/clickhouse_e2e_test.go @@ -72,7 +72,7 @@ var _ = Describe("ClickHouse controller", Label("clickhouse"), func() { }, }, DataVolumeClaimSpec: &defaultStorage, - KeeperClusterRef: &corev1.LocalObjectReference{ + KeeperClusterRef: &v1.KeeperClusterReference{ Name: keeper.Name, }, }, @@ -129,7 +129,7 @@ var _ = Describe("ClickHouse controller", Label("clickhouse"), func() { }, }, DataVolumeClaimSpec: &defaultStorage, - KeeperClusterRef: &corev1.LocalObjectReference{ + KeeperClusterRef: &v1.KeeperClusterReference{ Name: keeper.Name, }, }, @@ -176,7 +176,7 @@ var _ = Describe("ClickHouse controller", Label("clickhouse"), func() { Spec: v1.ClickHouseClusterSpec{ Replicas: new(int32(1)), DataVolumeClaimSpec: nil, // Diskless configuration - KeeperClusterRef: &corev1.LocalObjectReference{ + KeeperClusterRef: &v1.KeeperClusterReference{ Name: keeper.Name, }, PodTemplate: v1.PodTemplateSpec{ @@ -219,7 +219,7 @@ var _ = Describe("ClickHouse controller", Label("clickhouse"), func() { Spec: v1.ClickHouseClusterSpec{ Replicas: new(int32(1)), DataVolumeClaimSpec: nil, // Diskless configuration - KeeperClusterRef: &corev1.LocalObjectReference{ + KeeperClusterRef: &v1.KeeperClusterReference{ Name: keeper.Name, }, ContainerTemplate: v1.ContainerTemplateSpec{ @@ -281,7 +281,7 @@ var _ = Describe("ClickHouse controller", Label("clickhouse"), func() { }, Spec: v1.ClickHouseClusterSpec{ Replicas: new(int32(2)), - KeeperClusterRef: &corev1.LocalObjectReference{ + KeeperClusterRef: &v1.KeeperClusterReference{ Name: keeper.Name, }, ContainerTemplate: v1.ContainerTemplateSpec{ @@ -388,7 +388,7 @@ var _ = Describe("ClickHouse controller", Label("clickhouse"), func() { }, Spec: v1.ClickHouseClusterSpec{ Replicas: new(int32(2)), - KeeperClusterRef: &corev1.LocalObjectReference{ + KeeperClusterRef: &v1.KeeperClusterReference{ Name: keeper.Name, }, PodTemplate: v1.PodTemplateSpec{ @@ -495,7 +495,7 @@ var _ = Describe("ClickHouse controller", Label("clickhouse"), func() { Spec: v1.ClickHouseClusterSpec{ Replicas: new(int32(2)), Shards: new(int32(2)), - KeeperClusterRef: &corev1.LocalObjectReference{ + KeeperClusterRef: &v1.KeeperClusterReference{ Name: keeper.Name, }, ContainerTemplate: v1.ContainerTemplateSpec{ @@ -581,7 +581,7 @@ var _ = Describe("ClickHouse controller", Label("clickhouse"), func() { Image: v1.ContainerImage{Tag: BaseVersion}, }, DataVolumeClaimSpec: &defaultStorage, - KeeperClusterRef: &corev1.LocalObjectReference{Name: keeper.Name}, + KeeperClusterRef: &v1.KeeperClusterReference{Name: keeper.Name}, }, } @@ -646,7 +646,7 @@ var _ = Describe("ClickHouse controller", Label("clickhouse"), func() { Replicas: new(int32(1)), ContainerTemplate: v1.ContainerTemplateSpec{Image: v1.ContainerImage{Tag: BaseVersion}}, DataVolumeClaimSpec: &defaultStorage, - KeeperClusterRef: &corev1.LocalObjectReference{Name: keeper.Name}, + KeeperClusterRef: &v1.KeeperClusterReference{Name: keeper.Name}, ExternalSecret: &v1.ExternalSecret{ Name: secretName, Policy: v1.ExternalSecretPolicyObserve, @@ -781,7 +781,7 @@ var _ = Describe("ClickHouse controller", Label("clickhouse"), func() { }, Spec: v1.ClickHouseClusterSpec{ Replicas: new(int32(2)), - KeeperClusterRef: &corev1.LocalObjectReference{ + KeeperClusterRef: &v1.KeeperClusterReference{ Name: keeperCR.Name, }, ContainerTemplate: v1.ContainerTemplateSpec{ @@ -949,7 +949,7 @@ var _ = Describe("ClickHouse controller", Label("clickhouse"), func() { Spec: v1.ClickHouseClusterSpec{ Replicas: new(int32(3)), DataVolumeClaimSpec: &defaultStorage, - KeeperClusterRef: &corev1.LocalObjectReference{ + KeeperClusterRef: &v1.KeeperClusterReference{ Name: keeperName, }, PodTemplate: v1.PodTemplateSpec{ From 50288fe54bef7efdbed3704b6be14bd9e52572fd Mon Sep 17 00:00:00 2001 From: Pervakov Grigorii Date: Tue, 28 Apr 2026 15:47:15 +0900 Subject: [PATCH 2/2] Validation keeper ref fields in API, minor changes --- api/v1alpha1/clickhousecluster_types.go | 6 +- api/v1alpha1/types_test.go | 4 +- api/v1alpha1/zz_generated.deepcopy.go | 6 +- .../clickhouse.com_clickhouseclusters.yaml | 4 + .../clickhouseclusters.clickhouse.com.yaml | 4 + .../config/vocabularies/ClickHouse/accept.txt | 2 +- .../controller/clickhouse/controller_test.go | 126 +++++++++--------- .../v1alpha1/clickhousecluster_webhook.go | 16 --- .../clickhousecluster_webhook_test.go | 8 +- test/deploy/deploy_test.go | 2 +- test/e2e/clickhouse_e2e_test.go | 22 +-- 11 files changed, 96 insertions(+), 104 deletions(-) diff --git a/api/v1alpha1/clickhousecluster_types.go b/api/v1alpha1/clickhousecluster_types.go index 2ccc20ef..eacdafa2 100644 --- a/api/v1alpha1/clickhousecluster_types.go +++ b/api/v1alpha1/clickhousecluster_types.go @@ -34,7 +34,7 @@ type ClickHouseClusterSpec struct { // Reference to the KeeperCluster that is used for ClickHouse coordination. // When namespace is omitted, the ClickHouseCluster namespace is used. // +operator-sdk:csv:customresourcedefinitions:type=spec,displayName="Keeper Cluster Reference" - KeeperClusterRef *KeeperClusterReference `json:"keeperClusterRef"` + KeeperClusterRef KeeperClusterReference `json:"keeperClusterRef"` // Parameters passed to the ClickHouse pod spec. // +optional @@ -210,9 +210,13 @@ type ClickHouseClusterStatus struct { // KeeperClusterReference identifies the KeeperCluster used by a ClickHouseCluster. type KeeperClusterReference struct { // Name of the KeeperCluster resource. + // +kubebuilder:validation:Pattern=`^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$` + // +kubebuilder:validation:MaxLength=63 Name string `json:"name"` // Namespace of the KeeperCluster resource. // When omitted, the ClickHouseCluster namespace is used. + // +kubebuilder:validation:Pattern=`^[a-z0-9]([-a-z0-9]*[a-z0-9])?$` + // +kubebuilder:validation:MaxLength=63 // +optional Namespace string `json:"namespace,omitempty"` } diff --git a/api/v1alpha1/types_test.go b/api/v1alpha1/types_test.go index a9b21a94..eb184578 100644 --- a/api/v1alpha1/types_test.go +++ b/api/v1alpha1/types_test.go @@ -66,7 +66,7 @@ var _ = Describe("ClickHouseCluster", func() { Namespace: "clickhouse-ns", }, Spec: ClickHouseClusterSpec{ - KeeperClusterRef: &KeeperClusterReference{ + KeeperClusterRef: KeeperClusterReference{ Name: "keeper", }, }, @@ -85,7 +85,7 @@ var _ = Describe("ClickHouseCluster", func() { Namespace: "clickhouse-ns", }, Spec: ClickHouseClusterSpec{ - KeeperClusterRef: &KeeperClusterReference{ + KeeperClusterRef: KeeperClusterReference{ Name: "keeper", Namespace: "keeper-ns", }, diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index 66f90fbe..7538a8bf 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -100,11 +100,7 @@ func (in *ClickHouseClusterSpec) DeepCopyInto(out *ClickHouseClusterSpec) { *out = new(int32) **out = **in } - if in.KeeperClusterRef != nil { - in, out := &in.KeeperClusterRef, &out.KeeperClusterRef - *out = new(KeeperClusterReference) - **out = **in - } + out.KeeperClusterRef = in.KeeperClusterRef in.PodTemplate.DeepCopyInto(&out.PodTemplate) in.ContainerTemplate.DeepCopyInto(&out.ContainerTemplate) if in.DataVolumeClaimSpec != nil { diff --git a/config/crd/bases/clickhouse.com_clickhouseclusters.yaml b/config/crd/bases/clickhouse.com_clickhouseclusters.yaml index cde155c1..e063e8bc 100644 --- a/config/crd/bases/clickhouse.com_clickhouseclusters.yaml +++ b/config/crd/bases/clickhouse.com_clickhouseclusters.yaml @@ -1119,11 +1119,15 @@ spec: properties: name: description: Name of the KeeperCluster resource. + maxLength: 63 + pattern: ^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$ type: string namespace: description: |- Namespace of the KeeperCluster resource. When omitted, the ClickHouseCluster namespace is used. + maxLength: 63 + pattern: ^[a-z0-9]([-a-z0-9]*[a-z0-9])?$ type: string required: - name diff --git a/dist/chart/templates/crd/clickhouseclusters.clickhouse.com.yaml b/dist/chart/templates/crd/clickhouseclusters.clickhouse.com.yaml index 842cd1ab..a869274e 100644 --- a/dist/chart/templates/crd/clickhouseclusters.clickhouse.com.yaml +++ b/dist/chart/templates/crd/clickhouseclusters.clickhouse.com.yaml @@ -1122,11 +1122,15 @@ spec: properties: name: description: Name of the KeeperCluster resource. + maxLength: 63 + pattern: ^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$ type: string namespace: description: |- Namespace of the KeeperCluster resource. When omitted, the ClickHouseCluster namespace is used. + maxLength: 63 + pattern: ^[a-z0-9]([-a-z0-9]*[a-z0-9])?$ type: string required: - name diff --git a/docs/styles/config/vocabularies/ClickHouse/accept.txt b/docs/styles/config/vocabularies/ClickHouse/accept.txt index 0255a1bc..45b8176e 100644 --- a/docs/styles/config/vocabularies/ClickHouse/accept.txt +++ b/docs/styles/config/vocabularies/ClickHouse/accept.txt @@ -1,7 +1,7 @@ kubectl hostname CRDs -namespace +[Nn]amespaces? uncomment PVCs PDBs diff --git a/internal/controller/clickhouse/controller_test.go b/internal/controller/clickhouse/controller_test.go index a6e82fc3..aced0fd7 100644 --- a/internal/controller/clickhouse/controller_test.go +++ b/internal/controller/clickhouse/controller_test.go @@ -40,65 +40,6 @@ func TestControllers(t *testing.T) { RunSpecs(t, "ClickHouse Controller Suite") } -var _ = Describe("keeper watch mapping", func() { - It("should enqueue ClickHouse clusters that explicitly reference a keeper in another namespace", func(ctx context.Context) { - testScheme := k8sruntime.NewScheme() - Expect(clientgoscheme.AddToScheme(testScheme)).To(Succeed()) - Expect(v1.AddToScheme(testScheme)).To(Succeed()) - - referencedCluster := &v1.ClickHouseCluster{ - ObjectMeta: metav1.ObjectMeta{ - Name: "cross-namespace-cluster", - Namespace: "clickhouse-ns", - }, - Spec: v1.ClickHouseClusterSpec{ - KeeperClusterRef: &v1.KeeperClusterReference{ - Name: "keeper", - Namespace: "keeper-ns", - }, - }, - } - sameNameDifferentNamespace := &v1.ClickHouseCluster{ - ObjectMeta: metav1.ObjectMeta{ - Name: "same-name-different-namespace", - Namespace: "other-ns", - }, - Spec: v1.ClickHouseClusterSpec{ - KeeperClusterRef: &v1.KeeperClusterReference{ - Name: "keeper", - }, - }, - } - - controller := &ClusterController{ - Client: fake.NewClientBuilder(). - WithScheme(testScheme). - WithObjects(referencedCluster, sameNameDifferentNamespace). - WithIndex(&v1.ClickHouseCluster{}, keeperClusterReferenceField, func(obj client.Object) []string { - cluster, ok := obj.(*v1.ClickHouseCluster) - if !ok { - return nil - } - - return keeperReferenceFieldValue(cluster) - }). - Build(), - } - - Expect(controller.clickHouseClustersForKeeper(ctx, &v1.KeeperCluster{ - ObjectMeta: metav1.ObjectMeta{ - Name: "keeper", - Namespace: "keeper-ns", - }, - })).To(ConsistOf(reconcile.Request{ - NamespacedName: types.NamespacedName{ - Name: referencedCluster.Name, - Namespace: referencedCluster.Namespace, - }, - })) - }) -}) - var _ = When("reconciling ClickHouseCluster", Ordered, func() { var ( suite testutil.TestSuit @@ -119,7 +60,7 @@ var _ = When("reconciling ClickHouseCluster", Ordered, func() { Spec: v1.ClickHouseClusterSpec{ Replicas: new(int32(2)), Shards: new(int32(2)), - KeeperClusterRef: &v1.KeeperClusterReference{Name: keeperName}, + KeeperClusterRef: v1.KeeperClusterReference{Name: keeperName}, Labels: map[string]string{ "test-label": "test-val", }, @@ -254,7 +195,7 @@ var _ = When("reconciling ClickHouseCluster", Ordered, func() { Spec: v1.ClickHouseClusterSpec{ Replicas: new(int32(1)), Shards: new(int32(1)), - KeeperClusterRef: &v1.KeeperClusterReference{ + KeeperClusterRef: v1.KeeperClusterReference{ Name: keeper.Name, Namespace: keeper.Namespace, }, @@ -565,7 +506,7 @@ var _ = When("reconciling ClickHouseCluster", Ordered, func() { Spec: v1.ClickHouseClusterSpec{ Replicas: new(int32(2)), Shards: new(int32(1)), - KeeperClusterRef: &v1.KeeperClusterReference{Name: keeperName}, + KeeperClusterRef: v1.KeeperClusterReference{Name: keeperName}, DataVolumeClaimSpec: &corev1.PersistentVolumeClaimSpec{ AccessModes: []corev1.PersistentVolumeAccessMode{corev1.ReadWriteOnce}, Resources: corev1.VolumeResourceRequirements{ @@ -669,7 +610,7 @@ var _ = When("reconciling ClickHouseCluster", Ordered, func() { Namespace: "default", }, Spec: v1.ClickHouseClusterSpec{ - KeeperClusterRef: &v1.KeeperClusterReference{Name: keeperName}, + KeeperClusterRef: v1.KeeperClusterReference{Name: keeperName}, ExternalSecret: &v1.ExternalSecret{ Name: secret.Name, }, @@ -729,3 +670,62 @@ var _ = When("reconciling ClickHouseCluster", Ordered, func() { Expect(secret.Data).To(HaveKey(SecretKeyClusterSecret)) }) }) + +var _ = Describe("keeper watch mapping", func() { + It("should enqueue ClickHouse clusters that explicitly reference a keeper in another namespace", func(ctx context.Context) { + testScheme := k8sruntime.NewScheme() + Expect(clientgoscheme.AddToScheme(testScheme)).To(Succeed()) + Expect(v1.AddToScheme(testScheme)).To(Succeed()) + + referencedCluster := &v1.ClickHouseCluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cross-namespace-cluster", + Namespace: "clickhouse-ns", + }, + Spec: v1.ClickHouseClusterSpec{ + KeeperClusterRef: v1.KeeperClusterReference{ + Name: "keeper", + Namespace: "keeper-ns", + }, + }, + } + sameNameDifferentNamespace := &v1.ClickHouseCluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "same-name-different-namespace", + Namespace: "other-ns", + }, + Spec: v1.ClickHouseClusterSpec{ + KeeperClusterRef: v1.KeeperClusterReference{ + Name: "keeper", + }, + }, + } + + controller := &ClusterController{ + Client: fake.NewClientBuilder(). + WithScheme(testScheme). + WithObjects(referencedCluster, sameNameDifferentNamespace). + WithIndex(&v1.ClickHouseCluster{}, keeperClusterReferenceField, func(obj client.Object) []string { + cluster, ok := obj.(*v1.ClickHouseCluster) + if !ok { + return nil + } + + return keeperReferenceFieldValue(cluster) + }). + Build(), + } + + Expect(controller.clickHouseClustersForKeeper(ctx, &v1.KeeperCluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "keeper", + Namespace: "keeper-ns", + }, + })).To(ConsistOf(reconcile.Request{ + NamespacedName: types.NamespacedName{ + Name: referencedCluster.Name, + Namespace: referencedCluster.Namespace, + }, + })) + }) +}) diff --git a/internal/webhook/v1alpha1/clickhousecluster_webhook.go b/internal/webhook/v1alpha1/clickhousecluster_webhook.go index ad373fe5..90470229 100644 --- a/internal/webhook/v1alpha1/clickhousecluster_webhook.go +++ b/internal/webhook/v1alpha1/clickhousecluster_webhook.go @@ -4,9 +4,7 @@ import ( "context" "errors" "fmt" - "strings" - "k8s.io/apimachinery/pkg/util/validation" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/webhook/admission" @@ -91,20 +89,6 @@ func (w *ClickHouseClusterWebhook) validateImpl(obj *chv1.ClickHouseCluster) (ad errs []error ) - if obj.Spec.KeeperClusterRef == nil || obj.Spec.KeeperClusterRef.Name == "" { - errs = append(errs, errors.New("keeperClusterRef name must not be empty")) - } - - if obj.Spec.KeeperClusterRef != nil && obj.Spec.KeeperClusterRef.Namespace != "" { - if validationErrs := validation.IsDNS1123Label(obj.Spec.KeeperClusterRef.Namespace); len(validationErrs) > 0 { - errs = append(errs, fmt.Errorf( - "keeperClusterRef namespace %q is invalid: %s", - obj.Spec.KeeperClusterRef.Namespace, - strings.Join(validationErrs, ", "), - )) - } - } - if err := obj.Spec.Settings.TLS.Validate(); err != nil { errs = append(errs, err) } diff --git a/internal/webhook/v1alpha1/clickhousecluster_webhook_test.go b/internal/webhook/v1alpha1/clickhousecluster_webhook_test.go index 69396bf7..052e23c6 100644 --- a/internal/webhook/v1alpha1/clickhousecluster_webhook_test.go +++ b/internal/webhook/v1alpha1/clickhousecluster_webhook_test.go @@ -24,7 +24,7 @@ var _ = Describe("ClickHouseCluster Webhook", func() { Name: "test-default", }, Spec: chv1.ClickHouseClusterSpec{ - KeeperClusterRef: &chv1.KeeperClusterReference{ + KeeperClusterRef: chv1.KeeperClusterReference{ Name: "some-keeper-cluster", }, }, @@ -47,7 +47,7 @@ var _ = Describe("ClickHouseCluster Webhook", func() { Name: "test-default", }, Spec: chv1.ClickHouseClusterSpec{ - KeeperClusterRef: &chv1.KeeperClusterReference{ + KeeperClusterRef: chv1.KeeperClusterReference{ Name: "some-keeper-cluster", }, DataVolumeClaimSpec: &corev1.PersistentVolumeClaimSpec{Resources: corev1.VolumeResourceRequirements{ @@ -73,7 +73,7 @@ var _ = Describe("ClickHouseCluster Webhook", func() { Name: "test-validate", }, Spec: chv1.ClickHouseClusterSpec{ - KeeperClusterRef: &chv1.KeeperClusterReference{ + KeeperClusterRef: chv1.KeeperClusterReference{ Name: "some-keeper-cluster", }, }, @@ -107,7 +107,7 @@ var _ = Describe("ClickHouseCluster Webhook", func() { err := k8sClient.Create(ctx, cluster) Expect(err).To(HaveOccurred()) - Expect(err.Error()).To(ContainSubstring("keeperClusterRef namespace")) + Expect(err.Error()).To(ContainSubstring("keeperClusterRef.namespace")) }) It("Should check certificate passed if TLS enabled", func(ctx context.Context) { diff --git a/test/deploy/deploy_test.go b/test/deploy/deploy_test.go index 14fdec95..9b5c1dfb 100644 --- a/test/deploy/deploy_test.go +++ b/test/deploy/deploy_test.go @@ -376,7 +376,7 @@ func testDeployment(namespace string) { }, Spec: v1.ClickHouseClusterSpec{ Replicas: new(int32(1)), - KeeperClusterRef: &v1.KeeperClusterReference{ + KeeperClusterRef: v1.KeeperClusterReference{ Name: keeper.Name, }, ContainerTemplate: v1.ContainerTemplateSpec{ diff --git a/test/e2e/clickhouse_e2e_test.go b/test/e2e/clickhouse_e2e_test.go index 97d66985..4460de03 100644 --- a/test/e2e/clickhouse_e2e_test.go +++ b/test/e2e/clickhouse_e2e_test.go @@ -72,7 +72,7 @@ var _ = Describe("ClickHouse controller", Label("clickhouse"), func() { }, }, DataVolumeClaimSpec: &defaultStorage, - KeeperClusterRef: &v1.KeeperClusterReference{ + KeeperClusterRef: v1.KeeperClusterReference{ Name: keeper.Name, }, }, @@ -129,7 +129,7 @@ var _ = Describe("ClickHouse controller", Label("clickhouse"), func() { }, }, DataVolumeClaimSpec: &defaultStorage, - KeeperClusterRef: &v1.KeeperClusterReference{ + KeeperClusterRef: v1.KeeperClusterReference{ Name: keeper.Name, }, }, @@ -176,7 +176,7 @@ var _ = Describe("ClickHouse controller", Label("clickhouse"), func() { Spec: v1.ClickHouseClusterSpec{ Replicas: new(int32(1)), DataVolumeClaimSpec: nil, // Diskless configuration - KeeperClusterRef: &v1.KeeperClusterReference{ + KeeperClusterRef: v1.KeeperClusterReference{ Name: keeper.Name, }, PodTemplate: v1.PodTemplateSpec{ @@ -219,7 +219,7 @@ var _ = Describe("ClickHouse controller", Label("clickhouse"), func() { Spec: v1.ClickHouseClusterSpec{ Replicas: new(int32(1)), DataVolumeClaimSpec: nil, // Diskless configuration - KeeperClusterRef: &v1.KeeperClusterReference{ + KeeperClusterRef: v1.KeeperClusterReference{ Name: keeper.Name, }, ContainerTemplate: v1.ContainerTemplateSpec{ @@ -281,7 +281,7 @@ var _ = Describe("ClickHouse controller", Label("clickhouse"), func() { }, Spec: v1.ClickHouseClusterSpec{ Replicas: new(int32(2)), - KeeperClusterRef: &v1.KeeperClusterReference{ + KeeperClusterRef: v1.KeeperClusterReference{ Name: keeper.Name, }, ContainerTemplate: v1.ContainerTemplateSpec{ @@ -388,7 +388,7 @@ var _ = Describe("ClickHouse controller", Label("clickhouse"), func() { }, Spec: v1.ClickHouseClusterSpec{ Replicas: new(int32(2)), - KeeperClusterRef: &v1.KeeperClusterReference{ + KeeperClusterRef: v1.KeeperClusterReference{ Name: keeper.Name, }, PodTemplate: v1.PodTemplateSpec{ @@ -495,7 +495,7 @@ var _ = Describe("ClickHouse controller", Label("clickhouse"), func() { Spec: v1.ClickHouseClusterSpec{ Replicas: new(int32(2)), Shards: new(int32(2)), - KeeperClusterRef: &v1.KeeperClusterReference{ + KeeperClusterRef: v1.KeeperClusterReference{ Name: keeper.Name, }, ContainerTemplate: v1.ContainerTemplateSpec{ @@ -581,7 +581,7 @@ var _ = Describe("ClickHouse controller", Label("clickhouse"), func() { Image: v1.ContainerImage{Tag: BaseVersion}, }, DataVolumeClaimSpec: &defaultStorage, - KeeperClusterRef: &v1.KeeperClusterReference{Name: keeper.Name}, + KeeperClusterRef: v1.KeeperClusterReference{Name: keeper.Name}, }, } @@ -646,7 +646,7 @@ var _ = Describe("ClickHouse controller", Label("clickhouse"), func() { Replicas: new(int32(1)), ContainerTemplate: v1.ContainerTemplateSpec{Image: v1.ContainerImage{Tag: BaseVersion}}, DataVolumeClaimSpec: &defaultStorage, - KeeperClusterRef: &v1.KeeperClusterReference{Name: keeper.Name}, + KeeperClusterRef: v1.KeeperClusterReference{Name: keeper.Name}, ExternalSecret: &v1.ExternalSecret{ Name: secretName, Policy: v1.ExternalSecretPolicyObserve, @@ -781,7 +781,7 @@ var _ = Describe("ClickHouse controller", Label("clickhouse"), func() { }, Spec: v1.ClickHouseClusterSpec{ Replicas: new(int32(2)), - KeeperClusterRef: &v1.KeeperClusterReference{ + KeeperClusterRef: v1.KeeperClusterReference{ Name: keeperCR.Name, }, ContainerTemplate: v1.ContainerTemplateSpec{ @@ -949,7 +949,7 @@ var _ = Describe("ClickHouse controller", Label("clickhouse"), func() { Spec: v1.ClickHouseClusterSpec{ Replicas: new(int32(3)), DataVolumeClaimSpec: &defaultStorage, - KeeperClusterRef: &v1.KeeperClusterReference{ + KeeperClusterRef: v1.KeeperClusterReference{ Name: keeperName, }, PodTemplate: v1.PodTemplateSpec{