Skip to content

fix(kubernetes): retry the tenant CNI install instead of uninstalling it - #3552

Merged
Aleksei Sviridkin (lexfrei) merged 4 commits into
mainfrom
fix/tenant-cni-nondestructive-install-retry
Aug 6, 2026
Merged

fix(kubernetes): retry the tenant CNI install instead of uninstalling it#3552
Aleksei Sviridkin (lexfrei) merged 4 commits into
mainfrom
fix/tenant-cni-nondestructive-install-retry

Conversation

@lexfrei

@lexfrei Aleksei Sviridkin (lexfrei) commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

What this PR does

The tenant Cilium HelmRelease carried no install strategy, so Flux applied its default, RemediateOnFailure, which uninstalls the release between attempts. Cilium is the tenant cluster's only CNI, so every cycle takes it away and gives it back when the next install applies the manifests again, and retries: -1 lets that repeat without end.

What sets a cycle off is an install that outruns its budget, which can happen while the tenant still has no registered node, since the operator Deployment cannot become Available until one exists. Remediation neither causes that delay nor is fixed by this change: what it adds is a repeated teardown of the CNI on top of a delay that resolves itself once the node arrives. This removes the amplifier, not the cause.

RetryOnFailure keeps the applied manifests in place and retries a failed install as an upgrade, so the agent DaemonSet rolls out as soon as a worker registers. Both actions carry the strategy because helm-controller selects the active retry from .status.lastAttemptedReleaseAction, and the retry of a failed install runs as an upgrade. With the strategy on install alone that retry still happens, but once it is the last attempted action its own failure is handled by upgrade remediation, whose default is a rollback with no previous release to roll back to. The remediation blocks stay because two branches consult the retry count rather than the strategy and refuse to act once it is exhausted: an absent release reads the install block, an out-of-sync one reads upgrade. retries: -1 keeps both open.

The cost is that a genuine upgrade failure no longer rolls back either, it retries on the failed manifests instead. That is forced, since the controller cannot tell a retried install from a real upgrade, and what it replaces was an unbounded rollback-and-retry flap rather than a stable safety net: upgrade.remediation.retries: -1 with the default rollback strategy already retried forever.

This is the strategy and retry interval that pkg/registry/apps/application/rest.go and internal/operator/package_reconciler.go already set on every HelmRelease they generate, except that they leave remediation nil where this release keeps it. The other tenant addons this chart renders still carry the remediation-only shape; this PR changes the CNI alone, where the teardown removes the cluster's only CNI rather than churning a single component.

Screenshots

Not a UI change.

Downstream repositories

Release note

fix(kubernetes): the tenant Cilium HelmRelease now retries a failed install instead of being uninstalled between attempts, so a failed install no longer takes the tenant cluster's only CNI away while it retries. The same strategy applies to the upgrade action, which it has to: a retried install runs as an upgrade. One consequence is operator-visible, a failed Cilium upgrade in a tenant cluster now retries the new manifests every 30s instead of rolling back to the previous release.

Summary by CodeRabbit

  • Bug Fixes

    • Improved Cilium installation and upgrade reliability by automatically retrying failed operations every 30 seconds.
    • Preserved deployed resources during retries and continued supporting unlimited remediation attempts.
  • Tests

    • Added coverage validating installation and upgrade retry behavior.

The tenant cilium HelmRelease carried no install strategy, so Flux
applied its default, RemediateOnFailure, which uninstalls the release
between attempts. Cilium is the tenant cluster's only CNI, so every
cycle takes it away and gives it back when the next install applies the
manifests again, and retries: -1 lets that repeat without end.

What sets a cycle off is an install that outruns its budget, which can
happen while the tenant still has no registered node, since the operator
Deployment cannot become Available until one exists. Remediation neither
causes that delay nor is fixed by this change; what it adds is a
repeated teardown of the CNI on top of a delay that resolves itself once
the node arrives. This removes the amplifier, not the cause.

RetryOnFailure keeps the applied manifests in place and retries a failed
install as an upgrade, so the rollout completes on its own once a worker
registers. Both actions carry the strategy because the retry of a failed
install runs as an upgrade, which would otherwise fall back to upgrade
remediation. It is the strategy and retry interval cozystack-api and
cozystack-operator already set on every HelmRelease they generate,
except that they leave remediation nil where this release keeps it. The
other tenant addons the chart renders still carry the remediation-only
shape; this one is where the teardown removes the cluster's only CNI
rather than churning a single component.

