diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 5438a8c1..f891b75e 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -238,7 +238,7 @@ jobs: deploy_target: test-compat-e2e-olm - name: supported-clickhouse-compatibility k8s_image: v1.30.13 - clickhouse_version: "26.3,26.2,26.1,25.8" + clickhouse_version: "26.3,26.3-distroless,26.2,26.1,25.8" deploy_target: test-compat-e2e-manifest - name: operator-upgrade k8s_image: v1.30.13 diff --git a/api/v1alpha1/keepercluster_types.go b/api/v1alpha1/keepercluster_types.go index 9a3bda65..885a4d4d 100644 --- a/api/v1alpha1/keepercluster_types.go +++ b/api/v1alpha1/keepercluster_types.go @@ -66,6 +66,8 @@ type KeeperClusterSpec struct { UpgradeChannel string `json:"upgradeChannel,omitempty"` // VersionProbeTemplate overrides for the version detection Job. + // + // Deprecated: Keeper version probe Jobs are not used; this field is retained for backward compatibility. // +optional VersionProbeTemplate *VersionProbeTemplate `json:"versionProbeTemplate,omitempty"` } @@ -161,12 +163,13 @@ type KeeperClusterStatus struct { // ObservedGeneration indicates latest generation observed by controller. // +operator-sdk:csv:customresourcedefinitions:type=status ObservedGeneration int64 `json:"observedGeneration,omitempty"` - // Version indicates the version reported by the container image. + // Version indicates the version reported by the Keeper server. // +optional // +operator-sdk:csv:customresourcedefinitions:type=status Version string `json:"version,omitempty"` // VersionProbeRevision is the image hash of the last successful version probe. - // When this matches the current image hash, the cached Version is used directly. + // + // Deprecated: Keeper version probe Jobs are not used; this field is retained for backward compatibility. // +optional // +operator-sdk:csv:customresourcedefinitions:type=status VersionProbeRevision string `json:"versionProbeRevision,omitempty"` diff --git a/config/crd/bases/clickhouse.com_keeperclusters.yaml b/config/crd/bases/clickhouse.com_keeperclusters.yaml index c7157d28..4e9bad54 100644 --- a/config/crd/bases/clickhouse.com_keeperclusters.yaml +++ b/config/crd/bases/clickhouse.com_keeperclusters.yaml @@ -6163,8 +6163,10 @@ spec: pattern: ^(lts|stable|\d+\.\d+)?$ type: string versionProbeTemplate: - description: VersionProbeTemplate overrides for the version detection - Job. + description: |- + VersionProbeTemplate overrides for the version detection Job. + + Deprecated: Keeper version probe Jobs are not used; this field is retained for backward compatibility. properties: metadata: description: Metadata applied to the version probe Job. @@ -6869,13 +6871,14 @@ spec: spec revision. type: string version: - description: Version indicates the version reported by the container - image. + description: Version indicates the version reported by the Keeper + server. type: string versionProbeRevision: description: |- VersionProbeRevision is the image hash of the last successful version probe. - When this matches the current image hash, the cached Version is used directly. + + Deprecated: Keeper version probe Jobs are not used; this field is retained for backward compatibility. type: string type: object type: object diff --git a/config/manifests/bases/clickhouse-operator.clusterserviceversion.yaml b/config/manifests/bases/clickhouse-operator.clusterserviceversion.yaml index d015c7ff..470f8913 100644 --- a/config/manifests/bases/clickhouse-operator.clusterserviceversion.yaml +++ b/config/manifests/bases/clickhouse-operator.clusterserviceversion.yaml @@ -141,12 +141,12 @@ spec: revision. displayName: Update Revision path: updateRevision - - description: Version indicates the version reported by the container image. + - description: Version indicates the version reported by the Keeper server. displayName: Version path: version - description: |- VersionProbeRevision is the image hash of the last successful version probe. - When this matches the current image hash, the cached Version is used directly. + Deprecated: Keeper version probe Jobs are not used; this field is retained for backward compatibility. displayName: Version Probe Revision path: versionProbeRevision version: v1alpha1 diff --git a/dist/chart-cluster/values.yaml b/dist/chart-cluster/values.yaml index e926f9dd..91b606ee 100644 --- a/dist/chart-cluster/values.yaml +++ b/dist/chart-cluster/values.yaml @@ -156,5 +156,7 @@ keeper: # upgradeChannel: "" # VersionProbeTemplate overrides for the version detection Job. + # + # Deprecated: Keeper version probe Jobs are not used; this field is retained for backward compatibility. # versionProbeTemplate: {} diff --git a/dist/chart/templates/crd/keeperclusters.clickhouse.com.yaml b/dist/chart/templates/crd/keeperclusters.clickhouse.com.yaml index a7b3d539..666c2fd5 100644 --- a/dist/chart/templates/crd/keeperclusters.clickhouse.com.yaml +++ b/dist/chart/templates/crd/keeperclusters.clickhouse.com.yaml @@ -6166,8 +6166,10 @@ spec: pattern: ^(lts|stable|\d+\.\d+)?$ type: string versionProbeTemplate: - description: VersionProbeTemplate overrides for the version detection - Job. + description: |- + VersionProbeTemplate overrides for the version detection Job. + + Deprecated: Keeper version probe Jobs are not used; this field is retained for backward compatibility. properties: metadata: description: Metadata applied to the version probe Job. @@ -6872,13 +6874,14 @@ spec: spec revision. type: string version: - description: Version indicates the version reported by the container - image. + description: Version indicates the version reported by the Keeper + server. type: string versionProbeRevision: description: |- VersionProbeRevision is the image hash of the last successful version probe. - When this matches the current image hash, the cached Version is used directly. + + Deprecated: Keeper version probe Jobs are not used; this field is retained for backward compatibility. type: string type: object type: object diff --git a/docs/guides/configuration.mdx b/docs/guides/configuration.mdx index 014b25f7..7210c373 100644 --- a/docs/guides/configuration.mdx +++ b/docs/guides/configuration.mdx @@ -158,12 +158,12 @@ For ClickHouse clusters with more than one shard, **one PDB is created per shard The operator picks safe defaults based on the cluster size so that a fresh `apply` already protects against accidental quorum loss. -| Resource | Topology | Default PDB | -|---|---|---| -| `ClickHouseCluster` | `replicas: 1` (single-replica shard) | `maxUnavailable: 1` — disruption is allowed for a single-node cluster so that node drains are not blocked | -| `ClickHouseCluster` | `replicas: 2+` (multi-replica shard) | `minAvailable: 1` — at least one replica per shard must stay up | -| `KeeperCluster` | `replicas: 1` | `maxUnavailable: 1` — disruption is allowed for a single-node cluster so that node drains are not blocked | -| `KeeperCluster` | `replicas: 3+` | `maxUnavailable: replicas/2` — preserves the RAFT quorum for a `2F+1` cluster (3 replicas tolerate 1 down, 5 replicas tolerate 2 down) | +| Resource | Topology | Default PDB | +|---------------------|--------------------------------------|----------------------------------------------------------------------------------------------------------------------------------------| +| `ClickHouseCluster` | `replicas: 1` (single-replica shard) | `maxUnavailable: 1` — disruption is allowed for a single-node cluster so that node drains are not blocked | +| `ClickHouseCluster` | `replicas: 2+` (multi-replica shard) | `minAvailable: 1` — at least one replica per shard must stay up | +| `KeeperCluster` | `replicas: 1` | `maxUnavailable: 1` — disruption is allowed for a single-node cluster so that node drains are not blocked | +| `KeeperCluster` | `replicas: 3+` | `maxUnavailable: replicas/2` — preserves the RAFT quorum for a `2F+1` cluster (3 replicas tolerate 1 down, 5 replicas tolerate 2 down) | For a 3-shard ClickHouseCluster with `replicas: 3`, the operator creates three PDBs, one per shard, each with `minAvailable: 1`. @@ -205,11 +205,11 @@ spec: `spec.podDisruptionBudget.policy` lets you choose **how aggressively** the operator manages PDBs: -| Policy | Behavior | -|---|---| -| `Enabled` (default) | The operator creates and updates the PDB on every reconcile. This is the safe production default. | -| `Disabled` | The operator does **not** create PDBs and **deletes** any existing ones with matching labels. Useful for development clusters where every voluntary disruption should be allowed. | -| `Ignored` | The operator neither creates nor deletes PDBs. Existing PDBs are left alone. Use this when another system (e.g. policy admission, GitOps tool) owns PDB management for you. | +| Policy | Behavior | +|---------------------|-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------| +| `Enabled` (default) | The operator creates and updates the PDB on every reconcile. This is the safe production default. | +| `Disabled` | The operator does **not** create PDBs and **deletes** any existing ones with matching labels. Useful for development clusters where every voluntary disruption should be allowed. | +| `Ignored` | The operator neither creates nor deletes PDBs. Existing PDBs are left alone. Use this when another system (e.g. policy admission, GitOps tool) owns PDB management for you. | Example — disable PDB management completely on a development cluster: @@ -385,12 +385,12 @@ The referenced Secret must reside in the **same namespace** as the ClickHouseClu The Secret must contain the following keys: -| Key | Format | When required | -|---|---|---| -| `interserver-password` | plaintext password | Always | -| `management-password` | plaintext password | Always | -| `keeper-identity` | `clickhouse:` | Always | -| `cluster-secret` | plaintext password | Always | +| Key | Format | When required | +|-------------------------|--------------------------------------------|----------------------------| +| `interserver-password` | plaintext password | Always | +| `management-password` | plaintext password | Always | +| `keeper-identity` | `clickhouse:` | Always | +| `cluster-secret` | plaintext password | Always | | `named-collections-key` | hex-encoded 16-byte AES key (32 hex chars) | ClickHouse `>= 25.12` only | A complete Secret looks like this: @@ -414,10 +414,10 @@ stringData: `spec.externalSecret.policy` controls how the operator handles missing required keys: -| Policy | Behavior on missing keys | -|---|---| -| `Observe` (default) | Reconciliation is **blocked** until every required key is present. The operator reports each missing key — and the format hint for it — via the `ExternalSecretValid` condition (with reason `ExternalSecretInvalid`) and a `Warning` event. | -| `Manage` | The operator **generates** any missing required keys and writes them back to the same Secret. Useful for bootstrapping: create an empty Secret, let the operator fill it, then optionally tighten access. The operator still never deletes the Secret. | +| Policy | Behavior on missing keys | +|---------------------|--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------| +| `Observe` (default) | Reconciliation is **blocked** until every required key is present. The operator reports each missing key — and the format hint for it — via the `ExternalSecretValid` condition (with reason `ExternalSecretInvalid`) and a `Warning` event. | +| `Manage` | The operator **generates** any missing required keys and writes them back to the same Secret. Useful for bootstrapping: create an empty Secret, let the operator fill it, then optionally tighten access. The operator still never deletes the Secret. | Even with `policy: Manage` the Secret must already exist in the namespace — the operator never creates the Secret itself, it only writes generated keys into an existing one. If the referenced Secret is missing, reconciliation is blocked with the `ExternalSecretNotFound` reason regardless of policy. @@ -442,11 +442,11 @@ kubectl get clickhousecluster sample -o jsonpath='{.status.conditions}' | jq Possible reasons: -| `reason` | Meaning | Fix | -|---|---|---| -| `ExternalSecretNotFound` | The referenced Secret does not exist in the namespace. | Create the Secret, or fix `spec.externalSecret.name`. | -| `ExternalSecretInvalid` | The Secret exists but lacks required keys (only with `Observe`). The message lists each missing key together with its expected format. | Add the missing keys, or switch to `policy: Manage`. | -| `ExternalSecretValid` | All required keys are present and the operator is using the Secret. | — | +| `reason` | Meaning | Fix | +|--------------------------|----------------------------------------------------------------------------------------------------------------------------------------|-------------------------------------------------------| +| `ExternalSecretNotFound` | The referenced Secret does not exist in the namespace. | Create the Secret, or fix `spec.externalSecret.name`. | +| `ExternalSecretInvalid` | The Secret exists but lacks required keys (only with `Observe`). The message lists each missing key together with its expected format. | Add the missing keys, or switch to `policy: Manage`. | +| `ExternalSecretValid` | All required keys are present and the operator is using the Secret. | — | The operator requeues reconciliation while the Secret is invalid, so once you add the missing keys the next reconcile picks them up automatically — no need to bounce pods. @@ -517,36 +517,36 @@ kubectl exec sample-clickhouse-0-0-0 -- \ ### Field constraints {#additional-ports-constraints} -| Field | Rule | -|---|---| +| Field | Rule | +|--------|------------------------------------------------------------------------------------------------------------------------------------------| | `name` | Must match the DNS_LABEL pattern `^[a-z]([-a-z0-9]*[a-z0-9])?$`, max 63 characters. Uniqueness is enforced by the CRD as a list-map key. | -| `port` | Integer in `[1, 65535]`. The webhook rejects duplicate port numbers within the list. | +| `port` | Integer in `[1, 65535]`. The webhook rejects duplicate port numbers within the list. | ### Reserved ports and names {#additional-ports-reserved} The validating webhook rejects `additionalPorts` entries that would collide with ports the operator binds itself. All TLS-related ports are reserved **unconditionally** so that flipping `spec.settings.tls.enabled` later cannot break a previously valid cluster. -| Port | Reserved for | -|---|---| -| `8123` | HTTP | -| `8443` | HTTPS | -| `9000` | native TCP | -| `9440` | native TLS | -| `9009` | interserver | -| `9001` | management | +| Port | Reserved for | +|--------|--------------------| +| `8123` | HTTP | +| `8443` | HTTPS | +| `9000` | native TCP | +| `9440` | native TLS | +| `9009` | interserver | +| `9001` | management | | `9363` | Prometheus metrics | The following names are also rejected — they are the operator's internal protocol-type identifiers (not the human-readable aliases): -| Name | -|---| -| `http` | +| Name | +|---------------| +| `http` | | `http-secure` | -| `tcp` | -| `tcp-secure` | +| `tcp` | +| `tcp-secure` | | `interserver` | -| `management` | -| `prometheus` | +| `management` | +| `prometheus` | A rejected request produces an error such as: @@ -559,7 +559,7 @@ spec.additionalPorts[0].name: "http" is reserved by the operator The operator does two independent things with cluster versions: -1. **Version probe** — a Kubernetes `Job` that runs the container image once to detect the running ClickHouse / Keeper version. The detected version is recorded in `.status.version` and used by other reconciliation steps (e.g. the `External Secret` named-collections key is only required from ClickHouse `25.12`). +1. **Version reporting** — for `ClickHouseCluster`, a Kubernetes `Job` runs the container image once to detect the running ClickHouse version; for `KeeperCluster`, the operator reads the server-reported version from running replicas. The detected version is recorded in `.status.version` and used by other reconciliation steps (e.g. the `External Secret` named-collections key is only required from ClickHouse `25.12`). 2. **Upgrade channel** — a periodic check against the public ClickHouse release feed (`https://clickhouse.com/data/version_date.tsv`). The operator reports whether a newer version is available via the `VersionUpgraded` status condition. It never upgrades the cluster on its own — the user is in control of the image tag. ### Choosing a release channel {#upgrade-channel-choosing} @@ -573,12 +573,12 @@ spec: Allowed values (validated by the CRD with the pattern `^(lts|stable|\d+\.\d+)?$`): -| Value | Behavior | -|---|---| -| _empty_ (default) | The operator proposes only **minor** updates within the currently-running major.minor line. A cluster on `25.8.3.1` will be told about `25.8.4.x` but not `25.9.x`. | -| `stable` | Tracks the upstream `stable` channel — the latest release that ClickHouse Inc. flags as stable on the main release line. Receives major upgrades sooner than the `lts` channel. | -| `lts` | Tracks the upstream `lts` channel — long-term support releases. Receives major upgrades less frequently, with longer support windows. | -| `25.8` (or any `.`) | Pins the channel to a specific major.minor line. Major upgrades beyond it are not proposed even if a newer version exists upstream. | +| Value | Behavior | +|-----------------------------------|---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------| +| _empty_ (default) | The operator proposes only **minor** updates within the currently-running major.minor line. A cluster on `25.8.3.1` will be told about `25.8.4.x` but not `25.9.x`. | +| `stable` | Tracks the upstream `stable` channel — the latest release that ClickHouse Inc. flags as stable on the main release line. Receives major upgrades sooner than the `lts` channel. | +| `lts` | Tracks the upstream `lts` channel — long-term support releases. Receives major upgrades less frequently, with longer support windows. | +| `25.8` (or any `.`) | Pins the channel to a specific major.minor line. Major upgrades beyond it are not proposed even if a newer version exists upstream. | For production, pinning the channel to an explicit `.` (e.g. `25.8`) is generally preferred. It locks the cluster to the intended major release line and lets the operator surface a `WrongReleaseChannel` warning if any replica somehow drifts onto a different major — which matters especially when the image is referenced by a digest (`@sha256:...`) rather than by a human-readable tag. The empty default is fine for development clusters where major-version jumps are not a concern. @@ -586,18 +586,18 @@ For production, pinning the channel to an explicit `.` (e.g. `25.8 Two conditions surface the result of the probe and the upgrade check: -| Condition | Reason | Meaning | -|---|---|---| -| `VersionInSync` | `VersionMatch` | All replicas report the same version as the image | -| `VersionInSync` | `VersionMismatch` | Replicas are running different versions. The warning event is suppressed during a planned rolling upgrade. It typically surfaces when a mutable image tag has been pinned (for example `latest` or a bare major like `26.3`) and the underlying registry has shifted between pulls, so different replicas ended up on different patches of the same tag. | -| `VersionInSync` | `VersionPending` | Version probe Job has not finished yet | -| `VersionInSync` | `VersionProbeFailed` | Probe Job failed; the operator cannot determine the running version | -| `VersionUpgraded` | `UpToDate` | The cluster is on the latest version available in the selected channel | -| `VersionUpgraded` | `MinorUpdateAvailable` | A newer patch is available in the same `major.minor` line | -| `VersionUpgraded` | `MajorUpdateAvailable` | A newer `major.minor` is available within the chosen channel | -| `VersionUpgraded` | `VersionOutdated` | The running version is out of date and will no longer receive fixes from the selected channel — typically because the major line has been dropped from `lts` or `stable` upstream | -| `VersionUpgraded` | `WrongReleaseChannel` | The running image does not belong to the selected `upgradeChannel`. Example: a cluster running `26.5` with `upgradeChannel: lts`, since `26.5` is not part of the upstream `lts` line. | -| `VersionUpgraded` | `UpgradeCheckFailed` | The operator could not reach the upstream release feed | +| Condition | Reason | Meaning | +|-------------------|------------------------|----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------| +| `VersionInSync` | `VersionMatch` | All replicas report the same version as the image | +| `VersionInSync` | `VersionMismatch` | Replicas are running different versions. This reason is suppressed during a planned rolling upgrade. It typically surfaces when a mutable image tag has been pinned (for example `latest` or a bare major like `26.3`) and the underlying registry has shifted between pulls, so different replicas ended up on different patches of the same tag. | +| `VersionInSync` | `VersionPending` | Version probe Job has not finished yet, or no Keeper replica version has been observed yet | +| `VersionInSync` | `VersionProbeFailed` | ClickHouse probe Job failed; the operator cannot determine the running version | +| `VersionUpgraded` | `UpToDate` | The cluster is on the latest version available in the selected channel | +| `VersionUpgraded` | `MinorUpdateAvailable` | A newer patch is available in the same `major.minor` line | +| `VersionUpgraded` | `MajorUpdateAvailable` | A newer `major.minor` is available within the chosen channel | +| `VersionUpgraded` | `VersionOutdated` | The running version is out of date and will no longer receive fixes from the selected channel — typically because the major line has been dropped from `lts` or `stable` upstream | +| `VersionUpgraded` | `WrongReleaseChannel` | The running image does not belong to the selected `upgradeChannel`. Example: a cluster running `26.5` with `upgradeChannel: lts`, since `26.5` is not part of the upstream `lts` line. | +| `VersionUpgraded` | `UpgradeCheckFailed` | The operator could not reach the upstream release feed | Inspect them with: @@ -637,9 +637,9 @@ The container name `version-probe` is the operator's default — the entry under Two flags on the operator manager control the upgrade-check loop globally: -| Flag | Default | Effect | -|---|---|---| -| `--version-update-interval` | `24h` | How often the operator re-fetches the upstream version list | +| Flag | Default | Effect | +|-----------------------------------|---------|--------------------------------------------------------------------------------------------------------------------------------------------------| +| `--version-update-interval` | `24h` | How often the operator re-fetches the upstream version list | | `--disable-version-update-checks` | `false` | Disables the upgrade checker entirely. The `VersionUpgraded` condition is not set, and no outbound HTTP traffic to `clickhouse.com` is generated | Set `--disable-version-update-checks=true` in air-gapped environments or when egress to `clickhouse.com` is not allowed. @@ -763,13 +763,13 @@ spec: count: 50 # Default: 50. Number of rotated files to keep ``` -| Field | Default | Description | -|---|---|---| -| `logToFile` | `true` | When `false`, the operator drops the file targets and the server logs only to the container console. | -| `jsonLogs` | `false` | When `true`, the operator adds `formatting.type: json` so each line is a JSON object. | -| `level` | `trace` | Log verbosity. One of `test`, `trace`, `debug`, `information`, `notice`, `warning`, `error`, `critical`, `fatal`. | -| `size` | `1000M` | Maximum size of a single log file before rotation. | -| `count` | `50` | Number of rotated log files the server retains. | +| Field | Default | Description | +|-------------|---------|-------------------------------------------------------------------------------------------------------------------| +| `logToFile` | `true` | When `false`, the operator drops the file targets and the server logs only to the container console. | +| `jsonLogs` | `false` | When `true`, the operator adds `formatting.type: json` so each line is a JSON object. | +| `level` | `trace` | Log verbosity. One of `test`, `trace`, `debug`, `information`, `notice`, `warning`, `error`, `critical`, `fatal`. | +| `size` | `1000M` | Maximum size of a single log file before rotation. | +| `count` | `50` | Number of rotated log files the server retains. | The operator always keeps console logging on so that `kubectl logs` works, and layers file logging on top when `logToFile` is `true`. A cluster with the defaults renders this `logger` block: diff --git a/docs/reference/api-reference.mdx b/docs/reference/api-reference.mdx index 45a6bc55..751af4ae 100644 --- a/docs/reference/api-reference.mdx +++ b/docs/reference/api-reference.mdx @@ -274,7 +274,7 @@ KeeperClusterSpec defines the desired state of KeeperCluster. | `settings` | [KeeperSettings](#keepersettings) | Configuration parameters for ClickHouse Keeper server. | false | | | `clusterDomain` | string | ClusterDomain is the Kubernetes cluster domain suffix used for DNS resolution. | false | cluster.local | | `upgradeChannel` | string | UpgradeChannel specifies the release channel for major version upgrade checks.
When empty, only minor updates will be proposed. Allowed values are: stable, lts or specific major.minor version (e.g. 25.8). | false | | -| `versionProbeTemplate` | [VersionProbeTemplate](#versionprobetemplate) | VersionProbeTemplate overrides for the version detection Job. | false | | +| `versionProbeTemplate` | [VersionProbeTemplate](#versionprobetemplate) | VersionProbeTemplate overrides for the version detection Job.
Deprecated: Keeper version probe Jobs are not used; this field is retained for backward compatibility. | false | | Appears in: - [KeeperCluster](#keepercluster) @@ -292,8 +292,8 @@ KeeperClusterStatus defines the observed state of KeeperCluster. | `currentRevision` | string | CurrentRevision indicates latest applied KeeperCluster spec revision. | true | | | `updateRevision` | string | UpdateRevision indicates latest requested KeeperCluster spec revision. | true | | | `observedGeneration` | integer | ObservedGeneration indicates latest generation observed by controller. | true | | -| `version` | string | Version indicates the version reported by the container image. | false | | -| `versionProbeRevision` | string | VersionProbeRevision is the image hash of the last successful version probe.
When this matches the current image hash, the cached Version is used directly. | false | | +| `version` | string | Version indicates the version reported by the Keeper server. | false | | +| `versionProbeRevision` | string | VersionProbeRevision is the image hash of the last successful version probe.
Deprecated: Keeper version probe Jobs are not used; this field is retained for backward compatibility. | false | | Appears in: - [KeeperCluster](#keepercluster) diff --git a/internal/controller/clickhouse/sync.go b/internal/controller/clickhouse/sync.go index 10ec9a3a..50378357 100644 --- a/internal/controller/clickhouse/sync.go +++ b/internal/controller/clickhouse/sync.go @@ -401,7 +401,6 @@ func (r *clickhouseReconciler) reconcileExternalSecret(ctx context.Context, log func (r *clickhouseReconciler) reconcileVersionProbe(ctx context.Context, log ctrlutil.Logger) (chctrl.StepResult, error) { probeResult, err := r.VersionProbe(ctx, log, chctrl.VersionProbeConfig{ - Binary: "clickhouse-server", Labels: r.Cluster.Spec.Labels, Annotations: r.Cluster.Spec.Annotations, PodTemplate: r.Cluster.Spec.PodTemplate, diff --git a/internal/controller/keeper/controller.go b/internal/controller/keeper/controller.go index ce1f3850..5d52ef1d 100644 --- a/internal/controller/keeper/controller.go +++ b/internal/controller/keeper/controller.go @@ -5,7 +5,6 @@ import ( "fmt" appsv1 "k8s.io/api/apps/v1" - batchv1 "k8s.io/api/batch/v1" corev1 "k8s.io/api/core/v1" policyv1 "k8s.io/api/policy/v1" "k8s.io/apimachinery/pkg/api/errors" @@ -47,7 +46,6 @@ type ClusterController struct { // +kubebuilder:rbac:groups=apps,resources=statefulsets/status,verbs=get // +kubebuilder:rbac:groups=policy,resources=poddisruptionbudgets,verbs=get;list;watch;create;update;delete // +kubebuilder:rbac:groups=events.k8s.io,resources=events,verbs=create;patch -// +kubebuilder:rbac:groups=batch,resources=jobs,verbs=get;list;watch;create;delete // Reconcile is part of the main kubernetes reconciliation loop which aims to // move the current state of the cluster closer to the desired state. @@ -156,8 +154,7 @@ func SetupWithManager(mgr ctrl.Manager, log controllerutil.Logger, checker *upgr Owns(&appsv1.StatefulSet{}). Owns(&corev1.ConfigMap{}). Owns(&corev1.Service{}). - Owns(&corev1.Pod{}). - Owns(&batchv1.Job{}) + Owns(&corev1.Pod{}) if enablePDB { controllerBuilder = controllerBuilder.Owns(&policyv1.PodDisruptionBudget{}) diff --git a/internal/controller/keeper/controller_test.go b/internal/controller/keeper/controller_test.go index 58dfe334..a4e83be6 100644 --- a/internal/controller/keeper/controller_test.go +++ b/internal/controller/keeper/controller_test.go @@ -12,13 +12,13 @@ import ( batchv1 "k8s.io/api/batch/v1" corev1 "k8s.io/api/core/v1" policyv1 "k8s.io/api/policy/v1" + "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" "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" v1 "github.com/ClickHouse/clickhouse-operator/api/v1alpha1" "github.com/ClickHouse/clickhouse-operator/internal/controller/testutil" @@ -41,7 +41,6 @@ var _ = When("reconciling standalone KeeperCluster resource", Ordered, func() { pdbs policyv1.PodDisruptionBudgetList configs corev1.ConfigMapList statefulsets appsv1.StatefulSetList - jobs batchv1.JobList cr = &v1.KeeperCluster{ ObjectMeta: metav1.ObjectMeta{ Name: "standalone", @@ -115,9 +114,10 @@ var _ = When("reconciling standalone KeeperCluster resource", Ordered, func() { Expect(suite.Client.List(ctx, &statefulsets, listOpts)).To(Succeed()) Expect(statefulsets.Items).To(HaveLen(1)) + var jobs batchv1.JobList Expect(suite.Client.List(ctx, &jobs, listOpts)).To(Succeed()) - Expect(jobs.Items).To(HaveLen(1)) - Expect(jobs.Items[0].Labels[controllerutil.LabelRoleKey]).To(Equal(controllerutil.LabelVersionProbe)) + Expect(jobs.Items).To(BeEmpty()) + Expect(meta.FindStatusCondition(cr.Status.Conditions, v1.ConditionTypeVersionUpgraded)).To(BeNil()) testutil.AssertEvents(recorder.Events, map[string]int{ "ReplicaCreated": 1, @@ -125,65 +125,6 @@ var _ = When("reconciling standalone KeeperCluster resource", Ordered, func() { }) }) - It("should propagate version probe overrides to the job", func(ctx context.Context) { - By("updating the CR with version probe overrides") - - updatedCR := cr.DeepCopy() - Expect(suite.Client.Get(ctx, cr.NamespacedName(), updatedCR)).To(Succeed()) - updatedCR.Spec.VersionProbeTemplate = &v1.VersionProbeTemplate{ - Spec: v1.VersionProbeJobSpec{ - Template: v1.VersionProbePodTemplate{ - Metadata: v1.TemplateMeta{ - Annotations: map[string]string{ - "sidecar.istio.io/inject": "false", - }, - Labels: map[string]string{ - "probe-label": "probe-value", - }, - }, - }, - }, - } - updatedCR.Spec.PodTemplate.Tolerations = []corev1.Toleration{ - {Key: "workload", Operator: corev1.TolerationOpEqual, Value: "system", Effect: corev1.TaintEffectNoSchedule}, - } - Expect(suite.Client.Update(ctx, updatedCR)).To(Succeed()) - - // Delete old job so new one is created with overrides. - for _, j := range jobs.Items { - Expect(suite.Client.Delete(ctx, &j, client.PropagationPolicy(metav1.DeletePropagationBackground))).To(Succeed()) - } - - _, err := controller.Reconcile(ctx, ctrl.Request{NamespacedName: cr.NamespacedName()}) - Expect(err).NotTo(HaveOccurred()) - Expect(suite.Client.Get(ctx, cr.NamespacedName(), updatedCR)).To(Succeed()) - - listOpts := controllerutil.AppRequirements(cr.Namespace, cr.SpecificName()) - Expect(suite.Client.List(ctx, &jobs, listOpts)).To(Succeed()) - Expect(jobs.Items).To(HaveLen(1)) - - By("verifying annotations on Pod template only") - Expect(jobs.Items[0].Spec.Template.Annotations).To(HaveKeyWithValue("sidecar.istio.io/inject", "false")) - - By("verifying probe-specific labels on Pod template only") - Expect(jobs.Items[0].Spec.Template.Labels).To(HaveKeyWithValue("probe-label", "probe-value")) - - By("verifying operator-reserved labels are preserved") - Expect(jobs.Items[0].Labels[controllerutil.LabelRoleKey]).To(Equal(controllerutil.LabelVersionProbe)) - Expect(jobs.Items[0].Labels[controllerutil.LabelAppKey]).To(Equal(cr.SpecificName())) - - By("verifying scheduling fields inherited from PodTemplate") - Expect(jobs.Items[0].Spec.Template.Spec.Tolerations).To(ContainElement(corev1.Toleration{ - Key: "workload", Operator: corev1.TolerationOpEqual, Value: "system", Effect: corev1.TaintEffectNoSchedule, - })) - - testutil.AssertEvents(recorder.Events, map[string]int{ - "HorizontalScaleBlocked": 1, - }) - - cr = updatedCR.DeepCopy() - }) - It("should propagate meta attributes for every resource", func() { expectedOwnerRef := metav1.OwnerReference{ Kind: "KeeperCluster", @@ -266,6 +207,10 @@ var _ = When("reconciling standalone KeeperCluster resource", Ordered, func() { Expect(updatedCR.Status.UpdateRevision).NotTo(Equal(updatedCR.Status.CurrentRevision)) Expect(updatedCR.Status.ConfigurationRevision).NotTo(Equal(cr.Status.ConfigurationRevision)) Expect(updatedCR.Status.StatefulSetRevision).NotTo(Equal(cr.Status.StatefulSetRevision)) + + testutil.AssertEvents(recorder.Events, map[string]int{ + "HorizontalScaleBlocked": 1, + }) }) It("should add extra config in configmap", func(ctx context.Context) { diff --git a/internal/controller/keeper/sync.go b/internal/controller/keeper/sync.go index 151ac686..48e52c72 100644 --- a/internal/controller/keeper/sync.go +++ b/internal/controller/keeper/sync.go @@ -86,8 +86,7 @@ type keeperReconciler struct { Cluster *v1.KeeperCluster ReplicaState map[v1.KeeperReplicaID]replicaState - versionProbe chctrl.VersionProbeResult - revs chctrl.RevisionState + revs chctrl.RevisionState // Computed by reconcileActiveReplicaStatus HorizontalScaleAllowed bool } @@ -155,7 +154,7 @@ func (r *keeperReconciler) sync(ctx context.Context, log ctrlutil.Logger) (ctrl. return result, nil } -func (r *keeperReconciler) reconcileClusterRevisions(ctx context.Context, log ctrlutil.Logger) (chctrl.StepResult, error) { +func (r *keeperReconciler) reconcileClusterRevisions(_ context.Context, log ctrlutil.Logger) (chctrl.StepResult, error) { if r.Cluster.Status.ObservedGeneration != r.Cluster.Generation { r.Cluster.Status.ObservedGeneration = r.Cluster.Generation log.Debug(fmt.Sprintf("observed new CR generation %d", r.Cluster.Generation)) @@ -203,33 +202,6 @@ func (r *keeperReconciler) reconcileClusterRevisions(ctx context.Context, log ct } } - probeResult, err := r.VersionProbe(ctx, log, chctrl.VersionProbeConfig{ - Binary: "clickhouse-keeper", - Labels: r.Cluster.Spec.Labels, - Annotations: r.Cluster.Spec.Annotations, - PodTemplate: r.Cluster.Spec.PodTemplate, - ContainerTemplate: r.Cluster.Spec.ContainerTemplate, - VersionProbe: r.Cluster.Spec.VersionProbeTemplate, - CachedVersion: r.Cluster.Status.Version, - CachedRevision: r.Cluster.Status.VersionProbeRevision, - }) - if err != nil { - return chctrl.StepResult{}, fmt.Errorf("run version probe: %w", err) - } - - r.versionProbe = probeResult - if probeResult.Completed() { - r.Cluster.Status.Version = probeResult.Version - r.Cluster.Status.VersionProbeRevision = probeResult.Revision - } - - if r.Checker != nil { - cond, event := chctrl.GetUpgradeCondition(*r.Checker, r.versionProbe, r.Cluster.Spec.UpgradeChannel) - r.SetCondition(cond, event...) - } else { - meta.RemoveStatusCondition(r.Cluster.GetStatus().GetConditions(), v1.ConditionTypeVersionUpgraded) - } - return chctrl.StepContinue(), nil } @@ -615,16 +587,15 @@ func (r *keeperReconciler) evaluateReplicaConditions() { notUpdatedIDs = append(notUpdatedIDs, idStr) } - replicaVersions[idStr] = replica.Status.Version + if replica.Status.Version != "" { + replicaVersions[idStr] = replica.Status.Version + } } r.SetCondition(chctrl.ReplicaStartupCondition(errorIDs)) r.SetCondition(chctrl.HealthyCondition(notReadyIDs)) r.SetCondition(chctrl.ConfigSyncCondition(nil, notUpdatedIDs, nil)) - { - cond, event := chctrl.GetVersionSyncCondition(r.versionProbe, replicaVersions, len(notUpdatedIDs) > 0) - r.SetCondition(cond, event...) - } + r.evaluateVersionConditions(len(notUpdatedIDs) > 0) // Ready condition — keeper-specific logic. exists := len(r.ReplicaState) @@ -703,6 +674,40 @@ func (r *keeperReconciler) evaluateReplicaConditions() { ) } +func (r *keeperReconciler) evaluateVersionConditions(isUpdating bool) { + versionByReplica := map[string]string{} + countByVersion := map[string]int{} + commonVersion := "" + + for i, s := range r.ReplicaState { + if s.Status.Version != "" { + versionByReplica[strconv.FormatInt(int64(i), 10)] = s.Status.Version + countByVersion[s.Status.Version]++ + commonVersion = s.Status.Version + } + } + + // Record the version only when all observed replicas agree, otherwise keep the last known one. + if len(countByVersion) == 1 { + r.Cluster.Status.Version = commonVersion + } + + probe := chctrl.VersionProbeResult{ + Version: r.Cluster.Status.Version, + Pending: len(versionByReplica) == 0, + } + + cond, event := chctrl.GetVersionSyncCondition(probe, versionByReplica, isUpdating) + r.SetCondition(cond, event...) + + if r.Checker != nil { + cond, event = chctrl.GetUpgradeCondition(*r.Checker, probe, r.Cluster.Spec.UpgradeChannel) + r.SetCondition(cond, event...) + } else { + meta.RemoveStatusCondition(r.Cluster.GetStatus().GetConditions(), v1.ConditionTypeVersionUpgraded) + } +} + func (r *keeperReconciler) updateReplica(ctx context.Context, log ctrlutil.Logger, replicaID v1.KeeperReplicaID) (*ctrl.Result, error) { log = log.With("replica_id", replicaID) log.Info("updating replica") diff --git a/internal/controller/keeper/sync_test.go b/internal/controller/keeper/sync_test.go index 773aaaf0..d5f64f27 100644 --- a/internal/controller/keeper/sync_test.go +++ b/internal/controller/keeper/sync_test.go @@ -5,12 +5,14 @@ import ( "errors" "net" "reflect" + "time" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" k8serrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" @@ -23,6 +25,7 @@ import ( v1 "github.com/ClickHouse/clickhouse-operator/api/v1alpha1" "github.com/ClickHouse/clickhouse-operator/internal/controller" util "github.com/ClickHouse/clickhouse-operator/internal/controllerutil" + "github.com/ClickHouse/clickhouse-operator/internal/upgrade" ) var _ = Describe("UpdateReplica", Ordered, func() { @@ -175,6 +178,99 @@ var _ = Describe("UpdateReplica", Ordered, func() { }) }) +var _ = Describe("Keeper version status", func() { + It("should report matching live replica versions", func() { + _, rec, cancelEvents := setupReconciler() + defer cancelEvents() + + rec.ReplicaState = map[v1.KeeperReplicaID]replicaState{ + 0: {Status: serverStatus{Version: "25.8.2.1"}}, + 1: {Status: serverStatus{Version: "25.8.2.1"}}, + } + + rec.evaluateReplicaConditions() + + Expect(rec.Cluster.Status.Version).To(Equal("25.8.2.1")) + cond := meta.FindStatusCondition(rec.Cluster.Status.Conditions, v1.ConditionTypeVersionInSync) + Expect(cond).NotTo(BeNil()) + Expect(cond.Status).To(Equal(metav1.ConditionTrue)) + Expect(cond.Reason).To(Equal(v1.ConditionReasonVersionMatch)) + Expect(meta.FindStatusCondition(rec.Cluster.Status.Conditions, v1.ConditionTypeVersionUpgraded)).To(BeNil()) + }) + + It("should keep last known version when no live replica version is observed", func() { + _, rec, cancelEvents := setupReconciler() + defer cancelEvents() + + rec.Cluster.Status.Version = "25.8.2.1" + rec.ReplicaState = map[v1.KeeperReplicaID]replicaState{ + 0: {Status: serverStatus{}}, + } + + rec.evaluateReplicaConditions() + + Expect(rec.Cluster.Status.Version).To(Equal("25.8.2.1")) + cond := meta.FindStatusCondition(rec.Cluster.Status.Conditions, v1.ConditionTypeVersionInSync) + Expect(cond).NotTo(BeNil()) + Expect(cond.Status).To(Equal(metav1.ConditionUnknown)) + Expect(cond.Reason).To(Equal(v1.ConditionReasonVersionPending)) + }) + + It("should keep last known aggregate version when live replica versions differ", func() { + _, rec, cancelEvents := setupReconciler() + defer cancelEvents() + + rec.Cluster.Status.Version = "25.8.2.1" + rec.ReplicaState = map[v1.KeeperReplicaID]replicaState{ + 0: {Status: serverStatus{Version: "25.8.2.1"}}, + 1: {Status: serverStatus{Version: "25.8.3.1"}}, + } + + rec.evaluateReplicaConditions() + + Expect(rec.Cluster.Status.Version).To(Equal("25.8.2.1")) + cond := meta.FindStatusCondition(rec.Cluster.Status.Conditions, v1.ConditionTypeVersionInSync) + Expect(cond).NotTo(BeNil()) + Expect(cond.Status).To(Equal(metav1.ConditionFalse)) + Expect(cond.Reason).To(Equal(v1.ConditionReasonVersionMismatch)) + }) + + It("should use the agreed live version for upgrade checks", func(ctx context.Context) { + log, rec, cancelEvents := setupReconciler() + defer cancelEvents() + + updater := upgrade.NewReleaseUpdater(&upgrade.StaticFetcher{Releases: map[string][]upgrade.ClickHouseVersion{ + upgrade.ChannelStable: { + {Major: 25, Minor: 8, Patch: 2, Build: 1}, + {Major: 25, Minor: 8, Patch: 3, Build: 1}, + }, + }}, time.Hour, log) + rec.Checker = upgrade.NewChecker(updater) + rec.ReplicaState = map[v1.KeeperReplicaID]replicaState{ + 0: {Status: serverStatus{Version: "25.8.2.1"}}, + 1: {Status: serverStatus{Version: "25.8.2.1"}}, + } + + updCtx, cancel := context.WithCancel(ctx) + defer cancel() + + go func() { + defer GinkgoRecover() + + Expect(updater.Start(updCtx)).To(Succeed()) + }() + + Eventually(updater.GetReleasesData).ShouldNot(BeNil()) + + rec.evaluateReplicaConditions() + + cond := meta.FindStatusCondition(rec.Cluster.Status.Conditions, v1.ConditionTypeVersionUpgraded) + Expect(cond).NotTo(BeNil()) + Expect(cond.Status).To(Equal(metav1.ConditionFalse)) + Expect(cond.Reason).To(Equal(v1.ConditionReasonMinorUpdateAvailable)) + }) +}) + func mustGet[T client.Object](ctx context.Context, c client.Client, key types.NamespacedName) T { var result T diff --git a/internal/controller/versionprobe.go b/internal/controller/versionprobe.go index 1aac3b62..9cdd23e7 100644 --- a/internal/controller/versionprobe.go +++ b/internal/controller/versionprobe.go @@ -26,12 +26,12 @@ const ( DefaultProbeCPURequest = "250m" DefaultProbeMemoryLimit = "256Mi" DefaultProbeMemoryRequest = "256Mi" + versionProbeBinary = "/usr/bin/clickhouse" + versionProbeQuery = "INSERT INTO FUNCTION file('/dev/termination-log', 'RawBLOB', 'version String') SELECT version()" ) // VersionProbeConfig holds parameters for the version probe Job. type VersionProbeConfig struct { - // Name of the binary to run. - Binary string // Labels to apply to the Job, inherited from the cluster spec. Labels map[string]string // Annotations to apply to the Job, inherited from the cluster spec. @@ -280,7 +280,8 @@ func (rm *ResourceManager) buildVersionProbeJob(cfg VersionProbeConfig, revision SecurityContext: DefaultContainerSecurityContext(), TerminationMessagePolicy: corev1.TerminationMessageReadFile, TerminationMessagePath: corev1.TerminationMessagePathDefault, - Command: []string{"sh", "-c", fmt.Sprintf("%s --version > %s 2>&1", cfg.Binary, corev1.TerminationMessagePathDefault)}, + Command: []string{versionProbeBinary}, + Args: []string{"local", "--query", versionProbeQuery}, Resources: corev1.ResourceRequirements{ Requests: corev1.ResourceList{ corev1.ResourceCPU: resource.MustParse(DefaultProbeCPURequest), diff --git a/internal/controller/versionprobe_test.go b/internal/controller/versionprobe_test.go index 2c6e73f4..081dccc6 100644 --- a/internal/controller/versionprobe_test.go +++ b/internal/controller/versionprobe_test.go @@ -54,7 +54,8 @@ func baseJob() batchv1.Job { { Name: v1.VersionProbeContainerName, Image: "clickhouse/clickhouse-server:latest", - Command: []string{"sh", "-c", "clickhouse-server --version"}, + Command: []string{versionProbeBinary}, + Args: []string{"local", "--query", versionProbeQuery}, SecurityContext: &corev1.SecurityContext{ RunAsNonRoot: new(true), }, @@ -189,7 +190,8 @@ var _ = Describe("patchResource with jobSchema (version probe overrides)", func( By("verifying container command is preserved") Expect(container.Image).To(Equal("clickhouse/clickhouse-server:latest")) - Expect(container.Command).To(Equal([]string{"sh", "-c", "clickhouse-server --version"})) + Expect(container.Command).To(Equal([]string{versionProbeBinary})) + Expect(container.Args).To(Equal([]string{"local", "--query", versionProbeQuery})) }) It("should deep-merge securityContext via SMP", func() { @@ -311,7 +313,6 @@ func (f *fakeController) GetRecorder() events.EventRecorder { return f.recorder // probeCfg returns a VersionProbeConfig with the given image and cache values. func probeCfg(image, cachedVersion, cachedRevision string) VersionProbeConfig { return VersionProbeConfig{ - Binary: "clickhouse-server", ContainerTemplate: v1.ContainerTemplateSpec{ Image: v1.ContainerImage{Repository: image, Tag: "latest"}, },