feat(operator): expose HelmRelease generation knobs (interval, retry-interval, timeouts, max-history) by myasnikovdaniil · Pull Request #2509 · cozystack/cozystack

myasnikovdaniil · 2026-04-28T07:47:57Z

Why

The cozystack-operator Package reconciler builds every HelmRelease it generates with a handful of hardcoded values. Three of those have caused recurring pain across the project:

Spec.Interval = 5m (line 220 before this PR) — too long for E2E install where dependent HRs reconcile out of order; previously addressed by a manual flux reconcile --force workaround in hack/e2e-install-cozystack.bats.
Install.Remediation{Retries: -1} + Upgrade.Remediation{Retries: -1} — semantically "retry forever, never remediate", but the retry-on-failure timing was tied to Spec.Interval (5m). Audit of the last 30 successful PR runs found that every run hit kubectl wait hr/seaweedfs-system timeouts at the 2-min mark, recovering only after the inline flux reconcile --force workaround.
Install.Timeout = 10m / Upgrade.Timeout = 10m — directly contradicts the per-Application timeout work in pkg/config/config.go + pkg/registry/apps/application/rest.go (commit 7b146cbe). packages/apps/tenant/templates/etcd.yaml hand-rolls its HR YAML to bypass this for cert rotation; harbor has had multiple commits chasing the same problem.
Spec.MaxHistory unset → Helm default 5. Test clusters bounce HRs frequently and accumulate release-history Secrets.

This PR exposes all four as operator flags + helm chart values, with defaults that preserve current behavior.

What

New operator flags

Flag	Default	HR field
`--helmrelease-interval`	`5m`	`Spec.Interval`
`--helmrelease-retry-interval`	`30s`	`Spec.{Install,Upgrade}.Strategy.RetryInterval`
`--helmrelease-install-timeout`	`10m`	`Spec.Install.Timeout`
`--helmrelease-upgrade-timeout`	`10m`	`Spec.Upgrade.Timeout`
`--helmrelease-max-history`	`5`	`Spec.MaxHistory`

Strategy switch

Install.Strategy.Name and Upgrade.Strategy.Name are now set to RetryOnFailure (instead of leaving the default RemediateOnFailure), with RetryInterval set from the new flag. Functionally equivalent to the previous Remediation{Retries:-1} ("retry forever") since remediation never fired anyway, but decouples retry-on-failure timing from Spec.Interval so failed releases recover at 30s without polling healthy releases at the same fast cadence.

Chart exposure

packages/core/installer/values.yaml gets five new optional values (cozystackOperator.helmRelease{Interval,RetryInterval,InstallTimeout,UpgradeTimeout,MaxHistory}), each defaulting to empty so the operator's own default applies. The chart conditionally renders the corresponding flag only when the value is set.

Production safety

All static defaults match the previous hardcoded values (5m / 10m / 10m / 5). No new code path runs until an admin explicitly sets a flag.
One intentional behavior change: failed-release retry cadence drops from ~5m (previously coupled to Spec.Interval) to 30s (RetryInterval default). This is the whole motivation for the strategy switch — the recurring kubectl wait hr/seaweedfs-system flakes documented above were caused by the previous coupling. Healthy-release reconcile cadence is unchanged at 5m in production; CI overrides this to 30s via --set cozystackOperator.helmReleaseInterval=30s in hack/e2e-install-cozystack.bats for faster convergence on E2E clusters.
RetryOnFailure is functionally equivalent to RemediateOnFailure + Retries:-1 since neither ever triggers remediation.
Empty value in the chart → flag not rendered → operator falls back to its built-in default. (helmReleaseMaxHistory: 0 — unlimited history per Helm semantics — is also honored; the chart guards on ne (toString …) "" rather than truthiness so int 0 is not silently dropped.)

Verification

go build ./... clean
go test ./internal/operator/... passes (covers Interval, MaxHistory, Install/Upgrade.Timeout, Strategy.Name, RetryInterval, ChartRef propagation, plus a regression test for MaxHistory=0)
helm template installer packages/core/installer renders cleanly with default and overridden values; --set cozystackOperator.helmReleaseMaxHistory=0 correctly emits --helmrelease-max-history=0

Followups (deliberately not bundled)