The cost is that a genuine upgrade failure no longer rolls back either,
it retries on the failed manifests instead. That is forced, since the
controller cannot tell a retried install from a real upgrade, and what
it replaces was an unbounded rollback-and-retry flap rather than a
stable safety net.

The remediation blocks stay, since they cover the branches no strategy
reaches: an absent release, which consults the install block, and an
out-of-sync one, which consults upgrade.

Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
@coderabbitai

coderabbitai Bot commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 88f4ac3b-0c4e-4f39-b2a1-61ff7d7154fd

📥 Commits

Reviewing files that changed from the base of the PR and between ee26a50 and 80a2090.

📒 Files selected for processing (2)
  • packages/apps/kubernetes/templates/helmreleases/cilium.yaml
  • packages/apps/kubernetes/tests/cilium_install_retry_test.yaml

📝 Walkthrough

Walkthrough

The Cilium HelmRelease now uses Flux RetryOnFailure strategies with 30-second intervals for installation and upgrades. Tests cover these settings and unlimited remediation retries.

Changes

Cilium Helm retries

Layer / File(s) Summary
Configure Helm retry strategies
packages/apps/kubernetes/templates/helmreleases/cilium.yaml
The HelmRelease uses RetryOnFailure with 30-second intervals for installation and upgrades. Remediation retries remain unlimited.
Validate retry behavior
packages/apps/kubernetes/tests/cilium_install_retry_test.yaml
Tests verify retry strategies, retry intervals, upgrade-path behavior, and unlimited remediation retries.

Estimated code review effort: 2 (Simple) | ~10 minutes

Suggested reviewers: kvaps, ivanhunters

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main change: retrying tenant CNI installation instead of uninstalling it between attempts.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/tenant-cni-nondestructive-install-retry

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.

@lexfrei
Aleksei Sviridkin (lexfrei) marked this pull request as ready for review August 5, 2026 10:27
@dosubot dosubot Bot added area/kubernetes Issues or PRs related to the tenant Kubernetes app backport Should change be backported on previous release kind/bug Categorizes issue or PR as related to a bug labels Aug 5, 2026
@github-actions github-actions Bot added the size/M This PR changes 30-99 lines, ignoring generated files label Aug 5, 2026
Signed-off-by: Aleksei Sviridkin <f@lex.la>
@lexfrei

Copy link
Copy Markdown
Contributor Author

This is the first run on this branch that actually executed the change. The two before it died during install, before any tenant cluster existed, so the modified HelmRelease was never created and the strategy never ran. Run 31009037915 on e9e56097b got past install and failed in the suites instead, which is what makes it worth reading.

The discriminating check comes out clean, and this time it is not vacuous. Across the whole run there are zero uninstall remediation events and zero UninstallSucceeded on any tenant cilium release, while those releases demonstrably existed and failed repeatedly. On the earlier runs a zero here meant nothing, because nothing had been created to tear down.

The observed behaviour matches what the change describes. One InstallFailed carrying Helm install failed for release cozy-cilium/cilium ... timeout waiting for: [Deployment/cozy-cilium/cilium-operator status: 'InProgress'], then a series of UpgradeFailed carrying failed early due to stalled resources: [Deployment/cozy-cilium/cilium-operator status: 'Failed'] at roughly one a minute. The manifests stay applied and the release is never torn down. The HelmRelease now reads either Unknown with Running 'upgrade' action while a retry is in flight, or False with the upgrade failure once it settles.

A branch without the change gives the contrast on the same day, on the same failure. PR #3466, run 31003691332, carries three uninstall remediation events on its tenant cilium releases, and the cycle is visible in the event ages: InstallFailed at 14m, UninstallSucceeded at 3m46s, InstallFailed again at 3m53s. Same class of failure on both branches, opposite remediation behaviour.

Both runs are still red, and both fail the same way, with node-join failed: fewer than 2 tenant nodes Ready within 18m on kubernetes-latest and kubernetes-previous. That is the expected outcome for a change scoped to removing the teardown rather than to fixing node join, which is what the comment in the diff already says. One run is not a series, so this shows the strategy taking effect, not that it improves the pass rate.

Signed-off-by: Aleksei Sviridkin <f@lex.la>
@lexfrei

Copy link
Copy Markdown
Contributor Author

