Skip to content

fix(falkordb): map backup image by service version - #3191

Merged
leon-ape merged 2 commits into
mainfrom
fix/falkordb-backup-image-version-mapping
Jul 25, 2026
Merged

fix(falkordb): map backup image by service version#3191
leon-ape merged 2 commits into
mainfrom
fix/falkordb-backup-image-version-mapping

Conversation

@weicao

@weicao weicao commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

Closes #3192

Problem

FalkorDB ActionSets rendered the chart-default engine image into every backup and restore job, so new backups did not follow the target Cluster service version. A direct replacement of the existing ActionSet image fields would introduce a second failure: old Backup objects persist the legacy ActionSet names, but do not persist FALKORDB_IMAGE, so their restores must continue to resolve the original fixed-image ActionSets.

This PR preserves both contracts:

  • old Backup objects keep resolving the unchanged legacy ActionSet names and their original v4.12.5 image;
  • new BackupPolicyTemplates select version-aware ActionSets whose engine image is mapped from the target service version.

Change

  • preserve falkordb-physical-br, falkordb-cluster-br, and falkordb-for-pitr as legacy fixed-image ActionSets for existing Backup restore compatibility;
  • add falkordb-physical-br-v2, falkordb-cluster-br-v2, and falkordb-for-pitr-v2 for new backups;
  • point the standalone and cluster BackupPolicyTemplates at the new -v2 ActionSets;
  • add FALKORDB_IMAGE versionMapping entries for standalone datafile, standalone aof, and cluster datafile methods;
  • derive mapped refs from the same registry, repository, and mirror-version values as ComponentVersion rendering;
  • leave the independent busybox rebuild and ape-dts cluster restore jobs unchanged;
  • add render-contract tests for the legacy names/images, the version-aware names/images, and exact 4.12.5/4.14.12 mappings.

Evidence

TDD causal check: applying the current spec to base b55707f0 produced 3 failing examples: the -v2 template was absent and both BackupPolicyTemplates still selected legacy ActionSets.

Current-head checks at e0e51ace56f20b43e6a8083a06fea44619670b32:

  • focused backup mapping ShellSpec: 4 examples, 0 failures;
  • full FalkorDB ShellSpec on bash 5.3: 210 examples, 0 failures;
  • full FalkorDB ShellSpec on system bash 3.2: 42 examples, 0 failures, 4 existing bash-version skips;
  • helm lint addons/falkordb: pass;
  • focused Helm render: 3 legacy ActionSets with 9 fixed v4.12.5 engine images, 3 version-aware ActionSets with 9 mapped engine images, and 3 policy references to -v2 with no legacy policy reference;
  • ShellCheck for the mapping spec: pass;
  • git diff --check: pass.

No Kubernetes environment was mutated by this code change. Runtime validation remains required for a fresh 4.14.12 provision, backup/restore, and service connectivity.

@weicao
weicao requested review from a team and leon-ape as code owners July 13, 2026 23:02
@codecov-commenter

codecov-commenter commented Jul 13, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 0% with 61 lines in your changes missing coverage. Please review.
✅ Project coverage is 0.00%. Comparing base (b55707f) to head (e0e51ac).

Files with missing lines Patch % Lines
...ripts-ut-spec/backup_image_version_mapping_spec.sh 0.00% 61 Missing ⚠️
Additional details and impacted files
@@          Coverage Diff          @@
##            main   #3191   +/-   ##
=====================================
  Coverage   0.00%   0.00%           
=====================================
  Files        147     148    +1     
  Lines      23282   23343   +61     
=====================================
- Misses     23282   23343   +61     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@weicao

weicao commented Jul 13, 2026

Copy link
Copy Markdown
Contributor Author

Focused review of corrected exact head 4b0525fc4ca96de15ebbee2edb6033b07aba76f6 (base b55707f0c373f2c1f4e119baff131f299caadfb2): PASS for the addon/render contract. The shared GitHub identity is the PR author, so this is recorded as a comment rather than a formal approval.

