The policies are right and nothing here changes them. What is missing is
the reason, and this file has now been changed in both directions inside
a week by people reasoning from the annotations alone, so the reason is
the part worth keeping.
The Job carries no hook-succeeded because the script exits 0 on the
degraded path as well as the clean one, and its log is the only record
that a finalizer outlived the wait; ttlSecondsAfterFinished reaps it on a
timescale a human can act within. The ServiceAccount, Role and
RoleBinding have no such clock, and before-hook-creation fires on a next
activation that after the last uninstall never comes, so without a
terminal policy they sit in the tenant namespace for the life of the
cluster still granting delete over every KafkaTopic in it.
Reaping them cannot strand a running pod, which is the objection the
symmetrical shape was written against: helm applies hook-succeeded only
once the hook is terminal, and activeDeadlineSeconds ends the Job inside
the uninstall hook timeout. hook-failed would decide nothing, because a
failing hook already reaps the earlier successful ones under
hook-succeeded.
Assisted-by: LLM
Signed-off-by: IvanHunters <xorokhotnikov@gmail.com>
(cherry picked from commit 3a43bcf391)
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
A non-zero --request-timeout makes kubectl skip its in-cluster credentials
altogether. client-go consults them only while the assembled config still
equals the default, and the flag lands in that config, so the branch that
would load the ServiceAccount token never runs. kubectl falls back to its
built-in default cluster, localhost:8080, is refused, and the script reads
the refusal as a failed delete and exits 1. A non-zero pre-delete hook
returns from Uninstall.Run before any resource is removed, so no Kafka
release could finish deleting and helm-controller retried forever.
Measured on the pinned image rather than argued from the docs: with the
token and ca.crt mounted and KUBERNETES_SERVICE_HOST set, the same delete
reaches the cluster without the flag, goes to localhost:8080 with
--request-timeout=20s, goes to localhost:8080 with 1m, and reaches the
cluster again with 0.
The three in-tree hooks that work do not pass it. apps/kubernetes names it
in a comment only, apps/harbor and apps/mariadb never mention it, and
apps/tenant records that its DELETE calls "carry kubectl's default
--request-timeout of 0". The watch stays bounded by --timeout, and the Job
by activeDeadlineSeconds, which is that same shape.
The suite now rejects the flag. It needs the "=" because the explanation
in the script names the flag too, and matching the bare name would fail on
the comment rather than on a use.
Assisted-by: LLM
Signed-off-by: IvanHunters <xorokhotnikov@gmail.com>
(cherry picked from commit e63b8595d3)
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
The Job carries no hook-succeeded, so it outlives the hook run and
ttlSecondsAfterFinished reclaims it, which is what leaves a readable
record when the finalizer outlives the wait. Its ServiceAccount, Role and
RoleBinding still carried hook-succeeded and hook-failed, so Helm reaped
them at the end of the run either way and left the Job behind them.
A Job whose ServiceAccount is gone still starts a pod. kubelet mounts the
projected volume without a token, client-go finds no in-cluster config,
kubectl falls back to localhost:8080, and the script takes its catch-all
branch and exits 1. A non-zero pre-delete hook returns from Uninstall.Run
before any resource goes, so the release never finishes deleting and
helm-controller retries it: the e2e run on the merge commit shows six
such pods in five minutes, in two namespaces, every one of them dead the
same way. The hook meant to prevent one stuck Kafka release was instead
stopping all of them.
Give all four objects the same before-hook-creation policy, so the
credentials live exactly as long as the consumer and the next run
replaces them. Both other in-tree hooks that reach the apiserver,
apps/kubernetes and apps/harbor, are symmetric this way already.
The suite now pins the policy on each of the three, because nothing
compared them: the asymmetry rendered cleanly, passed 37 helm-unittest
cases and 5 bats cases, and only a cluster showed it.
Assisted-by: LLM
Signed-off-by: IvanHunters <xorokhotnikov@gmail.com>
(cherry picked from commit 64d5689434)
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
Both suites matched "strimzi.io/cluster=kafka-test" as a bare substring,
and that is a prefix of the sibling release kafka-test2 and of
kafka-test-topics. Rendering the hook with either selector left both
suites green, so neither pinned the scoping they exist to pin. Anchor on
the leading "-l " and on the trailing delimiter; both variants now red in
both suites, and the six earlier hook mutations still red exactly where
they did.
Drop the notMatchRegex on a leading slash. It cannot fail on its own: the
anchored image pattern above rejects a leading slash first, and an empty
_cluster.images-registry renders no slash to begin with. Its rationale
moves into that pattern's comment.
Record why activeDeadlineSeconds and the 180s wait have to move together.
A deadline that fires mid-wait kills the Job, and with backoffLimit: 0 the
hook then returns non-zero, which is the wedged uninstall the deliberate
exit 0 on the timeout branch exists to avoid.
Assisted-by: LLM
Signed-off-by: IvanHunters <xorokhotnikov@gmail.com>
(cherry picked from commit 637098f222)
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
The suite used bats' run helper. make bats-unit-tests does not invoke the bats
binary: it runs hack/cozytest.sh over every hack/*.bats, and that runner
implements @test and little else, so the file died at 'run: command not found'
and took make unit-tests with it. Under bats it was 5/5, which is where the 5/5
in the previous commit came from. Save $? by hand instead, the shape
app-cleanup-hooks_test.bats already uses; verified under both runners.
The fake kubectl no longer defaults DELETE_RC to 0. An unset value made every
failure-path assertion vacuous; now it exits 64 and says so. Cover both
RESTMapper wordings for an absent CRD rather than just the first.
Widen the image pattern to v[0-9]+\.[0-9]+(\.[0-9]+)? like harbor and mariadb:
pinning v1\.32 reintroduced the breakage the digest was made pattern-based to
avoid, one Renovate bump later. Assert the _cluster.images-registry override
and the no-leading-slash default, neither of which was covered: the hook is the
one image in this chart whose pull failure surfaces at uninstall.
Assisted-by: LLM
Signed-off-by: IvanHunters <xorokhotnikov@gmail.com>
(cherry picked from commit 415aa82409)
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
The hook's exit code is the whole contract: a non-zero one returns from
Uninstall.Run before any release resource goes, keeping the topic operator
alive; a zero one lets Helm remove it. delete_test.yaml matches manifest text,
so it stayed green when the timeout branch was put back to exit 1, when the
CRD-absent branch was, and when the selector was widened past this release.
Add a bats suite in the shape hack/app-cleanup-hooks_test.bats already uses for
qdrant and harbor: render the script, run it against a fake kubectl, pin the
exit code of each branch. Each of the three mutations above now turns it red.
Drop hook-succeeded from the delete policy and reclaim the Job with
ttlSecondsAfterFinished instead. The script exits 0 on the degraded path too,
where topics still hold the finalizer, so deleting on success removed the only
record of that seconds after it was written. Assert both in delete_test.yaml.
Pin the hook image by shape rather than by literal digest: Renovate rewrites
digests under packages/*/templates/hooks/ and nothing rewrites tests/, so the
literal turns the next bump red for no defect.
Assisted-by: LLM
Signed-off-by: IvanHunters <xorokhotnikov@gmail.com>
(cherry picked from commit 3326e0fb9e)
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
## What this PR does
Manual backport of #3936 to `release-1.6`.
The automated backport, #4372, pushed conflict markers in
`packages/apps/rabbitmq/README.md` and
`packages/system/rabbitmq-rd/cozyrds/rabbitmq.yaml` as its only commit,
so it would ship them if merged. Both are generated files, and the
conflict is the `tls` block that landed on main after 1.6 was cut.
Everything else applies unchanged, so two differences follow:
- the README parameter table and the cozyrd schema are regenerated from
the 1.6 values with cozyvalues-gen v1.6.0, so they carry the new default
and descriptions without `tls`;
- the 1.6 rabbitmq `Makefile` has no `test` target, so the helm-unittest
sweep would never enter the package and the new preset suite would not
run. The second commit adds the same two lines main already has.
Verified on this branch: the rabbitmq suite passes 4/4, `make -n test`
answers the sweep probe, and a second `make generate` produces no drift.
Change summary, unchanged from #3936: the default `resourcesPreset`
moves from `t1.nano` (128Mi) to `s1.nano` (250m CPU, 512Mi), because a
default-sized RabbitMQ 4.2 broker was OOMKilled on `t1.nano` and never
became Ready. The `resources` and `resourcesPreset` descriptions say the
preset fills every resource `resources` leaves unset.
Close#4372 in favour of this one, or tell me to take a different route
and I will.
### Screenshots
Not a UI change.
### Downstream repositories
Walked the trigger map against the diff: one app chart with a changed
default. The provider follow-up is
cozystack/terraform-provider-cozystack#34, labelled `target/1.7`.
- [ ] No downstream repository is affected by this change
- [ ] [cozystack/website](https://github.com/cozystack/website) -
follow-up:
- [x]
[cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack)
- follow-up: cozystack/terraform-provider-cozystack#34
- [ ]
[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:
- [ ] [cozystack/community](https://github.com/cozystack/community) -
follow-up:
### Release note
```release-note
fix(rabbitmq): raise the default resources preset from t1.nano to s1.nano so a default-sized broker is not OOMKilled. Existing brokers that never set resourcesPreset move to s1.nano on upgrade and roll their pods.
```
With watch granted, kubectl delete waits for the claims to be gone, and
its default for that wait is a week. A claim held by a pod that will not
stop, or a stuck detach, would hold the post-delete hook until
helm-controller's 300s uninstall budget ran out and leave the
HelmRelease stuck uninstalling.
The PVC deletes in the clickhouse, harbor, mariadb and qdrant hooks now
wait 120s at most and report the claims still terminating by name, and
each Job carries activeDeadlineSeconds 270 with a 10s termination grace
period, the numbers packages/apps/tenant/templates/cleanup-job.yaml
documents against that budget, so a kubectl that never returns cannot
hold the hook either.
The mariadb Role no longer grants watch on the named Secrets: they carry
no finalizer, so the delete never reaches the watch, and a resourceNames
rule does not authorize a collection watch anyway.
Signed-off-by: Yan Bondarenko <202671653+yankawai@users.noreply.github.com>
Assisted-by: LLM
(cherry picked from commit f886f26ad4)
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
kubectl delete waits for the object to disappear unless told otherwise,
and that wait is a watch. The post-delete hook Roles of harbor,
clickhouse, mariadb and qdrant granted get, list and delete only, so
every uninstall logged a forbidden watch, the wait was skipped and the
hook reported success before the PVCs were actually gone.
The rules now carry watch for every resource the hooks delete, including
the named Secrets the mariadb hook removes.
Signed-off-by: Yan Bondarenko <202671653+yankawai@users.noreply.github.com>
(cherry picked from commit 711fcc168c)
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
Assisted-by: LLM
The 1.6 rabbitmq Makefile has no test target, so the helm-unittest sweep
never enters the package and the preset suite would sit there unrun.
main gained the target earlier; this brings the same two lines.
Signed-off-by: Yan Bondarenko <202671653+yankawai@users.noreply.github.com>
A default-sized RabbitMQ 4.2 broker was OOMKilled on t1.nano (128Mi)
and never became Ready; its working set peaked at 125.8Mi. s1.nano keeps
the 250m CPU and gives 512Mi. The resources and resourcesPreset
descriptions now say the preset fills every resource that resources
leaves unset. The parameter table and the cozyrd schema are regenerated
from the 1.6 values, which carry no tls block.
(cherry picked from commit 12e64a99cd)
Signed-off-by: Yan Bondarenko <202671653+yankawai@users.noreply.github.com>
kubectl delete waits for the claims to be gone unless told otherwise,
and OpenSearch data pods stop slowly, so the post-delete hook could
outlive helm-controller's 300s uninstall budget on a claim that would
not release and leave the HelmRelease stuck uninstalling.
The delete now runs with --wait=false, the shape the monitoring, etcd
and seaweedfs hooks use: the deletion is requested and the claims go
once their pods have stopped, which the log now says instead of
"complete". The Job carries activeDeadlineSeconds 120 as the backstop
for a kubectl that never returns and keeps only before-hook-creation,
so a failed run and its log survive. The Role drops watch, which
nothing needs once the wait is gone.
Signed-off-by: Yan Bondarenko <202671653+yankawai@users.noreply.github.com>
Assisted-by: LLM
(cherry picked from commit 54ac8ed135)
The OpenSearch operator creates its data volumes through a StatefulSet,
so the PVCs carry no ownerReference, and the chart had no cleanup hook.
Removing the application left every data volume Bound and counting
against the tenant quota, unlike clickhouse, kafka and harbor, whose
post-delete hooks reclaim the operator-owned storage.
A post-delete hook in the same shape as the clickhouse one now deletes
the PVCs labelled opster.io/opensearch-cluster=<release>, with a Role
limited to persistentvolumeclaims and the kubectl image pinned by digest.
Signed-off-by: Yan Bondarenko <202671653+yankawai@users.noreply.github.com>
(cherry picked from commit 13be303fbb)
The Keycloak comment and the helper doc claimed the tenant LimitRange as the
reason for the resources, but cozy-keycloak has no LimitRange and neither
does a tenant that leaves resourceQuotas empty: there the sidecar had no
requests and no limit at all, and the change gives it a 1Gi ceiling where
there was none. The comments and both doc blocks now say what applies to
each caller. No rendered object changes.
Assisted-by: LLM
Signed-off-by: Yan Bondarenko <202671653+yankawai@users.noreply.github.com>
(cherry picked from commit 4dc6065ec9)
The barman-cloud plugin injects its sidecar without resource requests or
limits. Tenant namespaces ship a LimitRange that defaults containers to
128Mi, so the sidecar inherits that default and is OOMKilled mid-backup:
the ObjectStore stays healthy, the Cluster stays Ready, and only the
backup fails. A cgroup high-water mark of 254 MiB on a 38 MB database
shows the default cannot hold it.
The shared cozy-lib helper and the Go path that builds the platform's own
ObjectStore now request 100m/256Mi and cap memory at 1Gi. No CPU limit is
set, matching the entityOperator precedent in the kafka chart: a
throttled sidecar stalls WAL archiving instead of failing it.
The helper is renamed from checksumSidecarConfiguration to
sidecarConfiguration, since it no longer carries only the checksum pin.
Signed-off-by: Yan Bondarenko <202671653+yankawai@users.noreply.github.com>
(cherry picked from commit 9214430bd8)
The upgrade section still promised a password rotation and a regenerated
credentials Secret. With the operator-copy fallback the chart reads the
current passwords from internal-<release>-users and writes the same values,
so nothing rotates and no session breaks.
Assisted-by: LLM
Signed-off-by: Yan Bondarenko <202671653+yankawai@users.noreply.github.com>
(cherry picked from commit 9fac0add84)
Every test rendered with an empty lookup, so the branch that keeps a live
password could be deleted with the suite green. Three fixtures through
kubernetesProvider cover what a fresh render never reaches: a helm-owned
users Secret is reused, a Secret the release does not own stays out of the
manifest while its password still reaches the credentials, and the operator
copy rebuilds a missing users Secret. Values with URI-reserved characters
cover the userinfo encoding, and both Secrets are asserted Opaque again.
Assisted-by: LLM
Signed-off-by: Yan Bondarenko <202671653+yankawai@users.noreply.github.com>
(cherry picked from commit 0e17c6f65e)
On the not-owned path a missing password key produced a generated password
that reached <release>-credentials but never the users Secret the operator
reads, so the dashboard advertised a password MongoDB never had. The render
now fails and names the Secret and the key. The operator fills every key
itself, so only a hand-made partial Secret reaches this branch.
Assisted-by: LLM
Signed-off-by: Yan Bondarenko <202671653+yankawai@users.noreply.github.com>
(cherry picked from commit f0da352b30)
Two paragraphs under Credentials were hard-wrapped at 80 columns, the one
place in this README that was; the rest follows the one-line-per-paragraph
rule the repository asks for in markdown.
Signed-off-by: Yan Bondarenko <202671653+yankawai@users.noreply.github.com>
Assisted-by: LLM
(cherry picked from commit 348a65d18b)
With users set the chart emitted accounts.A.users from the generated
passwords and then appended .Values.config.merge verbatim. A tenant that
also put an accounts block into config.merge, which is the only place to
enable JetStream for the generated account, got the key twice in the
rendered values and the release failed at post-render.
The generated users are now merged into config.merge before it is
rendered: the generated list wins for account A, everything else the
tenant put under config.merge is kept, and a config.merge without users
passes through unchanged.
Signed-off-by: Yan Bondarenko <202671653+yankawai@users.noreply.github.com>
(cherry picked from commit 334f6adac6)
The operator stores the cluster file in the <release>-config ConfigMap,
but the dashboard resource map did not list it and no RBAC granted
tenant subjects access to it, so a tenant could not retrieve the
connection string for its own FoundationDB at all.
Add the ConfigMap to the dashboard resource map and create a
Role/RoleBinding scoped to exactly that ConfigMap (get/list/watch)
for the tenant use-level subjects. Add helm unittest coverage.
Signed-off-by: Yan Bondarenko <202671653+yankawai@users.noreply.github.com>
(cherry picked from commit 9ee2a2375c)
clickhouse-operator derives the Keeper config volume name as
chk-<chk>-deploy-confd-<cluster>-<shard>-<replica>. The CHK is named
<release>-keeper and Release.Name already carries the clickhouse- prefix,
so an application name longer than 15 characters pushes that volume name
past 63 characters: the StatefulSet is rejected, no Keeper pod appears,
and the CHK still reports Completed.
The CHK name is now shortened only when it would overflow, with a short
hash of the release name so two long names stay distinct, and the
ClickHouse zookeeper hosts are built from the same helper. Short names
render exactly as before.
Signed-off-by: Yan Bondarenko <202671653+yankawai@users.noreply.github.com>
(cherry picked from commit 24e43fcdf9)
Address review of the version generator (hack/update-versions.sh):
- Fetch tags with `skopeo list-tags` (registry v2 /tags/list) instead of
the Docker Hub v2 API, whose anonymous pagination caps at 1000 entries
and then returns an error page with no `.results`, silently truncating
the list so the frozen 24.9.2.42 fell outside the window and the run
failed. skopeo is already in build-deps and is how the sibling
generators (postgres, redis, mongodb, opensearch, kubernetes) fetch.
- Make a frozen default tag missing from the registry a hard error
instead of silently falling back to a newer patch, which would move
the default image for existing installs.
- Restore 0644 on the regenerated files (mktemp's 0600 survived the mv).
- Exclude /hack from the chart tarball via .helmignore.
- Refresh files/versions.yaml: v25.8 -> 25.8.32.4, the true latest 25.8
patch now visible with the complete tag list.
- bats: add a missing-frozen-tag hard-error case and make the atomicity
test reach the resolution phase after a valid default rather than
failing at the default-in-set check.
Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io>
(cherry picked from commit e707475f96)
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
Assisted-By: LLM
Add a `version` value (v25.8, v25.3, v24.9; default v24.9) to the
ClickHouse chart, backed by files/versions.yaml and resolved by
templates/_versions.tpl for both the ClickHouse server and Keeper
images, so the Cozystack API returns the engine version (spec.version)
like the other managed-DB charts. Default v24.9 -> 24.9.2.42 keeps
existing installations byte-for-byte identical.
The maintainer-run hack/update-versions.sh generator intersects the
server/keeper Docker Hub tags byte-collated (LC_ALL=C) so comm keeps
tags across the X.9 -> X.10 boundary; freezes the default major to the
tag it already ships (read from versions.yaml, so the freeze follows the
default); errors on a dropped default or an unresolvable major; writes
atomically; and runs on stock macOS bash 3.2 / BSD. Covered by
hack/clickhouse-update-versions_test.bats (10 cases) and the
tests/version_test.yaml helm-unittest suite.
Refs: #1246
Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io>
(cherry picked from commit 3c78c0be56)
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
Assisted-By: LLM
Backport of #3552 to `release-1.6`.
Tenant cilium HelmRelease on this branch sets
`install.remediation.retries: -1` with no `strategy:`, so
helm-controller uses its default install remediation, which is
uninstall. Cilium is the tenant cluster's only CNI, so a tenant whose
install misses its budget gets the CNI torn down and reinstalled once
per cycle, and `-1` lets that repeat without end. `RetryOnFailure` keeps
applied manifests in place and retries the failed install as an upgrade
instead. Strategy is set on both actions because the retry of a failed
install runs as an upgrade.
Shipped broken in v1.6.0, v1.6.1 and v1.6.2. `RetryOnFailure` appears in
none of the 19 helmrelease templates on this branch, and the Flux CRD
release-1.6 already ships accepts it - enum at
`spec.install.strategy.name` and `spec.upgrade.strategy.name`, with the
`retryInterval` CEL guard.
Cost is carried over from the original PR: a genuine upgrade failure now
retries on failed manifests instead of rolling back. Controller can't
tell a retried install from a real upgrade, and what it replaces was an
unbounded rollback-and-retry flap.
Auto backport did not apply here, and it was not a conflict. Bot picked
four commits from the PR branch, three of them empty `ci: re-run`
commits, and `git cherry-pick` exits non-zero on an empty pick with no
unmerged paths, which the action reads as a conflict. Picking
`80a209044` alone is the whole backport and applies with a line offset
only. Resulting tree matches the bot's four-SHA-with-three-skips recipe
byte for byte.
### Testing
- `make test` in `packages/apps/kubernetes` - 18 suites, 186 tests
green.
- New suite is mutation-proven rather than revert-proven: setting
`install.strategy.name` to `Rollback` reds exactly the install test,
removing the `upgrade.strategy` block reds exactly the upgrade test,
other 185 unaffected both times.
- `make unit-tests` and `make test-controllers` green, `make generate`
leaves no drift.
- No migration touched, `targetVersion` still 54.
```release-note
fix(kubernetes): retry the tenant CNI install instead of uninstalling it
```
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>
(cherry picked from commit 80a2090444)
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
An in-place RestoreJob (targetApplicationRef omitted) re-bootstraps the
Postgres app under its own name. On stock v1.6.2 it never comes back up,
for two independent reasons in the same restore patch - both surface only
in place, because a to-copy restore targets a different app.
1. Archive-vs-recovery serverName collision. The driver set the restored
cluster's WAL-archive serverName equal to the recovery source
(bootstrap.serverName = bootstrap.oldName = sourceServerName) while the
chart pinned the archiver serverName to the release name. In place those
coincide, so the cluster archived onto the source's own WAL prefix and
barman-cloud-check-wal-archive refused to start ("Expected empty
archive"). Fixed by giving the restored cluster a fresh
bootstrap.newServerName, keyed by the RestoreJob UID (unique per
restore, stable across reconciles); the chart archives there while
externalClusters still recovers from bootstrap.serverName. This is what
the manual CNPG strategies hit (serverName defaults to the cluster
name); the cozy-default flow dodged it (serverName is <ns>-<app>).
2. useSystemBucket guard. The restore populates explicit S3 coordinates
(destinationPath / s3CredentialsSecret / endpointCA) but left
backup.useSystemBucket=true, so the chart's "bootstrap.enabled is
incompatible with useSystemBucket" guard failed the HelmRelease upgrade
and the cluster never re-rendered. This is what the cozy-default
(system-bucket) flow hits on an in-place restore. Fixed by clearing
useSystemBucket in the restore patch, since the app is now on the
explicit-creds path. The driver's app view (postgresapp.Backup) gains
the field, without omitempty so a false survives the merge patch.
Both are exercised only by an in-place restore; the postgres backup e2e
drove only to-copy, so neither surfaced in CI. A step 50 in-place restore
now guards reason 1 (manual-strategy flow); reason 2 is covered by a Go
unit assertion (a system-bucket in-place e2e needs the platform bucket in
CI and is left as a follow-up).
Signed-off-by: Andrey Kolkov <androndo@gmail.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
(cherry picked from commit 5bcdb8cfb8)
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
Assisted-By: LLM
On the useSystemBucket / cozy-default flow the chart pointed the
clickhouse-backup sidecar's S3_ENDPOINT straight at the projected
cozy-backups-creds.endpoint, which the backup credentials projector
stores as a bare host by design. altinity/clickhouse-backup runs
S3_ENDPOINT through the aws-sdk-go-v2 endpoint resolver, which rejects a
scheme-less value with "Custom endpoint 's3.host' was not a valid URI",
so every backup and restore on the default flow failed (reproduced on a
live v1.6.2 cluster). ClickHouse was the only cozy-default app hit by
this: the CNPG/etcd/velero strategies get a scheme'd URL from the
backupstrategy-controller.endpoint helper, and mariadb/foundationdb
deliberately pair a bare host with a TLS flag.
Prepend https:// on the useSystemBucket path via a helper env plus
$(VAR) expansion (a secretKeyRef value can't be string-interpolated),
mirroring the platform's endpoint helper which force-prepends https for
the always-TLS external S3 ingress. The legacy and bring-your-own-Secret
paths keep reading endpoint verbatim, since that value is tenant-supplied
and already carries its own scheme.
Signed-off-by: Andrey Kolkov <androndo@gmail.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
(cherry picked from commit d646c15f8d)