Third run on this branch, 31025557528 on 4d16f4fdb. The new thing in it is that the cilium release converged, instead of only staying applied. Events on kubernetes-test-previous-version-cilium are InstallFailed and then UpgradeSucceeded on the same release, one minute apart in the event ages, with no teardown between them. The earlier run showed the failed install being retried and never finishing; this one finished.

kubernetes-latest passed outright and kubernetes-previous got past node join, both firsts on this branch. Both tenant workers reached Ready and the LoadBalancer check served its nginx page.

Two other branches ran the same day without this change, 31014597354 and 31020254620, and both carry uninstall remediation on the tenant cilium releases of both versions. They do not fail the same way. 31014597354 fails with node-join failed: fewer than 2 tenant nodes Ready within 18m on kubernetes-latest and kubernetes-previous. 31020254620 gets past node join and fails later, on a tenant workload Deployment that missed its 300s budget.

One run against two, and the two differ. This shows the strategy converging, and says nothing about the pass rate.

The run is still red. kubernetes-previous died later, at kubectl wait hr kubernetes-test-previous-version-csi --timeout=10m, with InstallFailed and UninstallSucceeded ... Helm uninstall remediation on the csi release inside that window. csi.yaml carries the same retries: -1 with no strategy that this PR fixes for cilium, so with cilium out of the way it is the one that now ends the suite. Filing that separately.

@IvanHunters IvanHunters left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM.

Correctly fixes a real bug: the default Flux strategy (RemediateOnFailure) uninstalled the tenant CNI between install attempts, taking the cluster's only network away each cycle. The RetryOnFailure + retryInterval:30s pattern matches what the operator already generates for every HelmRelease (retryInterval 30s = operator default). 194 existing chart tests plus the new test (3/3) pass.

I verified the central rationale against the helm-controller source (internal/reconcile/atomic_release.go), not just the docs: remediation is bypassed only on the retry path of a failed release, but the absent-release branch consults install.remediation.RetriesExhausted() and the out-of-sync branch consults upgrade.remediation.RetriesExhausted() independently of strategy. So retries: -1 is load-bearing — it keeps both admission branches open, a deliberate belt-and-suspenders for the cluster's only network. The user-facing "remediation is ignored for RetryOnFailure" wording only covers the retry path.

Non-blocking note: RetryOnFailure drops rollback on a genuine upgrade failure (broken manifests reapply every 30s instead of rolling back). This is the deliberate trade-off already stated in the release note and consistent with the operator's own convention.

@lexfrei
Aleksei Sviridkin (lexfrei) merged commit a4a8e41 into main Aug 6, 2026
15 of 16 checks passed
@lexfrei
Aleksei Sviridkin (lexfrei) deleted the fix/tenant-cni-nondestructive-install-retry branch August 6, 2026 12:25
@github-actions

github-actions Bot commented Aug 6, 2026

Copy link
Copy Markdown

Backport failed for release-1.6, because it was unable to cherry-pick the commit(s).

Please cherry-pick the changes locally and resolve any conflicts.

git fetch origin release-1.6
git worktree add -d .worktree/backport-3552-to-release-1.6 origin/release-1.6
cd .worktree/backport-3552-to-release-1.6
git switch --create backport-3552-to-release-1.6
git cherry-pick -x 80a20904446e5a574708fdb964f7be03042c2946 4ca64a99f4482610e83d0e65053d6222ea3e01eb e9e56097bc2aa43a57d6946f7434f5ddcaf89fb1 4d16f4fdb2f9cfaed91d4a0e9bd0807aae6a0e5a