Independent evidence:

  • Canonical issue #3192 now records the problem, reproduction, red/green scenarios, and separate repository-publication boundary; the PR closes it and GitHub API reports closingIssuesReferences=[3192].
  • Base→head is four-file/root-cause-only scope. The three BackupPolicyTemplate consumers close exactly: standalone datafilefalkordb-physical-br, standalone aoffalkordb-for-pitr, cluster datafilefalkordb-cluster-br.
  • Rendered ActionSets contain exactly nine $(FALKORDB_IMAGE) engine jobs. The independent ape-dts job remains one fixed ape-dts ref and rebuild remains two busybox refs; their image fields are not replaced.
  • All three mappings render exact 4.12.5 and 4.14.12 refs from the same global registry/repository plus mirrorVersions[].imageTag writer used by the matching standalone/cluster ComponentVersions.
  • I independently copied the new focused spec onto base and got the expected 3/3 RED failures: fixed 4.12.5 ActionSet images and zero mappings. On head, focused ShellSpec is 3/0; full system-bash suite is 41/0 with four existing version skips; full Homebrew bash 5 suite exits zero. helm lint, ShellCheck, git diff --check, and explicit Helm renders pass. All visible CI checks are terminal SUCCESS except the expected label job SKIPPED.

Fail-closed boundary: on current KB main, an empty/unmatched versionMapping result is omitted from BackupPolicy env; expansion then preserves literal $(FALKORDB_IMAGE) rather than substituting the old chart-default image. That prevents a silent wrong-version fallback, but failure occurs late when the Job image is consumed rather than as an early policy-validation error. This PR does not change that controller behavior.

XP contract walk: current head/scope/API-runtime writer pass; class 1 has no old-image fallback; class 2 maps every advertised mirror version and all consumers; class 3 uses the target component serviceVersion through BackupPolicy resolution; class 4 is fail-closed but late as noted; classes 5/6/7/8 introduce no cleanup/NotFound/terminating/precedence path.

Runtime boundary remains separate: this PASS does not prove registry publication parity and does not authorize reuse/mutation of the preserved failed backup scene. Validate only on a correctly published fresh 4.14.12 install after the normal owner/test handoff.

@leon-ape leon-ape left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • This mutates the existing falkordb-physical-br, falkordb-cluster-br, and falkordb-for-pitr ActionSet image contracts to require FALKORDB_IMAGE. Backups created before this change persist the same ActionSet names but no FALKORDB_IMAGE in status.backupMethod.env; restore resolves the current ActionSet by that persisted name, so those backups now produce unresolved restore image references after addon upgrade.
  • The current head has no successful fresh 4.14.12 provision, backup/restore, and service-connectivity result. The supplied verification covers only rendering and explicitly leaves runtime validation at N=0.

@weicao

weicao commented Jul 14, 2026

Copy link
Copy Markdown
Contributor Author

Addressed the persisted-name compatibility blocker at exact head e0e51ace56f20b43e6a8083a06fea44619670b32:

  • restored the three legacy ActionSet names to their original fixed v4.12.5 image contract, so old Backup objects continue to resolve a complete restore image;
  • added separate -v2 ActionSets for version-mapped new backups;
  • changed only the new BackupPolicyTemplate selections to the -v2 names;
  • added a causal render-contract spec covering both the legacy and version-aware paths.

Local checks are green: focused 4/4, full bash 5.3 210/210, full bash 3.2 42 examples with 0 failures and 4 existing skips, Helm lint/render, ShellCheck, and diff check. Current substantive GitHub checks are green; an earlier label-check failure is superseded by the later successful run.

Scoped runtime acceptance is now closed for one clean fixed-4.14.12 backup/restore cycle. The live contract uses the three -v2 ActionSets introduced by this PR and absent from base b55707f0; the tested Backup selected falkordb-physical-br-v2 with FALKORDB_IMAGE=v4.14.12 and /data.

Direct evidence from the clean run (raw SHA256 a789665da9d9fcb01cfdd774d66e0126b7b3c60cc45b47ce33f63f1cb9faffb4, identity supplement SHA256 0de7d167990083d157153d60c3fc13daa64eb141ebc8d0195615770482ee430e):

  • source and restore Clusters reached Running; source and restore data Pods converged to DBSIZE=22 on all three replicas;
  • Backup fdb-e2e-backup reached Completed with a non-empty path;
  • all three FalkorDB data PVCs reached Populating=True/Succeed and Restore=True/Succeed; the three sentinel PVCs correctly used provision-without-data-restore;
  • four Restore objects completed (three data-populate objects plus one post-ready object);
  • the restored graph returned 10 nodes, and representative first/last KV values matched;
  • three vcluster SyncError events coexisted with this successful restore, so their presence is not treated as a sufficient failure condition;
  • the live syncer was fixed at image docker.io/local/vcluster-kb12-restore:d87e04cf5-amd64, imageID sha256:6d672ba5a272856a82efe82adcc9ff26f1fe6e4488525e603c7ebd22a582d5e4.

