feat: Operator v2 - #20
Open
as51340 wants to merge 33 commits into
Open
Conversation
as51340
changed the title
Replace the discarded prior attempt (preserved on archive/pre-operator-mvp) with a clean kubebuilder v4 scaffold: - MemgraphCluster API skeleton: group memgraph.com, version v1alpha1, short name mgc; spec fields land in subsequent slices - Hello-world reconciler wired through the manager - Generated CRD manifests (verified to install on a local kind cluster; kubectl get mgc resolves) - CI skeleton: lint, unit, and envtest suites run on every pull request - specs/operator-mvp/ (PRD + issue slices) carried into the new history Implements specs/operator-mvp/issues/01-repo-reset-scaffold.md
…ain auto-switching go test -cover needs the covdata tool; Go 1.26 builds it on demand, and that build fails when the go command auto-switched toolchains (base Go older than go.mod's version), breaking local runs on distro Go installs. Nothing consumes the coverage profiles yet; pass COVER_FLAGS to opt in.
go install pkg@version ignores go.mod, so under GOTOOLCHAIN=auto golangci-lint gets built with its own older minimum toolchain (go1.25) and then refuses to lint a project targeting go 1.26. Export the project's active toolchain version for the install/custom-build recipe.
Applying a minimal MemgraphCluster now provisions one StatefulSet per role (coordinators, data instances), each backed by a headless Service: - API types grow the walking-skeleton spec: coordinator and data-instance counts, image (repository/tag/pullPolicy), and the chart-compatible secrets block. All fields carry CRD schema defaults, mirrored as Go constants so builders behave on specs that never passed admission. - internal/resources holds pure builders (spec in, desired objects out). Coordinator identity (--coordinator-id, advertised --coordinator-hostname FQDN) is derived from the pod ordinal at startup via a uniform shell wrapper, so all pods within a role share one template. Flags, ports, probes, and the uid 101 / gid 103 restricted security context mirror the HA Helm chart. Storage is emptyDir for now; PVC templates land with the storage slice. - The controller server-side-applies builder output with controller owner references (CR deletion cascades to the workloads) and watches the owned StatefulSets and Services. - Builders are covered by golden-style unit tests; the controller by envtest (objects created, spec propagation, schema defaults and validation, idempotent re-reconcile). Slice 02 of specs/operator-mvp (02-provisioning-walking-skeleton.md).
* Bootstrap registration: HA client, planner, controller wiring Make a freshly provisioned cluster become an actual HA cluster without human action (specs/operator-mvp/issues/03-bootstrap-registration.md): - internal/memgraph: narrow HA client interface (show instances, add coordinator, register instance, set main) over the Bolt driver; the only place the driver is referenced, and the mock seam for tests. - internal/planner: pure diff of declared topology against observed SHOW INSTANCES output into ordered registration commands, empty when converged. SET INSTANCE TO MAIN is planned only when no MAIN exists. - internal/resources: DeclaredTopology derives per-pod registration identity (coordinator IDs, instance names, advertised FQDNs) from pod ordinals, sharing the FQDN derivation with the coordinator start script so advertised and registered addresses cannot drift. - internal/controller: once both role StatefulSets report all pods ready, find the coordinator leader (follower views redirect to the leader they report; a fresh cluster bootstraps against the first reachable coordinator), plan, and execute. All interaction is read-before-write, so operator restarts mid-bootstrap are harmless. Planner and topology derivation are covered by pure unit tests; the controller registration flow by envtest with an in-memory fake cluster that rejects duplicate registrations and second MAIN promotions. * feat: 0-based indexing for data instances
* KinD e2e harness: licensed MemgraphCluster bootstrap proof, PR-gated A true end-to-end suite for the provisioning and bootstrap slices: CI boots a multi-node Kind cluster, builds and deploys the operator image into it, applies a MemgraphCluster running real Memgraph images, and asserts every declared instance appears registered in SHOW INSTANCES with exactly one MAIN (queried through mgconsole inside a coordinator pod, the HA chart CI's established practice). Operator deployment (namespace, CRDs, controller) moves from the Manager container into BeforeSuite/AfterSuite, so scenario containers only exercise MemgraphCluster behavior and later slices (re-registration, storage retention) add test cases, not pipeline. The enterprise license flows from the MEMGRAPH_ENTERPRISE_LICENSE / MEMGRAPH_ORGANIZATION_NAME repository secrets into a Kubernetes Secret applied over stdin, so no secret material reaches logged command lines or the repo. Part of specs/operator-mvp/issues/04-kind-e2e-harness.md. * testing: Fix checking for is main * refactor: Move role main under the constants block
as51340
force-pushed
the
01-repo-reset-scaffold
branch
from
July 24, 2026 13:15
60d1422 to
9cd1163
Compare
* feat: continuous re-registration reconciliation (#24) Extend registration reconciliation from one-shot bootstrap to continuous drift recovery: a converged cluster now reschedules a periodic resync so a registration a pod loses (rescheduled onto a fresh node, wiped storage) is re-issued without human action. A lost registration on a still-running pod produces no watch event, so this resync is the mechanism that detects drift. The planner already computed full-diff semantics; the gap was the controller returning no requeue once converged. Add a resyncInterval and requeue on it from the converged branch. Tests: - planner: multiple lost registrations re-issued without a second MAIN - controller envtest: re-register a lost data instance (MAIN untouched), re-add a lost coordinator, no-op across repeated resyncs - e2e: wipe a data instance's registration on the leader, assert the operator converges the cluster back to fully registered * testing: Add test for removal of coordinators
* feat: CEL immutability and creation-time validation on the CRD Pin the v1alpha1 topology contract in the CRD schema, with no admission webhook: CEL transition rules reject any change to spec.coordinators or spec.dataInstances on a live MemgraphCluster, and creation-time rules bound the counts and reject unusable image and secret references. - coordinators: 1-7 and odd, so the Raft quorum cannot split; an even count is never more fault tolerant than the odd count below it, and the field cannot be corrected once the cluster exists - dataInstances: 1-15, a typo guard on an equally immutable field - image.repository/tag and secrets.name/licenseKey/organizationKey get length and format rules; a tag or digest smuggled into repository is rejected with a message pointing at image.tag - secrets.licenseKey and secrets.organizationKey must name different keys of the Secret Envtest coverage lands in a dedicated validation suite: rejected mutations, accepted valid creates and updates that leave the counts alone, and the defaults a minimal CR materializes checked against the Go constants the resource builders fall back to. * feat: Remove upper bounds
Replace the ephemeral lib and log emptyDirs with StatefulSet volumeClaimTemplates, configurable per role with the HA chart's storage vocabulary (libPVCSize / libStorageAccessMode / libStorageClassName and the log counterparts). Add storage.retentionPolicy (Retain | Delete, default Retain) mapped straight onto the StatefulSets' persistentVolumeClaimRetentionPolicy, so deleting the CR keeps the data by default while dev clusters can opt into self-cleanup. whenScaled stays Retain: both replica counts are immutable in v1alpha1, so nothing ever scales down. The StatefulSet machinery is the only deleter — the operator owns no finalizer. Builder golden tests cover storage defaults, per-role overrides and both retention policies; envtest pins the schema defaults against the Go constants and asserts no operator finalizer; e2e covers both the Retain (PVCs survive CR deletion) and Delete (PVCs go with it) paths.
…main, env/args (#28) * feat: pod-tuning knobs (probes, resources, labels, ports, env/args) Add the remaining v1 configuration surface to the MemgraphCluster spec, mirroring the memgraph-high-availability Helm chart's vocabulary: - clusterDomain and ports (bolt, management, replication, coordinator), which propagate to container ports, Services, the coordinator start script and every advertised address the operator registers - probes per role and per probe (timings only; the probe stays a TCP-socket check against the role's own port) - resources per role - labels per role for pods, StatefulSets and Services - extraEnv and extraArgs per role, non-secret by construction The two ports the registration commands are built from cannot be overridden through extraArgs, and extraEnv cannot shadow the license or the pod's own identity: admission rejects both. A CR with every knob defaulted produces the same objects as before this slice. * fix: Improve sample config file
…Deployment) (#29) * feat: operator install chart with generated CRDs and least-privilege RBAC Add charts/memgraph-operator, the chart users install the operator with: the MemgraphCluster CRD, the RBAC the controller needs, and the controller Deployment. `helm install` from the local chart on a clean cluster is the complete install story. The chart cannot drift from the controller version it ships with, because the two manifests that encode the contract are generated by `make chart-sync`: crds/ from the Go types, and rbac/manager-rules.yaml from the +kubebuilder:rbac markers, rendered into the manager ClusterRole with .Files.Get. `make chart-verify` regenerates into a scratch directory and diffs, so CI fails on a stale copy. Tighten the controller's RBAC markers to what the reconciler issues, since they are now the chart's permission surface: read MemgraphClusters, patch their status, and create plus patch (server-side apply) the StatefulSets and Services — no update, no delete, deletion belongs to garbage collection via the owner references. The finalizers subresource stays for the blockOwnerDeletion owner references. Leader election is a namespaced Role (Lease, Events) gated on the value that enables it, and no rule grants Secret access: the license Secret is referenced from the CR and mounted by the kubelet, never read by the operator. Install and uninstall are exercised twice in CI. hack/chart-install-test.sh (`make test-chart`, license-free) installs the chart on a clean Kind cluster into a namespace enforcing the restricted Pod Security Standard, asserts the operator runs non-root and provisions the workloads and status of a MemgraphCluster with no authorization failure in its log, then uninstalls the release and the CRDs. The e2e suite now installs the operator through the same chart instead of `make deploy`, so every scenario runs under exactly the RBAC users get. * fix: Improve chart configuration
…m-charts (#30) * feat: release pipeline — operator image, chart cross-published to helm-charts Pushing a version tag now produces installable artifacts: the multi-arch operator image on Docker Hub, and the install chart packaged into the existing memgraph.github.io/helm-charts index, so users install from the helm repository they already have configured. Chart source and CRDs stay here. The chart's version and its appVersion move independently, the Helm-standard arrangement, so a chart-only fix does not force a redundant operator release. A tag therefore names the artifact it releases: v<version> the operator (and the chart alongside it), chart-<version> the chart alone, with no image build. The workflow refuses a tag whose version the tagged tree does not declare. Issue 11's fourth acceptance criterion is amended accordingly. Publishing writes into another repository, so hack/chart-publish.sh is exercised offline on every pull request by hack/chart-publish-test.sh, against a bare-git stand-in for helm-charts and a gh stub: dry runs publish nothing, publishing cuts the release and indexes it, re-publishing is a no-op, and prereleases are marked as such. Every publishing step skips when its result already exists, so re-running a half-finished release completes it rather than duplicating it — including the image, which is reused only when its revision label shows this same commit built it. After publishing, the run installs the chart from the public index on Kind and asserts the running Deployment is the image just released. A dry run from the Actions tab does the same against the local package with a Kind-loaded image, publishing nothing; a SemVer prerelease tag rehearses the real thing while staying hidden from helm install. Releasing needs HELM_CHARTS_TOKEN, a fine-grained PAT scoped to contents write on memgraph/helm-charts; docs/releasing.md covers its scope and rotation. The run checks the credentials exist before it builds anything. * testing: disable the Install chart job on pull requests It boots a Kind cluster on every pull request. Its chart-version-check step is also premature until the first release: it enforces a version bump against a chart version that has never been published, so any change to the chart trips it. Removing the 'if: false' brings the job back.
) The minutes-to-cluster story for a developer evaluating Memgraph: install the operator chart, create the license Secret, apply one manifest, watch `kubectl get mgc` converge, and connect over Bolt. The README also states the v1alpha1 contract outright — immutable counts, hands-off failover, and the deferred features (day-2 ops, external access, TLS, auth, monitoring, standalone) — and its relationship to the HA Helm chart: successor, parity then freeze, fresh-cluster migration only. The quickstart manifest is a file, examples/minimal-cluster.yaml, and the e2e suite applies it verbatim instead of an inlined copy, deriving the cluster name, topology, image and license Secret it asserts on from the file itself. The example a newcomer copies is therefore the one CI proves boots a registered, MAIN-elected cluster, and it cannot rot in place.
observeCluster treated a coordinator reporting no leader as the fresh-cluster bootstrap case and planned against its SHOW INSTANCES view. The premise was wrong: a coordinator's initial Raft configuration holds itself alone and it starts as the leader of that one-member cluster, so a fresh coordinator always names itself. An absent leader means the coordinator lost track of one — quorum gone, or it stepped down or was removed — and it then answers from its own state machine, which can be arbitrarily stale and would reject every mutating query the operator could issue anyway. Such a view is now skipped and the next coordinator tried. When no coordinator names a leader, nothing is issued, Ready and Converged go False with the new NoCoordinatorLeader reason — kept apart from CoordinatorUnreachable, since up-but-quorumless pods and unreachable pods need different remedies — and the reconcile requeues. The leader-by-name lookup now falls back to the reported view's bolt server, so a leader outside the declared coordinator set resolves instead of dead-ending. That is also what coordinator scale-down will need, where the leader can be a coordinator on its way out. The fallback was masking an unfaithful test seam: fakeMemgraph modelled a fresh cluster as an empty SHOW INSTANCES, so no fake cluster ever had a leader and the planner's empty-bolt_server rule was never exercised in the controller tests. The fake now serves the connected coordinator's own row as leader with an empty bolt_server, and ADD COORDINATOR fills that server in rather than inventing a row.
* feat: mutable topology with scale-up support Reverses the v1alpha1 contract that both replica counts are fixed at creation: the CEL immutability rules come off `coordinators` and `dataInstances`, and schema floors take their place — coordinators must be odd and at least 3 (enforced at creation and on every update, since this operator builds real HA clusters), data instances at least 1. Nothing else constrains a change: any target counts, both roles in one edit, any step size. Growth needs no new planning: the diff that restores a lost registration already emits `ADD COORDINATOR` and `REGISTER INSTANCE` for declared members it does not observe. What this adds is the structure the shrink slices hang off, so the API is cut once: - Resource builders take the replica count as an argument (`CoordinatorStatefulSet(cluster, replicas)`), staying pure while the controller decides the count: declared when growing or unchanged, and deliberately current when declared < current, so this slice can never shrink a StatefulSet. - `planner.Topology` gains `RetiringCoordinators` and `RetiringDataInstances`, empty here. - The promotion rule now prefers the lowest-ordinal declared instance observed `up`, falling back to `declared[0]` — which also closes the bootstrap hole where promoting a down instance only wrote Raft state and left the cluster MAIN-less until the coordinators retried. Observability and storage semantics move with the API. `status.coordinators` and `status.dataInstances` publish the registered counts as observed, as `priority=1` print columns. `Converged` widens to "registration matches and both StatefulSets' replicas equal the declared counts", so `kubectl wait --for=condition=Converged` means the scale is genuinely finished; a held-back count reports `ScaleInProgress`. `persistentVolumeClaimRetentionPolicy.whenScaled` now follows `spec.storage.retentionPolicy` instead of being hard-wired `Retain`. Tests: planner cases for a down `instance_0` and for a grown topology, plus `Registered`; builder cases for the argument-driven replica count and both retention values; envtest inverts the immutability cases to acceptance and adds floor rejections, a grow-both-roles spec, and the never-shrink spec. A new e2e container grows a dedicated 3/2 cluster to 5/3 in its own namespace and waits for `Converged`; the retention e2e and the chart install test move to the coordinator floor. The chart version is bumped to 0.2.0 because its generated CRDs changed. * fix: Don't bump chart version
* feat: data-instance scale-down with MAIN-safe retirement Carries out a lowered `dataInstances` count, which `14-topology-scale-up.md` accepted at admission but deliberately held back. A StatefulSet sheds only its highest ordinals, so the operator never picks a victim: the retiring set is `[spec.dataInstances, liveStatefulSet.spec.replicas)`, derived from the operator's own prior apply, which bounds the range exactly and keeps an instance a human registered out of it. The planner drives the whole removal in one pass against one leader connection, because it can predict every intermediate state: - `DEMOTE INSTANCE` is emitted only when a retiring instance is observed as MAIN — Memgraph refuses to unregister the MAIN — and it deliberately leaves the cluster MAIN-less, since failover only triggers on a coordinator leadership change or a failed ping. - The promotion that follows is the rule 14 installed (lowest-ordinal declared instance observed `up`), so the operator hands MAIN to a survivor itself. Only declared instances are candidates, so a member on its way out is never the target. Replicas are SYNC, so promoting after a clean demote is not a data-loss gamble, and the MAIN-less window is the time between two queries. - `UNREGISTER INSTANCE` then removes each retiring member that is still registered. Unregistration happens before the pods go: the smaller replica count is applied in exactly one place, at the end of the registration phase once the plan is empty. That keeps 14's pre-apply replica rule free of any cluster knowledge — it still never shrinks — and converges the shrink across two passes with no second observation source. The readiness gate stays strict, retiring pods included, so a retiring pod that cannot become ready blocks its own removal and the resource reports `WorkloadsNotReady` instead of the operator acting on a half-known cluster; that trade is documented. `Converged` is False with reason `RetirementInProgress`, naming the retiring instances, for as long as the StatefulSet still runs them. Errors are not special-cased: read-before-write means `ALREADY_REPLICA` or `NO_INSTANCE_WITH_NAME` only appear when something raced the operator, so they surface and the plan is recomputed from a fresh observation. Tests: planner cases for MAIN on a retiring ordinal, MAIN on a survivor, a down survivor, several retiring at once, shrink to a single instance, an already-unregistered retiring member, mixed grow-and-shrink across roles, and an undeclared instance outside the range; `RetiringDataInstances` bounds in both directions; envtest specs for the retirement order, the MAIN move, the unready retiring pod, and — rewritten for the role that still holds — the lowered coordinator count. The scaling e2e container parks MAIN on `instance_2`, drops the count to 2, and asserts the instance leaves the cluster before its pod is shed, that MAIN moved to a survivor, that the cluster converges, and that the retired claim is kept by the default retention policy. The fake cluster gains demote and unregister, rejecting a non-MAIN demote and an unregistration aimed at a MAIN, so any plan that is not read-before-write fails the suite loudly. No CRD or RBAC change, so the chart is untouched. * testing: Make more reliable e2e test
* feat: coordinator scale-down with leadership-safe Raft removal Carries out a lowered `coordinators` count, the last scale `14-topology-scale-up.md` accepted at admission but held back. Removing a coordinator means removing a Raft member, and Raft refuses to remove its own leader (`RAFT_CANNOT_REMOVE_LEADER`) — while a StatefulSet sheds only its highest ordinals, so the leader may well sit on one of them. Retiring ordinals are `[spec.coordinators, liveStatefulSet.spec.replicas)`, and because the count must stay odd a shrink always retires an even number of members, so the surviving Raft cluster keeps an odd membership throughout. `YIELD LEADERSHIP` is the lever, and it is the one command whose outcome the planner cannot predict: it must be issued on the current leader — the connection the controller already holds — and it names no successor, because `yield_leadership()` is called without one and NuRaft's election decides. So it is always a plan's **last** command and terminal: the controller stops after it and requeues to re-observe under whichever coordinator won, asking again if the election happened to pick another retiring member. Everything the planner can still order safely goes out ahead of it in that same pass — the retiring coordinators that are not the leader, and the whole data-instance retirement. `REMOVE COORDINATOR` is therefore never aimed at the observed leader. Raft membership is given up before the pods are, so no removed member's vote outlives its pod: the shrink is applied in the one place a replica count is ever lowered, at the end of the registration phase once the plan is empty. The readiness gate stays strict, retiring pods included. `Converged` is False with reason `RetirementInProgress` — now naming the retiring members of both roles — or `LeadershipTransferInProgress` while a yield is pending. A coordinator removed from Raft keeps running and keeps its state on purpose: NuRaft fires `RemovedFromCluster`, stops it campaigning after two election timeouts, and never calls `system_exit`, so the container does not die and the readiness gate is not tripped. It appends nothing, so its log stays a prefix of the leader's and cannot diverge, and a later `ADD COORDINATOR` is accepted unconditionally — a re-added coordinator on a retained volume is in the same position as one whose pod crashed and stayed down. No PVC wipe, no removal bookkeeping, no re-add guard. Its stale view is already handled by `13-coordinator-leader-required.md`. `ScaleInProgress` goes away with this: `applied` is `max(declared, current)`, so a mismatch between the two is now exactly a retirement in flight, and the reason became unreachable rather than merely unused. Tests: planner cases for the leader on a retiring ordinal, on a survivor and outside the retiring set, two coordinators retiring at once, an already-removed retiring member, a retiring leader with nothing else to order, and a retiring coordinator alongside retiring data instances; `RetiringCoordinators` bounds in both directions plus its declared-form equality; envtest specs for the removal order, the yield and the pass that removes under the new leader, the raised-back count, and both roles retiring in one edit. The fake cluster gains `REMOVE COORDINATOR` and `YIELD LEADERSHIP`, refusing a removal aimed at its own leader, so a plan that skipped the yield fails the suite loudly. The scaling e2e container forces leadership onto `coordinator_4`, drops the count to 3, and asserts both members leave the Raft cluster before their pods are shed, that leadership lands on a survivor, that the cluster converges, and that the retired claims are kept by the default retention policy. No CRD or RBAC change, so the chart is untouched. * fix: Finding out who is the leader
The promotion is the one command of a retirement with no second chance: the demotion has already landed when it runs, and a later pass cannot repeat the reasoning because SHOW REPLICATION LAG is served by the MAIN. A refused promotion therefore left the cluster MAIN-less with nothing to fall back on. SetInstanceToMain now carries every survivor handoverCandidates proved caught up, tried in order, plus the demoted MAIN as the last resort. Restoring it is safe by construction — a MAIN-less cluster accepts no writes, so nothing has advanced past the instance that was MAIN a moment ago — and it deliberately fails the pass: the cluster serves again, but the retirement made no progress and the unregistration planned behind the promotion would be aimed at a MAIN. Every attempt is logged, including the fallback that succeeds after an earlier candidate was refused, which the return value alone would hide.
The sample carried dataInstances: 0, which admission rejects — the schema floors the count at 1, because a cluster with no data instance has nothing to serve.
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
1 participant
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.
Some details:
Important decision around coordinator vs operator interacting. We rely on the fact that only when operator/human does demote will all instances have state replica. On the normal failover, the old main keeps the role main until the failover actually occurred. When the operator wants to do the failover, it records all instances that are viable and we try on every each of them. If it fails on all of them, then we revert old main as a new main again and scheduke a new reconciliation step.