An absent namespace resolves to the root store, and before the
informer's first list every namespace is absent, so a request served
in that window read a labeled namespace's logs from the wrong store.
Readiness keeps the Service from routing there, but a client that
addresses the pod directly is not stopped by it.
The resolver now fails while the cache is unsynced, which answers
503 like any other resolution error.
Assisted-by: LLM
Signed-off-by: ZverGuy <maximbel2003@gmail.com>
vlselect widens the window to step boundaries and returns a
zero-filled bucket for each one, so a start off a boundary yields one
bucket more than the span divided by the step. Sizing the step from
the unaligned span let hits and stats_query_range return MaxBuckets+1
buckets.
The minimum step now spans MaxBuckets-1 steps, and the smallest
accepted --logs-max-buckets rises to 4 so that this step still fits
the third-of-range cap that keeps alignment inside the window.
Assisted-by: LLM
Signed-off-by: ZverGuy <maximbel2003@gmail.com>
When the bucket map was full and the sweep freed nothing, the new
user's bucket was added anyway, so the map grew past its bound and
every further newcomer paid a full scan under the limiter lock.
A newcomer is now refused with 429 until the first bucket refills,
and the Retry-After tells it when. Resetting the map instead would
hand every throttled user a fresh burst.
Assisted-by: LLM
Signed-off-by: ZverGuy <maximbel2003@gmail.com>
A TokenReview whose authenticator failed comes back unauthenticated
with status.error set, and a SubjectAccessReview whose authorizer
failed comes back with an evaluation error and no decision. Both were
cached as refusals, so a valid caller stayed locked out for the cache
TTL after the outage ended.
Such answers are now refused for the current request only. A review
that failed to decide still answers 401 or 403 rather than 503: RBAC
sets an evaluation error for any binding to a missing role, which a
retryable status would turn into a permanent 503 for that user.
Assisted-by: LLM
Signed-off-by: ZverGuy <maximbel2003@gmail.com>
Once the status line has gone out, a read error from the store left
the handler returning normally, so the chunked body ended cleanly and
the client took a partial answer for a complete one.
The handler now aborts the response in that case. The request is
counted before the abort, because the abort skips the rest of the
handler.
Assisted-by: LLM
Signed-off-by: ZverGuy <maximbel2003@gmail.com>
The tail idle timer started only once the store had sent its headers,
and a tail request carries no deadline, so a store that accepted the
request and stayed silent held the tail slot until the client left.
The timer now starts before the request, and the same timer bounds the
header wait and the gap between lines. A store that never answers
gets 504, like a non-tail request past its deadline.
Assisted-by: LLM
Signed-off-by: ZverGuy <maximbel2003@gmail.com>
A client that stopped reading kept its slot for good: the first flush
ran before the tail idle timer started, and cancelling the upstream
request does not interrupt a write that is blocked on the client.
Every response now carries a write deadline. A tail renews it by the
idle timeout before each line, so a tail that keeps receiving stays
open while a stalled reader is cut and frees its slot; any other
response must be written by the request deadline.
Assisted-by: LLM
Signed-off-by: ZverGuy <maximbel2003@gmail.com>
The form was parsed before the concurrency slot was taken, and the
listener sets no body-read deadline, so a client that never finished a
POST body held a handler outside every limit for as long as it liked.
Tenant workloads reach the Service directly, so nginx buffering does
not cover this path.
The slot is now taken first and the body is read under the request
timeout; a body that does not arrive in time answers 408 and frees the
slot. The deadline is lifted once the body is read, because the
server's background read would take a later expiry for a client that
went away and cancel a live tail.
Assisted-by: LLM
Signed-off-by: ZverGuy <maximbel2003@gmail.com>
Only tails opt out of the transport's gzip negotiation; bounded
requests keep it on purpose. Assert that side too, so a change that
applies identity to every request fails a test instead of silently
dropping compression between the proxy and the store.
Assisted-by: LLM
Signed-off-by: ZverGuy <maximbel2003@gmail.com>
Go's transport asks for gzip on its own when a request sets no
Accept-Encoding. vlselect answers a gzip-negotiated tail through a gzip
writer that holds even the response headers until the first line, so the
proxy could not forward its 200 and the console kept showing a
connecting state for quiet applications; buffered lines were also
delayed.
The tail request now sends Accept-Encoding: identity, which also turns
off the transport's transparent gzip. Other endpoints keep it: they are
bounded responses read to the end, compression saves bandwidth between
the proxy and the store, and the transport decompresses before the body
is copied to the client.
Assisted-by: LLM
Signed-off-by: ZverGuy <maximbel2003@gmail.com>
The wait until the next token was converted from float to Duration
without a range check, so a rate limit below about 1e-10 requests per
second produced a negative Retry-After on amd64. The wait now saturates
at the largest Duration.
The earlier 429 change said range, step, bucket and limit violations
keep 422; on the logs root they are clamped or answer 400, and nothing
answers 422.
Assisted-by: LLM
Signed-off-by: ZverGuy <maximbel2003@gmail.com>
parseDuration summed duration parts without an overflow check and
converted out-of-range floats to Duration, which Go leaves
implementation-defined. A start_offset such as
99999999999h99999999999h1s wrapped to 999ms and was accepted, and a
huge step saturated on some architectures and wrapped on others. Such
values now answer 400 like any other invalid duration.
Assisted-by: LLM
Signed-off-by: ZverGuy <maximbel2003@gmail.com>
Clients such as Grafana and browser retry logic back off on 429 Too
Many Requests, not on 422, which they treat as a final rejection of
the query. The per-user rate limit, the per-user concurrency limit and
the open-tail caps are about the caller's pace, not the query, so they
now answer 429. The rate limiter knows when its bucket refills, so that
answer carries Retry-After; the concurrency caps cannot know when a
slot frees and omit it. Range, step, bucket and limit violations keep
422.
Assisted-by: LLM
Signed-off-by: ZverGuy <maximbel2003@gmail.com>
Two of the three windows divided into whole milliseconds, so they
passed with the old truncating step and guarded nothing. Each window
now leaves a sub-millisecond step remainder.
Assisted-by: LLM
Signed-off-by: ZverGuy <maximbel2003@gmail.com>
The step is sent to vlselect in milliseconds and was truncated, so a
window that the bucket cap does not divide evenly got one bucket more
than --logs-max-buckets allows. Round it up before the window is
shrunk to fit the step, so both caps still hold.
Bare-seconds steps and start offsets are now tested on the accepting
side too: only the refused forms were, and disabling the digit branch
passed the suite.
Assisted-by: LLM
Signed-off-by: ZverGuy <maximbel2003@gmail.com>
Bare seconds were parsed with ParseFloat, which also takes signs,
exponents, Inf and NaN. Such a start fell through to the relative
duration form and silently moved the window, so 1.7937864e9 meant
1970 rather than an error, and NaN converted differently per CPU
architecture. Only digits with an optional fraction are taken now;
everything else is a 400.
Assisted-by: LLM
Signed-off-by: ZverGuy <maximbel2003@gmail.com>
vlselect widens [start, end] outward to whole steps before scanning, so
a step of a hundred years turned the clamped week into the whole
retention. The step is now capped at the window and at a third of the
maximum range, and the window is shrunk from its start until the
widened span still fits the maximum range.
Numeric times are accepted only as unambiguous unix seconds, because
vlselect reads the unit from the digit count and a millisecond stamp or
a bare year would otherwise land in the wrong place. The standalone
ignore_global_time_filter substring check is dropped: the options block
check already covers it, and the substring only refused plain searches
for that word.
Assisted-by: LLM
Signed-off-by: ZverGuy <maximbel2003@gmail.com>
A request that hits the proxy's own deadline was answered like an
unreachable store, so a heavy query looked like an outage; it now
answers 504, and a client that went away is no longer logged as a
store failure. A tail line is read through a fixed buffer, so a single
huge log line cannot make one stream hold unbounded memory.
Assisted-by: LLM
Signed-off-by: ZverGuy <maximbel2003@gmail.com>
The label value is spliced into the vlselect host name. A value with a
dot would address a host in some other namespace instead of a tenant's
store, so it is treated as a resolution error rather than followed.
Assisted-by: LLM
Signed-off-by: ZverGuy <maximbel2003@gmail.com>
The console nginx selects its location on the normalized path but
forwards the original request URI. A path such as
namespaces/tenant-b/../tenant-a/logs/ reaches the proxy as written, so
any interpretation the proxy makes of dot segments, empty segments or
percent-encoding could make the namespace in the SubjectAccessReview
differ from the one a normalizer downstream would see. Such paths are
answered 400 instead of being interpreted.
Assisted-by: LLM
Signed-off-by: ZverGuy <maximbel2003@gmail.com>
A substring match on ignore_global_time_filter does not see the option
once it is written as a quoted name with an escape, or split by a
comment: the LogsQL lexer unquotes option names and joins words across
a comment, so the option still applies and lifts the time window the
proxy enforces. None of the query options is needed by a tenant, so
the options keyword followed by a parenthesis is refused outright,
skipping whitespace and comments exactly as the lexer does.
Assisted-by: LLM
Signed-off-by: ZverGuy <maximbel2003@gmail.com>
Tenants get a way to read their namespace's logs without Grafana and
without reaching vlselect directly, which would give them neither an
RBAC check nor a forced scope. The handler authenticates, authorizes
from the path, rebuilds the upstream request with fixed tenant headers
and no client headers, and returns VictoriaLogs' native response.
A live tail is flushed after every line, is not subject to the request
timeout, and is closed once idle, so it survives far past the apiserver
request timeout without leaving forgotten browser tabs holding streams
open. An unreachable store answers 502 so the dashboard can tell an
outage from an empty result.
Assisted-by: LLM
Signed-off-by: ZverGuy <maximbel2003@gmail.com>
The log stores are shared by every namespace routed to them, and the
root store also serves platform Grafana, so one tenant's dashboard must
not be able to monopolise vlselect. Live tails are counted apart from
request concurrency because they hold their slot for minutes and would
otherwise starve the same user's panels.
Assisted-by: LLM
Signed-off-by: ZverGuy <maximbel2003@gmail.com>
vlselect has no notion of a tenant: whatever filter reaches it decides
what is returned. The upstream parameters are therefore rebuilt from a
per-endpoint allowlist with the namespace scope as the only
extra_stream_filters value, so a client cannot widen it through
extra_filters, tenant headers or an alternate parameter spelling.
VictoriaLogs ANDs extra_stream_filters into every subquery, so union
and in() queries pass through unparsed. What the query can still do is
cost: a missing start scans the whole retention, a tiny step returns a
bucket per millisecond, query has no default limit, and
ignore_global_time_filter lifts the window altogether. Each of those is
bounded or refused here, and a tail resume point becomes a capped
start_offset.
Assisted-by: LLM
Signed-off-by: ZverGuy <maximbel2003@gmail.com>
Fluent Bit writes a namespace's logs to the store its monitoring label
names, falling back to the root tenant, so exactly one store answers for
a namespace. Reading with any other rule would show an empty log view
or the wrong tenant's store, so the resolver repeats the collector's
rule and is tested against the same namespace set as its chart test.
It reads a namespace lister, keeping store lookups off the apiserver.
Assisted-by: LLM
Signed-off-by: ZverGuy <maximbel2003@gmail.com>
The proxy holds no credentials of its own: kube-apiserver answers who a
bearer token belongs to and whether that identity may read a scope, the
same way an aggregated API server would ask through delegated auth.
Both answers are cached briefly so a dashboard polling several panels
does not turn into a review per request, while transport errors are
left uncached so an apiserver blip does not lock users out for the TTL.
Assisted-by: LLM
Signed-off-by: ZverGuy <maximbel2003@gmail.com>
The proxy derives its single SubjectAccessReview from the request path,
so the path has to be split and validated before anything else runs.
Names are DNS-1123 checked because they end up quoted inside LogsQL and
PromQL scopes, and percent-encoding is refused so a decoded slash
cannot shift the segments authorization is built from.
Assisted-by: LLM
Signed-off-by: ZverGuy <maximbel2003@gmail.com>
## What this PR does
Seven guides under `docs/` were operator facing and written alongside
the code, and left in the source tree where no user looks. They are on
the website now, so this deletes them here and repoints every reference.
```
docs/operations/backup-classes.md -> operations/services/backup-classes
docs/operations/resource-presets.md -> guides/resource-management
docs/storage-immutability.md -> operations/storage-class-immutability
docs/operations/system-memory-limits.md -> operations/configuration/system-memory-limits
docs/gateway-classes.md -> networking/gateway-classes
docs/observability/distributed-tracing.md -> operations/services/monitoring/distributed-tracing
docs/operations/seaweedfs-capacity-for-backups.md -> operations/services/object-storage/capacity-for-backups
```
`docs/release.md`, `docs/deprecating-applications.md` and the two
SeaweedFS runbooks in `docs/operations/` stay. A workflow, `AGENTS.md`,
a migration and a bats suite read them by path, which is the test from
#3777.
References point at `https://cozystack.io/docs/v1/...`. The site
rewrites that short prefix to the latest version in `layouts/404.html`,
so the links follow releases and pin none. Backup Classes, resource
management and `storageClass` immutability exist in the latest version
already. The other four pages are only in `next/` until 1.7.0 is
promoted, and their links start to resolve then. The generated mongodb
files come from `values.yaml` through `make generate` with
cozyvalues-gen v1.7.0, the version pinned in
`.github/workflows/pre-commit.yml`.
Tests: helm unit tests for mongodb (151), postgres (209), vm-disk (114)
and kubernetes-nodes (223), go tests for backupcontroller, presets and
the operator, rd-presets-check, the migration-50 bats suite.
### Downstream repositories
- [x] [cozystack/website](https://github.com/cozystack/website) -
follow-up: cozystack/website#763
Walked the rest of the trigger map against the diff. No provider
follow-up: the mongodb `values.schema.json` diff is one `description`
string, with no field added, removed or renamed, no enum, no default,
and no `kind`, `plural` or `release.prefix` change.
`packages/core/platform/values.yaml` changed in comments only and no
key, so the hand written platform-package table on the site is
unaffected.
Please merge after cozystack/website#763, the links in the code only
resolve once those pages deploy.
### Release note
```release-note
docs: seven guides move from the docs/ directory of this repository to cozystack.io (Backup Classes, resource presets, storageClass immutability, system component memory limits, GatewayClass selection, distributed tracing, SeaweedFS capacity for backups), and in-repo references point there.
```
Reconcile the tap digest-pin change with #5071 (no-install-on-connect), now
merged into main. Keep this branch's resetTapPinState, the verify-before-pull
Trust text, and the index-README trust model, while adopting #5071's wording
that installing (not tapping) runs charts and dropping the stale
"re-registration" phrasing from the shared tap.go and rest.go comments.
Conflicts resolved in cmd/cozypkg/cmd/tap.go, pkg/registry/core/tap/rest.go,
and hack/packages-index/README.md.
Assisted-by: LLM
Signed-off-by: IvanHunters <xorokhotnikov@gmail.com>
<!-- 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 `kind/backport` if it's a bugfix that needs to be
backported to a previous version.
-->
## What this PR does
Fixes#5014.
A bucket's `Workload` was named `"bucket-" + <BucketClaim name>`, and a
Bucket application names its claim after its release, which already
starts with `bucket-`. A bucket an operator knows as `cozy-backups`
therefore appeared as `bucket-bucket-cozy-backups`, and that string is
the `workload` of the bucket's consumer in every billing usage report.
- **Naming.** The `bucket-` prefix is now added only when the claim name
does not already carry it. A Bucket application's claim
`bucket-cozy-backups` gives the `Workload` `bucket-cozy-backups`; a
claim without the prefix, such as `my-bucket`, still gives
`bucket-my-bucket`. In the tree today every monitored BucketClaim comes
from the Bucket application (Harbor's registry claim carries no labels
its monitors select, so it has no `Workload` on either side of this
change).
- **Migration of existing Workloads.** Nothing garbage-collects bucket
`Workload`s by name: they are owned by their BucketClaim and live as
long as it does. Without a migration the old `bucket-bucket-…` object
would stay next to the new one and the bucket would be metered twice.
When the new `Workload` is reconciled, the controller looks up the
legacy name and, only if the same BucketClaim (matched by UID) is its
sole owner, uses its recorded sizes as the starting point and then
deletes it, with UID and resourceVersion preconditions so a `Workload`
that took over the name in the meantime survives. If the legacy
`Workload` has other owners as well, it is another bucket's renamed
`Workload` that a rolled-back controller attached this claim to: the
controller takes neither its sizes nor the object and only drops its own
owner reference. A legacy `Workload` with no owners left (a claim
deleted with `--cascade=orphan` and recreated) that still carries this
monitor's label is deleted the same way, without taking its sizes. Sizes
already recorded at the new name win over the legacy ones only when the
claim already owned that `Workload`. Carrying the sizes over matters
because the controller deliberately keeps the last known size when the
object-store metrics are unavailable; a fresh `Workload` would otherwise
report no size until the next successful poll, which billing reads as an
empty bucket.
- **Never taking over another claim's Workload.** With Bucket
applications `x` and `bucket-x` in one namespace, the legacy name of the
first (`bucket-bucket-x`) is the new name of the second. The controller
never takes over a `Workload` that has owners when the claim is not one
of them (in practice another BucketClaim). While the new name is held
that way, it keeps metering the bucket under its legacy name, without
migrating, and emits a `BucketWorkloadNameTaken` warning on the monitor.
For Bucket applications the other claim renames its `Workload` away on
its own reconcile and the migration completes on the next pass. Only
when the legacy name is held as well — which for a claim without the
prefix, whose two names coincide, means a claim `x` outside the Bucket
application next to the application `x` — is the bucket skipped, and the
warning says it is not metered. Without this, whichever claim reconciled
first decided whether the second bucket ended up with the first one's
size or with no size at all. The same rule keeps two claims that map to
one name (`x` and `bucket-x`) from sharing a `Workload`.
**Rollback.** The migration is one-way. A controller rolled back to a
previous version recreates `bucket-bucket-<name>` next to the renamed
`bucket-<name>`, which its monitor keeps alive, so the bucket is metered
twice until the `bucket-<name>` Workloads of Bucket applications are
deleted by hand. Upgrading again after such a rollback converges without
deleting another bucket's `Workload` (see above).
**Known limitation.** Taking over an ownerless `Workload` keeps the
sizes recorded on it as the last known ones, as the controller always
has. If such an object was left behind by another bucket (for example
`kubectl delete bucketclaim --cascade=orphan`), its sizes count for the
new claim until the next successful size poll.
**Effect on billing, worth knowing before the upgrade:** `Workload`
names are the identity billing integrates over. A bucket's consumer
changes name once, at the upgrade, so a usage report whose window spans
the upgrade lists the bucket twice: under the old name up to the switch
and under the new one after it. The totals are right; the split is
cosmetic. Reports for windows entirely before or after the upgrade are
unaffected.
Tests (`internal/controller/workloadmonitor_controller_test.go`):
- `TestBucketWorkloadName`: the naming rule for a Bucket application
claim, a plain claim and a Harbor claim.
- `TestReconcileBucketApp_NoDoublePrefix`: a Bucket application gets
`bucket-cozy-backups` and no `bucket-bucket-cozy-backups`.
- `TestReconcileBucketApp_ReplacesLegacyWorkload`: an existing
`bucket-bucket-cozy-backups` owned by the claim is deleted, and its
recorded logical and physical sizes (no metrics endpoint in the test)
are carried over to the new `Workload`.
- `TestReconcileBucketApp_KeepsLegacyNameOwnedByAnotherClaim`: a
`Workload` under the legacy name owned by a different claim is left
alone and its size is not taken.
- `TestReconcileBucketApps_LegacyNameIsAnotherAppsNewName`: Bucket
applications `x` and `bucket-x` with their legacy `Workload`s,
reconciled in both orders, end with `bucket-x` and `bucket-bucket-x`,
each with one owner and its own size, and the legacy
`bucket-bucket-bucket-x` gone.
- `TestReconcileBucketClaim_DoesNotAdoptAnotherClaimsWorkload`: a claim
whose name is held by another claim's `Workload` leaves it untouched,
emits `BucketWorkloadNameTaken` and is metered under its legacy name.
-
`TestReconcileBucketClaim_KeepsMeteringUnderLegacyNameWhileNewNameIsHeld`:
the legacy `Workload` keeps being updated, with its size and owner,
across repeated reconciles while the new name is held.
- `TestReconcileBucketClaim_UnprefixedClaimWithHeldNameIsSkipped` and
`TestReconcileBucketClaim_BothNamesHeldIsSkipped`: when no free name is
left, nothing is taken over and the event says the bucket is not
metered.
- `TestReconcileBucketClaim_UnprefixedClaimKeepsItsWorkload`: for a
claim without the prefix, where the legacy and the new name coincide,
its own `Workload` and size survive repeated reconciles.
- `TestReconcileBucketApp_RecordedSizeWinsOverLegacy`: a size already
recorded on the new `Workload` takes precedence over the legacy one.
- `TestReconcileBucketClaim_UpdatesItsOwnWorkload`: a claim's own
`Workload`, with its owner reference carrying the `BucketClaim` kind as
a live cluster's cache records it, keeps being updated and raises no
warning (the fake client leaves that kind empty on objects the
controller creates, so the other tests do not exercise this).
- `TestReconcileBucketClaim_AdoptsOwnerlessWorkload`: a `Workload` that
lost all owner references is taken over by the claim whose name it
carries, rather than blocking that name forever.
- `TestReconcileBucketApp_ReleasesSharedLegacyWorkload`: a legacy
`Workload` that another claim also owns (the rollback case) keeps its
size and its other owner; only this claim's reference is dropped, and
its size is not carried over.
- `TestReconcileBucketApp_KeepsWorkloadThatReplacedLegacy`: a `Workload`
that replaces the legacy one under the same name between the lookup and
the delete survives.
- `TestReconcileBucketApp_RemovesOrphanedLegacyWorkload`,
`TestReconcileBucketApp_KeepsOrphanOfAnotherMonitor`: an ownerless
legacy `Workload` with this monitor's label is removed without carrying
its sizes; one labelled for another monitor is left alone.
- `TestReconcileBucketApp_LegacySizeWinsOverOwnerlessNewWorkload`: a
stale size on an ownerless `Workload` at the new name loses to the
migrating legacy `Workload`'s size.
- `TestReconcileBucketApp_LegacyAlreadyGone`: a legacy `Workload` that
disappears between the lookup and the delete (NotFound) does not fail
the per-claim reconcile, and the size is still carried over.
Reverting the naming rule, the legacy delete, the size carry-over, the
owner check, the refusal to adopt, the same-name guard, the size
precedence ignoring NotFound on the legacy delete, the own-claim match
in the adoption check, the ownerless case, the physical size carry-over,
the sole-owner rule for legacy Workloads, the delete preconditions, the
legacy-name fallback or its own held check, the ownerless-legacy rule or
the size precedence on takeover each fails one of these. `go test
./internal/... -count=1` and `go vet ./internal/controller/` pass.
### Screenshots
N/A — no UI changes.
### Downstream repositories
Walked the trigger map in `docs/agents/contributing.md` against the
diff, which touches only
`internal/controller/workloadmonitor_controller.go` and its test: no
package, values, schema, CRD, variant, `hack/` tooling, telemetry metric
or cozy-proxy contract changes.
- [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:
- [ ] [cozystack/community](https://github.com/cozystack/community) -
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(workloadmonitor): Workloads of Bucket applications are no longer named with a doubled "bucket-" prefix (bucket-bucket-<name> becomes bucket-<name>). Existing Workloads are replaced on upgrade, keeping their recorded size; billing usage reports whose window spans the upgrade list such a bucket once under each name. Rolling the controller back afterwards requires deleting the bucket-<name> Workloads of Bucket applications, or the buckets are metered twice.
```
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
<!-- coderabbit-ai-generated:v1 -->
## Summary by CodeRabbit
* **Bug Fixes**
* Bucket workloads now use a consistent name without duplicating the
`bucket-` prefix.
* Existing workload size data is retained when workloads are renamed or
updated.
* Workloads owned by another claim are preserved. When a naming conflict
occurs, metering continues under the legacy name when possible; if both
names are unavailable, metering is skipped and a warning is emitted.
* Legacy workloads are cleaned up or updated according to their
ownership, helping prevent duplicate or orphaned workloads.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
## What this PR does
This PR syncs `healthCheckExprs` and `waitStrategy` from
ApplicationDefinition to HelmReleases that already exist. cozystack-api
writes both fields only when it builds a release, and the definition
reconciler copied chartRef, valuesFrom, disable-wait setting and labels
but not these two, so release created before its definition got
expressions keeps waiting on objects the definition already exempts.
Most visible case is vm-disk. Every disk created before the DataVolume
`current: "true"` expression was added still waits on its DataVolume, so
after platform upgrade an existing disk with import still in progress
fails the upgrade after 10 minutes with `timeout waiting for:
[DataVolume/<namespace>/vm-disk-<name> status: 'InProgress']`, then
retries forever and holds one of 5 workers of its helm-controller shard.
Reconciler now derives both fields same way the API does
(`config.ResolveWaitStrategy`) and syncs them in both directions, so
existing release ends up same as freshly written one. On current
definitions only vm-disk (DataVolume) and postgres (KEDA ScaledObject)
set expressions, both `current: "true"`, so this can only relax waits.
Releases of other kinds have nothing to sync and are not rewritten.
HelmReleases rendered by another chart (tenant modules like Etcd,
Ingress, Monitoring) carry Flux origin label
`helm.toolkit.fluxcd.io/name` and are skipped, their gate comes from
their own chart. Their chartRef is still synced as before.
Failed update was logged and dropped before, and nothing requeues the
definition for hours after that. Now errors are returned, so a conflict
is retried.
Unit tests cover both sync directions, `waitStrategy: legacy` without
expressions, no rewrite of in-sync release (nil vs empty slice too),
chart-rendered and other-kind releases left alone, and update failure
returned as error.
Fixes#4409
### 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:
- [ ] [cozystack/community](https://github.com/cozystack/community) -
follow-up:
### Release note
```release-note
fix(controller): existing HelmReleases now get `healthCheckExprs` and `waitStrategy` from their ApplicationDefinition. vm-disk and postgres releases created before these expressions were added stop waiting on DataVolume and KEDA ScaledObject during upgrades.
```
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
<!-- coderabbit-ai-generated:v1 -->
## Summary by CodeRabbit
* **Bug Fixes**
* Helm releases now synchronize health-check expressions and wait
strategies configured in their application definition, including when
those values are removed or no expressions are specified.
* Releases with chart-rendered health-check settings retain those
settings, and releases outside the matching application kind are
unaffected.
* Update failures are now reported during reconciliation, while updates
to other releases continue. Reconciliation can retry after a failed
update.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
## What this PR does
Connecting a marketplace repository materialized its `PackageSource`s
and then auto-created a tap-managed `Package` per source. The Package
reconciler installs a `HelmRelease` for every install-marked component
of the default variant, and Flux runs those under its `flux` service
account, which is bound to `cluster-admin` with no impersonation
(`--default-service-account` is not set, and neither `internal/operator`
nor `pkg/registry/apps` set `serviceAccountName` on the HelmRelease). A
publisher's install chart therefore ran as `cluster-admin` on connect,
with no operator step, contradicting the documented behaviour that
tapping installs nothing. The `privileged` flag is a prompt to the admin
and does not protect against a publisher acting in bad faith.
This removes the on-connect auto-registration (`ensureRegistration`).
Connecting now only materializes `PackageSource`s. Installing an app is
an explicit `cozypkg add`, which becomes the sole creator of a
`Package`; every other `Package` reference in the operator is a read or
a watch. Removing the call also fixes the second effect of the same
path: a `Package` the admin removed with `cozypkg del` is no longer
re-created on the next source revision.
A second commit reconciles the texts this behaviour change made wrong:
they promised that tapping registers a repository's apps in the catalog
and runs its charts. Reworded the `cozypkg tap`/`untap` help, the tap
success output (it now prints a `cozypkg add <name>` line per
materialized PackageSource), the `cozypkg init` scaffold README, the
packages-index trust model, and the dashboard Repositories banner and
per-repository empty state.
Accepted consequence: a connected repository's apps appear in the
catalog only after an admin runs `cozypkg add`, since the dashboard
catalog is computed from `ApplicationDefinition`s. Install from a
connected repository is CLI-only for now; the console connects and
disconnects repositories but does not create a `Package`. Populating the
catalog from `PackageSource` metadata without an install is the
long-term shape in #4359 and is out of scope here. The
privileged-confirmation gaps #4358 and #4359 stay open.
Upgrade note: a `Package` that Cozystack 1.7.0 auto-created on connect
stays after the upgrade, and the privileged-confirmation gate still
applies to it, so this is not a regression for existing installs. For a
newly connected repository no tap-labelled `Package` is created, so the
gate does not come into play on that path. An operator who does not want
a 1.7.0 auto-created package can remove it with `cozypkg del`.
Addresses #4978.
### Screenshots
Dashboard text-only change (Repositories banner and per-repository empty
state). No layout change.
### Downstream repositories
Walked the trigger map in `docs/agents/contributing.md` against the
diff. The diff is operator logic plus CLI/dashboard/doc text; it matches
no concrete trigger (no package, API, schema, tooling, annotation,
metric or commit-convention change). The dashboard console is vendored
in this repo (`packages/system/dashboard`), so its text change travels
in this PR. Leaving every box empty rather than ticking "No downstream
repository is affected".
### Release note
```release-note
fix(marketplace): connecting a marketplace repository no longer installs its components. A connected repository's apps are materialized as PackageSources but not installed until an explicit `cozypkg add`; install from a connected repository is CLI-only for now. Previously, connecting auto-created a Package whose install-marked components were deployed by Flux under its cluster-admin service account. A Package that Cozystack 1.7.0 auto-created on connect stays after upgrade and can be removed with `cozypkg del`.
```
A verified cosign signature whose manifest digest could not be parsed fell back
to an unpinned tag pull, which reopens the tag-mutation window the verification
was meant to close. Return an error instead.
Add a test that isolates the status.observedGeneration == metadata.generation
conjunct of artifactReady; the existing tests exercised only the tag-prefix
conjunct, so a mutation of the generation check went uncaught.
Assisted-by: LLM
Signed-off-by: IvanHunters <xorokhotnikov@gmail.com>
The materializer pinned whatever digest status.artifact reported, even when the
artifact still described the previous reference. On a re-connect to a new tag,
source-controller has not re-resolved yet, so the operator materialized and
pinned the old tag's content (issue #4979). Gate materialization and pinning on
the artifact reflecting the current spec: status.observedGeneration equals
metadata.generation, and for a tag-only source a revision that belongs to that
tag (<tag>@<digest>).
A tap connected before digest pinning existed already has its
materialized-revision equal to the artifact revision, so Reconcile short-circuited
and never pinned, letting it follow the tag one more time. Pin the current digest
before that early return.
Assisted-by: LLM
Signed-off-by: IvanHunters <xorokhotnikov@gmail.com>
Removing on-connect auto-registration made several user-facing and doc texts
wrong: they promised that tapping registers a repository's apps in the catalog
and runs its charts. Tapping now installs nothing, so the texts told operators to
wait for apps that never appear.
Reword the `cozypkg tap`/`untap` help, the tap success output (now printing a
`cozypkg add <name>` line per materialized PackageSource), the `cozypkg init`
scaffold README, the packages-index trust model, the dashboard Repositories
banner and per-repository empty state, and the stale re-registration code
comments. No behaviour changes beyond the added `cozypkg add` hint in the tap
output.
Assisted-by: LLM
Signed-off-by: IvanHunters <xorokhotnikov@gmail.com>
cozystack-api writes healthCheckExprs and the resolved wait strategy
from the ApplicationDefinition only when it builds a HelmRelease, and
the definition reconciler that keeps existing releases in step copied
neither. A release created before its definition exempted an object
kept waiting on that object at every upgrade. For vm-disk that is
every disk created before the DataVolume expression was added: a disk
whose import is still running after a platform upgrade fails the
upgrade at the timeout and, under RetryOnFailure, retries forever
while holding a helm-controller worker.
The reconciler now derives both fields the way the API does, in both
directions, so an existing release ends up with what a fresh write
would produce. On the shipped definitions this only relaxes waits: the
vm-disk DataVolume and the postgres KEDA ScaledObject. A release that
another chart renders carries Flux's origin label and keeps the gate
its chart sets.
A failed update used to be logged and dropped, and nothing else
re-enqueues the definition for hours, so that release stayed out of
sync. The errors are now returned and the definition is retried.
Fixes https://github.com/cozystack/cozystack/issues/4409
Assisted-by: LLM
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
A BucketClaim deleted with --cascade=orphan and recreated leaves its
bucket-bucket-<name> Workload without owners. The migration recognised a
legacy Workload only by the claim's UID among its owners, so that object
stayed next to the new one with a frozen size and the bucket was metered
twice. Treat an ownerless legacy Workload that carries the monitor's
label as the claim's: delete it with the same preconditions, but take no
sizes from it, since they may predate the claim.
The size carry-over kept whatever the Workload at the new name had
recorded. When that Workload was not yet owned by the claim, an ownerless
leftover, its sizes were not written for this bucket, yet they won over
the migrating legacy Workload's. Prefer the recorded sizes only when the
claim already owned the Workload.
Assisted-by: LLM
Signed-off-by: Andrey Kolkov <androndo@gmail.com>
When the new Workload name was held by another owner the claim was
skipped altogether, so its legacy Workload stopped being refreshed and
billing kept reading the last size it had recorded. That happens briefly
with Bucket applications "x" and "bucket-x" until the first one has
migrated, and for good with a claim "x" outside the Bucket application
next to the application "x", which map to the same name.
Keep reconciling the bucket under its legacy name instead, without
migrating, until the new name is free. Skip it only when the legacy name
is held by another owner as well, which is the only option left for a
claim without the prefix, whose two names coincide; the warning event
then says that the bucket is not metered.
Assisted-by: LLM
Signed-off-by: Andrey Kolkov <androndo@gmail.com>
A controller rolled back to the doubled prefix runs CreateOrUpdate on
bucket-bucket-x, which by then is the renamed Workload of the Bucket
application "bucket-x", and appends claim bucket-x as a second owner.
After the next upgrade claim bucket-x took that object for its own
legacy Workload: it carried the other bucket's size over and deleted the
object, and the other bucket's Workload came back without a size.
Treat a legacy Workload as the claim's own only when the claim is its
sole owner. When it has other owners, take neither its sizes nor the
object: drop the claim's owner reference and leave the rest.
Delete the legacy Workload with UID and resourceVersion preconditions,
so a Workload that took over the name between the lookup and the delete
survives; a conflict is not an error and the next reconcile retries.
Assisted-by: LLM
Signed-off-by: Andrey Kolkov <androndo@gmail.com>
The adoption check only refused a Workload owned by another BucketClaim,
and the kind test in it had no case that could tell it apart: a bucket
Workload only ever has BucketClaim owners. Refuse any Workload that has
owners when the claim is not among them, which needs no kind at all, and
keep taking over one that has lost every owner reference, since nothing
else would ever release its name. Pin the ownerless case.
Assisted-by: LLM
Signed-off-by: Andrey Kolkov <androndo@gmail.com>
On a live cluster the cache records the BucketClaim kind in a Workload's
owner reference, so a claim's own Workload passes through the "owned by
another claim" check on every reconcile, and only the own-UID match keeps
it from being skipped. The fake client leaves that kind empty, so no test
exercised it. Seed a Workload whose owner reference carries the kind and
the claim's UID, and expect it to be updated with no warning event.
Pin the physical size carry-over alongside the logical one, and word the
warning event so it holds when two claims map to one name for good.
Assisted-by: LLM
Signed-off-by: Andrey Kolkov <androndo@gmail.com>
## What this PR does
After a Velero restore of a VMDisk or of a VMInstance (`RestoreJob` with
the Velero strategy), the disk's HelmRelease fails on its next upgrade
and stays failed:
```
Helm upgrade failed for release <ns>/vm-disk-<name> with chart vm-disk@...: failed to create resource:
admission webhook "datavolume-validate.cdi.kubevirt.io" denied the request: Destination PVC <ns>/vm-disk-<name> already exists
```
The VMInstance release that references the disk then fails in turn
(`Specified disk not exists in cluster`). Seen on a v1.6.5 cluster with
CDI v1.66.1, where each failed upgrade is retried and the release
history of one such disk had grown past 46,000 revisions.
### Cause
The restore controller deletes the disk's DataVolume
(`prepareForRestore`) and Velero restores the PVC under the same name,
with no owner. The restored release matches its chart and values, so
Flux does not upgrade it and the DataVolume is not recreated right away;
the next Helm upgrade (a platform update, an edit of the VMDisk) does
recreate it, and CDI's webhook refuses it.
In CDI v1.66.1 the webhook admits a DataVolume over an existing claim
only when `ClaimMayExistBeforeDataVolume` is true
(`pkg/controller/common/util.go`): either the PVC carries
`cdi.kubevirt.io/storage.populatedFor: <dv name>`, or
`AllowClaimAdoption` returns true, which reads
`cdi.kubevirt.io/allowClaimAdoption` **from the DataVolume** and
otherwise falls back to the `DataVolumeClaimAdoption` feature gate. The
Velero strategy's resource modifiers
(`createResourceModifiersConfigMap`) put `allowClaimAdoption=true` on
the restored **PVC**, where CDI never reads it, and the `CDI` CR in
`packages/system/kubevirt-cdi` enables neither the gate nor anything
equivalent. Migration 29 creates its PVCs with the same PVC-side
annotation.
### Fix
The vm-disk chart sets `cdi.kubevirt.io/allowClaimAdoption: "true"` on
the DataVolume it renders, on the creation path and on the
existing-DataVolume path alike, so the annotation does not flap between
upgrades.
- A fresh disk is unaffected: no PVC exists when the DataVolume is
admitted, and the PVC CDI then creates carries
`cdi.kubevirt.io/createdForDataVolume: <dv uid>`, for which
`AllowClaimAdoption` returns false, so CDI imports, clones or uploads as
before.
- An existing DataVolume already controls its PVC, so the annotation
changes nothing there; a Helm upgrade only adds it to the DataVolume's
metadata, which CDI allows (only the spec is immutable).
- After a restore, the webhook admits the recreated DataVolume and the
DataVolume controller adopts the claim: `pvcRequiresWork` is false, it
takes controller ownership of the PVC and the DataVolume goes
`Succeeded` without populating it again, so the restored data is kept.
Checked against a v1.6.5 cluster carrying the broken state, by rendering
the chart for an affected disk and running `kubectl create
--dry-run=server`: the DataVolume without the annotation is refused with
the error above, the same one with the annotation is admitted. On that
cluster two affected disks were later recovered through the
claim-adoption path (the same `AllowClaimAdoption` check): the PVC kept
its uid, became controlled by the new DataVolume, and the DataVolume
reached `Succeeded` with no import.
### Alternatives considered
- **Enable the `DataVolumeClaimAdoption` feature gate in the `CDI` CR.**
It works too, but for every DataVolume in the cluster (tenant Kubernetes
worker disks, golden images, KubeVirt `dataVolumeTemplates`), each of
which would then silently take over any unowned PVC of its name. The
per-DataVolume annotation keeps the behaviour to the one chart whose
DataVolumes are expected to meet a restored claim, and CDI has said it
means to retire the gate (kubevirt/containerized-data-importer#3480)
while the annotation is checked first and independently of it.
- **Stamp `cdi.kubevirt.io/storage.populatedFor` on the restored PVC**
from the resource modifiers. That would also pass the webhook, but only
for disks restored by the Velero strategy, not for claims created by
migration 29 or by hand, and it would need a controller image change
where a chart change suffices.
The PVC-side `allowClaimAdoption` rule in
`createResourceModifiersConfigMap` (and the one in migration 29) is
inert under CDI v1.66.1. This PR leaves the rule and its unit-test
assertions in place, and corrects the comments that credited it with the
adoption (the collector and `createResourceModifiersConfigMap` in
`velerostrategy_controller.go`, and step 5 of
`examples/backups/vmi/scenario-user-restore.md`): they now say CDI
adopts the restored PVC through the annotation the vm-disk chart sets on
the DataVolume. The backports carry only the template and the unittest
suite, so these comment and doc edits do not reach them.
One behaviour change to be aware of: a PVC named `vm-disk-<name>` left
behind without an owner (a restored disk deleted before this fix, whose
PVC Velero restored without an owner) is now adopted by a new VMDisk of
the same name, with its old contents, instead of blocking that VMDisk
forever. That PVC lives in the same tenant namespace.
### Tests
- `packages/apps/vm-disk/tests/dv_claim_adoption_test.yaml`: the
post-restore state (a restored PVC, no DataVolume) renders the
DataVolume through the creation path with the annotation, and the
existing-DataVolume path keeps it. Both fail with the template change
reverted. `helm unittest packages/apps/vm-disk`: 116 passed.
- `examples/backups/vmdisk/05-restore-in-place.sh` (run by the
`vmdisk-2-backup-roundtrip` Chainsaw suite): after the restore, the
HelmRelease was already reported Ready because no upgrade ran, which is
why CI did not catch this. The script now forces a Helm upgrade
(`reconcile.fluxcd.io/forceAt` + `requestedAt`), then requires the
release Ready, the DataVolume `Succeeded`, and the PVC to keep its
restored uid and be controlled by the DataVolume.
### Backports
Opened by hand as #4956 (`release-1.6`) and #4957 (`release-1.7`) rather
than through the `kind/backport` labels: `examples/backups/vmdisk/` does
not exist on either branch, so the bot's cherry-pick would conflict.
They carry the template change and the helm unittest suite.
### Screenshots
Not a UI change.
### Downstream repositories
Walked the trigger map against the diff: a vm-disk template change (no
`values.yaml`, schema, `README.md` or `Chart.yaml` change), a new helm
unittest file, and a backup walkthrough script. No app, field, default,
CRD, platform value or tooling changes, and no documented workaround on
the website is made obsolete (the only mention of `allowClaimAdoption`
there is in the archived v0 docs, about a different manual procedure).
- [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:
- [ ] [cozystack/community](https://github.com/cozystack/community) -
follow-up:
### Release note
```release-note
fix(vm-disk): a VMDisk restored by Velero, alone or as part of a VMInstance, no longer fails its next upgrade with "Destination PVC ... already exists": the DataVolume now carries cdi.kubevirt.io/allowClaimAdoption, so CDI adopts the restored PVC and keeps its data.
```
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
<!-- coderabbit-ai-generated:v1 -->
## Summary by CodeRabbit
* **Bug Fixes**
* Improved in-place VM disk restores so restored storage is correctly
adopted when the disk resource is recreated.
* The restore process now waits for the disk to become ready and reports
an error if it fails or the restored storage is not retained.
* Clarified the adoption behavior in restore documentation and ensured
recreated disks use the required adoption setting.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
The legacy Workload can disappear between the lookup and the delete, for
example through owner garbage collection, and the delete ignores
NotFound for that reason. Reconcile only logs per-claim errors, so check
reconcileBucketClaimForMonitor's own result with a delete that reports
NotFound.
Assisted-by: LLM
Signed-off-by: Andrey Kolkov <androndo@gmail.com>
With Bucket applications "x" and "bucket-x" in one namespace, the legacy
name of the first, bucket-bucket-x, is the new name of the second. When
the second reconciled first, CreateOrUpdate took over the first one's
legacy Workload, which already carried bucket x's size, and appended a
second owner. The first claim then found its own UID among the owners,
treated the object as its legacy Workload and deleted it, and the
second bucket's Workload came back without a size.
Leave a Workload alone while it is owned by a different BucketClaim, log
it and emit a BucketWorkloadNameTaken warning on the monitor. In the
case above the other claim renames its Workload away on its own
reconcile, and the name is free on the next pass. The same refusal keeps
two claims that map to one name, such as "x" and "bucket-x", from
sharing one Workload.
Pin the edge cases the migration depends on: the reconcile order above
in both directions, a claim without the prefix keeping its own Workload,
and a recorded size taking precedence over the legacy one.
Assisted-by: LLM
Signed-off-by: Andrey Kolkov <androndo@gmail.com>
A bucket's Workload was named "bucket-" + the BucketClaim name, and a
Bucket application names its claim after its release, which already
starts with "bucket-". A bucket an operator knows as cozy-backups
showed up as bucket-bucket-cozy-backups in every billing usage report.
Add the prefix only when the claim name does not carry it. Claims from
other charts, such as Harbor's <release>-registry, keep their names.
Existing Workloads under the doubled name are owned by their claim and
would otherwise live on next to the new one, metering the bucket twice.
When the replacement is reconciled, take the legacy Workload's recorded
sizes as the starting point, so the rename leaves no gap in the metered
size, and delete the legacy Workload. Only a Workload owned by the same
BucketClaim is touched.
Billing sees the renamed Workload as a new consumer: a usage report
spanning the upgrade lists the bucket once under each name.
Fixes#5014
Assisted-by: LLM
Signed-off-by: Andrey Kolkov <androndo@gmail.com>
A tapped repository followed a mutable tag: the OCIRepository carried only
spec.ref.tag with a 5m interval, so Flux re-pulled whatever the tag pointed
at, and a moved tag reached running tenant application instances within the
interval. Whoever controlled the tag controlled what ran in the cluster.
The materializer now pins the source to the digest Flux resolved for the
artifact it just verified: it reads the manifest digest from
status.artifact.revision (source-controller formats an OCI tag revision as
'<tag>@<digest>') and writes it to spec.ref.digest, which Flux resolves
ahead of the tag. The tag is kept informational. Moving the tag upstream no
longer changes what the cluster pulls; an explicit re-connect is required to
move the pin. A user-supplied explicit digest is never rewritten, and a
revision without a well-formed digest leaves the source following its tag
rather than writing a malformed reference.
Assisted-by: LLM
Signed-off-by: IvanHunters <xorokhotnikov@gmail.com>
Connecting a marketplace repository materialized its PackageSources and then
auto-created a tap-managed Package per source. The Package reconciler installed
a HelmRelease for every install-marked component of the default variant, which
Flux runs under its cluster-admin service account with no impersonation. A
publisher's install chart therefore ran as cluster-admin on connect, with no
operator step, contradicting the documented behaviour that tapping installs
nothing.
Remove the on-connect auto-registration. Connecting now only materializes
PackageSources; installing an app is an explicit cozypkg add (or dashboard
action), which is the sole remaining creator of a Package. This also stops a
Package the admin removed with cozypkg del from being re-created on the next
source revision.
Catalog population from PackageSource metadata without an install is left to
the long-term design in #4359, and the privileged-confirmation gaps #4358 and
#4359 stay open.
Assisted-by: LLM
Signed-off-by: IvanHunters <xorokhotnikov@gmail.com>
Seven guides in docs/ were written next to the code and left in the
source tree, where no operator looks. The website carries them now, so
a second copy here only lets the two drift again: the Backup Classes
page had already fallen behind its original.
docs/release.md, docs/deprecating-applications.md and the two SeaweedFS
runbooks in docs/operations stay. A migration, a bats suite, a workflow
or AGENTS.md reads each of them by path, which is the test for keeping
a document in this repository.
Assisted-by: LLM
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
The restore controller and the user-restore example described the
allowClaimAdoption annotation its resource modifiers put on restored
PVCs as what lets CDI adopt them. CDI v1.66 reads that annotation only
on the DataVolume; on a PVC it reads nothing but createdForDataVolume.
Adoption rests on the vm-disk chart annotating the DataVolume Flux
recreates, and a reader trusting the PVC rule could rely on it for a
restore without such a DataVolume, or drop the DataVolume annotation as
redundant.
Assisted-by: LLM
Signed-off-by: Mattia Eleuteri <mattia@hidora.io>
## What this PR does
Follow-up to the flux-plunger uninstall fence: the points the last
review round raised and left for after the merge, plus four found by a
later independent review. None changes the fence's design; each closes a
gap in what it reports, how long it waits, or what its documentation
says.
**Pre-delete hook.**
- A release being deleted that only the fence still holds is no longer
unfenced in the same step that lifts its suspension. helm-controller
runs the uninstall of a release being deleted once it is no longer
suspended, but removing the fence at once deleted the object before
helm-controller could reconcile it. The hook now keeps that fence while
helm-controller handles the generation the lifted suspension wrote,
until shortly before its retry window closes, and names a release it
still had to let go of as before. A release whose state cannot be read
keeps its fence for the wait the same way, rather than being let go of
on one failed read.
- A release being deleted while still suspended, by anyone but
flux-plunger, was reported as cleanly released, although helm-controller
skips the uninstall of a suspended release. It is now named as deleted
without an uninstall and counted with the others.
- Each release gets its own share of the retry window, capped at the
window's end. One release that could never be read used to take the
whole window and leave every release listed after it a single attempt.
The share is a cap, not a floor of attempts: a floor would let enough
failing releases run the Job past its deadline, which kills it before
its report is written.
- The hook stops, waits for and lists again only for the flux-plunger of
its own release. Another one (a second installation under another
release name, changed labels, a binary outside the cluster) is not
stopped by the hook, keeps the fences it sets and gives them up itself;
the README now says so instead of the hook re-listing unconditionally,
which would not close that case and would fail the uninstall for as long
as a second installation is recovering anything.
- helm-controller's finalizer, which the hook reads to tell whether a
release being deleted is still held, is tied to the constant the
controller uses, as the fence already was. Two comments are corrected:
the retry window counts from the script's start, not the Job's, and
kubectl scale's internal conflict retry never runs here.
**Shutdown drain.**
- A drain deadline of zero or less is refused at startup, and the
chart's lowest accepted grace period is held to the same constants as
the drain subtraction, so raising the manager's stop bound cannot make
the chart render a deadline that is already spent. The timings the
README and the values comment state are held to those constants too.
- Fences left behind are reported as an error naming each release, not
an Info line; a drain that never got a listing answered says so instead
of reporting a count of -1; a flux-plunger that never led no longer
claims it released every fence.
- The drain's Events fold repeats into one counted Event per release and
reason, as the manager's recorder does, instead of one object per pass.
The fold is a patch, the verb the chart grants on events, and every
write is bounded by the drain's deadline. It does not add a per-release
Event for fences left at the end: by then the drain's deadline is spent,
and writing one would reach past it.
- Tests now hold the outcome line, the retry of a release whose fence
cannot be given up yet, and the pause between passes, none of which any
suite noticed when removed.
**Controller.** The `SuspensionNotOwned` and `TerminatingWhileSuspended`
Warnings were keyed on the processed-version annotation, which stays on
every release flux-plunger ever recovered, so a later restore, migration
or operator suspension of a healthy release drew a Warning telling the
reader to resume it, and a migration's deliberate suspend-then-delete
was called an accident. Both now key on the state a leftover actually
freezes the release in, `Ready=False` with `has no deployed releases`,
since helm-controller rewrites no condition of a suspended release; the
README's candidate query follows. Two comments that claimed more than
holds are qualified.
**Chart and README.** A `shutdownGracePeriodSeconds` of a million or
more now gets the range message rather than being refused as a fraction.
The README gives the recovery steps in the order the controller runs
them (the annotation is written before the Secret is deleted), and names
the window in which a backup restore started during a recovery is
unsuspended under it, since the etcd, CNPG and Velero restore steps
write `spec.suspend` only when it differs.
### Testing
Each change was written test first: a test that builds the binary and
runs it with a deadline of zero, helm-unittest for the chart, unit tests
with a captured logger and the fake client for the drain's outcome,
Events and passes (the Event fold under a client that refuses the update
verb, as the chart's role does), table tests for both Warnings, and the
bats suite for the hook, whose fake kubectl now models helm-controller
handling a release some reads after its suspension is lifted and a state
read that fails. Every fix was then reverted on its own, and the suite
went red each time. The envtest suites pass unchanged. The README's
candidate query was run against HelmReleases read from a live cluster,
with a stuck, a healthy, an owned and an unsuspended variant, and lists
only the stuck one.
### Screenshots
Not applicable: no UI change.
### Downstream repositories
The trigger map was walked against the diff:
`internal/controller/fluxplunger`, `cmd/flux-plunger`, the
`packages/system/flux-plunger` chart (templates, tests, README) and its
bats test under `hack/`. The website's FluxPlunger section in
`guides/concepts.md` draws the recovery with the Secret deleted before
the annotation is written, which is the order before the fence change;
it needs the same correction as the chart README. That change is held as
a draft until v1.7.0 is released and the website's `next/` is
re-initialized: the fence change is not part of release-1.7, so the
corrected order would be wrong for v1.7 docs, which are copied from
`next/`. No other repository is reached by this diff.
- [ ] No downstream repository is affected by this change
- [x] [cozystack/website](https://github.com/cozystack/website) -
follow-up: https://github.com/cozystack/website/pull/752 (draft, merge
after v1.7.0)
- [ ]
[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:
- [ ] [cozystack/community](https://github.com/cozystack/community) -
follow-up:
### Release note
```release-note
fix(fluxcd): flux-plunger's pre-delete hook now lets helm-controller run the uninstall of a release being deleted before it removes the release's fence, gives each release its own share of its retries, and reports releases deleted under another manager's suspension; the shutdown drain reports the fences it leaves as errors and refuses a deadline that leaves no time
```
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit
* **Bug Fixes**
* Shutdown drain timeouts must be positive, and invalid values now stop
startup with a clear error.
* Uninstall cleanup waits for Helm to handle releases before removing
their fences, while bounding retries and reporting releases that remain
incomplete.
* Recovery and suspension warnings now reflect the release’s actual
failure state, and recovery can retry stale storage cleanup against the
same revision.
* **Documentation**
* Clarified shutdown grace-period requirements, uninstall behavior, and
guidance for backup restores and existing suspensions.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
ZverGuy