Scope boundary: this is N=1 current-candidate runtime support, not repeated/full-suite/release-readiness or cryptographic publication proof. Earlier ProvisionOnly -> RestoreData transitions changed DP pod lifecycle/config state together, so their unique causal mechanism remains unclassified and is not used to justify this acceptance.

The remaining PR gate is formal maintainer review/approval; runtime restore and post-restore data validation are no longer marked environment-blocked.

@weicao weicao added the nopick Not auto cherry-pick when PR merged label Jul 14, 2026
@weicao

weicao commented Jul 14, 2026

Copy link
Copy Markdown
Contributor Author

Focused re-review of exact head e0e51ace56f20b43e6a8083a06fea44619670b32 (base b55707f0c373f2c1f4e119baff131f299caadfb2): PASS for the persisted-name compatibility code/render contract. This does not close the separate fresh 4.14.12 runtime gate.

Independent evidence:

  • Local HEAD, remote PR head, and GitHub PR API agree on e0e51ace...; base-to-head is five FalkorDB files, all serving the backup image/version compatibility change.
  • The three legacy ActionSets remain rendered exactly once under their persisted names: falkordb-physical-br, falkordb-cluster-br, and falkordb-for-pitr. Ignoring the pre-existing missing final newline, backupactionset.yaml is semantically unchanged from base and retains nine fixed 4.12.5 FalkorDB job images.
  • New policies reference only the three -v2 ActionSets. The new ActionSets are source-byte-equivalent to the first three legacy definitions after normalizing only resource names and the nine FalkorDB image fields; the independent ape-dts job is unchanged. Full render contains seven unique ActionSets: four legacy/rebuild plus three v2.
  • Standalone datafile/AOF and cluster datafile each map both advertised serviceVersions, 4.12.5 and 4.14.12, from the same registry/repository values. No legacy ActionSet is referenced by a new BackupPolicyTemplate, and no v2 ActionSet contains a fixed FalkorDB image fallback.
  • Independent RED/GREEN: base cannot render backupactionset-v2.yaml, references only legacy names, and has no FALKORDB_IMAGE version mappings. Current head focused ShellSpec is 4/0; full system bash suite is 42 examples / 0 failures / 4 existing version skips; Helm lint and representative full renders pass. Current substantive CI is terminal SUCCESS; the earlier label failure is superseded by a later SUCCESS.

Contract anchors: docs/addon-api/09b-backup-extensions.md:14-16,31,39, docs/addon-api/12a-minimum-acceptance.md:27-30,38-39, and docs/addon-api/01-define-scope.md:60-66 require explicit restore compatibility boundaries, final env/versionMapping verification, stable resource-name closure, and upgrade resource governance.

XP contract walk: class 1 clean (v2 has no fixed-image fallback); class 2 clean for the two advertised serviceVersions and all three consumers; class 3 clean because old persisted names and new policy names are disjoint; class 4/4a clean because missing/unmatched mapping leaves the literal image unresolved rather than silently selecting legacy; classes 5-7 add no cleanup/NotFound/terminating flow; class 8 has no complex condition.

Remaining gate: per docs/addon-api/12a-minimum-acceptance.md:27,32,38, static/render review cannot replace a fresh 4.14.12 provision + backup + restore + post-restore connectivity/data validation. Until that test-owned evidence exists, this is not a runtime PASS or merge-readiness conclusion.

@leon-ape leon-ape left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Current head still has no 4.14.12 provision, backup, restore, and post-restore connectivity/data validation. Static/render checks do not validate the version-mapped ActionSets at runtime.

@weicao

weicao commented Jul 23, 2026

Copy link
Copy Markdown
Contributor Author

Accepted. I rechecked the current PR identity: head is still e0e51ace56f20b43e6a8083a06fea44619670b32 on base b55707f0c373f2c1f4e119baff131f299caadfb2.