Per-component timeout overrides on ComponentInstall (mirroring the UpgradeCRDs pattern from feat(operator): add per-package upgradeCRDs policy for HelmRelease #2427). Charts like etcd that need a 30m timeout would set it on their Component rather than tuning a global flag. Touches the Package CR API + codegen — bigger lift, separate PR.
Remediation.Retries as a flag. No current pain; skip.
createOrUpdateHelmRelease field preservation. Currently preserves Suspend but not Test/Rollback/Uninstall. Would silently overwrite an SRE's manual edit. Minor cleanliness, separate PR.

Test plan

CI passes E2E with helmReleaseInterval=30s set in install bats (existing T1 test path)
No regression on production install path (default values)
HR objects in a healthy cluster render with Strategy.Name=RetryOnFailure and RetryInterval=30s

Summary by CodeRabbit

New Features
- Operator and Helm chart now accept CLI flags and Helm values to configure HelmRelease reconciliation intervals, retry behavior, install/upgrade timeouts, and history retention.
Bug Fixes
- Operator now validates HelmRelease timing/history inputs on startup and will exit if values are invalid or non-positive.
Tests
- Added unit and e2e coverage to verify HelmRelease configuration and a chart install path using a custom reconciliation interval.

…0s in E2E Adds --helmrelease-interval (default 5m, matching today's behaviour) to the operator and a corresponding cozystackOperator.helmReleaseInterval value on the cozy-installer chart (default empty -> no flag rendered -> operator default 5m applies). E2E install sets it to 30s. Rationale: with the 5m default, dependency- blocked HRs (waiting on cert-manager webhooks, CRDs) are requeued only every 5 min, producing an 8-10 min dead zone in the E2E install where the fast pack of HRs is Ready but the slow tail hasn't been retried yet. 30s collapses the gap. Production defaults are unchanged. The override is opt-in per install. Signed-off-by: Myasnikov Daniil <[email protected]>

… with configurable RetryInterval The operator generated HelmReleases with `Install.Remediation{Retries: -1}` and `Upgrade.Remediation{Retries: -1}`, which is the v2 way of saying "retry forever, never remediate". Combined with the spec-level `Interval: 5m`, this meant a failed install/upgrade waited a full 5 minutes before the next attempt — even when the underlying race that caused the failure (e.g. a chart artifact not quite Ready, a HR dependency reconciling out of order) cleared in seconds. Audit of the last 30 successful PR runs found that every single run had its `Install Cozystack` step's `kubectl wait hr/seaweedfs-system` time out at the 2-minute mark and recover only after an inline `flux reconcile --force` workaround. Same root cause: 5-minute retry interval was longer than the test's wait window. That workaround can be removed once this change lands. Switch to `Strategy.Name=RetryOnFailure` with `RetryInterval` exposed via a new `--helmrelease-retry-interval` operator flag (default 30s): - Functionally equivalent to the previous configuration ("retry forever on failure") since `Retries: -1` meant remediation never fired anyway. - Decouples retry-on-failure timing from `spec.Interval` so failed releases recover at 30s without polling healthy releases at the same cadence (which would multiply controller load by ~10x for no benefit). - Helm chart exposes `cozystackOperator.helmReleaseRetryInterval`, defaulting to empty (operator uses its own 30s default). - Same flag pattern as the existing `--helmrelease-interval` (5m default), also added in this PR via cherry-pick. Verified: go build green, go test ./internal/operator/... pass, helm template renders both flags conditionally on values. Signed-off-by: Myasnikov Daniil <[email protected]>

The HelmRelease generation in package_reconciler hardcoded three values that have caused recurring pain: - Install.Timeout = 10m (line 214) - Upgrade.Timeout = 10m (line 220) - MaxHistory = unset (Helm default 5) The 10m timeout in particular contradicts the per-Application timeout work done in `pkg/config/config.go` and `pkg/registry/apps/application/rest.go` (commit 7b146cb), which lets individual Applications override the install/upgrade timeout via annotation. The PackageSource side had no equivalent — etcd works around it by hand-rolling its HR YAML in `packages/apps/tenant/templates/etcd.yaml` to set a 30m timeout, harbor has had multiple commits chasing the same problem from the other side. This closes that gap. New operator flags (each maps to a chart value with empty default; operator uses its own default when value is empty): - `--helmrelease-install-timeout` (default 10m) → Spec.Install.Timeout - `--helmrelease-upgrade-timeout` (default 10m) → Spec.Upgrade.Timeout - `--helmrelease-max-history` (default 5) → Spec.MaxHistory Production behaviour unchanged. E2E and edge cases (cert rotation, slow-installing charts) can override per-cluster without modifying chart manifests. Per-component overrides on `ComponentInstall` (mirroring the existing `UpgradeCRDs` pattern) would be a strictly better fix for charts like etcd that need an outlier timeout — left as a deliberate followup since it touches the Package CR API. Signed-off-by: Myasnikov Daniil <[email protected]>

coderabbitai · 2026-04-28T07:48:04Z

📝 Walkthrough

Walkthrough

Adds CLI flags and Helm chart values for HelmRelease timing/history, validates and parses duration flags at startup, injects parsed values into PackageReconciler, and uses them to build HelmRelease specs; templates, e2e invocation, and unit tests updated to exercise the new configuration path.

Changes

HelmRelease Configuration Pipeline

Layer / File(s)	Summary
Data Shape `internal/operator/package_reconciler.go`	`PackageReconciler` gains exported fields: `HelmReleaseInterval`, `HelmReleaseRetryInterval`, `HelmReleaseInstallTimeout`, `HelmReleaseUpgradeTimeout`, `HelmReleaseMaxHistory`.
Core Implementation `internal/operator/package_reconciler.go`	Added `buildHelmReleaseSpec(...)` which constructs `helmv2.HelmReleaseSpec` using the new fields: sets `Interval`, pointer `MaxHistory`, Install/Upgrade timeouts, and `RetryOnFailure` strategies (with `RetryInterval`); `Upgrade.CRDs` set from `parseCRDPolicy`.
Spec Application `internal/operator/package_reconciler.go`	Reconciliation now assigns `Spec: r.buildHelmReleaseSpec(...)` when creating HelmRelease objects instead of a previously hardcoded spec.
CLI Parsing & Validation `cmd/cozystack-operator/main.go`	New `--helmrelease-*` flags registered; startup parses durations with `parsePositiveDuration`, enforces durations > 0 and `--helmrelease-max-history >= 0`, and exits on invalid values.
Operator Initialization `cmd/cozystack-operator/main.go`	Parsed HelmRelease duration/MaxHistory values are set onto the `operator.PackageReconciler` instance during initialization.
Helm Values & Template `packages/core/installer/values.yaml`, `packages/core/installer/templates/cozystack-operator.yaml`	New `cozystackOperator.` Helm values added; template conditionally emits corresponding `--helmrelease-` CLI args when values are provided.
E2E Test Configuration `hack/e2e-install-cozystack.bats`	Installer Helm upgrade call now sets `cozystackOperator.helmReleaseInterval=30s`; post-tenant app readiness wait simplified to a direct `kubectl wait` for HelmReleases.
Unit Tests `internal/operator/package_reconciler_test.go`, `cmd/cozystack-operator/main_test.go`	Added tests: `TestBuildHelmReleaseSpec` and `TestBuildHelmReleaseSpecZeroMaxHistory` for spec fields and pointer behavior; `TestParsePositiveDuration` for duration parsing/validation.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

🐰 I count the ticks, I parse the time,

Flags in rows, durations in rhyme.
I build the charts that roll and try,
Retry, timeout — horizons high.
A hop, a spec, the operator's chime.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 37.50% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and specifically describes the main change: exposing HelmRelease configuration parameters (interval, retry-interval, timeouts, max-history) as operator knobs.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

Create stacked PR
Commit on current branch

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch feat/operator-helmrelease-config

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.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

IvanHunters

Blocking: helmReleaseMaxHistory: 0 is silently ignored

In packages/core/installer/templates/cozystack-operator.yaml:

{{- if .Values.cozystackOperator.helmReleaseMaxHistory }}
- --helmrelease-max-history={{ .Values.cozystackOperator.helmReleaseMaxHistory }}
{{- end }}

In Helm, 0 is falsy. A user setting helmReleaseMaxHistory: 0 (unlimited history) will never have the flag passed to the operator — it silently falls back to the default of 5. Fix:

{{- if not (eq (toString .Values.cozystackOperator.helmReleaseMaxHistory) "") }}
- --helmrelease-max-history={{ .Values.cozystackOperator.helmReleaseMaxHistory }}
{{- end }}

Non-blocking notes

The PR description states "all defaults match the previous hardcoded values", but failed install retry cadence changes from ~5m (tied to Spec.Interval) to 30s (RetryInterval default). This is intentional and beneficial, but the description is misleading — worth a clarifying sentence.
No new unit tests for the PackageReconciler fields. Existing tests may not cover that Strategy.Name, RetryInterval, MaxHistory, etc. are actually applied to created HelmRelease objects.
The inline comment at package_reconciler.go:236 is 5 lines — consider condensing to one line since the Why belongs in the commit/PR rather than the source.

coderabbitai

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@cmd/cozystack-operator/main.go`:
- Around line 152-175: The parsed duration variables (hrIntervalDuration,
hrRetryIntervalDuration, hrInstallTimeoutDuration, hrUpgradeTimeoutDuration)
must be validated to be strictly positive because time.ParseDuration accepts
zero and negative values; after parsing each flag (helmReleaseInterval,
helmReleaseRetryInterval, helmReleaseInstallTimeout, helmReleaseUpgradeTimeout)
add a check that the resulting duration > 0 and, if not, call setupLog.Error
with a descriptive message including the flag value and exit (os.Exit(1)); keep
the existing error handling for parse failures and leave the existing
helmReleaseMaxHistory check as-is.

🪄 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: a32ff2b3-8ae2-4c16-a5be-5994ed01329c

📥 Commits

Reviewing files that changed from the base of the PR and between 82d1f8a and e410b02.

📒 Files selected for processing (5)

cmd/cozystack-operator/main.go
hack/e2e-install-cozystack.bats
internal/operator/package_reconciler.go
packages/core/installer/templates/cozystack-operator.yaml
packages/core/installer/values.yaml

Chart template now uses `ne (toString …) ""` for the five helmRelease* knobs, so integer 0 (notably MaxHistory=0 = unlimited history per Helm semantics, documented in values.yaml) is no longer silently dropped. Extract buildHelmReleaseSpec from Reconcile and add unit tests asserting Interval, MaxHistory, Install/Upgrade timeouts, Strategy=RetryOnFailure, RetryInterval, and ChartRef propagate from PackageReconciler config to the generated HelmRelease. Signed-off-by: Myasnikov Daniil <[email protected]>

myasnikovdaniil · 2026-05-04T06:17:00Z

Thanks for the review IvanHunters — addressed in 6ca5566.

Blocking: `helmReleaseMaxHistory: 0` silently ignored

Confirmed and fixed. Helm treats integer 0 as falsy, so {{- if .Values.cozystackOperator.helmReleaseMaxHistory }} dropped the flag for the one value (0 = unlimited history, documented in values.yaml) where users would realistically pass an int rather than a string. Switched all five helmRelease* conditionals to ne (toString …) "" so the same trap is impossible for the duration knobs too:

-{{- if .Values.cozystackOperator.helmReleaseMaxHistory }}
+{{- if ne (toString .Values.cozystackOperator.helmReleaseMaxHistory) "" }}
 - --helmrelease-max-history={{ .Values.cozystackOperator.helmReleaseMaxHistory }}
 {{- end }}

Verified locally:

helm template installer packages/core/installer --set cozystackOperator.helmReleaseMaxHistory=0 now renders --helmrelease-max-history=0.
Default values (empty strings) still skip all five flags.
Explicit overrides (--set helmReleaseInterval=30s etc.) still render.

PR description glosses over retry-cadence change

Fair — "Production behavior unchanged" is technically wrong since the failed-release retry cadence drops from ~5m to 30s on day 1 with no override. That's the intended improvement (the whole reason for the strategy switch), but the framing oversells. Will tighten the PR description to call it out as the one intentional behavior delta.

No unit tests for the new fields

Added in 6ca5566. Extracted buildHelmReleaseSpec from the inline literal in Reconcile so the spec build is unit-testable without a fake-client harness, then added two tests in internal/operator/package_reconciler_test.go:

TestBuildHelmReleaseSpec — asserts Interval, MaxHistory, Install.Timeout, Install.Strategy.{Name,RetryInterval}, Upgrade.Timeout, Upgrade.Strategy.{Name,RetryInterval}, Upgrade.CRDs, and ChartRef all propagate from PackageReconciler config.
TestBuildHelmReleaseSpecZeroMaxHistory — pins MaxHistory=0 to a non-nil pointer-to-zero (mirrors the chart-template trap on the Go side).

go test ./internal/operator/... passes locally.

5-line comment in `package_reconciler.go`

Removed by the same refactor — the comment lived in the inline HR literal that's now gone. The new helper has only a 3-line doc-comment describing what the spec is; the rationale stays in the commit + PR body where it belongs.

gemini-code-assist · 2026-05-04T06:21:40Z

Summary of Changes

Hello, I'm Gemini Code Assist¹! 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 introduces granular control over HelmRelease generation within the cozystack-operator. By exposing key reconciliation parameters as configurable flags and Helm values, it addresses recurring issues with long timeouts and rigid retry cadences, particularly in E2E environments. The changes maintain backward compatibility by preserving existing hardcoded defaults while allowing for fine-tuned performance adjustments in production and testing scenarios.

Highlights

Configurable HelmRelease Parameters: Added new operator flags and Helm chart values to customize HelmRelease reconciliation interval, retry interval, install/upgrade timeouts, and max history.
Decoupled Retry Logic: Switched to RetryOnFailure strategy, decoupling retry-on-failure timing from the main reconciliation interval to improve recovery speed for failed releases.
E2E Test Optimization: Updated E2E installation scripts to utilize the new configurable interval, reducing test flakiness.

🧠 New Feature in Public Preview: You can now enable Memory 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 Assist

The 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 /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	`/gemini review`	Performs a code review for the current pull request in its current state.
Pull Request Summary	`/gemini summary`	Provides a summary of the current pull request in its current state.
Comment	Gemini (@gemini-code-assist)	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	`/gemini help`	Displays a list of available commands.

Customization

To customize 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 .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on Gemini (@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.

Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

gemini-code-assist

Code Review

This pull request makes several HelmRelease parameters configurable within the PackageReconciler, including reconciliation intervals, retry intervals, install/upgrade timeouts, and maximum history. These settings are exposed via new CLI flags in the operator and corresponding Helm values in the installer. The logic for constructing the HelmRelease specification has been refactored into a dedicated helper method with associated unit tests. Feedback was provided to ensure that the MaxHistory pointer is handled safely by using a local variable to avoid potential side effects from sharing a pointer to a struct field across multiple resources.

gemini-code-assist · 2026-05-04T06:26:13Z

+func (r *PackageReconciler) buildHelmReleaseSpec(componentInstall *cozyv1alpha1.ComponentInstall, artifactName string) helmv2.HelmReleaseSpec {
+	return helmv2.HelmReleaseSpec{
+		Interval:   metav1.Duration{Duration: r.HelmReleaseInterval},
+		MaxHistory: &r.HelmReleaseMaxHistory,


Taking the address of a struct field (&r.HelmReleaseMaxHistory) means all generated HelmRelease objects will share the same pointer. While the value is currently only set at startup, it's safer to create a local copy to avoid accidental side effects if the value is ever modified through the pointer elsewhere.

func (r *PackageReconciler) buildHelmReleaseSpec(componentInstall *cozyv1alpha1.ComponentInstall, artifactName string) helmv2.HelmReleaseSpec { maxHistory := r.HelmReleaseMaxHistory return helmv2.HelmReleaseSpec{ Interval: metav1.Duration{Duration: r.HelmReleaseInterval}, MaxHistory: &maxHistory,

The aliasing concern is technically correct, but it can't manifest in this codebase: HelmReleaseMaxHistory is set once from a CLI flag at startup and never mutated, and the controller-runtime client serializes the HR to JSON before sending it to the API server, which dereferences the pointer to a value copy. So every dispatched HelmRelease carries an independent value, not a live reference back to the field.

Adding a local-copy shim would be defensive code for a scenario that can't happen, against project guidance to keep the code minimal. Happy to revisit if anything ever starts mutating that field at runtime.

Fixed in 266465b4 — copying r.HelmReleaseMaxHistory into a local before taking its address so each generated HelmRelease holds an independent *int. Agreed it's safer than relying on the field staying write-once.

time.ParseDuration accepts "0s" and negative values, but Flux HelmRelease.Interval / Install.Timeout / Strategy.RetryInterval require strictly positive durations to function. Validate at startup so a misconfigured flag fails fast instead of propagating into every generated HelmRelease. Signed-off-by: Myasnikov Daniil <[email protected]>

The bats step had two waits for tenant-root child HRs: kubectl wait hr/etcd hr/ingress hr/tenant-root hr/seaweedfs --timeout=4m kubectl wait hr/monitoring hr/seaweedfs-system --timeout=2m The second line was the residue of a flux-reconcile-force workaround removed in 7d292eb; the mechanical cleanup kept the same HRs on a separate 2m wait without revisiting whether the line still made sense. After PR #2509 fixed the underlying timing bug: - monitoring is a top-level tenant-root child, identical shape to etcd and ingress; its Ready is already implied by hr/tenant-root Ready on the first line. - seaweedfs-system is an internal HR of the seaweedfs umbrella chart; its Ready is already implied by hr/seaweedfs Ready on the first line. So the second wait was always tautologically true by the time it ran, and the naming inconsistency (top-level monitoring next to internal seaweedfs-system) was an accident of the historical workaround. Promote monitoring to the first line where it belongs and drop the redundant second wait. Also bump the wait from 4m to 10m. seaweedfs now installs as a serial chain seaweedfs-db (CNPG bootstrap) -> seaweedfs-system (raft quorum) -> seaweedfs wrapper, so the parent tenant-root HR's Ready flip lags ~5-6 min. The previous 4m budget was sized for the old single-HR seaweedfs and is the immediate cause of the e2e failure on PR #2619. The 10m budget stays inside tenant-root HR.spec.timeout (15m). Assisted-By: Claude <[email protected]> Signed-off-by: Myasnikov Daniil <[email protected]>

Audit of 30 successful PR runs found that across 5 sampled failure attempts, **25/25 retries** on `Run E2E tests` failed — the retry loop never recovered a flake, only stretched deterministic failures and tripled diagnostic wall-time. Same data shape on `Install Cozystack`. Drop both. `Prepare environment` keeps its 3x retry — that step is pure infrastructure (Talos image download, sandbox VM boot, network) where transient runner hiccups warrant a retry. On failure, the test step now captures `kubectl get hr -A -o wide` and `kubectl get events -A` under a collapsible group so triage starts with the actual broken-state snapshot. Depends on: - #2508 — installer namespace bootstrap (Helm namespace-ownership conflict) - #2509 — operator HelmRelease config knobs (`seaweedfs-system` 2-min wait race) - #2528 — harbor bucket-secret + BucketInfo gating (harbor ValuesError) - #2529 — objectstorage-controller BucketAccess conflict retry - the daniil/split-vminstance PR (vminstance disk race + VM IP/ready timeouts) - the daniil/split-event-driven PR (existence backstops surfacing real errors) Until those land, dropping the retry will fail CI for unrelated PRs that hit the seaweedfs / harbor / installer / vminstance races. Signed-off-by: Myasnikov Daniil <[email protected]> Assisted-By: Claude <[email protected]> Signed-off-by: Myasnikov Daniil <[email protected]>

The bats step had two waits for tenant-root child HRs: kubectl wait hr/etcd hr/ingress hr/tenant-root hr/seaweedfs --timeout=4m kubectl wait hr/monitoring hr/seaweedfs-system --timeout=2m The second line was the residue of a flux-reconcile-force workaround removed in 7d292eb; the mechanical cleanup kept the same HRs on a separate 2m wait without revisiting whether the line still made sense. After PR #2509 fixed the underlying timing bug: - monitoring is a top-level tenant-root child, identical shape to etcd and ingress; its Ready is already implied by hr/tenant-root Ready on the first line. - seaweedfs-system is an internal HR of the seaweedfs umbrella chart; its Ready is already implied by hr/seaweedfs Ready on the first line. So the second wait was always tautologically true by the time it ran, and the naming inconsistency (top-level monitoring next to internal seaweedfs-system) was an accident of the historical workaround. Promote monitoring to the first line where it belongs and drop the redundant second wait. Also bump the wait from 4m to 10m. seaweedfs now installs as a serial chain seaweedfs-db (CNPG bootstrap) -> seaweedfs-system (raft quorum) -> seaweedfs wrapper, so the parent tenant-root HR's Ready flip lags ~5-6 min. The previous 4m budget was sized for the old single-HR seaweedfs and is the immediate cause of the e2e failure on PR #2619. The 10m budget stays inside tenant-root HR.spec.timeout (15m). Assisted-By: Claude <[email protected]> Signed-off-by: Myasnikov Daniil <[email protected]>

The bats step had two waits for tenant-root child HRs: kubectl wait hr/etcd hr/ingress hr/tenant-root hr/seaweedfs --timeout=4m kubectl wait hr/monitoring hr/seaweedfs-system --timeout=2m The second line was the residue of a flux-reconcile-force workaround removed in 7d292eb; the mechanical cleanup kept the same HRs on a separate 2m wait without revisiting whether the line still made sense. After PR #2509 fixed the underlying timing bug: - monitoring is a top-level tenant-root child, identical shape to etcd and ingress; its Ready is already implied by hr/tenant-root Ready on the first line. - seaweedfs-system is an internal HR of the seaweedfs umbrella chart; its Ready is already implied by hr/seaweedfs Ready on the first line. So the second wait was always tautologically true by the time it ran, and the naming inconsistency (top-level monitoring next to internal seaweedfs-system) was an accident of the historical workaround. Promote monitoring to the first line where it belongs and drop the redundant second wait. Also bump the wait from 4m to 10m. seaweedfs now installs as a serial chain seaweedfs-db (CNPG bootstrap) -> seaweedfs-system (raft quorum) -> seaweedfs wrapper, so the parent tenant-root HR's Ready flip lags ~5-6 min. The previous 4m budget was sized for the old single-HR seaweedfs and is the immediate cause of the e2e failure on PR #2619. The 10m budget stays inside tenant-root HR.spec.timeout (15m). Assisted-By: Claude <[email protected]> Signed-off-by: Myasnikov Daniil <[email protected]> (cherry picked from commit df65b0a)

Audit of 30 successful PR runs found that across 5 sampled failure attempts, **25/25 retries** on `Run E2E tests` failed — the retry loop never recovered a flake, only stretched deterministic failures and tripled diagnostic wall-time. Same data shape on `Install Cozystack`. Drop both. `Prepare environment` keeps its 3x retry — that step is pure infrastructure (Talos image download, sandbox VM boot, network) where transient runner hiccups warrant a retry. On failure, the test step now captures `kubectl get hr -A -o wide` and `kubectl get events -A` under a collapsible group so triage starts with the actual broken-state snapshot. Depends on: - #2508 — installer namespace bootstrap (Helm namespace-ownership conflict) - #2509 — operator HelmRelease config knobs (`seaweedfs-system` 2-min wait race) - #2528 — harbor bucket-secret + BucketInfo gating (harbor ValuesError) - #2529 — objectstorage-controller BucketAccess conflict retry - the daniil/split-vminstance PR (vminstance disk race + VM IP/ready timeouts) - the daniil/split-event-driven PR (existence backstops surfacing real errors) Until those land, dropping the retry will fail CI for unrelated PRs that hit the seaweedfs / harbor / installer / vminstance races. Signed-off-by: Myasnikov Daniil <[email protected]> Assisted-By: Claude <[email protected]>

…edfs-system (#2601) ## What this PR does Splits the `seaweedfs-system` HelmRelease into two: the CNPG `Cluster/seaweedfs-db` moves to 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. ## Stacked on #2602 This PR depends on **#2602** (Flux v2.8.0 upgrade) — `healthCheckExprs` is a helm-controller v1.5.0 feature. GitHub base set to `flux-bump-v048` so the diff cleanly shows only this PR's changes; will auto-rebase onto `main` once #2602 merges. ## Commits 1. **`refactor(seaweedfs)`** — the split itself. Squash of the three iterative commits from the consolidated branch (#2619): the initial split, a `dependsOn` merge-collision fix (collapses unconditional `seaweedfs-db` dependency with the namespace-guarded ingress dependency into one list), and the `healthCheckExprs` three-predicate form (route CNPG's `ClusterIsNotReady` bootstrap condition to `inProgress`, reserve `failed` for other `Ready=False` reasons, lean on the HR `timeout: 10m` as the real backstop). 2. **`fix(tenant)`** — bump parent HR `spec.timeout` from 5m → 15m so the serialized `seaweedfs-db → seaweedfs-system → seaweedfs` chain has enough wall-clock budget. Two complementary changes: the static `tenant-root` HR (created by `cozystack-basics`) gets its `spec.timeout` bumped directly; sub-tenants (Tenant CRs whose HR is built by `cozystack-api` from the `tenant` ApplicationDefinition) get the `release.cozystack.io/helm-install-timeout=15m` annotation that `kubernetes-rd` already uses to opt into the longer budget. 3. **`test(e2e)`** — consolidate two redundant child-HR waits in `hack/e2e-install-cozystack.bats` into one budgeted assert. The second `kubectl wait` (`hr/monitoring hr/seaweedfs-system --timeout=2m`) was a residue of a `flux-reconcile-force` workaround removed in #2509; `monitoring`'s Ready is already implied by `hr/tenant-root` Ready and `seaweedfs-system`'s by `hr/seaweedfs` Ready, so the line was tautologically true by the time it ran. ## Migration `packages/core/platform/images/migrations/migrations/42` adopts existing `Cluster/seaweedfs-db` resources into the new release on upgrade — rewrites `meta.helm.sh/release-name: seaweedfs-system → seaweedfs-db` and stamps `helm.sh/resource-policy: keep` so the `seaweedfs-system` upgrade (which no longer renders the Cluster) does not delete it during the transition. Renumbered from the consolidated branch's slot 40 to fit fresh main (last migration is 41). `targetVersion` bumped 42 → 43. The new HR's `upgrade.force: true` was dropped to match #2602's platform-wide cleanup (Helm v4 in helm-controller v1.5.0 rejects combining `--force-replace` with SSA's `--force-conflicts`). ## Verification - `helm template` renders `seaweedfs-db` HR cleanly with `healthCheckExprs`, `waitStrategy: poller`, and `upgrade: { remediation: { retries: -1 } }` (no `force: true`). - Migration 42 syntax-checks under `sh -n`. - All four PR commits GPG-signed + DCO-signed-off. ### Release note ```release-note NONE ```  ## Summary by CodeRabbit * **New Features** * SeaweedFS database is now available as a separate, independently configurable component with customizable replicas, storage size, and storage class options. * **Chores** * Restructured database configuration from the main application into a dedicated component for improved modularity and initialization order.  [![Review Change Stack](https://storage.googleapis.com/coderabbit_public_assets/review-stack-in-coderabbit-ui.svg)](https://app.coderabbit.ai/change-stack/cozystack/cozystack/pull/2601?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack)

Audit of 30 successful PR runs found that across 5 sampled failure attempts, **25/25 retries** on `Run E2E tests` failed — the retry loop never recovered a flake, only stretched deterministic failures and tripled diagnostic wall-time. Same data shape on `Install Cozystack`. Drop both. `Prepare environment` keeps its 3x retry — that step is pure infrastructure (Talos image download, sandbox VM boot, network) where transient runner hiccups warrant a retry. On failure, the test step now captures `kubectl get hr -A -o wide` and `kubectl get events -A` under a collapsible group so triage starts with the actual broken-state snapshot. Depends on: - #2508 — installer namespace bootstrap (Helm namespace-ownership conflict) - #2509 — operator HelmRelease config knobs (`seaweedfs-system` 2-min wait race) - #2528 — harbor bucket-secret + BucketInfo gating (harbor ValuesError) - #2529 — objectstorage-controller BucketAccess conflict retry - the daniil/split-vminstance PR (vminstance disk race + VM IP/ready timeouts) - the daniil/split-event-driven PR (existence backstops surfacing real errors) Until those land, dropping the retry will fail CI for unrelated PRs that hit the seaweedfs / harbor / installer / vminstance races. Signed-off-by: Myasnikov Daniil <[email protected]> Assisted-By: Claude <[email protected]>

…ay.bats tenant teardown (#2558) ## What this PR does Drops the 3× retry loop on `Run E2E tests` and `Install Cozystack into sandbox`. `Prepare environment` keeps its 3× retry — that step is pure infrastructure (Talos image download, sandbox VM boot, network) where transient runner hiccups warrant a retry. On failure, the test step now captures `kubectl get hr -A -o wide` and `kubectl get events -A` under a collapsible group so triage starts with the actual broken-state snapshot. > [!NOTE] > An earlier revision of this PR also doubled every bats timeout. That commit was dropped in a rebase and is intentionally **not restored**: the timeout class that actually matters (per-app HR-Ready waits) has since been standardized at 5m on `main` (7b9f286), making a blanket 2× redundant. **Fixes gateway.bats teardown leakage.** The nested-tenant tests deleted tenants fire-and-forget, parent and child back-to-back. The leftover uninstalls (each blocked on a cleanup Job, parents wedged on still-terminating child namespaces) plus one mid-install child HR occupied exactly 5 workers on the `--concurrent=5` tenants helm-controller shard, starving whichever app test ran next — observed as the harbor HR sitting unreconciled for its whole 5m HR-Ready budget in [run 27020081550](https://github.com/cozystack/cozystack/actions/runs/27020081550), surfaced by this PR's own retry removal + diagnostics dump. Teardown now deletes child→parent with hard `wait hr --for=delete` between, so a wedged tenant uninstall fails gateway.bats itself, not an innocent neighbor. ## Why Audit of 30 successful PR runs found that across 5 sampled failure attempts, **25/25 retries** on `Run E2E tests` failed — the retry loop never recovered a flake, only stretched deterministic failures and tripled diagnostic wall-time. Same data shape on `Install Cozystack`. Beyond wasted CI time, the retry was hiding ~10 deterministic bugs (Helm namespace-ownership conflict, seaweedfs HR timeout, harbor BucketInfo wiring, vminstance disk race, etc.). Each failure looked like a "flake" because the retry sometimes coincided with whatever transient state had cleared — the retry never fixed the bug, just delayed surfacing. ## Dependencies The deterministic bugs the retry was masking are now fixed on `main`: - ✅ **#2508** — installer namespace bootstrap (Helm namespace-ownership conflict on cold install) — merged - ✅ **#2509** — operator HelmRelease config knobs (`seaweedfs-system` 2-min wait race within Flux's 5-min reconcile windows) — merged - ✅ **#2528** — harbor bucket-secret + BucketInfo gating (harbor `ValuesError` on first install) — merged - ✅ **#2529** — objectstorage-controller BucketAccess conflict retry — merged Companion PRs in the #2619 split (independent of this PR, ordering-wise): - **#2602** — Flux v2.8.0 + chart fixes - **#2601** — seaweedfs-system split This PR does NOT depend on #2602/#2601 — it now touches only the workflow file and gateway.bats teardown, both on top of fresh `main`. Surfaced from #2500. ### Release note ```release-note NONE ```  ## Summary by CodeRabbit * **Chores** * CI prepare-environment step now reports plain attempt counts with clear success/failure messages. * Install and per-app test steps no longer retry; each runs once and fails immediately on error. Failed apps log diagnostics and job proceeds to remaining apps while overall job fails. * **Tests** * End-to-end tests and install/prepare flows use longer, more tolerant timeouts and added existence polling to reduce flakiness and improve diagnostics.  [![Review Change Stack](https://storage.googleapis.com/coderabbit_public_assets/review-stack-in-coderabbit-ui.svg)](https://app.coderabbit.ai/change-stack/cozystack/cozystack/pull/2558?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack)

Lift parsePositiveDuration from cmd/cozystack-operator/main.go into pkg/config so cozystack-api can reuse the same startup validator without duplication. Operator main.go now imports and delegates; the table-driven test moves to pkg/config/config_test.go. Extend ReleaseConfig with the five HelmRelease generation knobs the cozystack-operator already exposes via flags (PR #2509): HelmReleaseInterval, HelmReleaseRetryInterval, HelmReleaseInstallTimeout, HelmReleaseUpgradeTimeout, HelmReleaseMaxHistory. The existing per-Application HelmInstallTimeout (set from the release.cozystack.io/helm-install-timeout annotation) keeps its override semantics - it still wins over the new global Install/Upgrade timeout defaults when set. No behaviour change in this commit; the new fields are wired up in follow-up commits. Signed-off-by: Myasnikov Daniil <[email protected]>

Wire five flags into cozystack-api that mirror the cozystack-operator flags introduced in PR #2509: --helmrelease-interval (default 5m) --helmrelease-retry-interval (default 30s) --helmrelease-install-timeout (default 10m) --helmrelease-upgrade-timeout (default 10m) --helmrelease-max-history (default 5) CozyServerOptions.Complete() validates each duration via config.ParsePositiveDuration so a misconfigured operator restarts loudly instead of waiting until the first Application is created, and rejects MaxHistory < 0 (0 is valid: unlimited per Helm semantics, matching the operator). Validated values are written into every ReleaseConfig built from the ApplicationDefinitionList. The per-Application HelmInstallTimeout (annotation-driven) still wins over the new global Install/Upgrade timeout defaults; this commit only plumbs the values - the consumer in convertApplicationToHelmRelease lands in the next commit. Signed-off-by: Myasnikov Daniil <[email protected]>

convertApplicationToHelmRelease now builds the HelmRelease spec from ReleaseConfig instead of hardcoding values, bringing it in lockstep with cozystack-operator's PackageReconciler.buildHelmReleaseSpec (PR #2509). The drift between the two HelmRelease-generating paths was the rationale for this follow-up. Changes to the generated spec: - Spec.Interval reads ReleaseConfig.HelmReleaseInterval (was hardcoded 5m). - Spec.MaxHistory set as &maxHistory pointer from ReleaseConfig.HelmReleaseMaxHistory; 0 survives as pointer-to-0 (unlimited per Helm semantics) - mirrors the operator's TestBuildHelmReleaseSpecZeroMaxHistory. - Install.Strategy{Name: RetryOnFailure, RetryInterval} replaces Install.Remediation{Retries: -1}. Same for Upgrade.Strategy. Functionally equivalent to "retry forever, never remediate" for the api-side path (which also never relied on remediation firing) but decouples failed-install retry timing from Spec.Interval. - Install.Remediation / Upgrade.Remediation stay nil: helm-controller's XValidation rejects RetryOnFailure + RetryInterval alongside a Remediation entry, so reintroducing Retries:-1 "for safety" would silently break every HR. Install/Upgrade.Timeout precedence: - per-Application HelmInstallTimeout (annotation-driven) wins when non-zero (preserves the kubernetes-rd opt-in path); - otherwise HelmReleaseInstallTimeout / HelmReleaseUpgradeTimeout globals apply; - when both are zero, the field stays nil and flux defaults apply (preserves the legacy code path for tests that hand-construct ReleaseConfig without populating the new globals). Tests: - rest_timeout_test.go: Remediation.Retries == -1 assertions replaced with Strategy.Name == RetryOnFailure + Remediation == nil, matching the operator's spec test. - rest_helmrelease_spec_test.go: new file mirroring TestBuildHelmReleaseSpec / TestBuildHelmReleaseSpecZeroMaxHistory from internal/operator/package_reconciler_test.go, plus a third case asserting per-Application HelmInstallTimeout overrides the global Install/Upgrade timeout defaults. Signed-off-by: Myasnikov Daniil <[email protected]>

…tor tests Production change: - convertApplicationToHelmRelease now sets Install/Upgrade.Timeout unconditionally (matching cozystack-operator's buildHelmReleaseSpec). The previous "stay nil if zero" branch was a leaky test shim — it kept fixtures with empty ReleaseConfig{} green at the cost of api/operator drift. Test changes: - newRESTForTimeout seeds the new HelmRelease* globals with production-shaped defaults so the unset-annotation case exercises "global default applies" rather than "field stays nil". - TestConvertApplicationToHelmRelease_PerAppTimeoutOverridesGlobal is table-driven and now covers asymmetric globals (install != upgrade) with and without per-app override — guards against a regression that only assigns one side under override. - New start_test.go: table-driven validator coverage for each malformed/non-positive duration flag, the negative MaxHistory branch, the MaxHistory=0 unlimited branch, asymmetric timeout acceptance, and a smoke test that NewCozyServerOptions defaults validate. Refactor: - Extracted parseAndValidateHelmReleaseFlags from Complete() so the flag-validation portion is unit-testable without kubernetes I/O. Documentation: - Reworded --helmrelease-install/upgrade-timeout help so admins reading --help see the dual-purpose annotation explicitly (one annotation overrides both timeouts together). - Dropped PR-#2509 references and "preserving the pre-#2509 behaviour for tests" comments — kept the technical rationale (Strategy.Name vs Remediation.Retries:-1 + 5m Interval coupling) on its own merits. Assisted-By: Claude <[email protected]> Signed-off-by: Myasnikov Daniil <[email protected]>