Aleksei Sviridkin (lexfrei) added a commit that referenced this pull request Aug 6, 2026
… it (#3581)

## What this PR does

The tenant CSI HelmRelease carried `remediation.retries: -1` with no
strategy, so Flux applied its default, `RemediateOnFailure`. Install
remediation is an uninstall in every case the API offers, so an install
that missed its budget was torn down between attempts: the tenant lost
its CSI node plugin and the StorageClasses this release propagates until
a later attempt applied them again, and that attempt restarted the
DaemonSet rollout instead of resuming one already under way.

That teardown races the rollout it exists to recover, and the run
recorded in #3576 shows how narrowly. Both DaemonSet pods were scheduled
at the same second on both workers, so nothing was waiting on capacity;
the ten minutes went to image pulls. The slower worker started its last
container six seconds after the install timeout fired, and the uninstall
removed it one second after that.

`RetryOnFailure` leaves the manifests applied and retries the failed
release. Both actions carry it because the controller picks the active
retry from `.status.lastAttemptedReleaseAction` and performs that retry
as an upgrade: set on install alone, the retry still happens, but its
own failure is then handled by upgrade remediation, whose default is a
rollback with no previous release to roll back to. The `remediation`
blocks stay, because an absent release consults `install.remediation`
and an out-of-sync one consults `upgrade`, both by retry count rather
than by strategy. This is the change #3552 makes for the tenant CNI, and
the reason for putting the strategy on both actions is the one given
there. It is also the shape upstream prescribes: the "Recommended
settings" block in the [HelmRelease spec shipped with the
helm-controller this
installs](https://github.com/fluxcd/helm-controller/blob/v1.5.1/docs/spec/v2/helmreleases.md#recommended-settings)
puts `strategy.name: RetryOnFailure` under both `install` and `upgrade`
for production deployments.

Nothing changes on the test side, and that is deliberate rather than
deferred. Because no manifest is removed at the attempt boundary, the
rollout continues across it and the kubelet finishes what it started,
while the retry re-applies the same manifests every 30s. The release
therefore reports Ready once the rollout completes rather than once a
whole attempt has fit inside the timeout, so what the existing e2e wait
has to outlast is the rollout and no longer an attempt boundary that
discards it. That widens what the wait tolerates without making it
unbounded: a rollout slower than the wait itself still fails, and this
does not claim otherwise.

Scope. #3576 records more than this fixes, so this does not close it.
The other tenant addon HelmReleases this chart renders keep the
remediation-only shape; CSI is the one this changes.

### Screenshots

Not a UI change.

### Downstream repositories

- [x] No downstream repository is affected by this change
- [ ] [cozystack/website](https://github.com/cozystack/website) -
follow-up:
- [ ]
[cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack)
- follow-up:
- [ ]
[cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack)
- follow-up:
- [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up:
- [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up:
- [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) -
follow-up:
- [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) -
follow-up:
- [ ]
[cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server)
- follow-up:
- [ ]
[cozystack/external-apps-example](https://github.com/cozystack/external-apps-example)
- follow-up:
- [ ] [cozystack/examples](https://github.com/cozystack/examples) -
follow-up:

The trigger map was walked against the diff. The change is a HelmRelease
template and a chart test, not a `values.schema.json` field, a
`values.yaml` default, a version enum, a `release.prefix`, an
`ApplicationDefinition`, a chart source kind or a namespace.

### Release note

```release-note
fix(kubernetes): the tenant CSI HelmRelease now retries a failed install instead of being uninstalled between attempts, so a failed install no longer takes the tenant's CSI node plugin and its propagated StorageClasses away while it retries. The same strategy applies to the upgrade action, which it has to: a retried install runs as an upgrade. One consequence is operator-visible — a failed CSI upgrade in a tenant cluster now retries the new manifests every 30s instead of rolling back to the previous release.
```


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

* **Bug Fixes**
* Improved reliability when installing or upgrading the CSI component by
automatically retrying failed actions every 30 seconds.
* Retains unlimited remediation retries to help recover from transient
failures.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
Aleksei Sviridkin (lexfrei) added a commit that referenced this pull request Aug 8, 2026
…it (#3621)

<!-- Thank you for making a contribution! Here are some tips for you:
- Use Conventional Commits for the PR title: `type(scope): description`
- Types: feat, fix, docs, style, refactor, perf, test, build, ci, chore
- Scopes are not an exhaustive list — pick the most specific scope for
the change and extend the list when a genuinely new area appears.
Examples:
- System components: dashboard, platform, operator, cilium, kube-ovn,
linstor, fluxcd, cluster-api
- Managed apps: postgres, mariadb, redis, kafka, clickhouse,
virtual-machine, kubernetes
- Development and maintenance: api, hack, tests, ci, docs, maintenance
- Breaking changes: append `!` after type/scope (`feat(api)!: ...`) or
add a `BREAKING CHANGE:` footer
- If it's a work in progress, consider creating this PR as a draft.
- Don't hesistate to ask for opinion and review in the community chats,
even if it's still a draft.
- Add the label `backport` if it's a bugfix that needs to be backported
to a previous version.
-->

## What this PR does

Harbor's app chart renders a nested HelmRelease that owns the whole
workload: the Deployments, the inline RedisFailover and its volume. It
had no install strategy, so it got Flux's default, and install
remediation is an uninstall in every case Flux offers. A first install
that misses its budget gets torn down, and the next one provisions the
redis volume and runs the master election again from scratch.

That happens on a loaded runner. harbor-core waits on redis, redis waits
on its volume and the master election, and the nginx Deployment hits the
default `progressDeadlineSeconds` of 600s. Flux reads that as a stalled
resource and fails the install early. The e2e wait budget is 10m, the
same as the deadline, so after a teardown the second attempt has less
time than a first install needs.

`RetryOnFailure` keeps the manifests applied and retries as an upgrade.
Nothing is deleted, so the volume, the elected master and harbor-core's
startup all survive, and the retry costs 30s instead of a full install.
A Deployment clears `ProgressDeadlineExceeded` by itself once its pods
go ready, so the retry lands as soon as the stack converges.

This drops the teardown, not the deadline. The suite applies the release
in one step and waits for it in later ones, so part of the budget is
still left when the fail-early hits. That leftover is useless to a fresh
install and usable to a retry that resumes. Whether the deadline or the
budget also needs moving is a separate question, and an early deadline
only becomes safe once a failed install stops being torn down. In
production the win holds without any of that: a slow first install no
longer deletes the Redis volume and the database.

Both actions carry the strategy. The controller runs the retry of a
failed install as an upgrade, so with install alone the retry's own
failure falls through to upgrade remediation. The remediation blocks
stay: an absent release consults install, an out-of-sync one consults
upgrade, both by retry count rather than by strategy.

It goes on the nested release, not the parent. The parent is built from
the Application by the apiserver, which already stamps this strategy on
both actions. The nested one owns the workload and the volume a
remediation deletes.

Same shape as the tenant CNI and CSI releases in #3552 and #3581, and
the unit test follows theirs. Relates to #3445.

### Screenshots

<!-- REQUIRED for UI changes: attach screenshots or screen recordings
demonstrating
the visual impact of your changes. PRs with UI changes without
screenshots will not be merged. -->

### Downstream repositories

<!--
Cozystack is upstream for repositories that are NOT kept in sync with it
automatically, and nothing in CI compares the two sides, so a change
here can break them silently.

Which change forces what, and which file to touch, is the trigger map in
docs/agents/contributing.md under "Downstream Repositories". Walk it
before you tick anything.

IF YOU ARE AN AI AGENT OR AN LLM, THIS IS PART OF THE TASK, NOT A FORM
TO FILL IN:

1. Walk the trigger map against the actual diff of this PR, file by
file. Do not decide from the PR title or from your memory of what you
changed.
2. For every repository you tick, open the follow-up PR there and link
it on the same line. A ticked box with no link claims work that does not
exist, which is worse than an empty box. Search that repository for an
open PR or issue covering it first, and link that instead of filing a
duplicate.
3. When the follow-up is out of scope, or needs a decision that is not
yours to make, open an issue in that repository instead, link that, and
say so in the PR body. Do not push a speculative PR into another
repository just to fill a line here.
4. Do not tick "No downstream repository is affected" to make the
checklist go away. If you are unsure, leave every box empty and say so
in the PR body, so a human decides.
-->

- [x] No downstream repository is affected by this change
- [ ] [cozystack/website](https://github.com/cozystack/website) -
follow-up:
- [ ]
[cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack)
- follow-up:
- [ ]
[cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack)
- follow-up:
- [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up:
- [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up:
- [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) -
follow-up:
- [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) -
follow-up:
- [ ]
[cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server)
- follow-up:
- [ ]
[cozystack/external-apps-example](https://github.com/cozystack/external-apps-example)
- follow-up:
- [ ] [cozystack/examples](https://github.com/cozystack/examples) -
follow-up:

### Release note

<!--  Write a release note:
- Explain what has changed internally and for users.
- Start with the same `type(scope):` prefix as in the PR title
- Follow the guidelines at
https://github.com/kubernetes/community/blob/master/contributors/guide/release-notes.md.
-->

```release-note
fix(harbor): retry a failed Harbor install instead of uninstalling the release, so a slow first install no longer deletes the Redis volume and starts over
```


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

* **Bug Fixes**
* Harbor installations and upgrades now automatically retry every 30
seconds when failures occur.
* Existing unlimited remediation retries remain available for both
operations.

* **Tests**
* Added automated coverage to verify Harbor retry behavior and
remediation settings.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
Aleksei Sviridkin (lexfrei) added a commit that referenced this pull request Aug 8, 2026
<!-- Thank you for making a contribution! Here are some tips for you:
- Use Conventional Commits for the PR title: `type(scope): description`
- Types: feat, fix, docs, style, refactor, perf, test, build, ci, chore
- Scopes are not an exhaustive list — pick the most specific scope for
the change and extend the list when a genuinely new area appears.
Examples:
- System components: dashboard, platform, operator, cilium, kube-ovn,
linstor, fluxcd, cluster-api
- Managed apps: postgres, mariadb, redis, kafka, clickhouse,
virtual-machine, kubernetes
- Development and maintenance: api, hack, tests, ci, docs, maintenance
- Breaking changes: append `!` after type/scope (`feat(api)!: ...`) or
add a `BREAKING CHANGE:` footer
- If it's a work in progress, consider creating this PR as a draft.
- Don't hesistate to ask for opinion and review in the community chats,
even if it's still a draft.
- Add the label `backport` if it's a bugfix that needs to be backported
to a previous version.
-->

## What this PR does

The tenant chart renders the etcd and seaweedfs releases with no install
strategy, so they get Flux's default, and install remediation is an
uninstall in every case Flux offers. Neither release sets
`spec.uninstall.disableHooks`, which defaults to false, so that
uninstall runs the app chart's `post-delete` cleanup Job. Nothing
unusual is configured anywhere along that chain: fail early, uninstall,
hooks.

Those Jobs delete data on purpose. The etcd one removes the
`data-etcd-<ordinal>` PVCs by name, because the etcd-operator does not
reclaim them when the EtcdCluster goes. The seaweedfs one removes the
volume PVCs by label and explicitly reclaims the keep-protected CNPG
Cluster holding the filer metadata, because nothing else would. Both are
right for a real uninstall, and neither hook is at fault here.
Remediation is what calls them, on a release that is only starting
slowly.

So an install that misses its budget does not restart. It destroys the
datastore the tenant control planes keep their state in, or the object
store's data together with the metadata database that indexes it, and
the next attempt begins from nothing.

That budget is not a comfortable margin. The seaweedfs release has
`timeout: 10m`, and `hack/e2e-install-cozystack.bats` already records
that on a loaded runner the tenant stack only starts creating pods
around 9 to 10 minutes in. The window CI observes and the window that
triggers this are the same window.

`RetryOnFailure` keeps the manifests applied and retries the failed
release as an upgrade. No uninstall happens, so neither cleanup Job runs
and the install already under way carries on instead of starting over.

Both actions carry the strategy. The controller runs the retry of a
failed install as an upgrade, so with install alone the retry's own
failure falls through to upgrade remediation. The remediation blocks
stay: an absent release consults install, an out-of-sync one consults
upgrade, both by retry count rather than by strategy.

One trade is that a real upgrade failure retries instead of rolling
back. That costs no stable safety net. With `retries: -1` and the
default strategy, upgrade remediation was already rolling back and
retrying without end, so the release never rested in the rolled-back
state either.

The other side of the trade is that the uninstall was also the only
automatic clean slate. On a tenant holding no data yet, an install that
wedges on a bad resource state now retries against that state until an
operator removes the release. For a release that owns data that is the
right exchange, because lost data does not come back and a wedged
install does.

This covers two of the tenant's data-owning releases. The monitoring
release carries the same default and a cleanup Job that deletes its PVCs
as well, and is left for its own change, as is the rest of the catalog,
whose nested releases want the question asked one at a time. Same shape
as #3552 and #3581. Relates to #3555 and #3576.

### Screenshots

<!-- REQUIRED for UI changes: attach screenshots or screen recordings
demonstrating
the visual impact of your changes. PRs with UI changes without
screenshots will not be merged. -->

### Downstream repositories

<!--
Cozystack is upstream for repositories that are NOT kept in sync with it
automatically, and nothing in CI compares the two sides, so a change
here can break them silently.

Which change forces what, and which file to touch, is the trigger map in
docs/agents/contributing.md under "Downstream Repositories". Walk it
before you tick anything.

IF YOU ARE AN AI AGENT OR AN LLM, THIS IS PART OF THE TASK, NOT A FORM
TO FILL IN:

1. Walk the trigger map against the actual diff of this PR, file by
file. Do not decide from the PR title or from your memory of what you
changed.
2. For every repository you tick, open the follow-up PR there and link
it on the same line. A ticked box with no link claims work that does not
exist, which is worse than an empty box. Search that repository for an
open PR or issue covering it first, and link that instead of filing a
duplicate.
3. When the follow-up is out of scope, or needs a decision that is not
yours to make, open an issue in that repository instead, link that, and
say so in the PR body. Do not push a speculative PR into another
repository just to fill a line here.
4. Do not tick "No downstream repository is affected" to make the
checklist go away. If you are unsure, leave every box empty and say so
in the PR body, so a human decides.
-->

- [x] No downstream repository is affected by this change
- [ ] [cozystack/website](https://github.com/cozystack/website) -
follow-up:
- [ ]
[cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack)
- follow-up:
- [ ]
[cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack)
- follow-up:
- [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up:
- [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up:
- [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) -
follow-up:
- [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) -
follow-up:
- [ ]
[cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server)
- follow-up:
- [ ]
[cozystack/external-apps-example](https://github.com/cozystack/external-apps-example)
- follow-up:
- [ ] [cozystack/examples](https://github.com/cozystack/examples) -
follow-up:

### Release note

<!--  Write a release note:
- Explain what has changed internally and for users.
- Start with the same `type(scope):` prefix as in the PR title
- Follow the guidelines at
https://github.com/kubernetes/community/blob/master/contributors/guide/release-notes.md.
-->

```release-note
fix(tenant): retry a failed etcd or seaweedfs install instead of uninstalling the release, so a slow first install no longer runs the cleanup hooks that delete the etcd data volumes, the SeaweedFS volumes and the filer metadata database
```


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

* **Reliability Improvements**
* Improved installation and upgrade recovery for etcd and SeaweedFS by
automatically retrying failed operations every 30 seconds.
* Preserved ongoing remediation attempts to help services recover
without uninstalling or rolling back after failures.

* **Tests**
* Added coverage to verify retry behavior and remediation settings for
both services.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
Aleksei Sviridkin (lexfrei) added a commit that referenced this pull request Aug 8, 2026
…3633)

<!-- Thank you for making a contribution! Here are some tips for you:
- Use Conventional Commits for the PR title: `type(scope): description`
- Types: feat, fix, docs, style, refactor, perf, test, build, ci, chore
- Scopes are not an exhaustive list — pick the most specific scope for
the change and extend the list when a genuinely new area appears.
Examples:
- System components: dashboard, platform, operator, cilium, kube-ovn,
linstor, fluxcd, cluster-api
- Managed apps: postgres, mariadb, redis, kafka, clickhouse,
virtual-machine, kubernetes
- Development and maintenance: api, hack, tests, ci, docs, maintenance
- Breaking changes: append `!` after type/scope (`feat(api)!: ...`) or
add a `BREAKING CHANGE:` footer
- If it's a work in progress, consider creating this PR as a draft.
- Don't hesistate to ask for opinion and review in the community chats,
even if it's still a draft.
- Add the label `backport` if it's a bugfix that needs to be backported
to a previous version.
-->

## What this PR does

The tenant chart renders monitoring and gateway with no install
strategy, so they get Flux's default, and install remediation is an
uninstall in every case Flux offers. Neither sets
`spec.uninstall.disableHooks`, which defaults to false. So a release
that missed nothing but a readiness budget gets uninstalled, and the app
chart's `post-delete` cleanup Job runs against it.

Those Jobs delete on purpose, and they are right to. The monitoring one
removes PVCs labelled
`apps.cozystack.io/application.name=<release>-system`. That label is not
incidental either: the metrics and logs claim templates stamp it so the
operator copies it onto the storage PVCs, and the Job selects on it to
stay release-scoped. Both ends were built to meet. The result is that a
slow first install deletes the tenant's metrics and logs, and nothing
reconstructs them.

The gateway Job deletes the `cozystack-acme-account` Secret. That key
regenerates, so this one costs no data. It costs a re-registration
against the ACME provider, and repeated cycles run into its rate limits,
which can hold certificate issuance for the tenant down for hours.
Smaller stake, same mechanism, and the two are not one class: only one
of them owns storage.

`RetryOnFailure` keeps the manifests applied and retries the failed
release, so no uninstall happens and neither Job runs. Both actions
carry the strategy because the controller performs that retry as an
upgrade, so with install alone the retry's own failure falls through to
upgrade remediation. The remediation blocks stay: an absent release
consults install, an out-of-sync one consults upgrade, both by retry
count rather than by strategy.

Two things are traded away. A real upgrade failure now retries instead
of rolling back, which gives up no stable safety net, since with
`retries: -1` the default strategy was already rolling back and
re-upgrading without end. And the uninstall was the only automatic clean
slate, so an install wedged on a bad resource state retries against that
state until an operator removes the release. For a release that owns
data that is the right exchange.

This removes the consequence rather than the trigger, and for monitoring
the trigger is worth naming separately: the parent HelmRelease here
waits `10m`, while the child `monitoring-system` release sets `15m`
deliberately, with a comment explaining that 10m is too short for the
OIDC users-Job inside its `activeDeadlineSeconds: 900`. The parent's
wait covers the child, so the parent can fail while the child is
behaving exactly as designed. That mismatch is untouched here and wants
its own change.

The chart's etcd and seaweedfs releases lose storage the same way and
are outside this change. Beyond those four, computeplane, ingress and
info stay on the default strategy, but their charts ship no destructive
`post-delete` hook, so remediation costs them a wasteful teardown rather
than data.

Same shape as #3552 and #3581. Relates to #3624.

### Screenshots

<!-- REQUIRED for UI changes: attach screenshots or screen recordings
demonstrating
the visual impact of your changes. PRs with UI changes without
screenshots will not be merged. -->

### Downstream repositories

<!--
Cozystack is upstream for repositories that are NOT kept in sync with it
automatically, and nothing in CI compares the two sides, so a change
here can break them silently.

Which change forces what, and which file to touch, is the trigger map in
docs/agents/contributing.md under "Downstream Repositories". Walk it
before you tick anything.

IF YOU ARE AN AI AGENT OR AN LLM, THIS IS PART OF THE TASK, NOT A FORM
TO FILL IN:

1. Walk the trigger map against the actual diff of this PR, file by
file. Do not decide from the PR title or from your memory of what you
changed.
2. For every repository you tick, open the follow-up PR there and link
it on the same line. A ticked box with no link claims work that does not
exist, which is worse than an empty box. Search that repository for an
open PR or issue covering it first, and link that instead of filing a
duplicate.
3. When the follow-up is out of scope, or needs a decision that is not
yours to make, open an issue in that repository instead, link that, and
say so in the PR body. Do not push a speculative PR into another
repository just to fill a line here.
4. Do not tick "No downstream repository is affected" to make the
checklist go away. If you are unsure, leave every box empty and say so
in the PR body, so a human decides.
-->

- [x] No downstream repository is affected by this change
- [ ] [cozystack/website](https://github.com/cozystack/website) -
follow-up:
- [ ]
[cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack)
- follow-up:
- [ ]
[cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack)
- follow-up:
- [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up:
- [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up:
- [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) -
follow-up:
- [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) -
follow-up:
- [ ]
[cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server)
- follow-up:
- [ ]
[cozystack/external-apps-example](https://github.com/cozystack/external-apps-example)
- follow-up:
- [ ] [cozystack/examples](https://github.com/cozystack/examples) -
follow-up:

### Release note

<!--  Write a release note:
- Explain what has changed internally and for users.
- Start with the same `type(scope):` prefix as in the PR title
- Follow the guidelines at
https://github.com/kubernetes/community/blob/master/contributors/guide/release-notes.md.
-->

```release-note
fix(tenant): retry a failed monitoring or gateway install instead of uninstalling the release, so a slow first install no longer runs the cleanup hooks that delete the tenant's metrics and logs volumes or its ACME account key
```


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

* **Reliability Improvements**
* Monitoring and gateway deployments now automatically retry failed
installs and upgrades every 30 seconds.
* Existing unlimited remediation retries remain available, improving
recovery from deployment issues.
* **Validation**
* Added automated coverage to verify retry behavior for both monitoring
and gateway deployments.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/kubernetes Issues or PRs related to the tenant Kubernetes app backport Should change be backported on previous release kind/bug Categorizes issue or PR as related to a bug size/M This PR changes 30-99 lines, ignoring generated files

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants