Skip to content

fix(harbor): retry the nested system install instead of uninstalling it - #3621

Merged
Aleksei Sviridkin (lexfrei) merged 1 commit into
mainfrom
fix/harbor-install-retry-in-place
Aug 8, 2026
Merged

Aleksei Sviridkin (lexfrei) merged 1 commit into
mainfrom
fix/harbor-install-retry-in-place

Conversation

@lexfrei

@lexfrei Aleksei Sviridkin (lexfrei) commented Aug 7, 2026 •

Copy link
Copy Markdown
Contributor

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

Downstream repositories

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

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.

The harbor app chart renders a nested HelmRelease that owns the whole
workload: the harbor Deployments, the inline RedisFailover and its
volume. It carried Flux's default install strategy, and install
remediation is an uninstall in every case Flux offers, so a first
install that went over budget was torn down between attempts and the
next one re-provisioned the redis volume and re-ran the master election
from zero.

That is reachable on a loaded runner. harbor-core blocks on redis while
redis waits for its volume and its master election, and the nginx
Deployment reaches the default progressDeadlineSeconds of 600s, which
Flux reads as a stalled resource and fails the install early. The e2e
wait budget for the release is the same 600s, so one teardown cycle
consumes all of it and the second attempt cannot finish however fast it
converges.

RetryOnFailure keeps the manifests applied and retries the failed
release, so the convergence already under way continues instead of
restarting. Both actions carry the strategy because the controller
performs the retry of a failed install as an upgrade, so on install
alone the retry's own failure falls through to upgrade remediation. The
cost is that a real upgrade failure retries rather than rolls back,
which gives up no stable safety net: with retries: -1 and the default
strategy, upgrade remediation already rolled back and retried without
end. The remediation blocks stay because an absent release consults
install and an out-of-sync one consults upgrade, both by retry count
rather than by strategy.

The change lands on the nested release rather than on the parent: the
parent is built from the Application by the apiserver, which already
stamps this strategy on both actions, while the nested release is the
one that owns the workload and the volume a remediation deletes.

Assisted-By: Claude <[email protected]>
Signed-off-by: Aleksei Sviridkin <[email protected]>
@github-actions github-actions Bot added area/storage Issues or PRs related to storage (linstor, seaweedfs, bucket, velero, harbor) kind/bug Categorizes issue or PR as related to a bug size/L This PR changes 100-499 lines, ignoring generated files labels Aug 7, 2026
@coderabbitai

coderabbitai Bot commented Aug 7, 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: 31a504fd-b216-4966-8c61-eca2c8e23419

📥 Commits

Reviewing files that changed from the base of the PR and between 605030b and f4dc5d4.

📒 Files selected for processing (2)
  • packages/apps/harbor/templates/harbor.yaml
  • packages/apps/harbor/tests/install_retry_test.yaml

📝 Walkthrough

Walkthrough

The nested Harbor HelmRelease now retries failed installs and upgrades every 30 seconds. Helm tests verify both strategies and unlimited remediation retries.

Changes

Harbor retry strategy

Layer / File(s) Summary
Configure Harbor retry behavior
packages/apps/harbor/templates/harbor.yaml
The nested Harbor HelmRelease uses RetryOnFailure with 30-second intervals for installs and upgrades. Both remediation policies retain unlimited retries.
Validate retry behavior
packages/apps/harbor/tests/install_retry_test.yaml
Helm tests render the nested release and verify the install and upgrade strategies, intervals, and unlimited remediation retries.

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

Possibly related PRs

Suggested reviewers: 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: the nested Harbor installation now retries instead of uninstalling.
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/harbor-install-retry-in-place

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) merged commit 79ceaf1 into main Aug 8, 2026
16 checks passed
@lexfrei
Aleksei Sviridkin (lexfrei) deleted the fix/harbor-install-retry-in-place branch August 8, 2026 11:04
myasnikovdaniil pushed a commit that referenced this pull request Oct 26, 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 -->

(cherry picked from commit 79ceaf1)
Signed-off-by: Myasnikov Daniil <[email protected]>
myasnikovdaniil added a commit that referenced this pull request Nov 2, 2026
## What this PR does

Backports to release-1.6, picked from main with `cherry-pick -x` in
merge order. Each commit keeps the original message and its `(cherry
picked from commit …)` line.

- [#3581](#3581)
fix(kubernetes): retry the tenant CSI install instead of uninstalling it
- [#3621](#3621) fix(harbor):
retry the nested system install instead of uninstalling it
- [#3630](#3630) fix(tenant):
retry failed etcd and seaweedfs installs in place
- [#3735](#3735)
fix(vm-instance): disable helm readiness wait on the VMInstance release
- [#4799](#4799) fix: typo in
monitoring-agents values.yaml

### Conflicts resolved by hand

- [#3735](#3735): only
`pkg/config/config_test.go` conflicted. release-1.6 has no
`HelmServerSideApplyAnnotation`, so that test entry is left out, and two
comments in `pkg/config/config.go` and
`internal/controller/applicationdefinition_controller.go` are adjusted
to what exists on this line. The rest is the original as is.

Checked on this branch before opening: `helm template` of
`monitoring-agents` and `vm-instance-rd`, `go build`, `go vet`, `go
test` and the commit trailer check pass. Not run locally: `helm
unittest` (plugin not installed) and `helm template` of `harbor`,
`kubernetes` and `tenant`, which need values to render at base and head
alike. CI runs the unit suites.

### 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:

### 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.

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

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

fix(vm-instance): VMInstance HelmReleases no longer hang InProgress or roll back healthy VMs; the release no longer waits on VirtualMachine readiness, which helm-controller's kstatus poller cannot assess correctly for KubeVirt VMs. Existing VMInstance releases are brought in line automatically on upgrade.

fix: typo in monitoring-agents values.yaml
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/storage Issues or PRs related to storage (linstor, seaweedfs, bucket, velero, harbor) kind/bug Categorizes issue or PR as related to a bug size/L This PR changes 100-499 lines, ignoring generated files

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant