Repository navigation
fix(installer): bootstrap cozy-system via --create-namespace + label hook - #2508
Conversation
dosubot
Bot
added
the
size/XS
github-actions
Bot
added
area/platform
📝 WalkthroughWalkthroughAdds a conditional ChangesNamespace rendering & labeler flow
Sequence Diagram(s)sequenceDiagram
participant Helm
participant KubeAPI as Kubernetes API
participant Scheduler as K8s Scheduler
participant JobPod as Labeler Pod
Helm->>KubeAPI: Apply SA, ClusterRole, ClusterRoleBinding, Job (pre-install hook)
KubeAPI-->>Scheduler: Admit Job -> create Pod
Scheduler->>KubeAPI: Create Pod (with hostNetwork/tolerations as applicable)
JobPod->>KubeAPI: kubectl label --overwrite namespace cozy-system (apply labels)
KubeAPI-->>JobPod: 200 OK (labels updated)
JobPod->>KubeAPI: Exit/succeed (Job completes)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request refactors how the cozy-system namespace is managed to avoid conflicts between Helm's native namespace creation and the chart's own resource definitions. By removing the static Namespace resource and replacing it with a dynamic labeling hook, the installation process becomes more robust and compatible with standard Helm deployment patterns. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on Gemini (@gemini-code-assist) comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request removes the static Namespace resource for cozy-system and introduces a pre-install/upgrade hook to label the namespace, addressing a bootstrap issue where Helm v3 cannot adopt pre-created namespaces. Feedback was provided regarding the new cozy-system-labeler Job, which lacks mandatory resource requests, limits, and a securityContext as required by the repository style guide. Additionally, the PR description is missing the required release-note block.
| spec: | ||
| serviceAccountName: cozy-system-labeler | ||
| restartPolicy: OnFailure | ||
| containers: | ||
| - name: kubectl | ||
| image: alpine/k8s:1.32.0 | ||
| imagePullPolicy: IfNotPresent | ||
| command: | ||
| - kubectl | ||
| - label | ||
| - --overwrite | ||
| - namespace | ||
| - cozy-system | ||
| - cozystack.io/system=true | ||
| - pod-security.kubernetes.io/enforce=privileged |
There was a problem hiding this comment.
The Job is missing resource requests/limits and a proper securityContext. According to the repository style guide (lines 82 and 99), new workloads must have resource limits and should not run as root without justification. Adding these ensures better cluster security and stability. Additionally, consider moving the hardcoded image alpine/k8s:1.32.0 to values.yaml to follow the project's image management pattern.
spec:
securityContext:
runAsNonRoot: true
runAsUser: 65534
seccompProfile:
type: RuntimeDefault
serviceAccountName: cozy-system-labeler
restartPolicy: OnFailure
containers:
- name: kubectl
image: alpine/k8s:1.32.0
imagePullPolicy: IfNotPresent
securityContext:
allowPrivilegeEscalation: false
capabilities:
drop:
- ALL
resources:
requests:
cpu: 10m
memory: 32Mi
limits:
cpu: 100m
memory: 128Mi
command:
- kubectl
- label
- --overwrite
- namespace
- cozy-system
- cozystack.io/system=true
- pod-security.kubernetes.io/enforce=privilegedThere was a problem hiding this comment.
Addressed in 85ced0d:
securityContext:capabilities.drop: [ALL],allowPrivilegeEscalation: false, pod-levelseccompProfile: RuntimeDefault.resources: requests10m / 32Mi, limits100m / 128Mi.- Image pinned by digest (
alpine/k8s:1.32.0@sha256:4c28b0f0c6accfa07fa449493ea936291e4ff1270500571d73f75bb406676c3b). Keeping it inline rather than promoting tovalues.yaml— this is a one-shot bootstrap Job that auto-deletes after success, so a values knob would be noise nobody touches.
Skipped runAsNonRoot / runAsUser: 65534 and readOnlyRootFilesystem from your suggestion: upstream alpine/k8s runs as root and kubectl writes its discovery cache under $HOME, so both would require either an image rebase or extra emptyDir mounts for a Job that lives ~1s and is deleted on success. Not worth the surface area here.
| @@ -0,0 +1,82 @@ | |||
| {{/* | |||
There was a problem hiding this comment.
The pull request description is missing the required release-note block. According to the repository style guide (line 51), every PR must include a block in the following format:
```release-note
type(scope): human-readable changelog entry
<details>
<summary>References</summary>
1. PR body must contain a release note block.
</details>
There was a problem hiding this comment.
Added the release-note block to the PR description.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/core/installer/templates/cozy-system-labels.yaml (1)
22-22: Clean up hook resources on failure as well.Current hook delete policy leaves temporary SA/RBAC/Job objects behind when the hook fails. Consider adding
hook-failedto avoid lingering cluster-scoped permissions after aborted installs.♻️ Suggested change
- helm.sh/hook-delete-policy: before-hook-creation,hook-succeeded + helm.sh/hook-delete-policy: before-hook-creation,hook-succeeded,hook-failedApply the same change to all four hook resources in this file.
Also applies to: 31-31, 45-45, 63-63
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/installer/templates/cozy-system-labels.yaml` at line 22, The helm hook delete policy currently only uses "before-hook-creation,hook-succeeded" which leaves ServiceAccount/RBAC/Job resources when a hook fails; update each hook resource's helm.sh/hook-delete-policy annotation (the lines with "helm.sh/hook-delete-policy: before-hook-creation,hook-succeeded") to include "hook-failed" so it becomes "before-hook-creation,hook-succeeded,hook-failed" for all four hook resources in this template.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/core/installer/templates/cozy-system-labels.yaml`:
- Line 22: The helm hook delete policy currently only uses
"before-hook-creation,hook-succeeded" which leaves ServiceAccount/RBAC/Job
resources when a hook fails; update each hook resource's
helm.sh/hook-delete-policy annotation (the lines with
"helm.sh/hook-delete-policy: before-hook-creation,hook-succeeded") to include
"hook-failed" so it becomes "before-hook-creation,hook-succeeded,hook-failed"
for all four hook resources in this template.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 0249f0cd-aaa1-4ca4-9551-bf22df075e1c
📒 Files selected for processing (1)
packages/core/installer/templates/cozy-system-labels.yaml
IvanHunters
left a comment
IvanHunters
left a comment
There was a problem hiding this comment.
The fix is correct — replacing the shipped Namespace resource with --create-namespace + a label hook is the standard pattern, and the root cause diagnosis (Helm pre-creates the namespace without ownership annotations, causing the chart's Namespace apply to fail) is accurate. Verified against the CI install script (e2e-install-cozystack.bats) — it already uses --create-namespace. RBAC is minimal (get/patch on named resource only), hook weights and delete policies look right, and the scheduling config (hostNetwork: true + tolerations) correctly mirrors cozystack-operator.yaml.
Two non-blocking notes:
-
Missing explicit
hostedvariant case in the hook'senvsection —cozystack-operator.yamlhandles all three variants explicitly, including{{- else if eq .Values.cozystackOperator.variant "hosted" }}with a# Hosted: use in-cluster service account, no env override neededcomment. The hook silently falls through to "no env block" forhosted, which is functionally correct (hosted cluster has working CNI and ClusterIP routing), but adding the explicit case would keep it consistent with the operator and document the intent. -
alpine/k8s:1.32.0is not pinned by digest — the operator image uses a@sha256:…pin. Given that this hook runs at install time with namespace-patch RBAC, pinning the image digest would be consistent with the rest of the chart.
LGTM
…hook
The chart used to ship `Namespace cozy-system` directly, which trips a
helm v3 chicken-and-egg whenever the install command uses
`--create-namespace`:
- WITH `--create-namespace`, helm pre-creates the namespace via plain
kubectl-create (no helm meta annotations); the chart's own Namespace
apply then fails with `already exists`.
- WITHOUT `--create-namespace`, helm fails immediately because it
cannot write its release-secret to a non-existent namespace.
The bug was hidden by the 3x retry on the install step in
`.github/workflows/pull-requests.yaml`: attempt 1 fails with the
conflict, attempt 2 sees the failed release and takes the upgrade code
path (patch-merge instead of strict-create). Surfaces cleanly when the
retry is dropped; reproducible on every cold install.
Fix has two pieces:
1. Drop the chart-side `Namespace cozy-system` resource. Install
commands now use the standard `helm upgrade --install --namespace
cozy-system --create-namespace` (matches kube-prometheus-stack /
argo-cd / cert-manager).
2. Add a pre-install/pre-upgrade Job hook (`cozy-system-labeler`) that
patches `cozystack.io/system=true` and
`pod-security.kubernetes.io/enforce=privileged` onto the namespace
so the cozystack-operator pod (hostNetwork=true) is admitted.
The hook runs in `kube-system`, not `cozy-system`. Reason: the operator
pod needs hostNetwork=true (no CNI yet at install time), which violates
PodSecurity baseline. cozy-system therefore needs enforce=privileged
*before* the operator pod is admitted. A labeler pod inside cozy-system
would itself need hostNetwork=true and hit the same admission denial:
pods "cozy-system-labeler-..." is forbidden: violates PodSecurity
"baseline:latest": host namespaces (hostNetwork=true)
Job admission denial means no pod is ever created, .status.failed never
increments and backoffLimit never trips. helm's --wait waits for
Complete=True forever and surfaces only the generic "timed out waiting
for the condition" at its budget.
`kube-system` is PSA-exempt on Talos
(`defaults.exemptions.namespaces` in the apiserver
PodSecurityConfiguration) and unlabeled on vanilla kubeadm/kind/k3s, so
hostNetwork pods are admitted there. The hook's ServiceAccount is bound
to a ClusterRole scoped via `resourceNames: ["cozy-system"]` to the one
target namespace, keeping the cluster permission grant minimal.
`hook-delete-policy: before-hook-creation,hook-succeeded` ensures the
SA, RBAC, and Job are cleaned up after a successful install/upgrade.
Verified end-to-end in CI on the parallel consolidation branch
feat/e2e-optimization (PR #2500): cold install passes admission, the
operator pod comes up, all downstream E2E tests pass.
Assisted-By: Claude <[email protected]>
Signed-off-by: Myasnikov Daniil <[email protected]>
c0b76b1 to
4527320
Compare
github-actions
Bot
added
size/L
Re-introduces the pre-install/pre-upgrade Job hook (cozy-system-labeler) that was added by 3a87485/4754f57f and dropped by be0a668, but moves it into kube-system to dodge the PSA admission denial that broke the in-namespace approach: pods "cozy-system-labeler-..." is forbidden: violates PodSecurity "baseline:latest": host namespaces (hostNetwork=true) The operator pod needs hostNetwork=true (no CNI yet at install time), which violates PodSecurity baseline. cozy-system therefore needs enforce=privileged before the operator pod can be admitted. A labeler pod inside cozy-system would itself need hostNetwork=true and hit the same admission denial — Job admission denial means no pod is ever created, .status.failed never increments and backoffLimit never trips, helm waits for Complete=True until its budget elapses. `kube-system` is PSA-exempt on Talos (defaults.exemptions.namespaces in the apiserver PodSecurityConfiguration) and unlabeled on vanilla kubeadm/kind/k3s, so hostNetwork pods are admitted there. The hook's ServiceAccount is bound to a ClusterRole scoped via `resourceNames: ["cozy-system"]` to the one target namespace, keeping the cluster permission grant minimal. Also revert the e2e bats workaround (ddc429c) that pre-applied cozy-system manually before helm install: the labeler hook handles the labels in-chart now, so the bats can go back to plain `helm upgrade --install --namespace cozy-system --create-namespace ...`. Mirrors the corresponding fix on PR #2508 (fix/installer-namespace-bootstrap) so the two branches converge on the same resolution. Verified end-to-end in CI on this branch: cold install passes admission, operator pod comes up, all downstream E2E tests pass. Assisted-By: Claude <[email protected]> Signed-off-by: Myasnikov Daniil <[email protected]>
The previous run on d3e3677 raced PR #2508's concurrent build for `backupstrategy-controller:latest` and lost: ERROR: failed to build: unknown: Conflicted with another upload of the same manifest. Unrelated to the chart change at HEAD — pure shared-registry collision on :latest. Empty commit to retrigger the workflow. Assisted-By: Claude <[email protected]> Signed-off-by: Myasnikov Daniil <[email protected]>
Address review feedback on PR #2508: - Pin alpine/k8s by digest (matches operator image pinning style). - Add resources requests/limits and a minimal securityContext to the labeler Job: capabilities drop ALL, allowPrivilegeEscalation: false, seccompProfile RuntimeDefault. - Include hook-failed in helm.sh/hook-delete-policy across all four hook resources so a failed install doesn't leave SA / ClusterRole / ClusterRoleBinding / Job objects behind. - Add explicit hosted variant case in the env block, mirroring the pattern in cozystack-operator.yaml. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]> Signed-off-by: Myasnikov Daniil <[email protected]>
|
IvanHunters thanks for the review. Both follow-ups from your notes are folded into 85ced0d — explicit |
dosubot
Bot
added
size:XS
and removed
size/L
There was a problem hiding this comment.
Code Review
This pull request replaces the static cozy-system namespace definition with a pre-install/upgrade hook to label the namespace, resolving a circular dependency with PodSecurity admission. Feedback focuses on improving troubleshooting by retaining hook resources on failure and moving the hardcoded image digest to values.yaml to comply with the project's style guide.
| annotations: | ||
| helm.sh/hook: pre-install,pre-upgrade | ||
| helm.sh/hook-weight: "-200" | ||
| helm.sh/hook-delete-policy: before-hook-creation,hook-succeeded,hook-failed |
There was a problem hiding this comment.
Including hook-failed in the helm.sh/hook-delete-policy will cause the ServiceAccount to be deleted even if the hook fails. This can make troubleshooting more difficult as the associated Job's environment is partially cleaned up. Since before-hook-creation is already present, previous failed attempts will still be cleaned up on the next run.
helm.sh/hook-delete-policy: before-hook-creation,hook-succeededThere was a problem hiding this comment.
See #2508 (comment) — same reasoning applies; cleanliness on failed installs outweighs the debug-window for the RBAC objects.
| annotations: | ||
| helm.sh/hook: pre-install,pre-upgrade | ||
| helm.sh/hook-weight: "-180" | ||
| helm.sh/hook-delete-policy: before-hook-creation,hook-succeeded,hook-failed |
There was a problem hiding this comment.
Including hook-failed in the helm.sh/hook-delete-policy for the ClusterRole hinders debugging if the hook fails. before-hook-creation is sufficient to ensure a clean state for subsequent attempts.
helm.sh/hook-delete-policy: before-hook-creation,hook-succeededThere was a problem hiding this comment.
See #2508 (comment) — same reasoning applies; cleanliness on failed installs outweighs the debug-window for the RBAC objects.
| annotations: | ||
| helm.sh/hook: pre-install,pre-upgrade | ||
| helm.sh/hook-weight: "-160" | ||
| helm.sh/hook-delete-policy: before-hook-creation,hook-succeeded,hook-failed |
There was a problem hiding this comment.
Including hook-failed in the helm.sh/hook-delete-policy for the ClusterRoleBinding hinders debugging if the hook fails. before-hook-creation is sufficient to ensure a clean state for subsequent attempts.
helm.sh/hook-delete-policy: before-hook-creation,hook-succeededThere was a problem hiding this comment.
See #2508 (comment) — same reasoning applies; cleanliness on failed installs outweighs the debug-window for the RBAC objects.
| annotations: | ||
| helm.sh/hook: pre-install,pre-upgrade | ||
| helm.sh/hook-weight: "-100" | ||
| helm.sh/hook-delete-policy: before-hook-creation,hook-succeeded,hook-failed |
There was a problem hiding this comment.
Including hook-failed in the helm.sh/hook-delete-policy for the Job will cause it to be deleted immediately upon failure, making it impossible to inspect the pod logs for troubleshooting. Removing hook-failed allows the failed pod to persist for inspection, while before-hook-creation ensures it is cleaned up before the next installation attempt.
helm.sh/hook-delete-policy: before-hook-creation,hook-succeededThere was a problem hiding this comment.
Keeping hook-failed on the Job. Trade-off acknowledged — a failed install loses the labeler pod's logs. But the Job is kubectl label namespace cozy-system ...; the failure modes are almost always (a) apiserver unreachable from a kube-system host-network pod or (b) RBAC mismatch — both surface in helm install's output, not in the pod logs. The previous reviewer asked for hook-failed precisely to avoid leaving SA / RBAC / Job objects behind on a failed install, and that argument wins for me here.
| type: RuntimeDefault | ||
| containers: | ||
| - name: kubectl | ||
| image: alpine/k8s:1.32.0@sha256:4c28b0f0c6accfa07fa449493ea936291e4ff1270500571d73f75bb406676c3b |
There was a problem hiding this comment.
The image and its digest are hardcoded directly in the template. Following the project's established pattern (as seen in cozystack-operator.yaml) and the repository style guide (line 31), images and digests should ideally be defined in values.yaml. This facilitates centralized management and allows CI to update digests automatically.
References
- Image digest values in values.yaml — set by CI via make image, not by humans. (link)
There was a problem hiding this comment.
Same call as on the line-139 thread: the labeler image is third-party (alpine/k8s), not managed by make image's CI digest-bump, and the Job is a one-shot pre-install hook that auto-deletes on success. A values.yaml knob here would be config nobody touches. Pinned by digest in the template covers the supply-chain concern.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/core/installer/templates/cozy-system-labels.yaml (1)
86-86: ⚡ Quick winSet
dnsPolicyfor the hostNetwork hook.Kubernetes recommends
ClusterFirstWithHostNetfor Pods that run withhostNetwork: true; without it, DNS falls back toDefault, which makes the generic API-server endpoint path more fragile ifcozystack.apiServerHostis a hostname. (kubernetes.io)Suggested fix
hostNetwork: true + dnsPolicy: ClusterFirstWithHostNet tolerations:🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/installer/templates/cozy-system-labels.yaml` at line 86, Add the recommended dnsPolicy when using hostNetwork: true in the pod spec to avoid DNS falling back to Default; specifically update the block containing hostNetwork: true to also set dnsPolicy: ClusterFirstWithHostNet so Kubernetes uses ClusterFirstWithHostNet for DNS resolution when cozystack.apiServerHost may be a hostname.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/core/installer/templates/cozy-system-labels.yaml`:
- Line 86: Add the recommended dnsPolicy when using hostNetwork: true in the pod
spec to avoid DNS falling back to Default; specifically update the block
containing hostNetwork: true to also set dnsPolicy: ClusterFirstWithHostNet so
Kubernetes uses ClusterFirstWithHostNet for DNS resolution when
cozystack.apiServerHost may be a hostname.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: be7f72b8-ac43-4d78-adb4-e84ff655325d
📒 Files selected for processing (2)
packages/core/installer/templates/cozy-system-labels.yamlpackages/core/installer/templates/cozystack-operator.yaml
💤 Files with no reviewable changes (1)
- packages/core/installer/templates/cozystack-operator.yaml
Aleksei Sviridkin (lexfrei)
left a comment
Aleksei Sviridkin (lexfrei)
left a comment
There was a problem hiding this comment.
Solid analysis and the approach is correct: replacing the chart-shipped Namespace cozy-system with --create-namespace + a kube-system pre-install hook is the standard pattern (mirrors kube-prometheus-stack / argo-cd / cert-manager), and the chicken-and-egg root cause is real (the conflict is masked today by the 3× retry on Install Cozystack in .github/workflows/pull-requests.yaml; hack/e2e-install-cozystack.bats already drives the --create-namespace path end-to-end). Hook hygiene is good: RBAC scoped via resourceNames: [cozy-system] to get,patch only, ordered hook weights, digest-pinned image, hardened pod (drop ALL caps, no privilege escalation, RuntimeDefault seccomp, requests/limits set), all three operator variants covered. Existing-cluster migration is safe — the live cozy-system carries helm.sh/resource-policy: keep, so the upgrade keeps it and the hook re-asserts labels.
One blocker before merge:
1. alpine/k8s:1.32.0 — image is from an EOL Kubernetes minor.
Kubernetes 1.32 went EOL on 2026-02-28 (per endoflife.date); supported minors as of today are 1.33 (EOL 2026-06-28), 1.34 (EOL 2026-10-27), 1.35 (current). Two additional concerns:
- The pin is
1.32.0— the very first patch. 13 patch releases (1.32.1 .. 1.32.13) shipped before EOL, all skipped. Image1.32.0was last updated 2024-12-15 (~17 months stale), so neither the kubectl binary nor the alpine base receives security backports. - kubectl is supported within ±1 minor of the apiserver. On clusters running 1.34/1.35 (typical: current Talos, hosted EKS/GKE/AKS) the 1.32 client is outside the formally-supported skew window.
kubectl labelis simple enough that it usually works, but the supply-chain and CVE arguments still apply because this Job runs at install time with namespace-patch RBAC. - Other in-tree hooks pin
alpine/k8s:1.33.4(packages/system/cozystack-api/templates/hook.yaml,packages/system/seaweedfs/templates/hook.yaml,packages/apps/vm-disk/templates/pvc-resize-hook.yaml) — itself not the freshest patch (1.33.11last_updated 2026-04-19), but at least in a supported minor. This PR introduces unnecessary version drift from that convention.
Minimum: bump to alpine/k8s:1.33.11@sha256:… (matches in-tree convention, supported until 2026-06-28). Better: 1.34.7 or 1.35.4 (both last_updated 2026-04-19) — aligns with the upstream support window and avoids needing another bump in 2 months.
Non-blocking follow-ups (consider for this PR or a fast-follow):
-
hostedvariant on managed clusters with strict-PSAkube-system. Template comment claims kube-system is PSA-exempt on Talos and unlabeled on vanilla kubeadm/kind/k3s, but says nothing about EKS/GKE/AKS/OpenShift. On a managed cluster where kube-system carriespod-security.kubernetes.io/enforce: baseline(or stricter), the Job is rejected becausehostNetwork: trueviolates baseline. Inhostedmode the CNI is already provided by the platform, so the hostNetwork bootstrap need does not apply — a cleaner split would have thehostedbranch run withouthostNetworkand without the not-ready/CNI tolerations. At minimum, expand the docstring to acknowledge the assumption. -
dnsPolicyon the hostNetwork pod (CodeRabbit's nitpick). Current paths are DNS-free (localhost:7445, kubelet-injectedKUBERNETES_SERVICE_HOST/PORT, IP-onlyapiServerHost), so this is cosmetic today. Note that on cold install the cluster DNS does not exist yet, soClusterFirstWithHostNetwould not actually help — keepingDefaultis defensible. If kept, a one-line comment explaining why would help future readers. -
E2E assertion for the PSA label. No helm-unittest fixtures for
packages/core/installer/(consistent with prior chart state — no precedent). Worth a one-line bats assertion ine2e-install-cozystack.batsafter the helm install so the contract is explicit and silent hook regressions surface:
kubectl get ns cozy-system -o jsonpath='{.metadata.labels.pod-security\.kubernetes\.io/enforce}' | grep -qx privileged-
make manifestsstatic-render path is incomplete.Makefile:45-58renders onlytemplates/cozystack-operator.yamlvia--show-onlyandhack/upload-assets.sh:7-9uploads those as release artifacts for users on a kubectl-apply path. The new hook is excluded. If kubectl-apply remains a supported install path on Talos (where namespace PSA enforce matters), users on that path won't get the hook and the operator pod won't be admitted. Either foldcozy-system-labels.yamlinto the rendered manifest (multi--show-onlyor drop--show-onlyand post-filter), or document that kubectl-apply path is deprecated in favor of helm. -
PR description nit. "shipped its own Namespace resource since the chart's introduction (Jan 2026)" — chart's introduction is
c43db3b72025-12-25 (December 2025). Doesn't change the analysis.
- Bump alpine/k8s to 1.33.4 (was 1.32.0, EOL); matches in-tree pin used by cozystack-api, seaweedfs, vm-disk, and vm-instance hooks. - Skip hostNetwork=true and not-ready/CNI tolerations on the hosted variant: managed clusters provide CNI before install, and kube-system may carry strict PSA there that would reject hostNetwork pods. - Add packages/core/installer/templates/cozy-system-namespace.yaml gated on .Values.bareNamespace (default false). The top-level Makefile's manifests target now passes --set bareNamespace=true and renders Namespace + operator into _out/assets/cozystack-operator-*.yaml so the kubectl-apply install path stays self-contained. Helm install/upgrade callers keep the default and bootstrap via --create-namespace + the pre-install labeler hook. - Add a bats assertion in hack/e2e-install-cozystack.bats checking cozy-system carries pod-security.kubernetes.io/enforce=privileged and cozystack.io/system=true after helm install. - Inline comment near hostNetwork=true documenting why dnsPolicy left at Default (cluster DNS not up at install time; ClusterFirstWithHostNet would not help). Assisted-By: Claude <[email protected]> Signed-off-by: Myasnikov Daniil <[email protected]>
github-actions
Bot
added
the
size/L
|
Addressed everything actionable in 730037c. 1. Image (blocker) — bumped to 2. Hosted variant — wrapped 3. dnsPolicy — kept 4. Bats assertion — added two 5. Manifests render path — new 6. PR description nit — fixed; chart was introduced 2025-12-25 (December 2025), not January. |
Aleksei Sviridkin (lexfrei)
left a comment
Aleksei Sviridkin (lexfrei)
left a comment
There was a problem hiding this comment.
LGTM. All findings from the prior review are addressed in 730037c85:
- Blocker (alpine/k8s:1.32.0 EOL'd): bumped to
docker.io/alpine/k8s:1.33.4@sha256:b0523f0a244ddc4c8e055aa335c040d3d78b3ead5528f4544395f7f9f69c7b68. Aligns with the in-tree convention (other hooks: cozystack-api, seaweedfs, vm-disk pvc-resize) and lands inside a supported Kubernetes minor (1.33 EOL 2026-06-28, ~7 weeks of cushion). The digest pin is preserved. - Non-blocking #1 (hosted variant on strict-PSA kube-system): the
hostNetwork: trueblock and the not-ready/CNI tolerations are now gated{{- if ne .Values.cozystackOperator.variant "hosted" }}. Verified locally:helm template ... --set cozystackOperator.variant=hosted --show-only templates/cozy-system-labels.yaml | grep hostNetwork→ empty. On managed clusters (EKS/GKE/AKS/OpenShift) where kube-system can carrypod-security.kubernetes.io/enforce: baseline, the labeler is now admissible. The docstring at the top of the file explains the asymmetry clearly. - Non-blocking #2 (dnsPolicy): inline comment explains why the default
ClusterFirstis correct for the cold-install scenario (kubectl reaches the apiserver via KubePrism/explicit IP/kubelet-injected env, no DNS lookups happen, andClusterFirstWithHostNetwould not help because cluster DNS is not up yet). No code change needed; the rationale is preserved for future readers. - Non-blocking #4 (E2E assertion for PSA label):
hack/e2e-install-cozystack.bats:19-25now assertskubectl get ns cozy-system -o jsonpath='{.metadata.labels.pod-security\.kubernetes\.io/enforce}' | grep -qx privilegedand the same forcozystack.io/system=true. Exact one-liner I called out. E2E Tests is green on this HEAD — the hook is actually stamping the labels in the real install path, not just rendering correctly in unit tests. - Non-blocking #5 (
make manifestskubectl-apply path): this is the cleaner solution than what I proposed. A newtemplates/cozy-system-namespace.yamlis gated on.Values.bareNamespace(defaultfalse), and the top-levelMakefilepasses--set bareNamespace=truetogether with multi--show-onlyfor the rendered release artifacts. Helm install/upgrade callers leavebareNamespace=falseand the pre-install hook handles namespace labeling after--create-namespacecreates it bare. kubectl-apply users get the Namespace resource alongside the operator. Two paths cleanly separated without thehelm install --create-namespacechicken-and-egg coming back through the manifest render. The doc comment in the new template makes the contract explicit.
Verification:
helm templateexercise of all three variants (default talos / generic / hosted) renders cleanly. Hosted variant correctly omits hostNetwork + tolerations; talos/generic preserve the original block.- CI green: pre-commit, Build, E2E Tests, CodeRabbit,
Verify generated code is up to date. - Migration safety preserved: existing clusters'
cozy-systemnamespace still carrieshelm.sh/resource-policy: keepfrom the prior chart, so the upgrade keeps it; the relabeled hook re-asserts the PSA + identity labels and the new bats assertion catches a future regression where the hook silently fails.
Pre-existing repo-level note from the prior review (still non-blocking, not introduced by this PR): the PR-body description nit "Jan 2026" → actual chart introduction is c43db3b7 2025-12-25 (December). Doesn't change the analysis.
Audit of 30 successful PR runs found that across 5 sampled failure attempts, **25/25 retries** on `Run E2E tests` failed — the retry loop never recovered a flake, only stretched deterministic failures and tripled diagnostic wall-time. Same data shape on `Install Cozystack`. Drop both. `Prepare environment` keeps its 3x retry — that step is pure infrastructure (Talos image download, sandbox VM boot, network) where transient runner hiccups warrant a retry. On failure, the test step now captures `kubectl get hr -A -o wide` and `kubectl get events -A` under a collapsible group so triage starts with the actual broken-state snapshot. Depends on: - #2508 — installer namespace bootstrap (Helm namespace-ownership conflict) - #2509 — operator HelmRelease config knobs (`seaweedfs-system` 2-min wait race) - #2528 — harbor bucket-secret + BucketInfo gating (harbor ValuesError) - #2529 — objectstorage-controller BucketAccess conflict retry - the daniil/split-vminstance PR (vminstance disk race + VM IP/ready timeouts) - the daniil/split-event-driven PR (existence backstops surfacing real errors) Until those land, dropping the retry will fail CI for unrelated PRs that hit the seaweedfs / harbor / installer / vminstance races. Signed-off-by: Myasnikov Daniil <[email protected]> Assisted-By: Claude <[email protected]>
Audit of 30 successful PR runs found that across 5 sampled failure attempts, **25/25 retries** on `Run E2E tests` failed — the retry loop never recovered a flake, only stretched deterministic failures and tripled diagnostic wall-time. Same data shape on `Install Cozystack`. Drop both. `Prepare environment` keeps its 3x retry — that step is pure infrastructure (Talos image download, sandbox VM boot, network) where transient runner hiccups warrant a retry. On failure, the test step now captures `kubectl get hr -A -o wide` and `kubectl get events -A` under a collapsible group so triage starts with the actual broken-state snapshot. Depends on: - #2508 — installer namespace bootstrap (Helm namespace-ownership conflict) - #2509 — operator HelmRelease config knobs (`seaweedfs-system` 2-min wait race) - #2528 — harbor bucket-secret + BucketInfo gating (harbor ValuesError) - #2529 — objectstorage-controller BucketAccess conflict retry - the daniil/split-vminstance PR (vminstance disk race + VM IP/ready timeouts) - the daniil/split-event-driven PR (existence backstops surfacing real errors) Until those land, dropping the retry will fail CI for unrelated PRs that hit the seaweedfs / harbor / installer / vminstance races. Signed-off-by: Myasnikov Daniil <[email protected]> Assisted-By: Claude <[email protected]> Signed-off-by: Myasnikov Daniil <[email protected]>
github-actions
Bot
added
the
size/S
Audit of 30 successful PR runs found that across 5 sampled failure attempts, **25/25 retries** on `Run E2E tests` failed — the retry loop never recovered a flake, only stretched deterministic failures and tripled diagnostic wall-time. Same data shape on `Install Cozystack`. Drop both. `Prepare environment` keeps its 3x retry — that step is pure infrastructure (Talos image download, sandbox VM boot, network) where transient runner hiccups warrant a retry. On failure, the test step now captures `kubectl get hr -A -o wide` and `kubectl get events -A` under a collapsible group so triage starts with the actual broken-state snapshot. Depends on: - #2508 — installer namespace bootstrap (Helm namespace-ownership conflict) - #2509 — operator HelmRelease config knobs (`seaweedfs-system` 2-min wait race) - #2528 — harbor bucket-secret + BucketInfo gating (harbor ValuesError) - #2529 — objectstorage-controller BucketAccess conflict retry - the daniil/split-vminstance PR (vminstance disk race + VM IP/ready timeouts) - the daniil/split-event-driven PR (existence backstops surfacing real errors) Until those land, dropping the retry will fail CI for unrelated PRs that hit the seaweedfs / harbor / installer / vminstance races. Signed-off-by: Myasnikov Daniil <[email protected]> Assisted-By: Claude <[email protected]> Signed-off-by: Myasnikov Daniil <[email protected]>
Audit of 30 successful PR runs found that across 5 sampled failure attempts, **25/25 retries** on `Run E2E tests` failed — the retry loop never recovered a flake, only stretched deterministic failures and tripled diagnostic wall-time. Same data shape on `Install Cozystack`. Drop both. `Prepare environment` keeps its 3x retry — that step is pure infrastructure (Talos image download, sandbox VM boot, network) where transient runner hiccups warrant a retry. On failure, the test step now captures `kubectl get hr -A -o wide` and `kubectl get events -A` under a collapsible group so triage starts with the actual broken-state snapshot. Depends on: - #2508 — installer namespace bootstrap (Helm namespace-ownership conflict) - #2509 — operator HelmRelease config knobs (`seaweedfs-system` 2-min wait race) - #2528 — harbor bucket-secret + BucketInfo gating (harbor ValuesError) - #2529 — objectstorage-controller BucketAccess conflict retry - the daniil/split-vminstance PR (vminstance disk race + VM IP/ready timeouts) - the daniil/split-event-driven PR (existence backstops surfacing real errors) Until those land, dropping the retry will fail CI for unrelated PRs that hit the seaweedfs / harbor / installer / vminstance races. Signed-off-by: Myasnikov Daniil <[email protected]> Assisted-By: Claude <[email protected]>
Audit of 30 successful PR runs found that across 5 sampled failure attempts, **25/25 retries** on `Run E2E tests` failed — the retry loop never recovered a flake, only stretched deterministic failures and tripled diagnostic wall-time. Same data shape on `Install Cozystack`. Drop both. `Prepare environment` keeps its 3x retry — that step is pure infrastructure (Talos image download, sandbox VM boot, network) where transient runner hiccups warrant a retry. On failure, the test step now captures `kubectl get hr -A -o wide` and `kubectl get events -A` under a collapsible group so triage starts with the actual broken-state snapshot. Depends on: - #2508 — installer namespace bootstrap (Helm namespace-ownership conflict) - #2509 — operator HelmRelease config knobs (`seaweedfs-system` 2-min wait race) - #2528 — harbor bucket-secret + BucketInfo gating (harbor ValuesError) - #2529 — objectstorage-controller BucketAccess conflict retry - the daniil/split-vminstance PR (vminstance disk race + VM IP/ready timeouts) - the daniil/split-event-driven PR (existence backstops surfacing real errors) Until those land, dropping the retry will fail CI for unrelated PRs that hit the seaweedfs / harbor / installer / vminstance races. Signed-off-by: Myasnikov Daniil <[email protected]> Assisted-By: Claude <[email protected]>
Audit of 30 successful PR runs found that across 5 sampled failure attempts, **25/25 retries** on `Run E2E tests` failed — the retry loop never recovered a flake, only stretched deterministic failures and tripled diagnostic wall-time. Same data shape on `Install Cozystack`. Drop both. `Prepare environment` keeps its 3x retry — that step is pure infrastructure (Talos image download, sandbox VM boot, network) where transient runner hiccups warrant a retry. On failure, the test step now captures `kubectl get hr -A -o wide` and `kubectl get events -A` under a collapsible group so triage starts with the actual broken-state snapshot. Depends on: - #2508 — installer namespace bootstrap (Helm namespace-ownership conflict) - #2509 — operator HelmRelease config knobs (`seaweedfs-system` 2-min wait race) - #2528 — harbor bucket-secret + BucketInfo gating (harbor ValuesError) - #2529 — objectstorage-controller BucketAccess conflict retry - the daniil/split-vminstance PR (vminstance disk race + VM IP/ready timeouts) - the daniil/split-event-driven PR (existence backstops surfacing real errors) Until those land, dropping the retry will fail CI for unrelated PRs that hit the seaweedfs / harbor / installer / vminstance races. Signed-off-by: Myasnikov Daniil <[email protected]> Assisted-By: Claude <[email protected]>
Audit of 30 successful PR runs found that across 5 sampled failure attempts, **25/25 retries** on `Run E2E tests` failed — the retry loop never recovered a flake, only stretched deterministic failures and tripled diagnostic wall-time. Same data shape on `Install Cozystack`. Drop both. `Prepare environment` keeps its 3x retry — that step is pure infrastructure (Talos image download, sandbox VM boot, network) where transient runner hiccups warrant a retry. On failure, the test step now captures `kubectl get hr -A -o wide` and `kubectl get events -A` under a collapsible group so triage starts with the actual broken-state snapshot. Depends on: - #2508 — installer namespace bootstrap (Helm namespace-ownership conflict) - #2509 — operator HelmRelease config knobs (`seaweedfs-system` 2-min wait race) - #2528 — harbor bucket-secret + BucketInfo gating (harbor ValuesError) - #2529 — objectstorage-controller BucketAccess conflict retry - the daniil/split-vminstance PR (vminstance disk race + VM IP/ready timeouts) - the daniil/split-event-driven PR (existence backstops surfacing real errors) Until those land, dropping the retry will fail CI for unrelated PRs that hit the seaweedfs / harbor / installer / vminstance races. Signed-off-by: Myasnikov Daniil <[email protected]> Assisted-By: Claude <[email protected]>
Audit of 30 successful PR runs found that across 5 sampled failure attempts, **25/25 retries** on `Run E2E tests` failed — the retry loop never recovered a flake, only stretched deterministic failures and tripled diagnostic wall-time. Same data shape on `Install Cozystack`. Drop both. `Prepare environment` keeps its 3x retry — that step is pure infrastructure (Talos image download, sandbox VM boot, network) where transient runner hiccups warrant a retry. On failure, the test step now captures `kubectl get hr -A -o wide` and `kubectl get events -A` under a collapsible group so triage starts with the actual broken-state snapshot. Depends on: - #2508 — installer namespace bootstrap (Helm namespace-ownership conflict) - #2509 — operator HelmRelease config knobs (`seaweedfs-system` 2-min wait race) - #2528 — harbor bucket-secret + BucketInfo gating (harbor ValuesError) - #2529 — objectstorage-controller BucketAccess conflict retry - the daniil/split-vminstance PR (vminstance disk race + VM IP/ready timeouts) - the daniil/split-event-driven PR (existence backstops surfacing real errors) Until those land, dropping the retry will fail CI for unrelated PRs that hit the seaweedfs / harbor / installer / vminstance races. Signed-off-by: Myasnikov Daniil <[email protected]> Assisted-By: Claude <[email protected]>
Audit of 30 successful PR runs found that across 5 sampled failure attempts, **25/25 retries** on `Run E2E tests` failed — the retry loop never recovered a flake, only stretched deterministic failures and tripled diagnostic wall-time. Same data shape on `Install Cozystack`. Drop both. `Prepare environment` keeps its 3x retry — that step is pure infrastructure (Talos image download, sandbox VM boot, network) where transient runner hiccups warrant a retry. On failure, the test step now captures `kubectl get hr -A -o wide` and `kubectl get events -A` under a collapsible group so triage starts with the actual broken-state snapshot. Depends on: - #2508 — installer namespace bootstrap (Helm namespace-ownership conflict) - #2509 — operator HelmRelease config knobs (`seaweedfs-system` 2-min wait race) - #2528 — harbor bucket-secret + BucketInfo gating (harbor ValuesError) - #2529 — objectstorage-controller BucketAccess conflict retry - the daniil/split-vminstance PR (vminstance disk race + VM IP/ready timeouts) - the daniil/split-event-driven PR (existence backstops surfacing real errors) Until those land, dropping the retry will fail CI for unrelated PRs that hit the seaweedfs / harbor / installer / vminstance races. Signed-off-by: Myasnikov Daniil <[email protected]> Assisted-By: Claude <[email protected]>
Audit of 30 successful PR runs found that across 5 sampled failure attempts, **25/25 retries** on `Run E2E tests` failed — the retry loop never recovered a flake, only stretched deterministic failures and tripled diagnostic wall-time. Same data shape on `Install Cozystack`. Drop both. `Prepare environment` keeps its 3x retry — that step is pure infrastructure (Talos image download, sandbox VM boot, network) where transient runner hiccups warrant a retry. On failure, the test step now captures `kubectl get hr -A -o wide` and `kubectl get events -A` under a collapsible group so triage starts with the actual broken-state snapshot. Depends on: - #2508 — installer namespace bootstrap (Helm namespace-ownership conflict) - #2509 — operator HelmRelease config knobs (`seaweedfs-system` 2-min wait race) - #2528 — harbor bucket-secret + BucketInfo gating (harbor ValuesError) - #2529 — objectstorage-controller BucketAccess conflict retry - the daniil/split-vminstance PR (vminstance disk race + VM IP/ready timeouts) - the daniil/split-event-driven PR (existence backstops surfacing real errors) Until those land, dropping the retry will fail CI for unrelated PRs that hit the seaweedfs / harbor / installer / vminstance races. Signed-off-by: Myasnikov Daniil <[email protected]> Assisted-By: Claude <[email protected]>
Audit of 30 successful PR runs found that across 5 sampled failure attempts, **25/25 retries** on `Run E2E tests` failed — the retry loop never recovered a flake, only stretched deterministic failures and tripled diagnostic wall-time. Same data shape on `Install Cozystack`. Drop both. `Prepare environment` keeps its 3x retry — that step is pure infrastructure (Talos image download, sandbox VM boot, network) where transient runner hiccups warrant a retry. On failure, the test step now captures `kubectl get hr -A -o wide` and `kubectl get events -A` under a collapsible group so triage starts with the actual broken-state snapshot. Depends on: - #2508 — installer namespace bootstrap (Helm namespace-ownership conflict) - #2509 — operator HelmRelease config knobs (`seaweedfs-system` 2-min wait race) - #2528 — harbor bucket-secret + BucketInfo gating (harbor ValuesError) - #2529 — objectstorage-controller BucketAccess conflict retry - the daniil/split-vminstance PR (vminstance disk race + VM IP/ready timeouts) - the daniil/split-event-driven PR (existence backstops surfacing real errors) Until those land, dropping the retry will fail CI for unrelated PRs that hit the seaweedfs / harbor / installer / vminstance races. Signed-off-by: Myasnikov Daniil <[email protected]> Assisted-By: Claude <[email protected]>
…ay.bats tenant teardown (#2558) ## What this PR does Drops the 3× retry loop on `Run E2E tests` and `Install Cozystack into sandbox`. `Prepare environment` keeps its 3× retry — that step is pure infrastructure (Talos image download, sandbox VM boot, network) where transient runner hiccups warrant a retry. On failure, the test step now captures `kubectl get hr -A -o wide` and `kubectl get events -A` under a collapsible group so triage starts with the actual broken-state snapshot. > [!NOTE] > An earlier revision of this PR also doubled every bats timeout. That commit was dropped in a rebase and is intentionally **not restored**: the timeout class that actually matters (per-app HR-Ready waits) has since been standardized at 5m on `main` (7b9f286), making a blanket 2× redundant. **Fixes gateway.bats teardown leakage.** The nested-tenant tests deleted tenants fire-and-forget, parent and child back-to-back. The leftover uninstalls (each blocked on a cleanup Job, parents wedged on still-terminating child namespaces) plus one mid-install child HR occupied exactly 5 workers on the `--concurrent=5` tenants helm-controller shard, starving whichever app test ran next — observed as the harbor HR sitting unreconciled for its whole 5m HR-Ready budget in [run 27020081550](https://github.com/cozystack/cozystack/actions/runs/27020081550), surfaced by this PR's own retry removal + diagnostics dump. Teardown now deletes child→parent with hard `wait hr --for=delete` between, so a wedged tenant uninstall fails gateway.bats itself, not an innocent neighbor. ## Why Audit of 30 successful PR runs found that across 5 sampled failure attempts, **25/25 retries** on `Run E2E tests` failed — the retry loop never recovered a flake, only stretched deterministic failures and tripled diagnostic wall-time. Same data shape on `Install Cozystack`. Beyond wasted CI time, the retry was hiding ~10 deterministic bugs (Helm namespace-ownership conflict, seaweedfs HR timeout, harbor BucketInfo wiring, vminstance disk race, etc.). Each failure looked like a "flake" because the retry sometimes coincided with whatever transient state had cleared — the retry never fixed the bug, just delayed surfacing. ## Dependencies The deterministic bugs the retry was masking are now fixed on `main`: - ✅ **#2508** — installer namespace bootstrap (Helm namespace-ownership conflict on cold install) — merged - ✅ **#2509** — operator HelmRelease config knobs (`seaweedfs-system` 2-min wait race within Flux's 5-min reconcile windows) — merged - ✅ **#2528** — harbor bucket-secret + BucketInfo gating (harbor `ValuesError` on first install) — merged - ✅ **#2529** — objectstorage-controller BucketAccess conflict retry — merged Companion PRs in the #2619 split (independent of this PR, ordering-wise): - **#2602** — Flux v2.8.0 + chart fixes - **#2601** — seaweedfs-system split This PR does NOT depend on #2602/#2601 — it now touches only the workflow file and gateway.bats teardown, both on top of fresh `main`. Surfaced from #2500. ### Release note ```release-note NONE ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Chores** * CI prepare-environment step now reports plain attempt counts with clear success/failure messages. * Install and per-app test steps no longer retry; each runs once and fails immediately on error. Failed apps log diagnostics and job proceeds to remaining apps while overall job fails. * **Tests** * End-to-end tests and install/prepare flows use longer, more tolerant timeouts and added existence polling to reduce flakiness and improve diagnostics. <!-- review_stack_entry_start --> [](https://app.coderabbit.ai/change-stack/cozystack/cozystack/pull/2558?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 -->
%5D(https%3A%2F%2Fapp.coderabbit.ai%2Fchange-stack%2Fcozystack%2Fcozystack%2Fpull%2F2558%3Futm_source%3Dgithub_walkthrough%26amp%3Butm_medium%3Dgithub%26amp%3Butm_campaign%3Dchange_stack){kind=link}
3 participants
Why
helm v3 has a chicken-and-egg with charts that ship their own Namespace resource:
--create-namespaceon the install: helm pre-creates the namespace via plain kubectl-create (no helm meta annotations); the chart's own Namespace apply then fails withalready exists.--create-namespace: helm fails immediately because it can't write its release-secret to a non-existent namespace.The cozy-installer chart shipped a
Namespace cozy-systemresource since the chart's introduction (December 2025). Every cold install since has hit this conflict. It was masked by the 3× retry onInstall Cozystackin.github/workflows/pull-requests.yaml: attempt 1 fails with the conflict, attempt 2 sees the existing failed release and takes the upgrade code path which patch-merges instead of strict-create.Surfaced when retries on the install step were dropped.
What
Namespace cozy-systemfrompackages/core/installer/templates/cozystack-operator.yaml.packages/core/installer/templates/cozy-system-labels.yaml: a pre-install/pre-upgrade hook (ServiceAccount + ClusterRole + ClusterRoleBinding + Job) that patchescozystack.io/system=trueandpod-security.kubernetes.io/enforce=privilegedonto the namespace.verbs: [get, patch]onresourceNames: [cozy-system]only.hook-delete-policy: before-hook-creation,hook-succeededso the hook surface only exists during install/upgrade.Install commands now use
helm upgrade --install --namespace cozy-system --create-namespace …— standard pattern, matches kube-prometheus-stack / argo-cd / cert-manager.Verification
End-to-end on a kind cluster:
Test plan
Summary by CodeRabbit
New Features
Chores