Conversation
Brings up minikube, installs the chart from the checkout and exercises a real RedisFailover with sentinel.enabled: false - master election, replication, an operator-driven failover, and the master Service endpoints afterwards. Logs are collected on failure. Operator-managed mode is the default since v4.0.0 and the recent bugs in it (Saremox#161, Saremox#167) were both found from a live cluster's logs rather than from CI. Saremox#165 covers the rollout at the integration level; this covers a running cluster. Triggers on every pull request rather than a single base branch, so a change to the workflow can be tested on its own pull request. Takes about 9 minutes.
…inalizer (Saremox#174) * Fix cluster_ok metric leak by handling RedisFailover deletion via a finalizer kooper v2's generic controller never calls Handle() on resource deletion: by the time its DeleteFunc fires, the object is already gone from the informer's local indexer, and the processor that resolves a dequeued key back to an object just no-ops when the key no longer resolves (see newIndexerProcessor in kooper/v2/controller/processor.go). Because of this, metrics.DeleteCluster - which correctly removes a RedisFailover's cluster_ok Prometheus gauge series - had zero callers, so cluster_ok leaked a stale series forever after every RedisFailover deletion. Add a finalizer (redisfailovers.databases.spotahome.com/finalizer) so a delete becomes an ordinary object update that Handle() does receive: the API server holds the object (DeletionTimestamp set, finalizer still present) until we remove it. Handle() now: - registers the finalizer on any RedisFailover that doesn't have it yet, before validation, so even one that never becomes valid still gets it - when DeletionTimestamp is set and the finalizer is present, calls mClient.DeleteCluster and removes just the finalizer from the list - when DeletionTimestamp is set but the finalizer is already gone, no-ops (cleanup already ran on a previous reconcile) Adds RedisFailover.PatchRedisFailoverFinalizers (service/k8s) doing a JSON merge patch against metadata.finalizers, following the same "always send the full desired value" pattern UpdateRedisFailoverStatus already uses for the status subresource. Mocks for both k8s.RedisFailover and k8s.Services are hand-patched to add the new method: the pinned mockery v2.20.0 panics on modern Go's export data, and regenerating with a newer mockery risked unrelated style-wide diff noise, so the new method was added by hand matching the existing generated style exactly. This is purely additive (new interface method, new object field usage) so it should not conflict with downstream forks that share this code's ancestry (e.g. buildio/redis-operator). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01G4mmyo3sqLwf23m5FE2nBE * Cover PatchRedisFailoverFinalizers and Handle's finalizer error paths Codecov flagged this PR's patch coverage at 40%: PatchRedisFailoverFinalizers (service/k8s/redisfailover.go) had zero test coverage, and Handle's two new error-propagation branches (finalizer-add failure, deletion-cleanup finalizer- removal failure) were untested. - TestRedisFailoverServicePatchRedisFailoverFinalizers (service/k8s): success, nil-finalizers-becomes-empty-array, and not-found-returns-error, mirroring the existing UpdateRedisFailoverStatus test's fake-clientset style. - TestHandleFinalizerRegistrationErrorPropagates: a failed finalizer-add patch stops Handle before Validate/Ensure/CheckAndHeal. - TestHandleDeletionFinalizerRemovalErrorPropagates: a failed finalizer-removal patch during deletion cleanup is still propagated, after DeleteCluster has already fired. Handle and PatchRedisFailoverFinalizers are now both at 100% coverage (verified with go tool cover -func). go build/vet/test and gofmt all pass. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01G4mmyo3sqLwf23m5FE2nBE --------- Co-authored-by: Claude <noreply@anthropic.com>
…emox#177) * Run the two integration tests in parallel to cut CI wall-clock time TestRedisFailover (creation_test.go, sentinel-managed) and TestRedisFailoverOperatorManagedModeRollout (operator-managed) ran back to back in the same go test process - 206.66s + 205.84s = ~412s combined, per a real CI run's -v output (Saremox#174, job 35540389923/106157089205). They don't share any state that would make that unsafe: - separate namespaces (rf-integration-tests vs rf-integration-tests-operator-managed) - separate in-process operator instances, each with its own leader-election lease scoped to its own namespace (confirmed in the same CI run's logs - no lease contention between them) - separate Secrets, separate k8s clientset instances (client-go clientsets are safe for concurrent use) - both use metrics.Dummy/log.Dummy, so no shared Prometheus registry Adding t.Parallel() as the first statement in both lets Go's test runner execute them concurrently instead of sequentially, which should cut roughly half of that ~412s off the Integration test job's wall time (the long pole of the whole CI run - every other job finishes in under 3.5 minutes). The same redis:7.2.12-alpine image backs both tests' pods, so running them concurrently doesn't double the image-pull cost either: kubelet dedupes concurrent pulls of the same image on a node. This can't be verified against a real cluster in this environment (no k8s available), so validation here is build/vet only: `go build/vet ./...` and `go build/vet -tags integration ./test/...` all pass. The real before/after timing comparison happens in this PR's own CI run. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01G4mmyo3sqLwf23m5FE2nBE * ci: fix e2e pod-readiness gate and shave wall time off the CI run - e2e.yml: kubectl rollout status only supports RollingUpdate, but the Redis StatefulSet uses OnDelete, so the "pods ready" gate errored out on every run and silently burned all 60 retries under bash's &&-list errexit exemption. Switch to kubectl wait on .status.readyReplicas, which the StatefulSet controller populates regardless of strategy. - ci.yaml: drop integration-test's needs: [check, unit-test] gate - the matrix doesn't depend on either job's output, so serializing them was pure wall time. - ci.yaml: pre-pull the redis image in the background right after checkout, only waiting on it just before the Go tests start, so the pull is hidden behind conntrack/minikube/CRD setup instead of paying for it inline while the tests poll for pods to become ready. - operator_managed_rollout_test.go: reduce ommRedisSize to 2, matching creation_test.go's already-lighter footprint and cutting one pod's worth of startup/rollout time from the heaviest subtest. - creation_test.go: fix a leaked operator goroutine in TestRedisFailover - Run() was called with context.Background() and the stopC channel used for "cleanup" was never wired to it, so closing stopC did nothing and the controller (plus its informers/leader-election) kept running for the rest of the test binary's life. Use a cancelable context instead. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01G4mmyo3sqLwf23m5FE2nBE --------- Co-authored-by: Claude <noreply@anthropic.com>
…aremox#178) The two integration tests construct their in-process operator with a zero-value redisfailover.Config{}, so Config.SyncInterval is 0. That flows into kooper's ResyncInterval, which falls back to *kooper's own* default of 3 minutes when unset - unlike production, which always sets one explicitly via -sync-interval (default 30, cmd/utils/flags.go). That 3-minute fallback is reachable in practice: CheckAndHeal's deferred status patch is sometimes byte-identical to what's already stored (a steady-state reconcile with no health-state transition), and the apiserver/etcd treat a truly no-op write as a no-op - no new resourceVersion, no watch event. Since the operator only watches the RedisFailover CR itself (not Pods/StatefulSets), that patch was the only thing re-triggering the next Handle() call. TestRedisFailoverOperatorManagedModeRollout needs two separate Handle() calls to replace both pods (slave, then master), so a missed self-trigger between them stalls for the full resync interval - matching the ~300-313s outliers seen in CI (~180s stall + ~130s real work), always in that one subtest and nowhere else, since it's the only test that depends on a second, chained reconcile. Giving both operators a short explicit SyncInterval keeps that same stall - if it happens at all - under a couple of seconds instead of 3 minutes, which should make the rollout subtest's timing consistent across runs. This masks the symptom in tests, matching what production already does via its own default; it doesn't fix the underlying gap (the controller's self-trigger depends on incidental status content changes rather than being guaranteed). That's tracked separately. Claude-Session: https://claude.ai/code/session_01G4mmyo3sqLwf23m5FE2nBE Co-authored-by: Claude <noreply@anthropic.com>
Bumps [actions/checkout](https://github.com/actions/checkout) from 6 to 7. - [Release notes](https://github.com/actions/checkout/releases) - [Changelog](https://github.com/actions/checkout/blob/main/CHANGELOG.md) - [Commits](actions/checkout@v6...v7) --- updated-dependencies: - dependency-name: actions/checkout dependency-version: '7' dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
…in permissions (Saremox#184) Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
…ce (Saremox#182) A Service only routes new connections, so clients connected through the master Service stay on a pod after it is demoted to a replica and keep getting READONLY errors. When a pod's role label changes from master to slave, the operator now waits (in the background, up to 10s) for the pod's IP to leave the master Service's EndpointSlices, then 2s for kube-proxy, and closes the pod's normal and pub/sub connections (CLIENT KILL TYPE normal|pubsub) so clients reconnect to the new master. Replication links are left alone. This covers Sentinel-driven demotions too, which the operator only relabels. - On by default; --disconnect-clients-on-demotion=false turns it off. - Needs get/list on discovery.k8s.io endpointslices (chart, kustomize and examples updated). Without it, or on timeout, it disconnects anyway. - e2e: a client writing through the master Service must end up on the new master after master and replica are swapped, in both directions. Claude-Session: https://claude.ai/code/session_01G4mmyo3sqLwf23m5FE2nBE Co-authored-by: Claude <noreply@anthropic.com>
…x#187) A plain `kind create cluster` fails in the sandbox, and a working cluster can't pull images. The skill's scripts: - patch the kind config for the sandbox's cgroup v1 kernel (failCgroupV1: false, containerd restrict_oom_score_adj); - mirror docker.io and quay.io through a local registry, since the node can't reach any registry and the operator defaults to pullPolicy Always; - route the pod subnet to the host for the integration tests; - build the operator image without docker/app/Dockerfile, whose apk step has no network here. .gitignore now lets .claude/skills be committed. Claude-Session: https://claude.ai/code/session_01G4mmyo3sqLwf23m5FE2nBE Co-authored-by: Claude <noreply@anthropic.com>
build-image.sh let docker build pull alpine straight from Docker Hub, which fails when Docker Hub rate limits (429). registry.sh already fell back to Docker Hub's copy and mirror.gcr.io for the images it pushes. Move that fallback into a `registry.sh pull` subcommand and use it for alpine, the registry:2 image and the kind node image. Claude-Session: https://claude.ai/code/session_01G4mmyo3sqLwf23m5FE2nBE Co-authored-by: Claude <noreply@anthropic.com>
…#186) With client-go 1.35+ the informer streams its initial list and waits for the initial-events-end bookmark. The RedisFailover watch filter dropped it, since it has no namespace, so with a restrictive --supported-namespaces-regex the operator never finished syncing and never reconciled. It also dropped watch errors such as 410 Gone. Let bookmark and error events through, and run the integration tests with a regex that only matches their namespace. Claude-Session: https://claude.ai/code/session_01G4mmyo3sqLwf23m5FE2nBE Co-authored-by: Claude <noreply@anthropic.com>
A rollout replaces one pod per reconcile, and each next step waited for the resync because nothing queued the RedisFailover again. Replace kooper's controller with one that has a single queue keyed by RedisFailover, fed by RedisFailover events and by events on the pods of handled RedisFailovers, so pod changes drive the next step. The pod cache keeps metadata only, and a pod watch that can't sync doesn't block reconciling. One queued event is counted per update unless a pod's owner changed, as with kooper. Because the next reconcile now follows a delete immediately: - replace another pod only once the last replacement has settled (all pods present, none terminating, updated pods ready), and log at Info why a rollout waits; - in operator-managed mode, wait for a terminating master that is still ready to exit before electing a new one. A stopping replica doesn't block the election; - don't dial terminating pods in CheckAllSlavesFromMaster; their IP is often gone and every rollout step would stall for the dial timeout. Claude-Session: https://claude.ai/code/session_01G4mmyo3sqLwf23m5FE2nBE Co-authored-by: Claude <noreply@anthropic.com>
Port what kooper still provided onto client-go and prometheus: - the controller metrics, with the same kooper_controller_* names, labels, help texts and buckets; - leader election, with the same lease, timings and identity, and no Events written. Leader election and shutdown: - On SIGTERM the controller stops taking queued reconciles and main waits up to 5s, kooper's grace period, for running ones. The lease is then released so another replica takes over at once; if they don't finish in time, the process exits without releasing it, as before. Stopping before the cache has synced is a clean stop. - When the lease is lost, Run returns errLeadershipLost at once, without waiting for a running reconcile, and the process exits. Kooper exited 5s after the loss. - A lost lease is never released, a lease another replica holds is left alone, and a lease acquired while shutting down is released. Claude-Session: https://claude.ai/code/session_01G4mmyo3sqLwf23m5FE2nBE Co-authored-by: Claude <noreply@anthropic.com>
…ng port (Saremox#128) SentinelCheckQuorum: real Sentinel's CKQUORUM NOQUORUM outcome comes back as a RESP error, not a successful string reply - so the intended NOQUORUM handling (parsing "(error)"/"NOQUORUM" out of a successful result) could never execute; callers only ever saw the raw driver error. NOQUORUM is now classified from err.Error() directly, restoring the intended "quorum Not available" message and NOQUORUM metrics tag. MakeSlaveOfWithPort(ip, masterIP, masterPort, password) connected to the *target* redis instance at (ip, masterPort) - using the master's port to reach the target, with no parameter for the target's own port. This only worked because every caller except SetExternalMasterOnAll happens to pass the same port for target and master (both come from this RedisFailover's own spec.redis.port). SetExternalMasterOnAll's externally-supplied bootstrap master port can genuinely differ, in which case the client silently connected to the wrong instance (or, when target and master share an IP, issued SLAVEOF to the master itself) instead of failing loudly. MakeSlaveOfWithPort now takes an explicit port for the target, separate from masterPort; all call sites, the mock, and MakeSlaveOf updated accordingly. Both found and confirmed against real Redis/Sentinel 7.0.15 while writing coverage tests for this file. Co-authored-by: Claude <noreply@anthropic.com>
Saremox#144) * feat(exporter): make the exporter metrics port configurable (#30) The sentinel exporter port was hardcoded to 9355 (and the redis exporter to 9121), so the listen port, container port and metrics service port could not be changed. Add an optional Exporter.port field that overrides the listen/container/service port for both the redis and sentinel exporters, defaulting to the previous values when left at 0. Regenerated CRD manifests for the new field. Refs upstream spotahome#531. Co-authored-by: Binh Nguyen <binh.nguyen@encapital.io> (cherry picked from commit 00c37f2) * Make the redis exporter listen on a custom port and bound exporter ports A custom spec.redis.exporter.port only changed the declared container and Service port; the exporter itself kept listening on 9121. Set REDIS_EXPORTER_WEB_LISTEN_ADDRESS when a custom port is set, like the sentinel exporter does. Reject exporter ports outside 0-65535 in the CRD schema and in Validate(). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01G4mmyo3sqLwf23m5FE2nBE --------- Co-authored-by: Binh Nguyen <37066217+binhnguyenduc@users.noreply.github.com> Co-authored-by: Binh Nguyen <binh.nguyen@encapital.io> Co-authored-by: Claude <noreply@anthropic.com>
…ion (dnse Saremox#35) (Saremox#147) * feat(heal): optionally protect the redis master from autoscaler eviction (Saremox#35) Add an opt-in redis.preventMasterEviction flag. When enabled, the operator annotates the current master pod with cluster-autoscaler.kubernetes.io/safe-to-evict=false and marks slaves true, so the cluster autoscaler will not drain the node running the master and force an avoidable failover. The annotation follows the master as the role moves, in both the checker and healer relabel paths. Adds a Pod.UpdatePodAnnotations service method (JSON merge patch) and the matching mock. Regenerated the CRD for the new field. Refs upstream spotahome#689. Co-authored-by: Binh Nguyen <binh.nguyen@encapital.io> (cherry picked from commit 85bc2f3) * Mark a demoted master evictable only after it is relabelled setSlaveLabel set safe-to-evict=true before changing the role label, so a failed relabel left a pod still labelled master, and still behind the master Service, that the autoscaler could evict. Relabel first and annotate afterwards. Also test UpdatePodAnnotations against the fake clientset. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01G4mmyo3sqLwf23m5FE2nBE * Test that a master is not relabelled when pinning it fails Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01G4mmyo3sqLwf23m5FE2nBE --------- Co-authored-by: Binh Nguyen <37066217+binhnguyenduc@users.noreply.github.com> Co-authored-by: Binh Nguyen <binh.nguyen@encapital.io> Co-authored-by: Claude <noreply@anthropic.com>
… (dnse Saremox#33) (Saremox#146) * feat(sentinel): configurable deployment strategy and PDB minAvailable (Saremox#33) Two rollout/availability knobs that were previously hardcoded: - sentinel.strategy overrides the sentinel Deployment update strategy (e.g. rollingUpdate maxSurge/maxUnavailable), avoiding a deadlocked rolling update when required anti-affinity plus replicas==nodes leaves no room to surge. - redis/sentinel.podDisruptionBudgetMinAvailable overrides the PDB minAvailable (previously fixed at 2, or 1 when replicas<=2). Regenerated CRD + deepcopy for the new fields. Refs upstream spotahome#662, spotahome#516, spotahome#598. Co-authored-by: Binh Nguyen <binh.nguyen@encapital.io> (cherry picked from commit a53177d) * Compare a configured sentinel strategy instead of discarding it deploymentUpToDate blanked the stored strategy before comparing, so a configured sentinel.strategy never matched and every reconcile updated the Deployment, while removing one was never applied. Compare both strategies with the API server's defaults filled in instead. Also fix the sentinel PDB default docs, which derive from the sentinel replicas rather than the redis ones, and cover the sentinel override. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01G4mmyo3sqLwf23m5FE2nBE --------- Co-authored-by: Binh Nguyen <37066217+binhnguyenduc@users.noreply.github.com> Co-authored-by: Binh Nguyen <binh.nguyen@encapital.io> Co-authored-by: Claude <noreply@anthropic.com>
…s (dnse Saremox#34) (Saremox#145) * feat(env): allow custom env vars on redis and sentinel main containers (Saremox#34) Only the exporter sidecar accepted custom environment variables. Add an optional env field to the redis and sentinel specs, injected into their main containers. On the redis container the user's env is placed before the operator-injected vars (REDIS_ADDR/PORT/USER/PASSWORD) so those keep precedence under Kubernetes last-wins semantics. Regenerated CRD + deepcopy for the new fields. Refs upstream spotahome#290. Co-authored-by: Binh Nguyen <binh.nguyen@encapital.io> (cherry picked from commit 4d51d46) * Test that DeepCopy clones the redis and sentinel env vars Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01G4mmyo3sqLwf23m5FE2nBE --------- Co-authored-by: Binh Nguyen <37066217+binhnguyenduc@users.noreply.github.com> Co-authored-by: Binh Nguyen <binh.nguyen@encapital.io> Co-authored-by: Claude <noreply@anthropic.com>
Removed ignore rule for Kubernetes dependencies in Dependabot configuration.
…#190) Add opt-in redis.maxMemory {percent, policy}: maxmemory is min(limit * percent / 100, limit - 32Mi) of the redis container's memory limit, percent defaults to 75 and policy to noeviction. Applied at runtime with CONFIG SET, so enabling it rolls no pods. - Upgrade safety: keys set in customConfig take precedence, so instances are unaffected until they opt in. Without a memory limit >= 64Mi, maxmemory is not managed and the reason is set in the status message instead of stopping the reconcile. replica-ignore-maxmemory no is rejected, and running pods are set to yes as removing it from customConfig does not reset it. - The target follows the smallest pod's limit, capped by the configured one (applied limit from the pod status, else the pod spec): replicas hold the whole dataset and any of them can be promoted. With OnDelete a raised limit applies once every pod runs with it, a lowered one before smaller pods are rolled out. While a pod runs below 64Mi, maxmemory is left alone. - maxmemory is only lowered below the memory in use under allkeys-* (volatile-* could evict every key with a TTL and still not fit), where it stays while Redis evicts down to it. Each lowering is verified on the master and rolled back if it stopped being master or, without eviction, the usage does not fit. A master without maxmemory, e.g. after a restart, whose data does not fit the target is capped at the smallest running container less 32Mi, or at the memory in use. - While a stale pod is about to be replaced with a smaller limit and the master's maxmemory or memory in use does not fit it yet, the rollout is held and the reason is set in the status message. Pods count as stale until the StatefulSet controller has observed the latest spec. - When raising, maxmemory is set before the policy; when lowering, after. Bootstrap mode applies managed maxmemory before replacing pods. Claude-Session: https://claude.ai/code/session_01G4mmyo3sqLwf23m5FE2nBE Co-authored-by: Claude <noreply@anthropic.com>
When a stale redis pod's revision differs from the update revision only in container cpu and memory, resize it through the pods/resize subresource instead of recreating it, then move its revision label to the update revision once the kubelet reports the resources as applied (safe with OnDelete). No restart, no data reload, no master failover; still one pod at a time, replicas first. On by default from Kubernetes 1.33; spec.redis.inPlaceResize: Disabled opts out. - The resize starts from the pod's own resources and sets the update revision's cpu and memory values, so admission defaults (e.g. a LimitRange) are kept and a resize superseded by a newer revision is corrected. Memory decreases are detected against the applied limits. - The pod is recreated as before when the update changes anything else (resource claims included), adds or removes requests or limits, lowers a memory limit before Kubernetes 1.35, or when the API server rejects the resize, e.g. for a QoS class change or without the new RBAC. It is also recreated when the kubelet does not report the container's resources, as one without in-place resize support would leave the pod spec, which maxmemory derives from, ahead of the container. - Kubelet state: infeasible, deferred for more than 5 minutes, failing for more than 5 minutes (e.g. a memory decrease below the usage it sees, which includes the page cache), or not applied within 5 minutes without any condition falls back to recreating the pod. Timeouts count from the latest resize request, kept in a pod annotation. Conditions from an earlier pod generation are ignored. An error detecting the cluster's support is retried rather than recreating the pod. The operator needs patch on pods/resize and get on controllerrevisions, granted in the chart, the kustomize and the example manifests. Claude-Session: https://claude.ai/code/session_01G4mmyo3sqLwf23m5FE2nBE Co-authored-by: Claude <noreply@anthropic.com>
…aremox#193) * fix(failover): don't promote while a ready Redis pod doesn't answer GetNumberMasters skipped a pod it could not query, so a master that was briefly unreachable counted as absent. Zero masters drives promotion, so the operator replaced a running master: in operator-managed mode a 20s CLIENT PAUSE on the master was enough. A ready pod that doesn't answer now makes the count unknown when no master answered, and the reconcile stops. A pod Kubernetes has marked not ready is still skipped, so failover after a node loss or a crashed Redis goes ahead once the pod leaves the master Service. Ported from powerhome#106. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01G4mmyo3sqLwf23m5FE2nBE * fix(redis): shorten the Redis client timeouts Every client used the go-redis defaults: a 3s read timeout, 5s dial timeout and 3 retries. A pod that accepts connections but never answers, like one on a frozen node, held each call for 12s, and a failover after a node loss waited on several of them. Use 2s timeouts and one retry, so such a call gives up after 4s. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01G4mmyo3sqLwf23m5FE2nBE --------- Co-authored-by: Claude <noreply@anthropic.com>
The CRD YAML is 1.09MB, over the 1MiB a ConfigMap holds, so with crds.upgradeHook.enabled every install and upgrade failed. Store it as compact JSON (587KB, descriptions kept) and apply it server-side, as it is also too large for a client-side apply's last-applied annotation. Claude-Session: https://claude.ai/code/session_01G4mmyo3sqLwf23m5FE2nBE Co-authored-by: Claude <noreply@anthropic.com>
…ox#196) fsGroup is a PodSecurityContext field. In the container securityContext the API server dropped it with an unknown-field warning, and strict validation rejects the Deployment. Move it to a new podSecurityContext value rendered on the pod spec. Claude-Session: https://claude.ai/code/session_01G4mmyo3sqLwf23m5FE2nBE Co-authored-by: Claude <noreply@anthropic.com>
* fix(auth): apply a changed Redis password in place Changing the password in the auth secret, or adding or removing auth.secretPath, left the RedisFailover wedged: Redis reads requirepass only at startup, so every pod refused the operator's new password, CheckAndHeal failed on its first check, and the rolling update that would have restarted the pods onto the secret was never reached. Each pass also tried to promote a master and failed only on the password. Restarting the pods one at a time can't apply it either: a restarted replica can't authenticate to a master still on the old password. The operator now remembers, per RedisFailover, the password every running Redis last accepted. When the secret differs, it logs in with that one and runs CONFIG SET masterauth and requirepass on each Redis, then gives the Sentinels the new auth-pass. Replication and existing connections stay up, and the rolling update then restarts the pods onto the secret. If the operator restarted in between it has no old password; the RedisFailover reports it and the pods have to be deleted by hand. The readiness probe authenticates with the password its container started with, so after the change it failed on every pod not yet restarted, the master included. A refused password now counts as ready. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01G4mmyo3sqLwf23m5FE2nBE * fix(auth): keep the password out of errors and converge Sentinel The Sentinel auth-pass went through SetCustomSentinelConfig as the config string "auth-pass <password>", and that function's parse error quotes the string, so the password could reach the reconcile error log (CodeQL go/clear-text-logging). Set it with a dedicated SetSentinelAuthPass instead. Give the Sentinels the password whenever the Redis pods all accept it, not only when the previous password is known. Otherwise an operator restart between changing the Redis pods and the Sentinels left the Sentinels on the old password for good. The step only runs when the password isn't already cached, so this is one SENTINEL SET per operator start. Count a Redis pod that isn't running yet as incomplete, so the password isn't cached while a pod may still start on the old pod template. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01G4mmyo3sqLwf23m5FE2nBE * fix(auth): use the shared client timeouts for the password calls Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01G4mmyo3sqLwf23m5FE2nBE * test(auth): cover the password change's error paths Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01G4mmyo3sqLwf23m5FE2nBE * fix(auth): don't let one pod or Sentinel hold up a password change - Give the Sentinels the new password as soon as every running Redis accepts it, instead of waiting for pods yet to start, and remember it separately so a Pending pod doesn't rewrite their config each sync. - Skip an unreachable Sentinel and retry it, rather than failing the whole reconcile. - With no password remembered after an operator restart, remember the one every running Redis accepts. A Redis without a password is changed without needing the old one. - Point to putting the previous password back, not deleting the pods, when the old password is unknown, and document that the exporter and pre-stop save keep the old password until their pod restarts. - Read the secret once per reconcile. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01G4mmyo3sqLwf23m5FE2nBE --------- Co-authored-by: Claude <noreply@anthropic.com>
…09-29 # Conflicts: # .github/workflows/e2e.yml # README.md # charts/redisoperator/crds/databases.spotahome.com_redisfailovers.yaml # cmd/utils/flags.go # go.mod # manifests/databases.spotahome.com_redisfailovers.yaml # manifests/kustomize/base/databases.spotahome.com_redisfailovers.yaml # operator/redisfailover/checker_test.go # operator/redisfailover/config.go # operator/redisfailover/service/generator_test.go
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
This branch has not been deployed
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
3 participants
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.
Brings the fork level with
Saremox/main— 0 commits behind after this.Why now
Three of these we actively want:
fix(chart): set fsGroup on the operator pod, not its containerhelm upgradeof the operatorDisconnect a demoted master's clients once it leaves the master Servicefix(failover): don't promote while a ready Redis pod doesn't answerAlso Saremox#191 in-place pod resize, Saremox#190 operator-managed maxmemory, Saremox#185 kooper dropped, Saremox#181 reconcile on pod changes, Saremox#195 CRD fits the upgrade-hook ConfigMap.
Conflict resolutions, and the judgement calls
cobra(our instance-manager CLI), droppedkoopersince Drop the kooper dependency Saremox/redis-operator#185 removed its use. Verified nothing imports kooper; only the metric names keep the prefix, deliberately, so dashboards keep working.InstanceManagerImageand theirDisconnectClientsOnDemotion/KeepClientsOnDemotion.instanceManagerImage, theirs addsmaxMemory. Both kept, alphabetical. Not regenerated —make generateneeds a docker build that 404s on mockery locally, so "Verify generated code" in CI is the real check here.redisPodsSettlednow arrives natively and does the same job better, by counting pods againstspec.redis.replicas. This supersedes Adopt upstream's settled check in place of our replica quorum #28, which did the same port by hand.TestUpdateRedisesPodsWaitsForTheLastReplacement, adapting only the master rows, where a stale master is promoted away rather than deleted.TestRedisShutdownConfigMapRetriesthen failed because sentinel is off by default in this fork, so it got the operator-managed script with no retry loops. Enabled sentinel explicitly in that test — a genuine fork adaptation, not a workaround.e2e-probe-behavior,e2e-instance-manager). Upstream has evolved thee2e-sentinel-freejob we contributed, so theirs wins for that one.### Persistenceheading my first pass created.What stays ours
Promote-before-replacing-the-master (#26), proposed upstream as Saremox#201. If that lands,
checker.goconverges entirely.Verification
Full suite green, integration test binary compiles,
go vetclean with and without the integration tag. Three files are flagged by my localgofmt(health.go,health_test.go,types.go) but are already unformatted onmainand Lint passes there, so I left them rather than bundle unrelated churn.Supersedes #28 — close that in favour of this if this lands first.