admin/cozystack
1
0 Fork 0
mirror of https://github.com/cozystack/cozystack.git synced 2026-11-07 04:23:50 +09:00
Code Issues Packages Projects Releases Wiki Activity
4,647 Commits 569 Branches 294 Tags
1f0c1949e72fddda40ea92e1ea850e2dba3303af
Commit Graph
4647 Commits
This Branch
This Branch
All Branches
Author SHA1 Message Date
Myasnikov DaniilandClaude Opus 4.8 1f0c1949e7 test(flux-shard-operator): add fuzz targets for shard config parsers 
Native Go fuzzing over the flag parsers in config.go, asserting the
invariants that matter rather than just no-panic:

  - ParseShardIndex round-trips with ShardName (accepted input is the canonical
    name of the returned index; index is non-negative).
  - ShardName round-trips back through ParseShardIndex for every valid index.
  - ParseShardCount: a non-auto success yields count >= 1; auto yields 0.
  - ParsePinnedTenants: every entry in a success has a non-empty key mapped to a
    valid shard name.

Adds an OSS-Fuzz-detectable fuzzing signal for the OpenSSF Scorecard Fuzzing
check while covering parsers that sit on the sharding hot path.

Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
Assisted-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
 2026-07-07 11:31:23 +05:00
Myasnikov DaniilandClaude Opus 4.8 a0bd53f84d ci: pin workflow actions, scope token permissions, add zizmor gate 
build-main.yaml and ui-test.yaml (the latter added by the in-tree console
move) landed without following the repo's workflow conventions: no top-level
permissions block and actions referenced by moving tags. Together they zero out
two OpenSSF Scorecard checks the rest of the tree already satisfies:

  - Token-Permissions: scored on the weakest workflow, so one workflow with an
    undeclared (default-write) top-level token drops the whole check to 0.
  - Pinned-Dependencies: the six tag-pinned action refs count as unpinned.

Fix both files (top-level read-only token; job-level packages:write kept where
needed; all action refs pinned to the SHA already used elsewhere in the tree)
and add a durable gate so it cannot recur:

  - .github/zizmor.yml enables the two Scorecard-relevant audits (unpinned-uses,
    excessive-permissions), clean tree-wide as of this commit. The remaining
    audits carry pre-existing debt and are disabled with a TODO to burn down and
    re-enable one at a time.
  - .pre-commit-config.yaml runs zizmor (offline) on changed workflows.
  - .github/workflows/zizmor.yml runs the same audit tree-wide on every PR that
    touches a workflow, failing on any reintroduced regression.

Also wire scorecard.yml to an optional SCORECARD_TOKEN secret: the default
GITHUB_TOKEN cannot read branch-protection settings, so the Branch-Protection
check is scored on partial data. The token is a no-op until the secret is added.

Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
Assisted-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
 2026-07-07 11:31:13 +05:00