The earlier 2026-07-14 N=1 packet was produced from this exact head and directly covered clean-slate 4.14.12 source provisioning, falkordb-physical-br-v2 backup completion, restore completion, and restored data readback. However, its connectivity checks were executed inside a restore data Pod, not through the restored Cluster's Service endpoint. It therefore does not fully close the requested post-restore service-connectivity gate.

A fresh exact-head cycle is now assigned to the test owner with the following required terminal evidence: clean-slate 4.14.12 provision; completed backup with the v2 ActionSet and mapped image; completed restore; and credentialed PING, representative KV readback, and graph readback through the restored Cluster Service endpoint. The first failure will be retained with raw evidence; a successful run will include normal-cleanup and empty-residue receipts.

Until that fresh packet is posted, I am not claiming runtime or merge readiness.

@weicao

weicao commented Jul 23, 2026

Copy link
Copy Markdown
Contributor Author

Fresh exact-head runtime follow-up for the missing Service-endpoint gate:

PASS for one scoped PR acceptance sample (N=1) at current head e0e51ace56f20b43e6a8083a06fea44619670b32 (base b55707f0c373f2c1f4e119baff131f299caadfb2).

Identity binding:

  • chart falkordb-1.2.0-alpha.0, SHA256 32bb79296190165e66d1c65c8efa24db823c2220be5e266661a72dcc600057d8;
  • KubeBlocks source dfa97006f546b969175782280acb92d813b24ae4, live exact controller image config/imageID sha256:7af29e02351276609fdb9646c2137cd7268d832174d14510132ce02485ee8c7a, Ready with zero restarts;
  • syncer image docker.io/local/vcluster-kb12-restore:d87e04cf5-amd64, imageID sha256:6d672ba5a272856a82efe82adcc9ff26f1fe6e4488525e603c7ebd22a582d5e4;
  • FalkorDB workload docker.io/falkordb/falkordb:v4.14.12.

Fresh r2 terminal evidence:

  • source Cluster reached Running with 6/6 Pods Ready; 20 KV records and graph count 10 read back from all three data Pods;
  • Backup UID 07fc23ec-b43e-4fa6-a5b9-14e9c302a4df reached Completed with a non-empty path and totalSize=3745, selecting falkordb-physical-br-v2 with the exact v4.14.12 image;
  • four Restore objects reached Completed; 6 PVCs were Bound and 6 restored Pods Ready; the same 20 KV records and graph count 10 read back from all three restored data Pods;
  • an independent Job connected through ClusterIP Service fdb-pr3191-restore-falkordb-falkordb:6379 using a generated credential SecretKeyRef: PING=PONG, first/last values were val-1/val-20, and graph count was 10. No credential values were written to the log;
  • fresh cleanup followed dependency order and completed normally: namespace NotFound, Cluster/Backup/PVC residue zero, and no force or finalizer removal. Three temporary host sideload helpers were also normally removed; the exact controller image remains live.

Canonical raw packet: SHA256 9581b57bc5cb605385a2537364a9acdf07a903d299d9a086a0c47b54b4d9d60b; safe relative tar; runtime manifest 70/70 and helper-cleanup manifest 20/20; credential-shaped key/JWT/Bearer scan zero; runner stderr zero bytes.

A preceding attempt stopped at an independently classified MinIO environment failure (Service existed but ready endpoints and Pods were zero) and is not counted in this PASS. Its cleanup-order recovery is also tracked separately and is not product evidence.

Scope boundary: this closes the requested fresh 4.14.12 provision + backup + restore + post-restore Cluster Service connectivity/data gate for N=1. It is not repeated/full-suite evidence and is not a release-readiness claim. The remaining PR gate is formal maintainer review/approval.

@leon-ape leon-ape left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@leon-ape
leon-ape merged commit 6eecbc4 into main Jul 25, 2026
18 of 19 checks passed
@leon-ape
leon-ape deleted the fix/falkordb-backup-image-version-mapping branch July 25, 2026 08:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

nopick Not auto cherry-pick when PR merged

Projects

None yet

Development

Successfully merging this pull request may close these issues.

FalkorDB backup helper image must follow Cluster serviceVersion

3 participants