0.27.3 - #2062
Merged
Merged
Conversation
A single transient apiserver/network error (connection refused/reset, timeout, 5xx) during a CHI reconcile currently propagates up as-is and aborts the entire reconcile, discarding progress on all remaining hosts. On installations with hundreds of hosts a reconcile makes tens of thousands of API calls over a long wall-clock window, so even a sub-second control-plane blip is likely to hit one of them and abort an otherwise healthy rollout, leaving the CR stuck in Aborted state. Wrap all Get calls in the chi kube drivers with a bounded exponential-backoff retry (5 attempts, 500ms..4s doubling, ~7.5s total budget) that retries only transient network/API errors. Terminal errors (NotFound, AlreadyExists, Conflict, Forbidden, Unauthorized, BadRequest, Invalid, MethodNotSupported) and context cancellation surface immediately, and when retries are exhausted the last error is returned unchanged - so callers behave exactly as before for non-transient errors and sustained outages. Signed-off-by: Aaron Harlap <aaron@chronosphere.io>
Update the race-only test to use the current metav1.Object-based Registry API. Signed-off-by: Rohan Thakkar <rohant@twitter.com>
Decode direct objects and DeletedFinalStateUnknown tombstones through one typed delete handler, reporting malformed events instead of panicking. Signed-off-by: Rohan Thakkar <rohant@twitter.com>
… calls Integrates aaron276h's PR #2025 (github.com//pull/2025, fixes #2026): bounded exponential-backoff retry of transient K8s API/network errors on the chi kube driver Get calls, so a sub-second control-plane blip no longer aborts an entire multi-hundred-host reconcile. Follow-up commit refactors the helper into controller/common/kube (shared with CHK), makes the back-off ctx-cancellable, and tightens the net.Error classifier.
- retire dead handleObject stub + messageUnableToDecode const (only callers were dead comments; deletedObject supersedes its tombstone decode branch) - document why deletedObject uses value-type T (typed-nil guard) - rename TestDeletedObject -> Test_deletedObject for sibling consistency
serviceMonitor.enabled now also creates a separate ServiceMonitor that scrapes native ClickHouse Keeper prometheus endpoints from every keeper service managed by the operator (matched by clickhouse-keeper.altinity.com/app=chop label, port name `metrics`). Opt-out via serviceMonitor.keeperMetrics.enabled=false. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Reworks PR #2046 (issue #1704) on three counts found by review: - A deferred host returned ErrCRUDAbort, which the shard[0] early-return turned into 'skip every sibling shard', and also skipped clean(), monitoring and the ancestor update. Deferral now has its own sentinel, is accumulated across shards and clusters, and is surfaced once at the end of the pass. It deliberately still reaches the top as an error: a pass that swallowed it would advance the ancestor past a host that was never rolled and let clean() purge that host. - hostMayRequireDisruption tested !Is(ObjectStatusSame), but at that point host statuses are only Requested/Modified/Found - Same is assigned later, inside ReconcileStatefulSet. Every converged host therefore looked disruptable, so a healthy replica in a degraded shard got no reconcile at all. Replaced by hostDisruptionWouldDegradeShard, evaluated in reconcileHostStatefulSet after PrepareHostStatefulSetWithStatus where the status is authoritative. - runConcurrently was last-writer-wins, so a benign deferral could mask a real shard failure. Deferrals are now tracked apart from errors. shouldExcludeHost keeps a shard-safety check, now for the opposite reason: since the restart is deferred later in the same pass, draining the host here would strip its ready label and drop it from the Service endpoints with nothing left to put it back - emptying the entrypoint Service of a shard whose only other replica is down. Deferral is surfaced via ReconcileAbortWithReason plus a dedicated event reason so 'Aborted' names its cause.
test_010083 covers one shard, so the deferral fix itself was untested: nothing proved that a host deferred for shard safety still lets the other shards update. Adds test_010083_1 - 2 shards x 2 replicas with shard 0 replica 0 parked on a broken image, so shard 0 replica 1 is its shard's last healthy replica. Rolling a new image to everything else must: - converge shard 1 (the fix; a deferral used to abort the pass) - leave shard 0 replica 1 serving (the protection test_010083 already covers) - still report Aborted (the deferral is surfaced, not swallowed) - NOT advance status-normalizedCompleted (else the pending roll is diffed away and lost) - NOT purge the deferred host's STS (else clean() deletes the replica being protected) Verified both ways: fails against the unreworked PR on exactly the sibling convergence assertion, passes with the rework. Known gap: the roll is driven by an image change, which leaves shouldForceRestartHost false, so the exclude/drain path is not exercised here.
…tarving sibling shards Brings in PR #2046 (issue #1704 - an interrupted roll could take down a shard's last healthy replica) together with a rework of it. The deferral now has its own sentinel and is accumulated across shards and clusters, so one deferred host no longer skips every sibling shard. It still reaches the top as an error on purpose: swallowing it would advance the ancestor past a host that was never rolled and let clean() purge that host's StatefulSet. The disruption gate moved into reconcileHostStatefulSet, after the desired StatefulSet is known - the previous predicate tested a status that is only assigned later, so every converged host looked disruptable and a healthy replica in a degraded shard received no reconcile at all. shouldExcludeHost keeps its shard-safety check so a host that is about to be deferred is not drained out of the Service with nothing left in the pass to restore it. Tests: unit coverage for the shard-safety predicate and the sentinel, plus e2e test_010083_1, verified to fail against the unreworked PR on exactly the sibling convergence assertion.
70-chop-config.yaml is described as "a fully-annotated example" by nine generated manifests, but it covers roughly 60% of the options the chopconf CRD declares and has never been the exhaustive reference those comments imply. It also ships: operatorhub.sh loads it into the OLM CSV alm-examples annotation, which is the YAML an OpenShift user gets pre-filled on "Create ClickHouseOperatorConfiguration" - a role that wants a short sample, not a complete catalogue. Adds 99-clickhouseoperatorconfiguration-max.yaml as the counterpart of 99-clickhouseinstallation-max.yaml, covering 81 of the 95 leaves the chopconf CRD declares, and points the docs at it for the full option list while 70-chop-config.yaml stays the short starting point. The 14 uncovered leaves are the reconcile.host.hooks fields, shown commented out: the chopconf CRD omits their `events` and `failurePolicy` sub-fields, so a hook set through the CR is accepted, stored and never fires. The file documents the merge rules up front, since they are easy to get wrong: explicitly-set values pin over future defaults, list-valued settings append rather than replace, and clickhouse.metrics.excludeRegexp is the one exception.
Document Go dependency licenses, add Apache NOTICE, and surface license metadata in README, CONTRIBUTING, and the Helm chart. Co-authored-by: Cursor <cursoragent@cursor.com>
Add third-party notices and clarify license attribution
alex-zaitsev
approved these changes
Aug 12, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
https://github.com/Altinity/clickhouse-operator/releases/tag/release-0.27.3