diff --git a/api/v1alpha1/clickhousecluster_types.go b/api/v1alpha1/clickhousecluster_types.go index 9cabf663..eacdafa2 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,37 @@ type ClickHouseClusterStatus struct { VersionProbeRevision string `json:"versionProbeRevision,omitempty"` } +// 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"` +} + +// 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 +406,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..eb184578 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..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(v1.LocalObjectReference) - **out = **in - } + out.KeeperClusterRef = in.KeeperClusterRef in.PodTemplate.DeepCopyInto(&out.PodTemplate) in.ContainerTemplate.DeepCopyInto(&out.ContainerTemplate) if in.DataVolumeClaimSpec != nil { @@ -400,6 +396,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..e063e8bc 100644 --- a/config/crd/bases/clickhouse.com_clickhouseclusters.yaml +++ b/config/crd/bases/clickhouse.com_clickhouseclusters.yaml @@ -1113,20 +1113,25 @@ 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. + 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: |- - 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. + maxLength: 63 + pattern: ^[a-z0-9]([-a-z0-9]*[a-z0-9])?$ 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..a869274e 100644 --- a/dist/chart/templates/crd/clickhouseclusters.clickhouse.com.yaml +++ b/dist/chart/templates/crd/clickhouseclusters.clickhouse.com.yaml @@ -1116,20 +1116,25 @@ 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. + 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: |- - 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. + maxLength: 63 + pattern: ^[a-z0-9]([-a-z0-9]*[a-z0-9])?$ 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/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.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..aced0fd7 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" @@ -54,7 +60,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 +80,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 +164,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 +506,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 +610,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, }, @@ -601,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/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..90470229 100644 --- a/internal/webhook/v1alpha1/clickhousecluster_webhook.go +++ b/internal/webhook/v1alpha1/clickhousecluster_webhook.go @@ -89,10 +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 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..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: &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..9b5c1dfb 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..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: &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{