refactor(seaweedfs): split seaweedfs-system into seaweedfs-db + seaweedfs-system#2601
refactor(seaweedfs): split seaweedfs-system into seaweedfs-db + seaweedfs-system#2601myasnikovdaniil wants to merge 1 commit into
Conversation
myasnikovdaniil
requested review from
IvanHunters,
androndo,
kvaps,
lexfrei,
lllamnyp and
sircthulhu
as code owners
github-actions
Bot
added
area/storage
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses a race condition during fresh tenant installations where seaweedfs-filer pods would crashloop due to the database not being ready. By decoupling the database infrastructure from the application components and utilizing Flux's dependency management and health check expressions, we ensure a reliable deployment order. This change requires Flux 2.8.x and includes a migration path for existing deployments. Highlights
New Features🧠 You can now enable Memory (public preview) to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThis pull request splits the SeaweedFS system into two coordinated HelmReleases to eliminate a startup race condition. The database release (seaweedfs-db) is now provisioned first with CNPG readiness gating via healthCheckExprs, and the workload release (seaweedfs-system) depends on successful database deployment. A migration script handles existing deployments. ChangesSeaweedFS System Split
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
dosubot
Bot
added
the
area/platform
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request splits the seaweedfs-system HelmRelease into seaweedfs-db and seaweedfs-system to ensure the database is ready before the application starts, resolving an EPERM crashloop issue. The changes introduce a new system chart, a migration script for existing clusters, and utilize FluxCD's healthCheckExprs for readiness gating. Feedback identifies a shell compatibility issue in the migration script, missing resource fields in the database values schema, and the presence of local absolute paths in the implementation plan.
| @@ -0,0 +1,32 @@ | |||
| #!/bin/sh | |||
There was a problem hiding this comment.
The script uses set -o pipefail (on line 13), which is a bash-specific option and not supported by POSIX-compliant shells like dash (the default /bin/sh on many systems). To ensure the script executes correctly with this option, the shebang should be changed to #!/bin/bash.
| #!/bin/sh | |
| #!/bin/bash |
| ## @typedef {struct} DB - Database configuration. | ||
| ## @field {int} [replicas] - Number of database replicas. | ||
| ## @field {quantity} [size] - Persistent Volume size. | ||
| ## @field {string} [storageClass] - StorageClass used to store the data. |
There was a problem hiding this comment.
The DB typedef is missing the resources and resourcesPreset fields. These are being passed by the wrapper template in packages/extra/seaweedfs/templates/seaweedfs-db.yaml. Adding them here ensures they are included in the generated JSON schema and documentation. Note that the database.yaml template in this chart also needs to be updated to use these values instead of hardcoded ones.
## @typedef {struct} DB - Database configuration.
## @field {int} [replicas] - Number of database replicas.
## @field {quantity} [size] - Persistent Volume size.
## @field {string} [storageClass] - StorageClass used to store the data.
## @field {string} [resourcesPreset] - Resource preset.
## @field {Resources} [resources] - Resource configuration.| db: | ||
| replicas: 2 | ||
| size: 10Gi | ||
| storageClass: "" |
There was a problem hiding this comment.
Include default values for resourcesPreset and resources in the db block to align with the updated typedef and provide defaults for the schema.
db:
replicas: 2
size: 10Gi
storageClass: ""
resourcesPreset: "nanosmall"
resources: {}|
|
||
| - [ ] **Step 3: Verify directory structure** | ||
|
|
||
| Run: `ls /home/daniil/aenix/cozystack-split-seaweedfs/packages/system/seaweedfs-db/` |
There was a problem hiding this comment.
This document contains absolute paths specific to the author's local environment (e.g., /home/daniil/aenix/cozystack-split-seaweedfs/). These should be replaced with relative paths or placeholders to ensure the documentation is portable and clean. This applies to multiple locations throughout the file.
| Run: `ls /home/daniil/aenix/cozystack-split-seaweedfs/packages/system/seaweedfs-db/` | |
| Run: ls packages/system/seaweedfs-db/ |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (5)
packages/system/seaweedfs-db/Chart.yaml (1)
1-3: 💤 Low valueConsider adding a description field.
While not strictly required, adding a
descriptionfield to the Chart.yaml improves discoverability and follows Helm best practices. The sibling charts in this repository may provide examples.📝 Suggested addition
apiVersion: v2 name: cozy-seaweedfs-db +description: SeaweedFS Database (CNPG Cluster) for Cozystack version: 0.0.0 # Placeholder, the actual version will be automatically set during the build process🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/system/seaweedfs-db/Chart.yaml` around lines 1 - 3, Add a descriptive "description" field to the Helm chart metadata in Chart.yaml for cozy-seaweedfs-db: update the Chart.yaml (the top-level apiVersion/name/version block) to include a concise human-readable description string explaining the chart purpose (e.g., what cozy-seaweedfs-db deploys and any notable behavior), matching style of sibling charts so discoverability and Helm best practices are followed.docs/superpowers/specs/2026-05-10-split-seaweedfs-system-design.md (1)
7-9: 💤 Low valueAdd language specifier to fenced code block.
The code block showing the error message should specify a language for proper syntax highlighting and to satisfy the markdown linter.
📝 Suggested fix
-``` +```text dial tcp <ClusterIP>:5432 (seaweedfs-db-rw): connect: operation not permitted</details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.In
@docs/superpowers/specs/2026-05-10-split-seaweedfs-system-design.mdaround
lines 7 - 9, The fenced code block containing the error message in the document
should include a language specifier for proper highlighting and linting; update
the fenced block around the line with the text "dial tcp :5432
(seaweedfs-db-rw): connect: operation not permitted" (the existing
triple-backtick block) to use a language tag such as text (i.e., change ``` todocs/superpowers/plans/2026-05-10-split-seaweedfs-system.md (1)
169-175: 💤 Low valueAdd language specifier to fenced code block.
For consistency with the rest of the plan and to satisfy the markdown linter, add a language specifier to this code block.
📝 Suggested fix
-``` +```yaml --- apiVersion: postgresql.cnpg.io/v1 kind: Cluster🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/superpowers/plans/2026-05-10-split-seaweedfs-system.md` around lines 169 - 175, The fenced code block containing the Kubernetes manifest (starting with "apiVersion: postgresql.cnpg.io/v1", "kind: Cluster", and "metadata: name: seaweedfs-db") needs a language specifier; update the opening fence from ``` to ```yaml so the block becomes a YAML code block to satisfy the markdown linter and maintain consistency with other plan examples.packages/system/seaweedfs-db/values.yaml (1)
5-7: 💤 Low valueUnused typedef: Resources is documented but not referenced.
The
Resourcestypedef definescpuandmemoryfields, but these are not used anywhere in thedbconfiguration below. This may be a copy-paste artifact from another values file.♻️ Suggested cleanup
If the Resources typedef is not needed:
## ## `@section` Database parameters ## -## `@typedef` {struct} Resources - Resource configuration. -## `@field` {quantity} [cpu] - Number of CPU cores allocated. -## `@field` {quantity} [memory] - Amount of memory allocated. - ## `@typedef` {struct} DB - Database configuration. ## `@field` {int} [replicas] - Number of database replicas. ## `@field` {quantity} [size] - Persistent Volume size. ## `@field` {string} [storageClass] - StorageClass used to store the data.However, if resources will be added to the db configuration in the future, keeping the typedef is fine.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/system/seaweedfs-db/values.yaml` around lines 5 - 7, The typedef "Resources" (struct Resources with cpu and memory) is declared but not referenced by the db configuration; either remove this unused typedef block or wire it into the db values by adding a resources entry that consumes cpu and memory (e.g., ensure the db chart's values structure includes a resources object and any templates reference values.db.resources.cpu/memory or similar). Locate the "Resources" typedef and either delete it or add a values key (e.g., db.resources) and update relevant templates/deployment specs to read values.db.resources.cpu and values.db.resources.memory so the fields are actually used.packages/extra/seaweedfs/templates/seaweedfs.yaml (1)
111-113: ⚡ Quick winDocument the rationale for the 10× faster reconciliation interval.
The
intervalwas reduced from5mto30s, which increases reconciliation frequency by 10×. This could impact cluster load, especially with many tenants. Please clarify whether this aggressive interval is necessary for quick recovery after the DB becomes ready, or if a more moderate value (e.g.,1mor2m) would suffice.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/extra/seaweedfs/templates/seaweedfs.yaml` around lines 111 - 113, Add a short in-file comment above the interval setting that explains why the reconciliation interval for the SeaweedFS resource was reduced from 5m to 30s (or change the value) — reference the interval key and the dependsOn entry ({{ .Release.Name }}-db) and state whether the 30s is required for fast recovery after the DB becomes ready or if a moderate value like 1m/2m is preferred to reduce load; update the interval to the chosen moderate value if you decide not to keep 30s and ensure the rationale comment documents the trade-offs for cluster load and tenant scale.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/superpowers/specs/2026-05-10-split-seaweedfs-system-design.md`:
- Around line 68-82: This PR adds HelmRelease.spec.healthCheckExprs (see
HelmRelease.spec.healthCheckExprs in
packages/extra/seaweedfs/templates/seaweedfs-db.yaml) which requires
helm-controller v1.5.0+/Flux 2.8.x; because the repo is pinned to Flux 2.7.x
(helm-controller v1.4.3) you must not merge this change until the Flux upgrade
lands—either (a) remove or revert the healthCheckExprs usage and instead use a
supported wait strategy/timeout fallback, or (b) gate this PR on the Flux
upgrade by confirming the Flux 2.8.x upgrade PR exists and will merge first and
add a note in this PR linking that upgrade and/or add CI gating that blocks
merge until the upgrade is merged.
In `@packages/extra/seaweedfs/templates/seaweedfs-db.yaml`:
- Around line 27-31: Add a comment immediately above the healthCheckExprs block
documenting that this field requires Flux v2.8.x (helm-controller v1.5.0+) and
that on older Flux (e.g., v2.7.x) the HelmRelease will either fail to parse or
silently ignore healthCheckExprs, removing the readiness check; update the
comment to mention the minimum Flux and helm-controller versions and the
resulting behavior so maintainers know to upgrade Flux before relying on the
healthCheckExprs in this HelmRelease.
---
Nitpick comments:
In `@docs/superpowers/plans/2026-05-10-split-seaweedfs-system.md`:
- Around line 169-175: The fenced code block containing the Kubernetes manifest
(starting with "apiVersion: postgresql.cnpg.io/v1", "kind: Cluster", and
"metadata: name: seaweedfs-db") needs a language specifier; update the opening
fence from ``` to ```yaml so the block becomes a YAML code block to satisfy the
markdown linter and maintain consistency with other plan examples.
In `@docs/superpowers/specs/2026-05-10-split-seaweedfs-system-design.md`:
- Around line 7-9: The fenced code block containing the error message in the
document should include a language specifier for proper highlighting and
linting; update the fenced block around the line with the text "dial tcp
<ClusterIP>:5432 (seaweedfs-db-rw): connect: operation not permitted" (the
existing triple-backtick block) to use a language tag such as text (i.e., change
``` to ```text) so the markdown linter and renderers treat it as plain text.
In `@packages/extra/seaweedfs/templates/seaweedfs.yaml`:
- Around line 111-113: Add a short in-file comment above the interval setting
that explains why the reconciliation interval for the SeaweedFS resource was
reduced from 5m to 30s (or change the value) — reference the interval key and
the dependsOn entry ({{ .Release.Name }}-db) and state whether the 30s is
required for fast recovery after the DB becomes ready or if a moderate value
like 1m/2m is preferred to reduce load; update the interval to the chosen
moderate value if you decide not to keep 30s and ensure the rationale comment
documents the trade-offs for cluster load and tenant scale.
In `@packages/system/seaweedfs-db/Chart.yaml`:
- Around line 1-3: Add a descriptive "description" field to the Helm chart
metadata in Chart.yaml for cozy-seaweedfs-db: update the Chart.yaml (the
top-level apiVersion/name/version block) to include a concise human-readable
description string explaining the chart purpose (e.g., what cozy-seaweedfs-db
deploys and any notable behavior), matching style of sibling charts so
discoverability and Helm best practices are followed.
In `@packages/system/seaweedfs-db/values.yaml`:
- Around line 5-7: The typedef "Resources" (struct Resources with cpu and
memory) is declared but not referenced by the db configuration; either remove
this unused typedef block or wire it into the db values by adding a resources
entry that consumes cpu and memory (e.g., ensure the db chart's values structure
includes a resources object and any templates reference
values.db.resources.cpu/memory or similar). Locate the "Resources" typedef and
either delete it or add a values key (e.g., db.resources) and update relevant
templates/deployment specs to read values.db.resources.cpu and
values.db.resources.memory so the fields are actually used.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: d9aed729-becb-4727-a3d1-b0a9ea155b00
📒 Files selected for processing (13)
docs/superpowers/plans/2026-05-10-split-seaweedfs-system.mddocs/superpowers/specs/2026-05-10-split-seaweedfs-system-design.mdpackages/core/platform/images/migrations/migrations/39packages/core/platform/sources/seaweedfs-application.yamlpackages/extra/seaweedfs/templates/seaweedfs-db.yamlpackages/extra/seaweedfs/templates/seaweedfs.yamlpackages/system/seaweedfs-db/Chart.yamlpackages/system/seaweedfs-db/Makefilepackages/system/seaweedfs-db/README.mdpackages/system/seaweedfs-db/templates/database.yamlpackages/system/seaweedfs-db/values.schema.jsonpackages/system/seaweedfs-db/values.yamlpackages/system/seaweedfs/values.yaml
💤 Files with no reviewable changes (1)
- packages/system/seaweedfs/values.yaml
| ```yaml | ||
| spec: | ||
| waitStrategy: | ||
| name: poller # required for healthCheckExprs to be evaluated | ||
| healthCheckExprs: | ||
| - apiVersion: postgresql.cnpg.io/v1 | ||
| kind: Cluster | ||
| current: has(status.conditions) && status.conditions.exists(e, e.type == 'Ready' && e.status == 'True') | ||
| failed: has(status.conditions) && status.conditions.exists(e, e.type == 'Ready' && e.status == 'False') | ||
| ``` | ||
|
|
||
| `exists` requires at least one matching condition element, so a missing `status.conditions` evaluates to `false` and the wait keeps polling — important early in the lifecycle when the CR has been applied but CNPG hasn't yet stamped any conditions. | ||
|
|
||
| The expression matches every `postgresql.cnpg.io/v1 Cluster` resource managed by the release (only `seaweedfs-db` here). The HR's `wait` phase blocks until `current` returns `true`, which only happens once CNPG sets `Ready=True` on the Cluster. Helm's existing `timeout: 10m` is the outer ceiling — if CNPG never reaches Ready, the HR fails with a clear message instead of silently letting the dependent HR proceed. | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check current Flux version in the repository
echo "Searching for Flux version references..."
rg -n "flux.*version|fluxcd.*v2\." --type=yaml -C2 -g '!docs/**'Repository: cozystack/cozystack
Length of output: 523
🏁 Script executed:
# Search for Flux version pins more broadly
find . -type f \( -name "*.yaml" -o -name "*.yml" -o -name "*.md" -o -name "*.json" \) ! -path "*/.*" | head -50 | xargs grep -l "flux.*2\." 2>/dev/null | head -20Repository: cozystack/cozystack
Length of output: 45
🏁 Script executed:
# Look for version information in common locations
echo "=== Checking for FluxCD version in various config files ==="
for file in fluxcd.yaml flux-release.yaml Chart.yaml kustomization.yaml values.yaml release.yaml; do
find . -name "$file" -type f ! -path "*/.*" -exec grep -H "version" {} \; 2>/dev/null
doneRepository: cozystack/cozystack
Length of output: 50375
🏁 Script executed:
# Search the docs directory for healthCheckExprs references
echo "=== Searching for healthCheckExprs feature documentation ==="
rg -n "healthCheckExprs" --type=yaml --type=markdownRepository: cozystack/cozystack
Length of output: 6096
🏁 Script executed:
# Check for Flux version in the main README or installation files
echo "=== Checking README and main docs ==="
rg -n "flux.*2\.[0-9]" --type=markdown -C 2 docs/ 2>/dev/null | head -40Repository: cozystack/cozystack
Length of output: 701
🏁 Script executed:
# Look at the actual seaweedfs-db template to see current implementation
head -50 ./packages/extra/seaweedfs/templates/seaweedfs-db.yamlRepository: cozystack/cozystack
Length of output: 1451
🏁 Script executed:
# Search for any version requirements or comments about healthCheckExprs
rg -i "healthcheckexprs|helm.*controller|v1.5.0" --type=yaml --type=markdown -B 2 -A 2Repository: cozystack/cozystack
Length of output: 50377
🏁 Script executed:
# Check Flux release notes or documentation in the repo
find . -type f -name "*.md" ! -path "*/.*" -exec rg -l "helm.controller|healthCheckExprs.*version|2\.8" {} \;Repository: cozystack/cozystack
Length of output: 407
🌐 Web query:
Flux 2.8.0 helm-controller v1.5.0 healthCheckExprs feature release
💡 Result:
Flux 2.8.0 (feature release) shipped with helm-controller v1.5.0 on 2026-02-24, and the helm-controller v1.5.0 release introduced the HelmRelease.spec.healthCheckExprs feature (custom health checks via CEL expressions) [1][2]. What changed (healthCheckExprs / CEL health checks): - helm-controller v1.5.0 added support for custom health checks on HelmRelease using CEL expressions via the.spec.healthCheckExprs field [2][3]. - The helm-controller API docs specify that.spec.healthCheckExprs evaluates CEL expressions for custom resources (with per-entry apiVersion/kind, and optional inProgress/failed and required current) and that evaluation order is inProgress → failed → current; the first expression that returns true determines the custom resource’s health status [4]. - The feature is tied to the Helm v4 “kstatus” based health checking flow: the GitHub issue notes it is only available for kstatus health checks implemented in Helm 4 (i.e., the waiter WaitStrategy), and the legacy WaitStrategy won’t be able to use it [5]. Release timing / versions: - Flux v2.8.0 release tag is dated 2026-02-24 and explicitly lists helm-controller v1.5.0 as part of the Flux 2.8.0 component versions [1]. - helm-controller v1.5.0 changelog shows release date 2026-02-20 and documents the defaults/behavior changes plus the addition of.spec.healthCheckExprs support [2].
Citations:
- 1: https://github.com/fluxcd/flux2/releases/tag/v2.8.0
- 2: https://github.com/fluxcd/helm-controller/blob/v1.5.1/CHANGELOG.md
- 3: https://fluxcd.io/blog/2026/02/flux-v2.8.0/
- 4: https://github.com/fluxcd/helm-controller/blob/main/docs/spec/v2/helmreleases.md
- 5: Add support for custom health checks via CEL expressions fluxcd/helm-controller#1382
🏁 Script executed:
# Check for any recent Flux version changes or upgrade PRs
rg -n "flux.*2\.8|helm.*controller.*v1\.5" --type=markdown --type=yaml -C 3 | head -60Repository: cozystack/cozystack
Length of output: 45
🏁 Script executed:
# Look for any issue or PR references about Flux upgrades
find . -type f \( -name "*.md" -o -name "*.yaml" \) ! -path "*/.*" -exec grep -l "flux.*upgrade\|2\.8\.0\|upgrade.*flux" {} \; 2>/dev/nullRepository: cozystack/cozystack
Length of output: 232
🏁 Script executed:
# Check the git history for recent Flux-related changes
git log --oneline --all -30 2>/dev/null | grep -i "flux\|helm.controller"Repository: cozystack/cozystack
Length of output: 45
🏁 Script executed:
# Check the v1.2.0 changelog for Flux upgrade information
grep -A 5 -B 5 "flux\|2\.8" docs/changelogs/v1.2.0.md | head -40Repository: cozystack/cozystack
Length of output: 3265
🏁 Script executed:
# Check if there are any recent version bumps in values
rg "version.*2\.[78]" packages/system/fluxcd/ packages/core/flux-aio/ -A 2 -B 2Repository: cozystack/cozystack
Length of output: 343
🏁 Script executed:
# Look at the fluxcd values to see what version is specified
cat packages/system/fluxcd/values.yaml | grep -A 10 -B 10 "version"Repository: cozystack/cozystack
Length of output: 522
Block merge until Flux is upgraded to 2.8.x.
The PR uses HelmRelease.spec.healthCheckExprs (in packages/extra/seaweedfs/templates/seaweedfs-db.yaml, lines 27–30) which requires helm-controller v1.5.0+, shipped with Flux 2.8.0. The repository is currently pinned to Flux 2.7.x with helm-controller v1.4.3, which does not support this feature. Verify that a Flux upgrade PR exists and will merge first, or gate this PR on the Flux upgrade completing.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/superpowers/specs/2026-05-10-split-seaweedfs-system-design.md` around
lines 68 - 82, This PR adds HelmRelease.spec.healthCheckExprs (see
HelmRelease.spec.healthCheckExprs in
packages/extra/seaweedfs/templates/seaweedfs-db.yaml) which requires
helm-controller v1.5.0+/Flux 2.8.x; because the repo is pinned to Flux 2.7.x
(helm-controller v1.4.3) you must not merge this change until the Flux upgrade
lands—either (a) remove or revert the healthCheckExprs usage and instead use a
supported wait strategy/timeout fallback, or (b) gate this PR on the Flux
upgrade by confirming the Flux 2.8.x upgrade PR exists and will merge first and
add a note in this PR linking that upgrade and/or add CI gating that blocks
merge until the upgrade is merged.
| healthCheckExprs: | ||
| - apiVersion: postgresql.cnpg.io/v1 | ||
| kind: Cluster | ||
| current: has(status.conditions) && status.conditions.exists(e, e.type == 'Ready' && e.status == 'True') | ||
| failed: has(status.conditions) && status.conditions.exists(e, e.type == 'Ready' && e.status == 'False') |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check deployed Flux version for helm-controller
kubectl get deployment -n flux-system helm-controller -o jsonpath='{.spec.template.spec.containers[0].image}' 2>/dev/null || echo "Flux not found"Repository: cozystack/cozystack
Length of output: 78
🏁 Script executed:
# First, let's examine the seaweedfs-db.yaml file
cat -n packages/extra/seaweedfs/templates/seaweedfs-db.yamlRepository: cozystack/cozystack
Length of output: 1752
🏁 Script executed:
# Search for any existing documentation or validation about Flux version in the seaweedfs package
rg -i "flux|helm-controller|version|2\.8" packages/extra/seaweedfs/ -A 2 -B 2Repository: cozystack/cozystack
Length of output: 38057
🏁 Script executed:
# Check if there are any migration or bootstrap scripts that validate Flux version
find packages/extra/seaweedfs -type f \( -name "*.yaml" -o -name "*.sh" -o -name "*.md" \) -exec grep -l "flux\|Flux\|validation\|preflight" {} \;Repository: cozystack/cozystack
Length of output: 166
🏁 Script executed:
# Verify the file structure to ensure umbrella pattern compliance
tree -L 3 packages/extra/seaweedfs/ 2>/dev/null || find packages/extra/seaweedfs -type f -o -type d | head -20Repository: cozystack/cozystack
Length of output: 695
🌐 Web query:
Flux helm-controller healthCheckExprs version requirement 2.8.0
💡 Result:
Flux’s HelmRelease .spec.healthCheckExprs (CEL-based custom health checks) requires helm-controller v1.5.0+; Flux 2.8.0 uses helm-controller v1.5.0 and introduces this support, so it is available starting with Flux 2.8.0 and any helm-controller >=1.5.0.[1][2] What you asked (“Flux helm-controller healthCheckExprs version requirement 2.8.0”): if your Flux version is 2.8.0 (released with helm-controller v1.5.0), you can use .spec.healthCheckExprs on HelmRelease resources.[1][3] Notes/constraints from the official docs: - .spec.healthCheckExprs is evaluated only when Helm wait is enabled (i.e., .spec.disableWait is false) and when the poller wait strategy is used (.spec.waitStrategy.name is poller).[3] - It is implemented via CEL expressions with current (required) and optional inProgress/failed.[3] - The implementation is only available for the kstatus-based health checks in Helm 4 (i.e., the poller wait strategy); users using the legacy wait strategy can’t use the feature.[4] Examples and field semantics are documented here: the HelmRelease “Health check expressions” section.[3]
Citations:
- 1: https://github.com/fluxcd/flux2/releases/tag/v2.8.0
- 2: https://github.com/fluxcd/helm-controller/blob/v1.5.1/CHANGELOG.md
- 3: https://fluxcd.io/flux/components/helm/helmreleases/
- 4: Add support for custom health checks via CEL expressions fluxcd/helm-controller#1382
🌐 Web query:
Flux HelmRelease healthCheckExprs supported version helm-controller
💡 Result:
Flux HelmRelease supports custom health check expressions via the .spec.healthCheckExprs field (CEL expressions), and this feature is tied to the Helm 4 + waitStrategy: poller flow using kstatus readers. [1][2] Supported Helm-controller version (where this feature landed): fluxcd/helm-controller v1.5.0 (released 2026-02-20), which includes “Add support for custom health checks via CEL expressions” and notes that “custom health checks via CEL expressions” are supported via .spec.healthCheckExprs. [2] Implementation/usage notes (to ensure the expressions actually evaluate): - .spec.healthCheckExprs is evaluated only when Helm action wait is enabled (.spec.disableWait is false) and when the poller wait strategy is used (.spec.waitStrategy.name is poller). [1] - The controller evaluates inProgress first, then failed, then current; the first expression that evaluates to true determines health status. [1]
Citations:
- 1: https://fluxcd.io/flux/components/helm/helmreleases/
- 2: https://github.com/fluxcd/helm-controller/blob/v1.5.0/CHANGELOG.md
Document the minimum Flux version requirement for healthCheckExprs.
The healthCheckExprs field on line 27 requires Flux 2.8.x (helm-controller v1.5.0+). If Flux is still on 2.7.x, the HelmRelease will either fail to parse or silently ignore the healthCheckExprs field, defeating the purpose of this readiness check.
Add a comment documenting this dependency:
Proposed documentation addition
+ # REQUIRES: Flux 2.8.x+ (helm-controller v1.5.0+) for healthCheckExprs support
# `poller` waitStrategy is required for healthCheckExprs to be evaluated.
# Without it, the HR flips Ready as soon as helm install applies the Cluster CR,
# before CNPG has bootstrapped postgres.
waitStrategy:🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/extra/seaweedfs/templates/seaweedfs-db.yaml` around lines 27 - 31,
Add a comment immediately above the healthCheckExprs block documenting that this
field requires Flux v2.8.x (helm-controller v1.5.0+) and that on older Flux
(e.g., v2.7.x) the HelmRelease will either fail to parse or silently ignore
healthCheckExprs, removing the readiness check; update the comment to mention
the minimum Flux and helm-controller versions and the resulting behavior so
maintainers know to upgrade Flux before relying on the healthCheckExprs in this
HelmRelease.
…collision Main has a conditional dependsOn:[ingress-nginx-system] in the application HR (added after #2601 branched). PR #2601 added an unconditional dependsOn:[seaweedfs-db]. The merge produced two `dependsOn:` keys in the same HelmRelease spec, which the Helm post-render rejected: Helm install failed for release tenant-root/seaweedfs ...: error while running post render on files: map[string]interface {}(nil): yaml: unmarshal errors Collapse into one list: the db dependency is unconditional; the ingress dependency stays inside the existing `if eq $ingress .Release.Namespace` guard so sub-tenants that inherit ingress from a parent namespace don't deadlock on a non-existent local HR. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
…edfs-system Splits the seaweedfs-system HelmRelease in two so the CNPG Cluster/seaweedfs-db lives in its own HR (seaweedfs-db), and the application HR (seaweedfs-system) dependsOn it. The new HR uses Flux v2 HelmRelease.spec.healthCheckExprs with a CEL expression on Cluster.status.conditions[type=Ready] plus waitStrategy.name: poller, so its Ready=True only flips after the postgres primary is actually serving connections — not just after helm install applied the Cluster CR. This eliminates the seaweedfs-filer CrashLoopBackOff race on a fresh tenant install. With Cilium kubeProxyReplacement: true, socket-LB returns EPERM from connect(2) to ClusterIPs with no Ready endpoints. Pre-split, the filer StatefulSet scheduled concurrently with the CNPG bootstrap (~55–70s of unavailable postgres), each connect() failed EPERM, kubelet exponential restart backoff pushed past the e2e bats 'kubectl wait hr/seaweedfs-system --timeout=2m' window, and the 'Configure Tenant and wait for applications' test failed. Migration 39 adopts existing Cluster/seaweedfs-db resources into the new release on upgrade by rewriting meta.helm.sh/release-name and stamping helm.sh/resource-policy: keep. Requires Flux 2.8.x (helm-controller v1.5.0+). Cozystack currently pins Flux to 2.7.x; a separate PR bumps the pin. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
myasnikovdaniil
force-pushed
the
split-seaweedfs-db
branch
from
28b02d5 to
2e50619
Compare
github-actions
Bot
added
size/L
…collision Main has a conditional dependsOn:[ingress-nginx-system] in the application HR (added after #2601 branched). PR #2601 added an unconditional dependsOn:[seaweedfs-db]. The merge produced two `dependsOn:` keys in the same HelmRelease spec, which the Helm post-render rejected: Helm install failed for release tenant-root/seaweedfs ...: error while running post render on files: map[string]interface {}(nil): yaml: unmarshal errors Collapse into one list: the db dependency is unconditional; the ingress dependency stays inside the existing `if eq $ingress .Release.Namespace` guard so sub-tenants that inherit ingress from a parent namespace don't deadlock on a non-existent local HR. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
1 participant
What this PR does
Splits the
seaweedfs-systemHelmRelease in two so the CNPGCluster/seaweedfs-dblives in its own HR (
seaweedfs-db), and the application HR (seaweedfs-system)dependsOnit. The new HR uses Flux v2HelmRelease.spec.healthCheckExprswith aCEL expression on
Cluster.status.conditions[type=Ready]pluswaitStrategy.name: poller, so itsReady=Trueonly flips after the postgresprimary is actually serving connections — not just after
helm installappliedthe Cluster CR.
This eliminates the
seaweedfs-filerCrashLoopBackOff race on a fresh tenantinstall. With Cilium
kubeProxyReplacement: true, socket-LB returnsEPERMfrom
connect(2)to ClusterIPs with no Ready endpoints. Pre-split, the filerStatefulSet schedule concurrently with the CNPG bootstrap (~55–70 s of unavailable
postgres), each
connect()failedEPERM, kubelet's exponential restart backoffpushed past the e2e bats
kubectl wait hr/seaweedfs-system --timeout=2mwindow,and the
Configure Tenant and wait for applicationstest failed.Changes
packages/system/seaweedfs-db/— chart with only theCluster/seaweedfs-dbtemplate.packages/extra/seaweedfs/templates/seaweedfs-db.yaml(new) — emits the HR withwaitStrategy: poller+healthCheckExprsforpostgresql.cnpg.io/v1 Cluster.packages/extra/seaweedfs/templates/seaweedfs.yaml— addeddependsOn: [seaweedfs-db],lowered
interval: 5m → 30sso the dependency check fires promptly, dropped thenow-unused
db.*pass-through.packages/system/seaweedfs/templates/database.yaml— moved intoseaweedfs-db.packages/system/seaweedfs/values.yaml— droppeddb:block (no longer consumed here).packages/core/platform/sources/seaweedfs-application.yaml— registered the new component.packages/core/platform/images/migrations/migrations/39— adopts existingCluster/seaweedfs-dbresources into the new release on upgrade by rewritingmeta.helm.sh/release-nameand stampinghelm.sh/resource-policy: keep.Same pattern for
monitoring-system(grafana-db, alerta-db) is filed as a follow-up.Release note