IvanHunters f1014264f7 ci(pr-labeler): drop 'edited' trigger to stop bot PR re-run churn (#3211) 
## Problem

The **PR Auto-Label** workflow (`.github/workflows/pr-labeler.yaml`)
triggers on `pull_request_target` with `types: [opened, edited,
reopened, synchronize]`.

The `edited` action fires on every PR title **and body** edit.
Long-lived bot PRs — notably Renovate — repeatedly edit their own
description on schedule (rebase checkbox, changelog tables,
dependency-dashboard state). Each body edit re-runs the `label` job
against an **unchanged commit**.

Example: on #2998 (a Renovate PR) the `label` job has run **~48 times**,
all on the same head SHA, from Jun 22 through today. It shows up on the
/checks page as a wall of duplicate labeler runs.

## Change

Drop `edited` from the trigger types:

```yaml
on:
  pull_request_target:
    types: [opened, reopened, synchronize]
```

## Trade-off

Labels derived from the PR title are re-applied on the next push
(`synchronize`) or on reopen. A **title-only** edit made between pushes
will no longer trigger an immediate relabel — an acceptable trade for
eliminating the constant bot-driven churn. (Title edits are rare and the
next push catches up.)

No behavior change for the common case: label-on-open and label-on-push
both still work.

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

## Summary by CodeRabbit

* **Chores**
* Improved pull request labeling behavior so labels are refreshed on new
commits or when a PR is reopened, while avoiding unnecessary updates
from body-only edits.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
 2026-07-06 09:30:22 +03:00
myasnikovdaniil e28681afb4 fix(kubevirt-instancetypes): restore persistent EFI/TPM state (#3154) 
## What this PR does

Reverts #3006, restoring upstream's persistent EFI/TPM firmware state
for the `windows.11` / `windows.2k22` / `windows.2k25` preferences (and
their `.virtio` variants).

#3006 dropped persistent EFI/TPM to "unblock live-migration", on the
assumption that an RWO Filesystem backend-storage PVC pins the VM to its
node. That assumption was outdated: KubeVirt has live-migrated
RWO-Filesystem backend storage since v1.4 (kubevirt/kubevirt#12629) — on
migration it creates a fresh target state PVC and copies the small state
blob. So persistent-EFI/TPM VMs on the default `replicated` storage
live-migrate fine; the regression described in #3005 was a misdiagnosis.

Verified on a KubeVirt v1.8.2 cluster: a persistent-EFI VM with an RWO
Filesystem backend PVC reports `LiveMigratable=True`, and the
copy-on-target backend PVC is created when a migration starts. KubeVirt
maintainers confirmed the same upstream.

This restores persistent firmware state so in-guest Microsoft UEFI CA /
Secure Boot enrollment survives reboots and migrations.

### Release note

```release-note
fix(kubevirt-instancetypes): restore persistent EFI/TPM state for Windows preferences — RWO-Filesystem backend storage already live-migrates (KubeVirt v1.4+), so persistence no longer needs to be stripped
```


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

* **Bug Fixes**
* Updated Windows instance type preferences to keep TPM persistence
enabled, improving compatibility and reducing storage-related issues.
* Ensured firmware and TPM persistence settings are preserved during
instance type preference generation for more reliable outcomes.

* **Chores**
* Streamlined the preference generation step so instance type templates
are produced consistently without removing persistence configuration.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
 2026-07-06 11:07:37 +05:00
Myasnikov DaniilandClaude Opus 4.8 e82c03a715 ci(pr-labeler): drop 'edited' trigger to stop bot PR re-run churn 
The PR Auto-Label workflow triggered on pull_request_target 'edited',
which fires on every PR title *and body* edit. Long-lived bot PRs such
as Renovate repeatedly edit their description on schedule, causing the
label job to re-run dozens of times against an unchanged commit (e.g.
~48 runs on #2998, all on the same head SHA).

Drop 'edited' so the job only runs on opened/reopened/synchronize.
Labels derived from the PR title are re-applied on the next push or on
reopen. Title-only edits between pushes no longer trigger a relabel,
which is an acceptable trade for eliminating the churn.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
 2026-07-06 10:24:32 +05:00
myasnikovdaniil 35025cf5f0 test(e2e): capture LINSTOR satellite ErrorReports in cozyreport (#3209) 
## What this PR does

Extends `hack/cozyreport.sh` linstor module to copy
`/var/log/linstor-satellite/ErrorReport-*.log` off every
`linstor-satellite` pod into `linstor/error-reports/<node>.tgz` in the
cozyreport archive.

### Why

The linstor-Satellite pod stdout log references its error reports only
by ID:

```
[DeviceManager] ERROR LINSTOR/Satellite/... SYSTEM - Failed to create zfsvolume [Report number 6A4A394E-E4EA0-000000]
```

The report body (actual `zfs create -V …` stderr, DRBD adjust output,
storage-driver stack trace, exit code) lives on the satellite pod's
ephemeral filesystem at `/var/log/linstor-satellite/ErrorReport-*.log`.
crust-gather only captures pod stdout, so the report body is lost when
the e2e sandbox is torn down at end-of-job.

Consequence for post-mortem of `ResourceExhausted` / `Not enough
available nodes` CreateVolume retry loops (see e.g. PR #3044 CI
history): cozyreport shows only the driver-level symptom. Root cause
distinction (out-of-space vs dataset name conflict vs ZFS module error
vs kernel signal) is not possible from the artefact alone, and the
sandbox is unrecoverable by that point.

### Scope

`hack/cozyreport.sh` only, +32 lines under the existing `linstor module`
block. Guarded so:
- a satellite with zero reports (healthy case) leaves nothing in the
tree
- a transient exec / tar failure never fails the surrounding cozyreport
run
- empty parent directory is pruned

### Verified on a live cluster

- Label selector `app.kubernetes.io/component=linstor-satellite` matches
all 3 satellite pods on the dev cluster used for verification.
- Bundle for one active satellite comes out at ~60 KiB (dozens of
historical reports tarred).

No product-side change, no dependency on capture happening during a
specific test phase; runs as part of the existing end-of-job cozyreport
collection.

### Release note

```release-note
NONE
```

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

* **New Features**
* Enhanced offline LINSTOR diagnostics collection by adding an
error-report index and bundling controller and satellite error-report
logs into downloadable archives.
* Generates per-node satellite bundles using stable node naming for
clearer organization of collected diagnostics.
* **Bug Fixes**
* Avoids creating empty or invalid error-report archives and
automatically cleans up unused bundles to prevent clutter in the
collected output.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
 2026-07-06 10:00:43 +05:00
myasnikovdaniil 6b6fad03fc fix(operator): force source-watcher drift on stuck ArtifactGenerator status (#3182) 
## Summary

`cozystack-platform` HelmRelease times out at 15m on fresh install when
a
PackageSource's derived ArtifactGenerator stalls in `Ready: Unknown`
even
though its ExternalArtifacts are produced and consumed successfully.
Root cause: fluxcd source-watcher patches `.status` and
`.status.conditions` as separate apiserver requests via `patch.Helper`,
so an etcd timeout under first-install load can leave `Inventory` +
`ObservedSourcesDigest` persisted while the Ready-condition write is
lost. Upstream tracker:
[fluxcd/pkg#934 — `patch.Helper` creates race conditions in downstream
controllers](https://github.com/fluxcd/pkg/issues/934).

The bug is present in the currently-deployed v2.1.0 (pinned at
`internal/fluxinstall/manifests/fluxcd.yaml:8230`), and a source-watcher
version bump alone does not fix it because the race lives in the shared
`fluxcd/pkg` runtime.

## What this PR does

`PackageSourceReconciler` now detects the stuck ArtifactGenerator
signature (Inventory populated, ObservedSourcesDigest set, Ready=Unknown
on the current generation, LastTransitionTime older than a 30s grace
period) and forces source-watcher onto its drifted-branch reconcile by
writing **both** signals it keys on:

1. **`Ready=False` on the AG's status subresource.** source-watcher's
   `detectDrift` (v2.1.0 `artifactgenerator_drift.go:52`) treats
   `IsFalse(Ready)` as `NotReady` drift, promoting the reconcile past
   the no-drift early-return at `artifactgenerator_controller.go:164`.
2. **`reconcile.fluxcd.io/requestedAt` bump on the AG's metadata.**
   source-watcher's ArtifactGenerator watch is registered with
`predicate.Or(GenerationChangedPredicate, ReconcileRequestedPredicate)`
   — the annotation is what enqueues the reconcile in the first place.
   Without it a status-only change never fires the watch and the AG
   would sit until the periodic self-requeue (hardcoded to 1h in
   `GetRequeueAfter`), well outside the 15m HR install timeout.

Both signals are load-bearing — either alone is a no-op. The status
write triggers the drifted branch inside source-watcher's reconcile;
the annotation write triggers the reconcile that inspects that drift.

On successful rebuild source-watcher writes `Ready=True` at
`controller.go:254`; on a genuine rebuild failure it writes
`Ready=False` with a real reason. Either way an honest condition from
upstream replaces our synthetic False, and the
`Owns(&ArtifactGenerator{})` watch fires this reconciler to copy the
resolved condition onto the PackageSource.

Bounded-retry harness sizes the total budget at ~11.5m
(`30s + 60s + 2m + 4m + 4m`) safely inside the 15m HR install timeout;
after `maxRecoveryAttempts` fruitless force-drift attempts the driver
surfaces `PackageSource.Ready=False, reason=SourceWatcherStalled` so an
operator can intervene, rather than lying about the state.

Retry state (attempt count + last-force timestamp) lives on the AG as
`cozystack.io/source-watcher-recovery-*` annotations so it survives
operator restarts. Counter reset is via the canonical
`clearRecoveryTracking` on the not-stuck path (Ready=True from
source-watcher, or Ready=False with an upstream reason), NOT keyed on
`LastTransitionTime` — source-watcher's Progressing write during
rebuild always advances that timestamp, and a heuristic reset there
would make `SourceWatcherStalled` unreachable under the very race this
driver targets.

## Owns()-re-entrancy handling

`Owns(&ArtifactGenerator{})` re-fires this reconciler on the
force-drift's own writes (both the status patch and the metadata
patch). A unified gate in `updateStatus` routes three overlapping
signals into the same state machine:

- **Our own marker reflecting back** — `Ready.Reason` carries
  `SourceWatcherRecoveryForced` (written only by
  `forceArtifactGeneratorDrift`).
- **source-watcher's Progressing during rebuild** — Ready=Unknown on
  the current generation, tracking annotations already present.
- **Classic first-time stuck detection** — the fluxcd/pkg#934 signature
  past the grace period.

All three route to `maybeRecoverArtifactGenerator`, whose
`decideRecovery` state machine is the single arbiter of wait vs
re-force vs give-up. The alternative — a separate "wait for
source-watcher" helper — was tried and rejected: if source-watcher
never responds (missing predicate, network partition, dead pod), a
wait-only path tight-loops without ever advancing to
SourceWatcherStalled. The unified gate lets the state machine's own
backoff schedule bound both the wait and the eventual give-up.

## Contract

`PackageSource.status.conditions[Ready]` contract is **unchanged**:

- `True` still means "source-watcher processed the current revision"
  (copied through from AG on the fast path).
- `False` from a genuine upstream failure still passes through
unchanged.
- `False, reason=SourceWatcherStalled` is new — surfaces after
  `maxRecoveryAttempts` fruitless force-drift attempts, an honest
  operator-actionable signal rather than a false-positive True.
- During force-drift and backoff windows, `Ready=Unknown,
  reason=AwaitingSourceWatcherRecovery` describes what the operator is
  waiting on.
- The synthetic `SourceWatcherRecoveryForced` marker is an
  operator-internal signal on the AG only; it never leaks to the
  PackageSource.

## Retirement

Once `fluxcd/pkg#934` lands and source-watcher consumes a `patch.Helper`
that either serialises or transactionally combines the `.status` /
`.status.conditions` writes, this whole recovery driver can be deleted
and `updateStatus` can copy the AG's Ready condition through
unconditionally. A `TODO(remove once fluxcd/pkg#934 lands and is rolled
out)` marker on the driver points at the upstream issue.

## HA note

The recovery driver stores its state as annotations on the
ArtifactGenerator, so it races on write when two cozystack-operator
replicas both see the same stuck target. Deploy the operator with
`--leader-elect=true` (or `replicas: 1`, which is the shipped default in
`packages/core/installer/templates/cozystack-operator.yaml`).

## Testing

**Unit tests** cover: stuck-detection predicate with grace-period cases,
bounded-retry state machine (Force / Wait / GiveUp branches), backoff
schedule, tracking-annotation parser hardening, mutation contract on
the two-subresource force-drift shape (success, status-patch failure
rollback, metadata-patch failure with Ready=False persisted and
annotations rolled back, rollback-restores-prior-bump), no-op
clear-tracking, Reconcile early-return on DeletionTimestamp, follow-up
scheduling on Unknown-within-grace to avoid the 10h informer resync
dormancy, Owns()-re-entrancy for both our own marker and Progressing
during recovery, canonical reset on Ready=True, and the
source-watcher-unresponsive path (stale own-marker triggers retry
through Force, exhaustion surfaces as SourceWatcherStalled). All pass
with `-race`; `go vet` and `golangci-lint` clean.

**E2E on dev3** — pending on the current tip. An earlier e2e ran on
`97b054bbb`, which observed a stuck AG recovering after our force-drift
but also had a dormancy bug (fixed since) and the requestedAt bump
missing (fixed since). Both of the current shape's load-bearing
mechanisms — requestedAt trigger + status-condition drift — need
verification against a live source-watcher on a fresh e2e run before
merge.
 2026-07-06 09:53:56 +05:00
Andrei Kvapil 59f035ff53 fix(kubevirt-instancetypes): restore persistent EFI/TPM state 
Reverts #3006. Dropping persistent EFI/TPM was motivated by a live-migration
concern that rested on an outdated assumption — that an RWO Filesystem
backend-storage PVC pins the VM to its node. KubeVirt has migrated
RWO-Filesystem backend storage since v1.4 (kubevirt/kubevirt#12629): on
migration it creates a fresh target state PVC and copies the small state blob,
so persistent-EFI/TPM VMs on the default replicated storage live-migrate fine
(verified: the VM reports LiveMigratable=True and the copy-on-target PVC is
created on migration). Restore upstream's persistent firmware state for the
windows.* preferences.

Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Andrei Kvapil <andrei.kvapil@aenix.io>
 2026-07-06 09:49:50 +05:00
myasnikovdaniil e27c586a5d test(e2e): wait for LINSTOR pool free capacity between tenant Kubernetes tests (#3210) 
## What this PR does

Adds a companion `cozy_wait_linstor_pool_free` helper to
`hack/e2e-apps/run-kubernetes.sh` and calls it from `cozy_cleanup` right
after `cozy_wait_tenant_drained`. The helper polls LINSTOR for the
smallest ZFS pool `FreeCapacity` across every satellite and blocks until
it reaches a per-satellite threshold (default 90 GiB), bounded to 300 s
with a loud warning on timeout.

### Root cause (proven)

`kubernetes-previous.bats` intermittently failed with `zfs create -V
20976048KB data/pvc-... returned with exitcode 1: cannot create '...':
out of space` on a satellite that measured **132 GiB free 35 minutes
later** — so this is not an absolute capacity issue but a timing race.

Root cause is captured in the cozyreport artefact of run 28751310913 (PR
#3044), satellite ErrorReport `6A4AADFD-349B2-000000`. The previous
tenant Kubernetes test's `cozy_wait_tenant_drained` gate returned as
soon as its PVCs disappeared from the API, but the satellite-side ZFS
`zvol destroy` for the just-deleted replicated (autoPlace=3, DRBD) 20
GiB worker disks lagged tens of seconds behind that signal as DRBD
adjusted, unref counts drained, and ZFS batch-destroyed the datasets.
The next tenant test's `CreateVolume` landed inside that window,
LINSTOR-CSI retried autoplace against the still-being-torn-down previous
placement (`Do not place with resource: pvc-<same-uid>` implicit
anti-collocation, no CSI bug), and worker Machine bringup stretched past
the MHC `nodeStartupTimeout` budget with no cluster-side fault.

The proximate observation (`zfs create out of space`) surfaced only
because of PR #3209 which teaches `hack/cozyreport.sh` to bundle
satellite ErrorReports into the artefact. Without that, the root cause
was ambiguous between several plausible LINSTOR internal races and I
nearly opened a wrong upstream issue.

### Threshold rationale

Two 20 GiB replicated worker targets = 60 GiB per satellite (autoPlace=3
places one replica per node) plus two 21 GiB CDI scratch PVCs (worst
case both landing on the same node via the `local` storageClass) = ~82
GiB per-satellite peak footprint. 90 GiB default threshold covers that
with margin.

### Scope

`hack/e2e-apps/run-kubernetes.sh` only. E2E-scope, no product or chart
change. Threshold and timeout are function args with sensible defaults;
caller can override.

### Verified

`jq` expression against a live LINSTOR 1.33.2 cluster:

```
$ kubectl -n cozy-linstor exec deploy/linstor-controller -- sh -c 'linstor --machine-readable sp l 2>/dev/null | jq -r "first | .[] | select((.provider_kind | test(\"^ZFS\")) and .free_capacity != null) | \"\(.free_capacity):\(.node_name)\"" | sort -n'
3088864:talos-5jt-iq3
42483036:talos-te2-p24
197991872:talos-6cn-cgg
```

Matches `linstor sp l` output exactly (2.95 GiB / 40.51 GiB / 188.82
GiB). ZFS-only filter excludes DISKLESS `Long.MAX_VALUE` sentinel; `and
.free_capacity != null` guard prevents OFFLINE satellite pool objects
(whose `free_capacity` is optional per REST schema) from ranking as
`null` in the sort and hanging the loop.

Contract mirrors `cozy_wait_tenant_drained`: bounded deadline, `|| true`
at the call site, loud timeout warning that includes the smallest
observed pool and its satellite. Timeout can never wedge the suite worse
than the status quo.

### Release note

```release-note
NONE
```

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

## Summary by CodeRabbit

* **Bug Fixes**
* Improved cleanup reliability by waiting for storage capacity to be
released before finishing teardown, reducing cases where resources
appear deleted but storage is still unavailable.
* Made teardown more resilient with a configurable timeout to avoid
premature completion.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
 2026-07-06 09:38:29 +05:00
Ivan Okhotnikov 287dbca600 test(e2e): wait for LINSTOR pool free capacity between tenant Kubernetes tests 
kubernetes-previous.bats intermittently fails with `zfs create -V ...
returned with exitcode 1: cannot create '...': out of space` on a
satellite even though the LINSTOR pool measured 30+ minutes later shows
100+ GiB free per satellite. Root cause proven from a satellite
ErrorReport captured in the cozyreport artefact (PR #3044 run
28751310913, report id 6A4AADFD-349B2-000000): the previous tenant
Kubernetes test's cozy_wait_tenant_drained gate returned as soon as
its PVCs disappeared from the API, but the satellite-side ZFS `zvol
destroy` for the just-deleted replicated (autoPlace=3, DRBD) 20 GiB
worker disks lagged tens of seconds behind that signal as DRBD
adjusted, unref counts drained, and ZFS batch-destroyed the datasets.
The next tenant test's CreateVolume then landed inside that window
against a pool that had not yet reclaimed the space, LINSTOR-CSI
retried autoplace against its own still-being-torn-down previous
placement, and worker Machine bringup stretched past the MHC
nodeStartupTimeout budget with no cluster-side fault.

Add a companion `cozy_wait_linstor_pool_free` helper that polls the
LINSTOR controller for the smallest FreeCapacity across every ZFS
satellite pool and blocks until it reaches a per-satellite threshold
(default 90 GiB — covers the ~82 GiB worst-case footprint of two
20 GiB replicated worker targets across all three satellites plus two
21 GiB CDI scratch PVCs landing on the same node via the local
storageClass). Called from cozy_cleanup immediately after
cozy_wait_tenant_drained. Bounded and best-effort: 300 s timeout,
`|| true` at the call site, warning printed with the observed
pool state if the threshold is not reached in budget so the next test
does not misdiagnose the pressure.

The threshold is a floor for the concrete failure mode observed in the
E2E suite; the surrounding kubernetes-latest -> kubernetes-previous
handoff is the only run in the current suite that immediately follows
a heavy tenant teardown with another tenant provisioning, so this
gate is the natural place for the check. E2E-scope only, no product
or chart change.

The proximate observation (`zfs create out of space`) surfaced because
of PR #3209 which taught hack/cozyreport.sh to bundle satellite
ErrorReports; without that artefact the root cause was ambiguous
between multiple plausible LINSTOR internal races. Ships alongside
that PR.

Signed-off-by: Ivan Okhotnikov <xorokhotnikov@gmail.com>
 2026-07-06 00:14:54 +03:00
Ivan Okhotnikov d5098d9430 test(e2e): capture LINSTOR satellite ErrorReports in cozyreport 
The linstor-Satellite pod stdout log references its error reports
only by ID:

    [DeviceManager] ERROR LINSTOR/Satellite/... SYSTEM - Failed to
    create zfsvolume [Report number 6A4A394E-E4EA0-000000]

The body of that report (which carries the actual `zfs create -V …`
stderr, the DRBD adjust output, the storage-driver stack trace, and
the exit code) lives at `/var/log/linstor-satellite/ErrorReport-*.log`
on the satellite pod. crust-gather only captures pod stdout, so the
report body stays on the pod's ephemeral filesystem and is lost when
the e2e sandbox is torn down at end-of-job.

Consequence for post-mortem: a run that failed on a
`ResourceExhausted` / `Not enough available nodes` CreateVolume retry
loop shows only the driver-level symptom in cozyreport. Root cause
attribution (out-of-space, dataset name conflict, ZFS module error,
kernel signal) is not distinguishable without the report body, and
without the sandbox is unrecoverable.

Extend the linstor module in `hack/cozyreport.sh` to tar the
`ErrorReport-*.log` set off every `linstor-satellite` pod into
`linstor/error-reports/<node>.tgz`. Guarded so a satellite with zero
reports (the healthy case) leaves nothing in the tree, and a
transient exec / tar failure never fails the surrounding cozyreport
run.

Verified against a live dev cluster: label selector
`app.kubernetes.io/component=linstor-satellite` matches all three
satellite pods, and the tar bundle for one active satellite comes
out at ~60 KiB. No product-side change.

Signed-off-by: Ivan Okhotnikov <xorokhotnikov@gmail.com>
 2026-07-05 21:50:46 +03:00
IvanHunters b6ac249faa test(e2e): bump sandbox tenant storage quota 100Gi -> 200Gi (#3206) 
## What this PR does

Raises the E2E sandbox tenant's `resourceQuotas.storage` from 100Gi to
200Gi so back-to-back tenant Kubernetes tests (`kubernetes-latest.bats`
-> `kubernetes-previous.bats`) do not collide with tenant quota during
the second cluster's CDI import.

The 100Gi default in `packages/apps/tenant` is unchanged — this touches
only the E2E test tenant CR in `hack/e2e-install-cozystack.bats`.

### Root cause

Both kubernetes-*.bats tests provision a 2-worker tenant control plane
whose worker root disks are populated by CDI. CDI's second-phase
importer allocates a scratch PVC of the same 20Gi as the target disk
before the qcow2 unpack, so per-tenant peak footprint is:

```
2 × 20Gi target + 2 × 21Gi scratch = ~82Gi during import
```

collapsing to 40Gi at rest.

`kubernetes-latest`'s `cozy_cleanup` deletes the Kubernetes CR and
blocks in `cozy_wait_tenant_drained` (300s) waiting for KubeVirt VMs /
VMIs / namespace PVCs to disappear. The API-level PVC delete returns
quickly, but the LINSTOR CSI detach + DRBD resource release on the
underlying nodes lags past that budget. During the overlap window the
deleted PVCs still count against
`ResourceQuota.status.used.requests.storage` because the storage
provisioner has not finished releasing the volume. `kubernetes-previous`
then starts its own CDI import with 40Gi of stale `kubernetes-latest`
quota still charged, and CDI's scratch PVC create for the second worker
(used 80Gi + scratch 21Gi = 101Gi) is rejected by the tenant-quota
admission check at 100Gi. The second worker's DataVolume stalls in
`ImportInProgress`, the tenant node never joins, and the 12-minute
node-join deadline in `run-kubernetes.sh` fails
`kubernetes-previous.bats`.

### Evidence

`cozyreport.tgz` from the failing E2E run of PR #3044 (run 28730757213):

```
cdi-deployment log:
  scratch PVC API create errored:
    persistentvolumeclaims "…-qxl8r-disk-system-scratch" is forbidden:
    exceeded quota: tenant-quota,
    requested: requests.storage=22763536384 (21Gi),
    used: requests.storage=85899345920 (80Gi),
    limited: requests.storage=100Gi
```

The tenant-quota `status.used.requests.storage` in the snapshot decays
back to 40Gi by end-of-run — confirming the leak is a transient window
during the second test's start-up, not a permanent leak.

### Relation to prior work

Overlaps in scope with #2947 (`ci(e2e): bump tenant storage quota 100Gi
-> 200Gi`), which bundles this same bump alongside three other e2e
stabilization changes (crust-gather tag, harbor BucketClaim timeout,
cilium IP-conflict healer). #2947 has been stale since 2026-06-17 with
the maintainer suggesting it be closed. This PR isolates just the
quota-bump so it can land on its own scope; the other three changes are
orthogonal and can ship separately if #2947 stays dormant.

### Release note

```release-note
NONE
```

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

## Summary by CodeRabbit

* **Tests**
* Updated end-to-end coverage for tenant creation with isolated mode to
reflect higher storage quota expectations.
* Improved quota validation in test scenarios to better match supported
storage behavior.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
 2026-07-05 13:37:10 +03:00
Ivan Okhotnikov 3ab9f95625 test(e2e): bump sandbox tenant storage quota to 200Gi 
The E2E sandbox tenant (tenant-test) provisions back-to-back tenant
Kubernetes control planes via kubernetes-latest.bats and
kubernetes-previous.bats. Each control plane brings up two worker VMs
whose 20Gi root disks are populated by CDI, and CDI's second-phase
importer allocates a scratch PVC of the same size before the qcow2
unpack. Peak per-tenant footprint is therefore ~82Gi during import
(2 × 20Gi target + 2 × 21Gi scratch), collapsing to 40Gi at rest.

kubernetes-latest teardown deletes the tenant Kubernetes CR and waits
via cozy_wait_tenant_drained for the KubeVirt VMs, VMIs and namespace
PVCs to disappear. The API-level PVC delete returns quickly, but the
LINSTOR CSI detach + DRBD resource release on the underlying nodes
lags past the 300s drain budget. During that window, the deleted
PVCs still count against ResourceQuota's requests.storage because
the storage-provisioner has not finished releasing the volume — so
kubernetes-previous starts its own CDI import with 40Gi of stale
kubernetes-latest quota still charged, and CDI's scratch PVC create
for the second worker (used=80Gi + scratch=21Gi = 101Gi) is
rejected by the tenant-quota admission check at 100Gi. The second
worker's DataVolume never leaves ImportInProgress, the tenant node
never joins, and the 12-minute node-join deadline in
run-kubernetes.sh trips kubernetes-previous.bats.

Raise the sandbox tenant-quota storage ceiling to 200Gi so the
overlap between one test's slow storage cleanup and the next test's
CDI import fits inside the quota. The failing-run diagnostic is
cozyreport artifact from run 28730757213:

  cdi-deployment log:
    scratch PVC API create errored: … forbidden: exceeded quota:
    tenant-quota, requested: requests.storage=22763536384,
    used: requests.storage=85899345920,
    limited: requests.storage=100Gi

This is a test-infrastructure fix; the 100Gi default in the tenant
chart itself is unchanged. The assertion below the CR update is
lifted from 100Gi to 200Gi in lockstep.

Signed-off-by: Ivan Okhotnikov <xorokhotnikov@gmail.com>
 2026-07-05 11:40:22 +03:00
Ivan Okhotnikov 0d98e27981 fix(operator): hold final backoff window before give-up, note upstream version skew 
Address lexfrei's non-blocking follow-ups on the LGTM approval:

Follow-up #1 — give-up preempted the final backoff. `decideRecovery`
returned `recoveryActionGiveUp` as soon as `attempts >= maxRecoveryAttempts`,
without checking whether the final-force backoff window had elapsed. The
Owns()-watch re-fire from the N-th force-drift's own writes triggered a
reconcile milliseconds after the force, so the effective time-to-give-up
was ~7.5m (30s+60s+2m+4m) — not the ~11.5m documented on the constants
block. The final 4-minute response window the last nudge was supposed to
grant source-watcher was never actually granted.

Fix: extend the exhaustion branch to hold `Wait` until
`now - lastRecoveryAt >= backoffFor(maxRecoveryAttempts)` before switching
to GiveUp. Now the documented ~11.5m budget matches the observed
behaviour, and the last force gets a real chance to succeed (Owns
watch → source-watcher Ready=True copy-through) before we surface
SourceWatcherStalled.

Tests:
* `TestDecideRecovery` gains a case pinning `Wait` when
  attempts=maxRecoveryAttempts but the final backoff has not yet
  elapsed (the exact re-fire shape the previous behaviour tripped on).
* `TestUpdateStatus_OwnMarkerAttemptsExhaustedButFinalBackoffPending`
  drives the full updateStatus path against the same shape, asserting
  the PackageSource stays in Unknown/AwaitingSourceWatcherRecovery and
  the RequeueAfter equals the remaining backoff, NOT zero (GiveUp).

Follow-up #3 — upstream version-skew note. The Go module dependency
tracks `fluxcd/source-watcher/api/v2 v2.0.3` (CRD types package only),
but the deployed binary this workaround targets is pinned to v2.1.0
at `internal/fluxinstall/manifests/fluxcd.yaml`. Comments referencing
`drift.go:52` / `controller.go:164/254` describe the deployed binary,
not the compiled API package. Documented on the constants block so a
future flux bump re-verifies the anchors AND the watch predicate /
drift-detection heuristics, which are the load-bearing upstream
assumptions.

Follow-up #2 (envtest over full Reconcile→Owns→updateStatus cycle) is
architectural and out of scope for this PR — noted for a follow-up.

Signed-off-by: Ivan Okhotnikov <xorokhotnikov@gmail.com>
 2026-07-05 08:30:19 +03:00
Ivan Okhotnikov 37421d2fec docs(operator): honest JSON-merge-patch tradeoff, clarify asymmetric OG check, warn on partial force-drift 
Self-review nits raised in a skeptical pass over the current tip:

* The status-patch docstring in forceArtifactGeneratorDrift described
  the JSON-merge-patch full-array-replace on `status.conditions[]` as
  "net-neutral". That was too optimistic: if source-watcher writes a
  concurrent Reconciling condition between our Get and our Patch, our
  patch does transiently wipe it. source-watcher restores it on its
  next reconcile (which our requestedAt bump is about to trigger), but
  downstream tooling watching for Reconciling can observe the momentary
  absence. Rewrite the comment to name that trade-off explicitly and
  point at server-side apply as the fuller fix if the operator ever
  migrates to SSA globally.

* The unified own-marker/progressing/stuck gate in updateStatus is
  asymmetric on ObservedGeneration — isProgressingInRecovery checks it,
  isOwnMarker does not. That asymmetry is deliberate (a stale own-marker
  after a spec edit must still route to the state machine to avoid
  leaking the synthetic reason to the PackageSource — exactly the B2
  regression), not an oversight. Add a comment explaining the
  reasoning so a future edit doesn't "symmetrise" the check and re-open
  B2.

* forceArtifactGeneratorDrift's step-2 (metadata patch) failure was
  silent when it happened after a successful step-1 status patch. In
  that state source-watcher has been nudged, but the tracking
  annotations never persisted, so the next reconcile reads attempts=0
  and re-forces from scratch instead of accumulating toward
  SourceWatcherStalled — a persistent metadata-write-only apiserver
  malfunction would loop forever. Log the failure loudly with the
  intended attempt count so operators can spot this specific mode
  from cozystack-operator logs.

Signed-off-by: Ivan Okhotnikov <ivan.okhotnikov@aenix.io>
 2026-07-05 08:30:19 +03:00
Ivan Okhotnikov 596427deb3 docs(operator): scrub stale awaitSourceWatcherResponse references from comments and test names 
Self-review after the previous commit removed the awaitSourceWatcherResponse
helper (routing unified through maybeRecoverArtifactGenerator's state machine
via a single own-marker / progressing-in-recovery / stuck gate). Four
comments and two test names still referenced the now-deleted symbol —
misleading anyone tracing the architecture.

* maybeRecoverArtifactGenerator docstring: rewrote the trade-off paragraph
  to describe the unified gate and Wait branch instead of the removed
  helper.
* TestMaybeRecoverArtifactGenerator_DoesNotResetCounterOnFreshTransition
  comment: same rewrite.
* TestUpdateStatus_OwnMarkerRoutesToAwait renamed to
  TestUpdateStatus_OwnMarkerHoldsTrackingViaStateMachine + comment
  rewritten to describe the state-machine Wait branch.
* TestUpdateStatus_ProgressingDuringRecoveryRoutesToAwait renamed to
  TestUpdateStatus_ProgressingDuringRecoveryHoldsTracking + comment
  rewritten in parallel.

Pure doc / naming; no code or test-body changes. Tests still pass with
-race, vet clean, golangci-lint 0 issues.

Signed-off-by: Ivan Okhotnikov <ivan.okhotnikov@aenix.io>
 2026-07-05 08:30:19 +03:00
Ivan Okhotnikov 7b08fcdca9 fix(operator): unify recovery gating through state machine, close source-watcher-unresponsive dormancy loop 
Self-review on 467b788b8 caught a MAJOR bug introduced by the separate
awaitSourceWatcherResponse helper: if source-watcher never responds to
a forced drift (network partition, missing predicate, dead pod), our
own reasonRecoveryForced marker keeps reflecting back via Owns() forever,
and the wait-forever helper schedules a 1-second requeue tight loop
without ever advancing the retry state machine. SourceWatcherStalled
give-up was unreachable.

Fix: remove awaitSourceWatcherResponse and unify all three recovery
entry conditions — our own marker, source-watcher's Progressing during
rebuild, and the classic first-time stuck detection — through a single
gate that routes to maybeRecoverArtifactGenerator. The state machine's
decideRecovery is the single arbiter of wait/force/give-up, and its
Wait branch is bounded by the same backoff schedule as Force. When
backoff elapses on a still-reflecting own-marker the machine escalates
to another force (bumps attempts, re-stamps requestedAt); when attempts
exhaust it surfaces SourceWatcherStalled honestly.

The own-marker filter also no longer requires attempts>0, so an
operator crash between step 1 (status patch) and step 2 (metadata
patch) of forceArtifactGeneratorDrift is still routed through the
state machine on the next reconcile rather than leaking the synthetic
marker onto the PackageSource.

Also address the non-blocking notes from the same self-review:

* Docstring on maybeRecoverArtifactGenerator was still describing the
  removed reset-counter branch and the "requestedAt alone is a no-op"
  framing. Rewrite to explain the current shape: status write triggers
  the drift signal, requestedAt bump triggers the reconcile event —
  both are required, neither alone works.
* TestForceArtifactGeneratorDrift_MetadataPatchFailure_RollbackAnnotations
  now asserts reconcile.fluxcd.io/requestedAt is also rolled back on
  the metadata-patch failure branch (symmetric with the tracking
  annotations).
* TestUpdateStatus_OwnMarkerRoutesToAwait asserts an exact 25s
  RequeueAfter (backoffFor(1)=30s minus 5s elapsed) so a silent
  regression in the backoff arithmetic is caught.

New regression tests for the source-watcher-unresponsive path:

* TestUpdateStatus_OwnMarkerStale_TriggersRetry: with attempts=1 and
  lastRecoveryAt older than backoffFor(1), the reconcile must
  escalate through the Force branch and bump attempts to 2 rather
  than idle in a tight requeue loop.
* TestUpdateStatus_OwnMarkerAttemptsExhausted_GivesUp: with
  attempts=maxRecoveryAttempts and the marker still reflecting back,
  the reconcile must set PackageSource Ready=False /
  SourceWatcherStalled instead of extending the loop indefinitely.

Signed-off-by: Ivan Okhotnikov <ivan.okhotnikov@aenix.io>
 2026-07-05 08:30:19 +03:00
Ivan Okhotnikov 934ca46a36 fix(operator): bump requestedAt with force-drift, filter own writes, remove reset-counter loop 
Address lexfrei's three blockers on the force-drift approach — as
committed, the recovery driver did not function on a live cluster.

**B1**: `forceArtifactGeneratorDrift` wrote `Ready=False` on the AG's
status subresource but did not touch its metadata annotations.
source-watcher v2.1.0 registers its ArtifactGenerator watch with
`predicate.Or(GenerationChangedPredicate, ReconcileRequestedPredicate)`
— a status-only patch changes neither the generation nor
`reconcile.fluxcd.io/requestedAt`, so it does NOT enqueue a
source-watcher reconcile. The AG's periodic self-requeue is hardcoded
to `time.Hour`, well outside the 15m HR install timeout. Fix: also
stamp `reconcile.fluxcd.io/requestedAt` in the metadata patch so the
ReconcileRequestedPredicate fires and source-watcher's next reconcile
picks up the `IsFalse(Ready)` drift signal.

**B2**: `Owns(&ArtifactGenerator{})` re-fires this reconciler on the
force-drift's own writes. Both the status patch (Ready=False, reason
`SourceWatcherRecoveryForced`) and the metadata patch enqueue a
PackageSource reconcile. Blindly running `clearRecoveryTracking` on
those re-entrant fires nuked the attempts counter — the give-up
budget was unreachable, and the synthetic marker leaked to
`PackageSource.status.conditions[Ready]`. Fix: in `updateStatus`,
detect our own reason and route into a new `awaitSourceWatcherResponse`
helper that holds tracking and reports Unknown /
AwaitingSourceWatcherRecovery on the PS. The same helper covers the
Progressing/Unknown write source-watcher issues during rebuild once
tracking is present (attempts > 0) — otherwise the transient
Progressing would trigger the same premature tracking clear.

**B3**: The attempt-counter reset keyed on
`Ready.LastTransitionTime > lastRecoveryAt`. That inequality is true
on every source-watcher response during recovery (the Progressing
write is always newer than our force), so under the very race this
driver targets the counter would reset every cycle and
`recoveryActionGiveUp` was unreachable. Fix: remove the reset. The
canonical counter reset is the natural `clearRecoveryTracking` on the
not-stuck path (Ready=True from source-watcher, or a real upstream
Ready=False), and the HA/leader-election requirement already
contains the "missed recovery event on operator restart" edge case
the reset was trying to paper over.

Extract `reasonRecoveryForced` to a named constant so both the
writer (forceArtifactGeneratorDrift) and the reader (updateStatus'
own-marker filter) use the same symbol.

New tests pin all three fixes:
- ForceArtifactGeneratorDrift_Success now asserts the
  reconcile.fluxcd.io/requestedAt annotation is set (B1).
- UpdateStatus_OwnMarkerRoutesToAwait asserts we do NOT clear
  tracking and do NOT copy the synthetic Ready=False through when we
  see our own marker (B2).
- UpdateStatus_ProgressingDuringRecoveryRoutesToAwait covers the
  second Owns() re-entrancy path — source-watcher's Progressing
  write during rebuild (B2).
- UpdateStatus_ReadyTrueClearsTracking pins that the natural not-
  stuck path IS the canonical counter reset after B3.
- The old ResetsCounterOnFreshTransition test is replaced with
  DoesNotResetCounterOnFreshTransition, asserting the opposite
  behaviour that closes B3.

Signed-off-by: Ivan Okhotnikov <ivan.okhotnikov@aenix.io>
 2026-07-05 08:30:19 +03:00
Ivan Okhotnikov f64081b232 fix(operator): schedule follow-up reconcile when Ready is Unknown within grace 
E2E on dev3 caught a dormancy bug that renders the whole recovery driver
inactive in practice: when updateStatus copies Ready=Unknown across but
the AG's LastTransitionTime is still within stuckGracePeriod,
artifactGeneratorStuck returns false and we fall through to the copy
path with no RequeueAfter set. If the reconcile happens to fire in the
[stuck_start, stuck_start+30s) window — a routine case since operator
restarts touch every PackageSource — the controller then sleeps
waiting for an AG-status change that will never come (that IS the
fluxcd/pkg#934 lost-write race), up to the 10h informer resync.

Observed on dev3: `cozystack.networking` stuck at Ready=Unknown for 9+
minutes with zero recovery attempts. Two reconciles fired at
stuck_start+23s and +24s — both correctly detected "not yet stuck" —
and then nothing until observation ended. Upstream OCIRepository
revision was unchanged throughout, ruling out a coincidental source
update as the cause.

Fix: whenever we copy Ready=Unknown to the PackageSource (or set a
synthetic Unknown because no Ready condition exists yet), compute the
remaining grace period against the AG's LastTransitionTime (or
CreationTimestamp for the missing-Ready branch) and schedule a
RequeueAfter at that expiry. A 1s floor prevents a zero-delay tight
loop when the anchor is right at or past grace expiry.

Extracted the delay computation to `agFollowUpDelay` for direct unit
testing. Adds three tests: the follow-up scheduling on Ready=Unknown
in-grace, the same on the missing-Ready branch, and a table test on
agFollowUpDelay covering the floor edge cases.

Signed-off-by: Ivan Okhotnikov <ivan.okhotnikov@aenix.io>
 2026-07-05 08:30:19 +03:00
Ivan Okhotnikov 4df18df5de test(operator): pin maybeRecoverArtifactGenerator branches, tighten wait assertions, polish comments 
Address the fourth-pass self-review's non-blocking findings:

* Add TestMaybeRecoverArtifactGenerator_{ForceBranch, WaitBranch,
  GiveUpBranch, ResetsCounterOnFreshTransition}. The three switch
  outcomes of the driver (force / wait / give-up) and the reset-counter
  guard on a fresh Ready.LastTransitionTime transition were previously
  only covered indirectly through their pure-function components. Now
  each outcome is asserted end-to-end against a fake client with a
  status subresource, verifying the Ready condition, tracking
  annotations, RequeueAfter, and — for the wait branch — that no AG
  writes are issued at all.
* Tighten TestDecideRecovery's wait-branch assertions from a loose lower
  bound to exact equality. decideRecovery is deterministic
  (needed - elapsed) so an exact assertion catches subtle backoff
  regressions a lower bound would miss (coderabbit's inline nit).
* Rename the local var `wantWaitMin` to `wantWait` for the tighter
  assertion shape.
* Fix two stale identifier references in comments
  (maybeRequeueArtifactGenerator, AwaitingSourceWatcherRequeue) left
  over from the rename pass.
* Add a docstring paragraph on forceArtifactGeneratorDrift explaining
  why the JSON-merge-patch full-array-replace on
  `.status.conditions[]` is net-neutral in this workaround's context.

Signed-off-by: Ivan Okhotnikov <ivan.okhotnikov@aenix.io>
 2026-07-05 08:30:19 +03:00
Ivan Okhotnikov 7c3af85e58 fix(operator): force source-watcher drift via status patch, not requeue bump 
Rework the fluxcd/pkg#934 recovery driver after myasnikovdaniil's B1
finding: bumping reconcile.fluxcd.io/requestedAt cannot recover a stuck
ArtifactGenerator because source-watcher v2.1.0's reconcile is
drift-gated. detectDrift (artifactgenerator_drift.go:52) returns
NoDriftDetected for exactly the stuck signature we target — Inventory
populated, ObservedSourcesDigest set, Ready=Unknown on the current
generation — so the ReconcileRequestedPredicate fires a reconcile that
lands on the no-drift early-return at controller.go:164 and MarkTrue at
:254 is never reached. Every attempt in the previous shape was a no-op.

New mechanism: force drift by patching Ready=False on the AG's status
subresource. detectDrift picks this up as NotReady drift, source-watcher
takes the drifted branch, and either MarkTrue (on healthy rebuild) or
MarkFalse with a real reason (on genuine failure) replaces our synthetic
False. Owns() watch fires this reconciler to copy the resolved condition
through. Same bounded-retry harness (30s / 60s / 2m / 4m / 4m budget
inside the 15m HR install timeout, SourceWatcherStalled surfaced on
exhaustion), same annotation-based state on the AG, same rollback
contract on Patch failure — only the action itself changed.

Additional fixes bundled in:

* Reset attempt counter when the AG's Ready.LastTransitionTime is newer
  than our lastRecoveryAt, so a fresh stall episode after a previous
  give-up gets a fresh budget rather than an immediate re-give-up
  (myasnikovdaniil's follow-up #3).
* Propagate updateStatus errors from Reconcile instead of swallowing them,
  so controller-runtime's exponential backoff retries transient failures
  on force-drift / status-update writes rather than dropping them silently
  (coderabbit's inline finding).
* forceArtifactGeneratorDrift documents the two-subresource shape (status
  patch first, metadata patch second) and the partial-failure semantics.
* New tests cover the status-patch success / status-patch failure /
  metadata-patch failure paths, all with fake client + interceptor. The
  successful case asserts Ready=False actually lands in the apiserver via
  WithStatusSubresource.
* All annotation / action / decision / helper names renamed from
  *Requeue* to *Recovery* to match the new semantics.

Signed-off-by: Ivan Okhotnikov <ivan.okhotnikov@aenix.io>
 2026-07-05 08:30:19 +03:00
Ivan Okhotnikov 4c3099d6f2 test(operator): pin overwrite-rollback contract, minor polish 
Address the fourth-pass self-review's non-blocking NITs:

* Add TestBumpArtifactGeneratorRequeue_PatchFailure_RollbackRestoresPriorBump
  to cover the subtle case the two existing rollback tests miss: when the
  tracking annotations were already set from a prior bump, a failed Patch
  must restore the PRIOR values, not delete the keys. A refactor that
  naively swapped rollback for `delete(ag.Annotations, key)` would pass
  the nil-rollback tests but silently regress this contract.
* Rename the cloneAnnotations subtest from "deep copy" to "independent
  copy" — the code documents shallow copy semantics and the misnomer was
  the only thing pointing readers at the wrong mental model.
* Hoist `now.UTC().Format(time.RFC3339Nano)` to a local so the two
  timestamp annotations use one formatted value instead of two identical
  Format calls.

Signed-off-by: Ivan Okhotnikov <ivan.okhotnikov@aenix.io>
 2026-07-05 08:30:19 +03:00
Ivan Okhotnikov 28834ca874 test(operator): cover rollback contract, deletion guard, and clone helper for the requeue driver 
Address the third-pass self-review's blocker: the mutation contract on
bumpArtifactGeneratorRequeue / clearRequeueTracking, the DeletionTimestamp
early-return in Reconcile, and the nil-preserving cloneAnnotations helper
had no regression tests, so a future refactor could silently break the
contracts documented in comments.

New fake-client + interceptor-based tests:

* TestCloneAnnotations — pins the "nil in / nil out" sentinel behaviour
  and mutation-independence of the shallow copy.
* TestBumpArtifactGeneratorRequeue_Success — asserts the persisted state
  matches the caller's in-memory `ag.Annotations` on success, including
  that reconcile.fluxcd.io/requestedAt is actually written (previously
  never asserted anywhere).
* TestBumpArtifactGeneratorRequeue_PatchFailure_Rollback and its
  PreservesUnrelated variant — drive the Patch-failure branch via an
  interceptor client and assert both that our tracking annotations are
  rolled back and that pre-existing unrelated annotations survive.
* TestClearRequeueTracking_Noop — asserts we don't issue a no-op Patch on
  every healthy reconcile when there are no tracking annotations to clear.
* TestClearRequeueTracking_PatchFailure_Rollback — same rollback contract
  as bump, on the clear path.
* TestReconcile_DeletionTimestamp_EarlyReturn — asserts a PackageSource
  with a deletionTimestamp does not receive any writes and does not
  schedule a RequeueAfter.

Doc tightening in the same commit:

* Reword clearRequeueTracking's comment on the flux annotation semantics —
  standard flux controllers do not clear reconcile.fluxcd.io/requestedAt,
  they only advance .status.lastHandledReconcileAt via the
  ReconcileRequestedPredicate.
* Note controller-runtime v0.15+'s default RecoverPanic behaviour where
  the retry-driver's unhandled-action panic is raised, so a future
  SetupWithManager flip that disables it is easier to spot.
* Explain why cloneAnnotations' shallow copy is sound (annotation values
  are immutable strings).

Signed-off-by: Ivan Okhotnikov <ivan.okhotnikov@aenix.io>
 2026-07-05 08:30:19 +03:00
Ivan Okhotnikov c3f9125163 fix(operator): skip deleted PackageSource, restore ag mutation contract, document HA + timing rationale 
Address second-pass self-review findings on the bounded-requeue driver:

* Skip reconcile early when the PackageSource has a deletionTimestamp so
  the retry driver stops writing "AwaitingSourceWatcherRequeue" onto an
  object about to be garbage collected. ownerReference cascade handles
  the ArtifactGenerator cleanup.
* Fix the mutation contract on bumpArtifactGeneratorRequeue and
  clearRequeueTracking: the previous deep-copy shape left the caller's
  `ag` desynchronised from the apiserver on success. Now mutations land
  on `ag` itself with a rollback to the pre-call annotation snapshot on
  Patch failure — so success reflects persisted state and failure does
  not leak unpersisted state to subsequent code paths.
* Convert the "unreachable" default in maybeRequeueArtifactGenerator to
  an explicit panic so a future requeueAction added without a matching
  case fails loudly instead of freezing the Ready condition silently.
* Document the timing constants' rationale (grace period matches the
  observed &lt;10s source-watcher settle time on dev3; total ~11.5m budget
  keeps SourceWatcherStalled inside the 15m HR install timeout) and note
  the operator must run with leader election when replicas > 1 so the
  annotation-based retry state does not race.

Signed-off-by: Ivan Okhotnikov <ivan.okhotnikov@aenix.io>
 2026-07-05 08:30:19 +03:00
Ivan Okhotnikov c5845db221 refactor(operator): thread single now through reconcile, harden patches, tighten status 
Address self-review nits on the bounded-requeue driver:

* Sample `now := time.Now()` once at Reconcile entry and thread it into
  `updateStatus`, `artifactGeneratorStuck` and `maybeRequeueArtifactGenerator`
  so every time-sensitive check inside a reconcile agrees on the same
  wall-clock reading.
* Set `ObservedGeneration` in the three early Ready-condition branches
  (`SourceRefNotSet`, `ArtifactGeneratorNotFound`, `ArtifactGeneratorNotReady`)
  for parity with the retry-driver branches.
* Patch the ArtifactGenerator via a deep copy in
  `bumpArtifactGeneratorRequeue` and `clearRequeueTracking` so a failed Patch
  no longer leaves the caller's in-memory `ag` desynchronised from the
  apiserver.
* Document the retry driver's dependency on source-watcher's
  ReconcileRequestedPredicate on ArtifactGenerator, so a future upstream
  change dropping the predicate is easier to trace.

Signed-off-by: Ivan Okhotnikov <ivan.okhotnikov@aenix.io>
 2026-07-05 08:30:19 +03:00
Ivan Okhotnikov ff314797e9 refactor(operator): drive source-watcher via bounded requeue instead of synthesising PackageSource Ready 
Replaces the Ready=True synthesis for PackageSources whose ArtifactGenerator
hit the fluxcd/pkg#934 patch.Helper split-write race with a bounded retry
driver: on detected stall, bump reconcile.fluxcd.io/requestedAt with
exponential backoff (30s to 4m cap) up to 5 attempts, then surface as
PackageSource Ready=False with reason SourceWatcherStalled.

The previous approach could mask a genuine regeneration failure if the same
patch.Helper race that the workaround exists for also swallowed a real
Ready=False write, and it permanently reinterpreted PackageSource.Ready
semantics from "current revision processed" to "valid consumable artifacts
exist". Requeue keeps the CRD contract intact and, on stall exhaustion,
produces an honest failure signal an operator can act on instead of a
false-positive True.

Retry state (attempts + last-bump timestamp) lives on the AG as
cozystack.io/source-watcher-* annotations so it survives operator restarts.

Signed-off-by: Ivan Okhotnikov <ivan.okhotnikov@aenix.io>
 2026-07-05 08:30:19 +03:00
Ivan Okhotnikov 88d62612fe docs(operator): correct meta.SetStatusCondition rewrite semantics in comment 
review-branch skill flagged that the comment claimed
meta.SetStatusCondition rewrites LastTransitionTime on Status OR
Reason change. Actually only Status change triggers a
LastTransitionTime rewrite — Reason and Message are updated in
place, LastTransitionTime is preserved. Outcome (repeated
identical calls are no-op-y on persisted state) is unchanged;
just tightening the technical claim to match apimachinery.

Signed-off-by: Ivan Okhotnikov <ivan.okhotnikov@aenix.io>
 2026-07-05 08:30:19 +03:00
Ivan Okhotnikov 122584045b docs(operator): link workaround to fluxcd/pkg#934 (existing upstream issue) 
Reviewer nit was to file an upstream issue so the workaround does
not silently outlive its cause. Checked first — fluxcd/pkg already
has one open: #934 \"patch.Helper creates race conditions in
downstream controllers\" (May 2025). Same root cause we traced:
patch.Helper writes .status and .status.conditions as separate
apiserver requests, so an etcd blip between the two leaves the
object half-persisted. The upstream issue is broader than just
source-watcher — every fluxcd downstream controller consuming
fluxcd/pkg's runtime patch helper is affected.

Update the TODO(remove) marker on our workaround to point at
that issue and note that both source-watcher v2.1.0 and v2.2.x
(flux-aio latest) pull the same fluxcd/pkg runtime, so bumping
source-watcher alone does not remove the need for this synthesis.
Compile + unit tests unchanged.

Signed-off-by: Ivan Okhotnikov <ivan.okhotnikov@aenix.io>
 2026-07-05 08:30:19 +03:00
Ivan Okhotnikov 2fcf0bf5be fix(operator): document PackageSource Ready contract change + address review nits 
kvaps and lexfrei confirmed the diagnosis by tracing source-watcher
v2.1.0 (same code paths in v2.2.x / flux-aio latest, so a simple
version bump does not remove the need for this workaround):

  * fluxcd's patch.Helper patches .status and .status.conditions as
    SEPARATE apiserver requests, so an etcd timeout during first-
    install load can land between them — leaving Inventory /
    ObservedSourcesDigest persisted while the Ready-condition write
    is lost.
  * On the next reconcile the !hasDrifted branch returns before
    touching conditions. The deferred summarizeStatus still runs and
    adjusts the Reconciling condition, but it never promotes a
    stuck Unknown Ready to True — that promotion only happens on the
    successful hasDrifted path.

Two substantive changes prompted by the reviews:

  1. Document the semantic change the workaround introduces on
     PackageSource.status.conditions[Ready]. fluxcd/pkg/runtime/
     conditions.Set writes condition.ObservedGeneration =
     ag.Generation on EVERY condition mutation including the
     Progressing/Unknown mark at the start of a rebuild. And
     ArtifactGeneratorStatus does not expose an object-level
     ObservedGeneration (only ReconcileRequestStatus is inlined).
     So Generation-matching cannot distinguish 'current revision
     fully processed' from 'rebuild in progress, previous
     Inventory/digest still persisted' — meaning the predicate can
     (and does) fire during a content-only regeneration (new OCI
     revision, same .metadata.generation).

     Net effect: PackageSource Ready no longer dips through Unknown
     while a new revision is being processed; it now means 'valid
     consumable artifacts exist' rather than 'the current revision
     has been processed'. Kvaps flagged this as the better contract
     for install/upgrade ordering (no InProgress flapping, and a
     genuine regeneration failure still surfaces as Ready=False on
     the successful hasDrifted path). Document it inline instead of
     implying the workaround only fires in the stuck-status window.

  2. Reworked the predicate signature to take the pre-resolved
     Ready condition as a parameter. The caller was already doing
     the FindStatusCondition lookup for the pass-through path, so
     the helper no longer duplicates it (kvaps' nit).

Also refreshed the pre-existing comments:
  * The 'early-exits before touching status again' step is now
    correctly worded as 'returns before touching conditions again;
    the deferred summarizeStatus never promotes a stuck Unknown
    Ready to True'.
  * Bug attribution now names the code paths ('!hasDrifted branch',
    'summarizeStatus deferred'), notes that v2.1.0 and v2.2.x
    ('flux-aio latest') both ship the affected code, and carries a
    TODO(remove once upstream is fixed) marker so the workaround
    cannot silently outlive its cause.

Tests: existing 8 subtests updated to construct the ready condition
manually so the call matches the new signature, plus one new
subtest locking in the intentional 'content-only regeneration
predicate fires' behaviour so a future refactor that tightens the
predicate back to 'current revision processed' — which would
re-introduce the flapping — fails loudly. 9/9 pass.

Signed-off-by: Ivan Okhotnikov <ivan.okhotnikov@aenix.io>
 2026-07-05 08:30:19 +03:00
Ivan Okhotnikov a03e879316 fix(operator): require Ready.ObservedGeneration == ag.Generation before synthesising 
Own-audit found the first pass had a spec-update race: if a user
edited PackageSource.spec.variants on a running cluster, cozystack
would bump the derived ArtifactGenerator to Generation=2, and
between "source-watcher started reconciling gen=2" and "source-
watcher wrote new Inventory + ObservedSourcesDigest for gen=2" the
ArtifactGenerator still exposes the previous generation's
Inventory / ObservedSourcesDigest AND Ready=Unknown (Progressing).
The workaround would synthesise Ready=True on stale artifacts, and
downstream Flux would consume the previous-generation
ExternalArtifacts as if the new spec had converged.

Tighten the predicate to require the Ready condition's
ObservedGeneration to match ag.Generation. Only when source-watcher
has actually processed the current spec (which happens strictly
after it writes the current-generation Inventory + Digest) does the
Ready condition carry ObservedGeneration == ag.Generation. That is
exactly the window where Inventory / Digest are guaranteed current-
spec AND only the status patch on Ready itself failed — the actual
upstream bug this workaround exists to paper over.

Consequence: if Ready is missing entirely we return false now
(was true). We cannot verify ObservedGeneration on an absent
condition, and the missing-Ready state is indistinguishable from
mid-reconcile-of-a-newer-generation. On the next reconcile source-
watcher will write at least a Progressing condition, at which point
the current-spec Generation check will be evaluable and the
workaround will kick in if — and only if — Inventory / Digest also
match the current spec.

Tests updated:
  * changed 'no Ready condition' case from want=true to want=false
    (reflecting the tighter invariant);
  * added a case where Ready=Unknown carries an OLD Generation
    (want=false — do not synthesise on stale artifacts);
  * stamped ObservedGeneration on every existing condition-bearing
    case so they exercise the matching-Generation branch;
  * introduced a small agWithGen helper to keep the fixtures
    readable now that Generation is load-bearing.

All 8 subtests pass; existing operator suite unaffected.

Signed-off-by: Ivan Okhotnikov <ivan.okhotnikov@aenix.io>
 2026-07-05 08:30:19 +03:00
Ivan Okhotnikov 73e88e8ff9 fix(operator): synthesise PackageSource Ready when source-watcher status-patch stalls 
Symptom
  cozystack-platform HelmRelease times out at 15m on fresh install
  with a single PackageSource stuck in InProgress. Snapshot from
  #3178 CI run 28587656430:

    PackageSource cozystack.vm-disk-application
      Ready: Unknown, reason: Progressing
      lastTransitionTime: 2026-07-02T13:39:26Z
      (snapshot generated 30+ minutes later — no state change)

    ArtifactGenerator cozystack.vm-disk-application
      status.inventory:              2 items with sha256 digests
      status.observedSourcesDigest:  populated
      status.conditions[Ready]:      Unknown, reason: Progressing
      status.conditions[Reconciling]: True, reason: Progressing

  ExternalArtifacts are produced. Downstream Flux consumes them and
  its HelmReleases install fine. But PackageSourceReconciler
  faithfully copies the Unknown Ready condition through, so
  cozystack-platform (which waits on PackageSource.Ready=True) never
  converges.

Root cause
  Traced in fluxcd/source-watcher:v2.1.0 log for the affected
  ArtifactGenerator:

    13:39:28.475  ExternalArtifact vm-disk       reconciled ✅
    13:39:28.641  ExternalArtifact vm-disk-rd    reconciled ✅
    13:39:45.719  ❌ failed to update status: etcdserver: request timed out
    13:39:45.720  ❌ Reconciler error (etcd timeout)
    13:39:46.485  ✅ No drift detected, 2 artifact(s) up to date
    ——— nothing else for this ArtifactGenerator ———

  Under first-install etcd pressure the patchStatus call times out.
  The reconciler returns the etcd error; controller-runtime prints
  the "returned both a result and non-nil error, RequeueAfter
  ignored" warning and requeues. The next reconcile hits the "No
  drift detected" short-circuit (artifacts are already up to date)
  and early-exits before touching status. Result: the Ready
  condition is stuck Unknown until either the process restarts or
  something forces a state change on the ArtifactGenerator. Neither
  happens in a real install run.

  This is an upstream bug in source-watcher's reconcile loop. It has
  been papered over once already at the runtime-tuning level in
  #3163 (source-watcher concurrency + CPU), but a bad etcd second
  during install still lands us here. Cozystack cannot afford to
  wait for an upstream release cycle to unblock every install.

Fix
  PackageSourceReconciler.updateStatus now checks two invariants
  before falling back to copying the (potentially stuck) upstream
  Ready condition:

    1. ArtifactGenerator.status.Inventory is non-empty
    2. ArtifactGenerator.status.ObservedSourcesDigest is set

  Both fields are only written by source-watcher after the artifacts
  have actually been produced — status.Inventory carries the
  ExternalArtifact digests downstream Flux is already consuming. If
  BOTH are present AND the upstream Ready condition is still Unknown
  (or absent), the PackageSource Ready condition is synthesised as
  True with a distinctive reason `ArtifactsGeneratedAwaitingUpstreamStatus`
  and a message that names the upstream bug and the observable state
  it relied on. If the upstream Ready ever catches up (True or False)
  the Owns() watch re-fires the reconciler and the real condition
  overwrites the synthetic one — True stays True, False surfaces the
  real failure. Failures are NEVER masked: the synthesis path only
  triggers when Ready is Unknown or missing.

Safety
  * Never synthesises True over a real False → upstream failure is
    surfaced unchanged.
  * Requires BOTH Inventory non-empty AND ObservedSourcesDigest set
    → a fresh ArtifactGenerator that has literally not started work
    cannot be prematurely marked Ready.
  * ObservedGeneration is stamped so a spec change re-invalidates
    the synthesis until the next observable-artifacts pass.
  * Reason string is greppable so operators can audit which
    PackageSources came up via the workaround.

Tests
  Added TestArtifactGeneratorObservablyReady with 7 cases covering
  every branch of the predicate:
    - fresh generator: false
    - inventory only:  false
    - digest only:     false
    - both set + Unknown Ready:   true  (the actual bug)
    - both set + no Ready cond:   true  (same window pre-stamp)
    - both set + Ready=True:      false (pass real condition)
    - both set + Ready=False:     false (never mask failure)
  All pass; existing operator unit tests are unaffected.

Signed-off-by: Ivan Okhotnikov <ivan.okhotnikov@aenix.io>
 2026-07-05 08:30:19 +03:00
Aleksei Sviridkin a44d11d2b6 fix(cozystack-basics): validate legacy Ingress hostnames against the tenant apex (#3200) 
## What this PR does

The Gateway (`cozystack-gateway-hostname-policy`) and Route
(`cozystack-route-hostname-policy`) admission policies constrain tenant
hostnames to the tenant apex on the Gateway API path. Gateway API is
opt-in and off by default, so in the default configuration tenant
applications publish through a legacy `networking.k8s.io/Ingress` on the
shared ingress-nginx, which had no equivalent policy.

This adds `cozystack-ingress-hostname-policy`, a fail-closed
`ValidatingAdmissionPolicy` + binding for `Ingress` in `tenant-*`
namespaces — the legacy-Ingress counterpart of the existing
Gateway/Route hostname policies, and a new Layer 8 in the gateway
security model. A hostname on `spec.rules[].host` or
`spec.tls[].hosts[]` is allowed when it is within the namespace's own
apex (`namespace.cozystack.io/host`), or when it is a concrete
(non-wildcard) name entirely outside the platform root apex
(`_cluster.root-host`). The second case keeps the `kubernetes` app's
Proxied `exposeMethod` working, since it routes a user-supplied external
domain to a nested cluster. A hostname under the platform root apex but
outside the namespace's own apex, a wildcard taken through the
outside-root path (which could otherwise match the platform apex
itself), a rule with no host (absent or empty — an unbounded catch-all),
and `spec.defaultBackend` are rejected.

The policy is fail-closed: a `tenant-*` namespace missing its own apex
label is denied, and the policy renders only when `_cluster.root-host`
is set, so it is never shipped in a state that would admit every
hostname. `cozy-*` namespaces are cluster-admin-managed and not gated.
This is defense-in-depth in the same family as the Gateway/Route
policies — by design tenants hold only read access to `Ingress`.

The change ships helm-unittest coverage (render, plus the root-host,
empty-host, and wildcard guards) and behavioral admission tests against
the live apiserver in `hack/e2e-apps/gateway.bats` (in-apex allow,
external-domain allow, under-root-outside-apex deny,
wildcard-matching-apex deny, hostless deny, empty-string-host deny,
defaultBackend deny), and documents the new layer in
`packages/extra/gateway/README.md`.

### Screenshots

N/A — no UI change.

### Release note

```release-note
fix(cozystack-basics): constrain legacy Ingress hostnames in tenant namespaces to the tenant apex
```


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

* **New Features**
* Added **fail-closed** defense-in-depth Layer 8
(`cozystack-ingress-hostname-policy`) to enforce allowed legacy Ingress
rule and TLS hostnames for tenant namespaces, while blocking unsafe
catch-all configurations (including missing/empty hosts and
`spec.defaultBackend`) when the platform root host is configured.
* **Tests**
* Added end-to-end and render/deny validation for permitted vs denied
hostname patterns, wildcard behavior, required namespace host labeling,
and correct deny/fail-closed rendering.
* **Documentation**
  * Updated the gateway security model to document Layer 8 behavior.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
 2026-07-05 00:18:16 +03:00
IvanHunters a1c779854b fix(vertical-pod-autoscaler): migrate admission-controller TLS to cert-manager to stop reconcile-driven ReplicaSet churn (#3178) 
## Summary

The VPA admission-controller webhook TLS Secret is currently rendered by
an in-chart `genCA` + `genSignedCert` template. Those functions are not
idempotent across `helm template` invocations — every Flux HelmRelease
reconcile produces a fresh CA + cert. A `checksum/tls-secret`
podAnnotation on the admission-controller Deployment then flips its
sha, materialising a new ReplicaSet on every reconcile.

Under load — the E2E install phase where cert-manager / CDI / LINSTOR
cascade races repeatedly refire the VPA HelmRelease — this produces
10+ admission-controller ReplicaSets in minutes and blocks the whole
cozystack-platform HR: the admission-controller pod livelocks in
startup (waits on its own election lease which its previous
ReplicaSet is still holding, hits livenessProbe timeout,
CrashLoopBackOffs), VPA never becomes Ready, PackageSources stay
InProgress, and cozystack-platform aborts at its 15-minute install
timeout.

Fix: replace the source of the TLS Secret with a namespace-scoped
cert-manager `Issuer` + `Certificate`. cert-manager only rewrites the
Secret when the certificate is close to expiry (`renewBefore`), so
reconcile-driven cert churn stops. Pod rotation is delegated to a
runtime-lookup-based `checksum/tls-cert` podAnnotation that only
changes when the Secret's `tls.crt` content actually changes — not on
every render, not on every reconcile.

## What

-
`charts/vertical-pod-autoscaler/templates/admission-controller/certificate.yaml`
  (new): namespace-scoped selfSigned Issuer + Certificate. The
  Certificate mirrors the legacy `genSignedCert` shape (single DNS SAN
  `vpa-webhook.<ns>.svc`, 365d validity, matching Secret name), so
  cert-manager on the first upgrade adopts the same Secret name the
  legacy path wrote — no rename cascade in the Deployment volume mount.
  Rendered only when `admissionController.tls.certManagerEnabled=true`
  AND no `existingSecret` is supplied.
-
`charts/vertical-pod-autoscaler/templates/admission-controller/tls-secret.yaml`
  (modified): wrapped in an extra `certManagerEnabled=false` guard so
  the legacy genCA path remains available as an explicit opt-out.
-
`charts/vertical-pod-autoscaler/templates/admission-controller/deployment.yaml`
  (modified): the legacy `checksum/tls-secret` annotation is gated
  behind `certManagerEnabled=false`. Under the cert-manager path the
  Deployment carries a `checksum/tls-cert` annotation computed from
  `lookup`-ing the runtime Secret's `tls.crt` — see the template
  comment for the full timing analysis and the day-350 renewal
  trade-off it documents.
- `charts/vertical-pod-autoscaler/values.yaml` (modified): new
  `admissionController.tls.certManagerEnabled` value defaulting to
  `true`. Values-block header documents the switch as effectively
  one-way (cert-manager writes `type: kubernetes.io/tls`;
  `Secret.type` is immutable, so falling back requires `kubectl
  delete` first).
- `patches/cert-manager-admission-tls.diff` (new) +
  `Makefile update` extended: the four subchart edits above live as a
  single reviewable diff that Makefile re-applies over the freshly
  vendored chart on every `make update`, matching the fluxcd-operator
  package convention. Prevents a bump from silently reverting the
  fix.
- `packages/core/platform/sources/vertical-pod-autoscaler.yaml`
  (modified): VPA PackageSource now depends on `cozystack.cert-manager`.
  Non-circular: cert-manager depends on `cozystack.networking` +
  `cozystack.gateway-api-crds`, neither reaches VPA.
- `tests/vpa_test.yaml` (modified): +11 helm-unittest cases (15 total,
  4 pre-existing) covering `certManagerEnabled={true,false}`,
  `existingSecret` bypass, disabled admission-controller, and
  the offline-render behaviour of the `checksum/tls-cert` lookup.

## Why (evidence)

Snapshot from CI run 28587656430 (this branch, pre-workaround-review):

```
admission-controller ReplicaSets after first-install churn window:
  vertical-pod-autoscaler-admission-controller-55d888d6d4
  vertical-pod-autoscaler-admission-controller-56f8d4c747
  vertical-pod-autoscaler-admission-controller-5775dbc9b7
  vertical-pod-autoscaler-admission-controller-585b7c56c9
  vertical-pod-autoscaler-admission-controller-6fc44944fb
  vertical-pod-autoscaler-admission-controller-74bccf4849
  vertical-pod-autoscaler-admission-controller-84df98f98b
  vertical-pod-autoscaler-admission-controller-8666dffb85
  vertical-pod-autoscaler-admission-controller-86b469457c
  vertical-pod-autoscaler-admission-controller-9b979f9d6
  → 10 ReplicaSets, one per Flux reconcile, each with a different
    genCA-rooted TLS Secret sha in checksum/tls-secret.
```

## Migration for existing clusters

On upgrade the legacy Opaque Secret disappears from the release
manifest, Helm garbage-collects it, and cert-manager immediately
writes a fresh `kubernetes.io/tls` Secret of the same name via the
new Certificate. `Secret.type` is immutable, so in-place adoption is
impossible — the delete/create pass is the correct path, and the
seconds-long window it produces is safe: the `vpa-webhook-config`
MutatingWebhookConfiguration ships with `failurePolicy: Ignore`
(verified on cozy-dev3), so Pod admissions in that window skip VPA
mutation but never get rejected — no cluster-wide disruption.

The Deployment rolls in the same upgrade because the new
`checksum/tls-cert` annotation differs from anything the pod had
before, so the admission-controller re-registers `vpa-webhook-config`
with the fresh caBundle on the same pass.

## Test plan

- [x] `helm unittest packages/system/vertical-pod-autoscaler` — 15/15
      pass.
- [x] `helm template` diff between main and this branch is scoped
      exactly to the cert-manager path (new Certificate + Issuer,
      new `checksum/tls-cert` annotation, guarded legacy Secret, new
      values key, patch file + Makefile step, PackageSource
      dependsOn). No unrelated lines change.
- [x] `make update`-style simulation (rm -rf charts && helm pull &&
      patch) produces a byte-identical subchart to what git tracks —
      the patch stays clean across future upstream bumps until it
      genuinely conflicts.
- [ ] Live migration on cozy-dev3: `helm upgrade` VPA release from
      pre-fix version → observe cert-manager write the new tls Secret
      → verify `checksum/tls-cert` on the new pod's podTemplate →
      verify MutatingWebhookConfiguration `vpa-webhook-config`
      caBundle updated in place → confirm no ReplicaSet churn on
      subsequent Flux reconciles. Blocking the draft's removal.
- [ ] Full E2E on this branch — verify only 1 admission-controller
      ReplicaSet after install, VPA HR reaches Ready inside
      `cozystack-platform`'s 15m install timeout, and
      `kubernetes-latest.bats` / `kubernetes-previous.bats` no longer
      time out on VPA-attributable resource churn.

## Review pass — kvaps' CHANGES_REQUESTED addressed

- ✅ Vendored chart mods now carried as `patches/*.diff` re-applied by
  `Makefile update` (matches fluxcd-operator convention).
- ✅ Retired the tls-migrator hook — kvaps correctly showed it was
  a no-op: Helm's delete decision is driven by the stored release
  manifest not by live labels (`ResourcePolicyAnno` is the only opt-
  out), and `Secret.type` is immutable so cert-manager could not have
  adopted the legacy Opaque Secret in place anyway. ~200 lines
  removed.
- ✅ PR description rewritten to match the actual diff (no more
  reference to the removed `tls-rollout.yaml` post-upgrade hook, real
  test count, dev3 verification honestly listed as pending).
- ✅ `checksum/tls-cert` timing comment corrected: `lookup` runs on
  Helm actions only, day-350 renewal trade-off spelled out.
- ✅ `certManagerEnabled=false` documented as effectively one-way
  (Secret.type immutable).
- ✅ Dropped the `secretTemplate` labels — `app.kubernetes.io/managed-
  by: Helm` on a cert-manager-owned Secret was misleading.
- ✅ Banner comment indentation restored in `vpa_test.yaml`.


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

* **New Features**
* VPA admission-controller webhook TLS now uses cert-manager by default,
with options to disable cert-manager or supply an existing TLS secret.
* Webhook pod restarts are now tied to cert-manager Secret contents via
a content-based checksum.
* **Bug Fixes**
* Improved webhook TLS provisioning reliability when TLS readiness is
delayed.
* Corrected conditional rendering/rollout behavior to avoid mismatched
TLS ownership.
* **Tests**
* Added rendering-invariant tests covering cert-manager, legacy TLS,
existing-secret bypass, and admission-controller disabled behavior.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
 2026-07-04 22:48:00 +03:00
Aleksei Sviridkin d89ad70698 fix(cozystack-basics): validate legacy Ingress hostnames against the tenant apex 
The Gateway (cozystack-gateway-hostname-policy) and Route
(cozystack-route-hostname-policy) admission policies constrain tenant
hostnames on the Gateway API path. Gateway API is opt-in and off by
default, so in the default configuration tenant applications publish
through a legacy networking.k8s.io/Ingress on the shared ingress-nginx,
which had no equivalent policy.

Add a fail-closed ValidatingAdmissionPolicy + Binding for Ingress in
tenant-* namespaces. A hostname on spec.rules[].host or spec.tls[].hosts[]
is allowed when it is within the namespace's own apex
(namespace.cozystack.io/host), or when it is a concrete (non-wildcard)
name entirely outside the platform root apex (_cluster.root-host); a
hostname under the platform root apex but outside the namespace's own
apex, and a wildcard taken through the outside-root path (which could
match the platform apex itself), are rejected. The outside-root branch
keeps the kubernetes app's Proxied exposeMethod working, since it routes
a user-supplied external domain to a nested cluster. A rule with no host
(absent or empty, an unbounded catch-all) and spec.defaultBackend are
also rejected.

The policy templates the platform root apex into its CEL and renders
only when _cluster.root-host is set, so it is never shipped in a state
that would admit every hostname. A tenant-* namespace missing its own
apex label is denied.

Cover the policy with helm-unittest (render, including the root-host,
empty-host, and wildcard guards) and behavioral admission tests against
the live apiserver (in-apex allow, external-domain allow,
own-apex-wildcard allow, external-tls allow, under-root-outside-apex
deny, prefix-adjacent-under-root deny, wildcard-matching-apex deny,
hostless deny, empty-string-host deny, defaultBackend deny), and document
it as an additional layer in the gateway security model.

Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
 2026-07-04 22:38:39 +03:00
Aleksei Sviridkin 2cfcdc265c fix(cert-manager): raise cainjector memory limit to unblock caBundle injection (#3199) 
## What this PR does

cert-manager's cainjector loads every ValidatingWebhookConfiguration,
MutatingWebhookConfiguration, APIService and CRD into its informer
caches at
startup. On a full cozystack install that working set exceeds the 128Mi
memory
limit the package sets today, so the single leader-elected cainjector
Pod is
OOMKilled (exit 137) during cache population and enters CrashLoopBackOff
before
it ever runs a reconcile.

Because cainjector is the component that injects the issuing CA into
webhook
`clientConfig.caBundle` (via `cert-manager.io/inject-ca-from`), a
cainjector
that never completes a reconcile leaves those caBundles empty. With
`failurePolicy: Fail`, the apiserver then rejects every call to an
affected
webhook with `x509: certificate signed by unknown authority` — even
though the
cert-manager PKI chain is healthy and the webhook Pods serve a valid
cert. In
practice this intermittently blocks `Ingress` creation in tenant
namespaces
(the ingress-nginx admission webhook), which stalls dependent
HelmReleases and
fails installs.

It behaves as a flake because it is a memory race: on a lighter run
cainjector
fits under 128Mi, injects the caBundle within seconds and everything
works; on
a heavier run (more CRDs/webhooks resident, slower leader election) peak
startup
memory crosses 128Mi and it is OOMKilled before the first injection,
stalling CA
injection cluster-wide for as long as it CrashLoops.

Fix: raise the cainjector memory limit to 512Mi (matching the
cert-manager
controller in the same values file) and lift the request from 32Mi to
128Mi so
injection completes deterministically instead of racing the OOM floor. A
helm-unittest pins the sizing so a blanket resource change or a `make
update`
cannot silently lower it again. The webhook and controller resources are
left
unchanged — only cainjector exhibited the OOM.

### Screenshots

N/A — no UI changes.

### Release note

```release-note
fix(cert-manager): raise cainjector memory limit so CA injection into webhook caBundles no longer fails under memory pressure (previously surfaced as intermittent "x509: certificate signed by unknown authority" admission errors)
```


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

## Summary by CodeRabbit

* **New Features**
* Increased cainjector memory settings for cert-manager, helping it run
more reliably during startup and cache population.
* Added automated chart tests to verify the cainjector resource settings
stay within the expected range.

* **Bug Fixes**
* Reduced the risk of cainjector being OOM-killed, improving stability
and consistency of CA injection.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
 2026-07-04 22:36:14 +03:00
Ivan Okhotnikov 223c0ef4fb fix(cert-manager): raise cainjector memory limit to unblock caBundle injection 
cainjector loads all ValidatingWebhookConfigurations, APIServices and CRDs
into its informer caches at startup. On a full cozystack install that working
set exceeds the 128Mi limit, so the single leader-elected cainjector Pod is
OOMKilled during cache population and CrashLoops before it injects any
caBundle. Webhook ValidatingWebhookConfigurations then keep an empty caBundle
and every admission call fails with "x509: certificate signed by unknown
authority" (failurePolicy: Fail), intermittently blocking Ingress creation
and stalling tenant installs.

Raise the cainjector memory limit to 512Mi (matching the controller) and the
request to 128Mi, and add a helm-unittest that pins the sizing so a blanket
resource change or a `make update` cannot silently lower it again.

Signed-off-by: Ivan Okhotnikov <xorokhotnikov@gmail.com>
 2026-07-04 15:57:49 +03:00
Aleksei Sviridkin 10f8e2de1f feat(kamaji): default to 2 replicas, drop telemetry webhook (#2671) 
## What this PR does

`kamaji-controller-manager` runs **9 sub-controllers per
TenantControlPlane** (bootstraptoken, coredns, konnectivity, kubeadm*,
kubelet, kubeproxy, …) inside a single Go process, **and** serves the
admission webhooks for `tenantcontrolplanes` + `datastores` from the
same pod via `kamaji-webhook-service`. With the upstream default
`replicaCount=1`, every webhook round-trip from the apiserver shares CPU
with the reconciler goroutines. On a multi-tenant cluster this pushes
the apiserver-observed p99 of `mtenantcontrolplane.kb.io` and
`vtenantcontrolplane.kb.io` from the kamaji-side `~10ms` (per
`controller_runtime_webhook_latency_seconds_count/sum`) up to `85–95ms`
— purely due to CPU contention.

Three safe defaults are changed:

- **`kamaji.replicaCount: 2`** — HA-safe baseline. `--leader-elect` is
already enabled upstream, so only one replica reconciles tenants; the
follower's CPU is fully available for webhook handlers. On a single-node
cluster both pods still schedule (just colocated).
- **`kamaji.telemetry.disabled: true`** — drops the
`--disable-telemetry` handler **and** removes the
`telemetry.kamaji.clastix.io` entry from the
`ValidatingWebhookConfiguration`. The latter requires `patches/1.diff`
to wrap the telemetry block in `{{- if not .Values.telemetry.disabled }}
… {{- end }}` inside the vendored
`controller-gen/validating-webhook.yaml`. Without the webhook entry, the
apiserver no longer round-trips to a no-op handler on every
TenantControlPlane CREATE/UPDATE/DELETE.
- **`kamaji.affinity` (soft `podAntiAffinity`)** — spread the two
replicas across nodes when possible, while keeping single-node clusters
working (`preferredDuringScheduling`, not required).

The `patches/1.diff` approach (re-applied by `make update`) mirrors the
pattern already used in `packages/system/fluxcd-operator/Makefile` and
`packages/system/ingress-nginx/Makefile`, and survives the next
`clastix/kamaji` chart bump.

## Measured impact

Hikube prod, 22 TenantControlPlanes, before/after a manual rollout of
the same configuration:

| webhook | p99 before | p99 after | Δ |
| --- | --- | --- | --- |
| `mtenantcontrolplane.kb.io` | 85 ms | 25 ms | **-71%** |
| `vtenantcontrolplane.kb.io` | 92 ms | 25 ms | **-73%** |
| `telemetry.kamaji.clastix.io` | 94 ms | _webhook removed_ | **-100%**
|

Cumulative Kamaji admission **impact** (`p99 × calls/s`) dropped from
**830 → 140 (-83%)**. The Kamaji webhooks moved from the top three of
the `apiserver_admission_webhook_admission_duration_seconds_*` impact
ranking down to positions #3-#4.

## Release note

```release-note
feat(kamaji): default to 2 replicas with soft pod anti-affinity and disable telemetry (handler + webhook entry). Reduces apiserver admission p99 for tenantcontrolplane mutating/validating webhooks by ~70% on multi-tenant clusters.
```

## Test plan

- [x] `helm template packages/system/kamaji` renders `replicas: 2`,
`--disable-telemetry`, the soft anti-affinity, and **no**
`telemetry.kamaji.clastix.io` entry in the
`ValidatingWebhookConfiguration`.
- [x] `patches/1.diff` applies cleanly against the freshly extracted
upstream chart (`patch --dry-run -p1 < patches/1.diff`).
- [x] `make update` re-applies the patch automatically.
- [x] Live deploy on a 22-TCP cluster: 3 pods Running (when overriden to
`replicaCount: 3` via cozystack-values), zero pod restarts, leader
election visible, `kamaji-webhook-service` Endpoints serving N IPs.

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

* **New Features**
  * Telemetry can be disabled via configuration.
  * Controller configured for high availability with 2 replicas.
* Pod anti-affinity added to spread replicas across nodes for better
resilience.

* **Bug Fixes**
* Validating webhook is now omitted when telemetry is disabled to avoid
unintended behavior.

* **Chores**
* Update process improved to re-apply chart patches after extraction for
reproducible updates.

<!-- review_stack_entry_start -->

[![Review Change
Stack](https://storage.googleapis.com/coderabbit_public_assets/review-stack-in-coderabbit-ui.svg)](https://app.coderabbit.ai/change-stack/cozystack/cozystack/pull/2671?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack)

<!-- review_stack_entry_end -->
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
 2026-07-03 19:31:51 +03:00
Aleksei Sviridkin 809a04a220 feat(kubevirt-cdi): make cloneStrategyOverride configurable (#2926) 
## What this PR does

- Exposes the CDI `cloneStrategyOverride` as a Helm value in
`packages/system/kubevirt-cdi` instead of hardcoding it in the CR
template.
- Default stays `csi-clone`, so **behavior is unchanged** for everyone.
- It can now be overridden via a cozystack `Package` (e.g.
`cloneStrategyOverride: copy`), the same way `uploadProxyURL` already is
— no fork needed.

### Why

The value was hardcoded, so the only way to change the clone strategy
was to fork the package. On some storage backends `csi-clone` is
problematic: with LINSTOR/DRBD a CSI clone is created co-located with
the source golden image and then relocated to optimal nodes
asynchronously; that async relocation (`migrate-disk`) can race the
post-clone resize CDI performs, occasionally leaving a volume whose DRBD
device is smaller than its PVC capacity. Being able to select `snapshot`
or `copy` per cluster, without maintaining a fork, is a useful escape
hatch.

This only adds a knob; it does not change the default or recommend a
strategy.

## Release note

```release-note
feat(kubevirt-cdi): make CDI cloneStrategyOverride configurable via values (default csi-clone)
```

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

* **New Features**
* The KubeVirt CDI clone strategy is now configurable via Helm chart
values, allowing selection of `csi-clone`, `snapshot`, or `copy` for
DataVolume cloning.
* The default remains `csi-clone` to preserve existing behavior and
ensure backward compatibility.
* **Documentation**
* Added the `cloneStrategyOverride` Helm value description and allowed
options to the chart’s configuration.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
 2026-07-03 19:31:47 +03:00
Aleksei Sviridkin 901bb981ae fix(seaweedfs): configure postgres2 connection pool for the filer (#2906) 
## What this PR does

- Adds `WEED_POSTGRES2_CONNECTION_MAX_IDLE=20`,
`WEED_POSTGRES2_CONNECTION_MAX_OPEN=40` and
`WEED_POSTGRES2_CONNECTION_MAX_LIFETIME_SECONDS=600` to the filer
`extraEnvironmentVars` in `packages/system/seaweedfs/values.yaml`.

The vendored seaweedfs chart only ships connection pool settings for the
**mysql** store (`WEED_MYSQL_CONNECTION_MAX_*`), which cozystack does
not use. The **postgres2** store, which cozystack enables, has no pool
settings at all. As a result the Go `database/sql` pool runs with
`SetMaxIdleConns(0)`: every filer metadata lookup opens a fresh
PostgreSQL connection (TCP + TLS + SCRAM, ~300 ms each). With ~5
path-component lookups per S3 operation, this adds roughly **2 seconds
of latency to every S3 request**, even on an idle cluster.

### Measurements (production cluster, 12 nodes, 2 filers, CNPG backend)

Before / after applying these three variables, same test (sequential ops
through the S3 gateway):

| Operation | Before | After |
|---|---|---|
| PUT 4 KiB (p50) | 10.0 s | 0.28 s |
| GET 4 KiB (p50) | 3.0 s | 0.14 s |
| HEAD (p50) | 2.6 s | 0.16 s |
| Bounded LIST, 100 keys (p50) | 3.8 s | 0.37 s |

`pg_stat_activity` confirmed the root cause: before the fix every
backend was younger than ~200 ms (permanent connection churn); after the
fix connections are reused for minutes. Real-traffic p95 of
`SeaweedFS_filerStore_request_seconds` dropped from 1.3–6.5 s to ~0.18
s.

`max_open` is capped at 40 so that the default two filer replicas stay
below the CNPG default `max_connections` of 100.

## Release note

```release-note
fix(seaweedfs): configure postgres2 connection pool for the filer, removing ~2s of per-request S3 latency caused by PostgreSQL connection churn
```

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

## Summary by CodeRabbit

* **Chores**
* Optimized PostgreSQL connection pool configuration in SeaweedFS to
reduce latency and improve performance during metadata operations.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
 2026-07-03 19:31:44 +03:00
Aleksei Sviridkin ec9c29948b [reloader] Set resources requests/limits on reloader container (#2631) 
## Summary

The upstream Stakater reloader chart ships with `resources: {}`, which
triggers a `no CPU limit` conformance warning on the `reloader`
Deployment.

Override `packages/system/reloader/values.yaml` at
`reloader.reloader.deployment.resources` with the exact values from the
chart's own commented example:

- `limits` : `cpu: 100m`, `memory: 512Mi`
- `requests` : `cpu: 10m`, `memory: 128Mi`

Reloader is a light controller that watches ConfigMap / Secret changes
and triggers workload restarts — small CPU envelope with enough memory
to track many objects in clusters with lots of namespaces.

## Test plan

- [ ] `helm template . --namespace cozy-reloader` from
`packages/system/reloader/` renders a `resources` block on the
`reloader-reloader` container of the Deployment (verified locally)
- [ ] Re-run the conformance/lint tool — the `reloader-reloader has no
CPU limit` warning should be gone
- [ ] CI passes

### Release note

\`\`\`release-note
Set CPU/memory requests and limits on the reloader Deployment to clear
the "no CPU limit" conformance warning. Values follow the upstream
chart's commented example (limits 100m/512Mi, requests 10m/128Mi).
\`\`\`

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

## Summary by CodeRabbit

* **Chores**
* Updated resource allocation configuration to define CPU and memory
parameters for system component deployments, enhancing infrastructure
stability and resource optimization.

[![Review Change
Stack](https://storage.googleapis.com/coderabbit_public_assets/review-stack-in-coderabbit-ui.svg)](https://app.coderabbit.ai/change-stack/cozystack/cozystack/pull/2631)

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
 2026-07-03 19:31:39 +03:00
Aleksei Sviridkin f2d66579f1 [metrics-server] Set resources requests/limits on metrics-server container (#2630) 
## Summary

The upstream metrics-server chart ships with `limits` commented out
(only `requests cpu 100m / memory 200Mi` are set), which triggers a `no
CPU limit` conformance warning on the `metrics-server` Deployment.

Override `packages/system/metrics-server/values.yaml` to keep the
upstream-recommended baseline `requests` and add `limits`:

- `limits` : `cpu: 500m`, `memory: 500Mi`
- `requests` : `cpu: 100m`, `memory: 200Mi`

Comfortable headroom on top of the [official sizing
formula](https://github.com/kubernetes-sigs/metrics-server#configuration)
(`100m + 1m/node`, `200Mi + 2Mi/node`), covering clusters up to ~150
nodes without needing to revisit. Larger clusters can override these
values further.

## Test plan

- [ ] `helm template . --namespace cozy-monitoring` from
`packages/system/metrics-server/` renders a `resources` block on the
`metrics-server` container of the Deployment (verified locally)
- [ ] Re-run the conformance/lint tool — the `metrics-server has no CPU
limit` warning should be gone
- [ ] CI passes

### Release note

\`\`\`release-note
Set CPU/memory requests and limits on the metrics-server Deployment to
clear the "no CPU limit" conformance warning. Values follow the upstream
sizing recommendation (requests 100m/200Mi) with a comfortable limits
cap (500m/500Mi).
\`\`\`

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

## Summary by CodeRabbit

* **Chores**
* Configured explicit resource requests and limits for the
metrics-server service (CPU: 100m-500m, Memory: 200Mi-500Mi) to ensure
consistent resource allocation.

[![Review Change
Stack](https://storage.googleapis.com/coderabbit_public_assets/review-stack-in-coderabbit-ui.svg)](https://app.coderabbit.ai/change-stack/cozystack/cozystack/pull/2630)

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
 2026-07-03 19:31:36 +03:00
Aleksei Sviridkin 4952909e4d [external-dns] Set resources requests/limits on external-dns container (#2629) 
## Summary

The upstream external-dns chart ships with `resources: {}`, which
triggers a `no CPU limit` conformance warning on the `external-dns`
Deployment.

Override `packages/system/external-dns/values.yaml` to set:

- `limits` : `cpu: 100m`, `memory: 128Mi`
- `requests` : `cpu: 10m`, `memory: 64Mi`

Modest sizing for a controller that just watches Service / Ingress
objects and pushes DNS records to the provider.

## Test plan

- [ ] `helm template . --namespace cozy-external-dns` from
`packages/system/external-dns/` renders a `resources` block on the
`external-dns` container of the Deployment (verified locally)
- [ ] Re-run the conformance/lint tool — the `external-dns has no CPU
limit` warning should be gone
- [ ] CI passes

### Release note

\`\`\`release-note
Set CPU/memory requests and limits on the external-dns Deployment to
clear the "no CPU limit" conformance warning.
\`\`\`

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

## Summary by CodeRabbit

* **Chores**
* Updated ExternalDNS resource allocation with optimized CPU and memory
limits and requests for improved resource efficiency.

[![Review Change
Stack](https://storage.googleapis.com/coderabbit_public_assets/review-stack-in-coderabbit-ui.svg)](https://app.coderabbit.ai/change-stack/cozystack/cozystack/pull/2629)

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
 2026-07-03 19:31:32 +03:00
Aleksei Sviridkin 24e3bf98b5 [bootbox] Add resources requests/limits to matchbox container (#2625) 
## Summary

The `matchbox` container in
`packages/extra/bootbox/templates/matchbox/deployment.yaml` had no
`resources` block, triggering a `no CPU limit` conformance warning on
the `bootbox-matchbox` Deployment (PXE/iPXE provisioning service).

Bootbox is a standalone chart (no vendored upstream), so the values are
set directly in the template — same pattern as #2614 (csi-resizer) and
#2624 (s3manager).

- `limits` : `cpu: 200m`, `memory: 256Mi`
- `requests` : `cpu: 50m`, `memory: 128Mi`

Modest sizing since matchbox is a lightweight HTTP server serving iPXE
profiles and boot assets from ConfigMaps.

## Test plan

- [ ] `helm template bootbox . --namespace tenant-root --show-only
templates/matchbox/deployment.yaml --set _namespace.host=example.com
--set _cluster.solver=letsencrypt` from `packages/extra/bootbox/`
renders the `resources` block on the `matchbox` container (verified
locally)
- [ ] Re-run the conformance/lint tool — the `matchbox has no CPU limit`
warning should be gone
- [ ] CI passes

### Release note

\`\`\`release-note
Add CPU and memory requests/limits to the matchbox container of the
bootbox Deployment to clear the "no CPU limit" conformance warning.
\`\`\`

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

## Summary by CodeRabbit

* **Chores**
* Added Kubernetes resource limits and requests configuration to
optimize container resource allocation and improve deployment stability.

[![Review Change
Stack](https://storage.googleapis.com/coderabbit_public_assets/review-stack-in-coderabbit-ui.svg)](https://app.coderabbit.ai/change-stack/cozystack/cozystack/pull/2625)

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
 2026-07-03 19:31:29 +03:00
Aleksei Sviridkin 04a05f623c [bucket] Add resources requests/limits to s3manager container (#2624) 
## Summary

The `s3manager` container in
`packages/system/bucket/templates/deployment.yaml` had no `resources`
block, triggering a `no CPU limit` conformance warning on the bucket UI
Deployment.

The bucket package is a standalone chart (no vendored upstream), so the
values are set directly in the template — same pattern as the
csi-resizer fix in #2614 and the existing `capi-providers-cpprovider`
template.

- `limits` : `cpu: 500m`, `memory: 512Mi`
- `requests` : `cpu: 100m`, `memory: 128Mi`

Comfortable headroom for the lightweight S3 browser UI while keeping a
small baseline request.

## Test plan

- [ ] `helm template . --show-only templates/deployment.yaml` from
`packages/system/bucket/` renders the `resources` block on the
`s3manager` container (verified locally)
- [ ] Re-run the conformance/lint tool — the `s3manager has no CPU
limit` warning should be gone
- [ ] CI passes

### Release note

\`\`\`release-note
Add CPU and memory requests/limits to the s3manager container of the
bucket UI Deployment to clear the "no CPU limit" conformance warning.
\`\`\`

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

## Summary by CodeRabbit

* **Chores**
* Improved deployment configuration with explicit resource allocation
settings to ensure optimal service stability and resource management.

[![Review Change
Stack](https://storage.googleapis.com/coderabbit_public_assets/review-stack-in-coderabbit-ui.svg)](https://app.coderabbit.ai/change-stack/cozystack/cozystack/pull/2624)

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
 2026-07-03 19:31:25 +03:00
Aleksei Sviridkin 6762d2d95c [goldpinger] Set resources requests/limits on goldpinger DaemonSet (#2623) 
## Summary

The upstream Bloomberg goldpinger chart ships with `resources: {}`,
which triggers a `no CPU limit` conformance warning on the
`goldpinger-daemon` container running on every node.

Override `packages/system/goldpinger/values.yaml` to set:

- `limits` : `cpu: 200m`, `memory: 256Mi`
- `requests` : `cpu: 50m`, `memory: 64Mi`

Comfortable margin for the ping-mesh fan-out on larger clusters while
keeping a small baseline request so the DaemonSet schedules everywhere.

## Test plan

- [ ] `helm template . --namespace cozy-goldpinger` from
`packages/system/goldpinger/` renders the `resources` block on the
`goldpinger-daemon` container of the DaemonSet (verified locally)
- [ ] Re-run the conformance/lint tool — the `goldpinger has no CPU
limit` warning should be gone
- [ ] CI passes

### Release note

\`\`\`release-note
Set CPU/memory requests and limits on the goldpinger DaemonSet to clear
the "no CPU limit" conformance warning.
\`\`\`

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

## Summary by CodeRabbit

* **Chores**
* Updated Kubernetes resource requirements for the goldpinger component,
defining CPU limits (200m), memory limits (256Mi), CPU requests (50m),
and memory requests (64Mi).

[![Review Change
Stack](https://storage.googleapis.com/coderabbit_public_assets/review-stack-in-coderabbit-ui.svg)](https://app.coderabbit.ai/change-stack/cozystack/cozystack/pull/2623)

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
 2026-07-03 19:31:22 +03:00
Aleksei Sviridkin 1aa6436922 [cert-manager] Set resources requests/limits on cert-manager pods (#2616) 
## Summary

The upstream cert-manager chart ships with `resources: {}` for all four
workloads, which triggers `no CPU limit` (and `no memory limit`)
conformance warnings on every pod.

Override `packages/system/cert-manager/values.yaml` (previously empty)
to set sane requests/limits sized to each component's role:

| Component | requests cpu/mem | limits cpu/mem | Notes |
|---|---|---|---|
| controller | 50m / 128Mi | 500m / 512Mi | Main workload — issues and
renews certs |
| webhook | 10m / 32Mi | 200m / 128Mi | Admission webhook, short-lived
requests |
| cainjector | 10m / 32Mi | 200m / 128Mi | CA bundle injection, light
watch loop |
| startupapicheck | 10m / 16Mi | 100m / 64Mi | One-shot Job at install
time |

Requests for `cainjector` (`10m` cpu / `32Mi` mem) match the example in
the upstream values.yaml comment.

## Test plan

- [ ] `helm template . --namespace cert-manager` from
`packages/system/cert-manager/` renders a `resources` block on all four
pods (verified locally for controller, webhook, cainjector and the
startupapicheck Job)
- [ ] Re-run the conformance/lint tool that originally flagged the
warnings on `cert-manager-cainjector` — should be clean on the other
three pods too
- [ ] CI passes

### Release note

\`\`\`release-note
Set CPU/memory requests and limits on cert-manager controller, webhook,
cainjector and startupapicheck pods to clear "no CPU limit" conformance
warnings.
\`\`\`

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

## Summary by CodeRabbit

* **Chores**
* Added resource allocation configuration for cert-manager deployment,
specifying CPU and memory requests and limits for improved system
stability and resource management across core components.

[![Review Change
Stack](https://storage.googleapis.com/coderabbit_public_assets/review-stack-in-coderabbit-ui.svg)](https://app.coderabbit.ai/change-stack/cozystack/cozystack/pull/2616)

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
 2026-07-03 19:31:19 +03:00
Aleksei Sviridkin b2d00af495 [kubernetes] Add CPU/memory limits to csi-resizer sidecar (#2614) 
## Summary

- The `csi-resizer` container in
`packages/apps/kubernetes/templates/csi/deploy.yaml` was the only
sidecar in the `kcsi-controller` Deployment without `resources.limits`,
which triggers a "no CPU limit" conformance warning.
- Aligned it with the other six CSI sidecars in the same Deployment:
`limits` `cpu: 512m` / `memory: 512Mi` and `requests` `cpu: 125m` /
`memory: 128Mi`.
- No other files touched; vendored charts and other CSI drivers
(`nfs-driver`, `piraeus-operator`) are out of scope for this PR.

## Test plan

- [ ] `make -C packages/apps/kubernetes show NAME=test NAMESPACE=test`
renders the Deployment with the new `limits` block
- [ ] Re-run the conformance/lint tool that originally flagged the
warning — `csi-resizer has no CPU limit` should be gone
- [ ] CI passes

### Release note

\`\`\`release-note
Set CPU and memory limits on the csi-resizer sidecar of the kubevirt-csi
controller to align with the other CSI sidecars and clear the "no CPU
limit" conformance warning.
\`\`\`

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

## Summary by CodeRabbit

* **Chores**
* Updated resource allocation for the CSI resizer component, increasing
CPU and memory requests to improve performance and reliability.

[![Review Change
Stack](https://storage.googleapis.com/coderabbit_public_assets/review-stack-in-coderabbit-ui.svg)](https://app.coderabbit.ai/change-stack/cozystack/cozystack/pull/2614)

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
 2026-07-03 19:31:15 +03:00
mattia-eleuteri 8bc62cca1c feat(kamaji): default to 2 replicas, drop telemetry webhook 
The kamaji-controller-manager runs ~9 sub-controllers per
TenantControlPlane (bootstraptoken, coredns, konnectivity, kubeadm*,
kubelet, kubeproxy, etc.) inside a single Go process, AND serves the
admission webhooks for tenantcontrolplanes/datastores via the
kamaji-webhook-service ClusterIP. With the upstream default
replicaCount=1, every webhook RT shares CPU with the reconciler
goroutines. On a multi-tenant cluster (~20 TCPs), this pushes
apiserver-observed p99 for mtenantcontrolplane.kb.io and
vtenantcontrolplane.kb.io from the kamaji-side ~10ms to 85-95ms.

Three changes, each safe on small clusters and high-impact on large ones:

* replicaCount=2 — HA-safe baseline. --leader-elect is already enabled
  upstream, so only one replica reconciles tenants; the follower's CPU
  is fully available for webhook handlers, cutting p99 ~70% on busy
  clusters. On a single-node cluster both pods schedule normally.

* telemetry.disabled=true — drops both the --disable-telemetry handler
  and the telemetry.kamaji.clastix.io admission webhook entry from the
  ValidatingWebhookConfiguration. The latter requires patches/1.diff to
  wrap the telemetry block in a {{ if not .Values.telemetry.disabled }}
  conditional inside the vendored controller-gen file. Without the
  webhook entry, the apiserver no longer issues a TCP round-trip to a
  no-op handler on every TenantControlPlane CREATE/UPDATE/DELETE.

* podAntiAffinity (preferred) — spread the two replicas across nodes
  when possible. preferredDuringScheduling so single-node clusters
  still schedule (both pods colocated, which is acceptable).

Measured on a 22-TCP production cluster (Hikube, before/after):
mtenantcontrolplane p99 85 → 25 ms (-71%), vtenantcontrolplane p99
92 → 25 ms (-73%), telemetry webhook impact 291 → 0. Total Kamaji
admission impact -83% (830 → 140).

The patches/1.diff approach (re-applied by `make update`) mirrors the
pattern already used in fluxcd-operator and ingress-nginx and survives
the next clastix/kamaji chart bump.

Signed-off-by: mattia-eleuteri <mattia@hidora.io>
 2026-07-03 19:14:36 +03:00
mattia-eleuteriandClaude Opus 4.8 c47bb6edb0 feat(kubevirt-cdi): make cloneStrategyOverride configurable 
The CDI cloneStrategyOverride was hardcoded to csi-clone in the CR
template, so operators could only change it by forking the package.
Expose it as a Helm value (default csi-clone, behavior unchanged) so it
can be set via a cozystack Package override, the same way uploadProxyURL
already is.

This is useful where the csi-clone path is problematic on a given
storage backend (e.g. LINSTOR/DRBD, where a clone is co-located with the
source and relocated afterwards; the async relocation can race the
post-clone resize), letting operators select snapshot or copy without
maintaining a fork.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: mattia-eleuteri <mattia@hidora.io>
 2026-07-03 19:14:33 +03:00
mattia-eleuteri 49cbea8c73 fix(seaweedfs): configure postgres2 connection pool for the filer 
The vendored seaweedfs chart ships connection pool settings only for the
mysql store, which cozystack does not use. The postgres2 store, which is
enabled, has no pool settings, so the Go sql pool keeps zero idle
connections and every filer metadata lookup opens a fresh PostgreSQL
connection (TCP + TLS + SCRAM, ~300ms each). With ~5 path lookups per S3
operation this adds roughly 2 seconds of latency to every S3 request,
regardless of cluster load.

Measured on a production cluster (12 nodes, 2 filers): PUT of a 4KiB
object went from 10s (p50) to 0.28s, HEAD from 2.6s to 0.16s after
setting these values; pg_stat_activity confirmed connections were
previously recycled every ~200ms and are now reused.

max_open is capped at 40 so that two filer replicas stay below the CNPG
default max_connections of 100.

Signed-off-by: mattia-eleuteri <mattia@hidora.io>
 2026-07-03 19:14:30 +03:00