Skip to content

feat(apps/kubernetes): overridable Talos image factory URL + in-sandbox e2e mirror#3244

Merged
Aleksei Sviridkin (lexfrei) merged 5 commits into
mainfrom
feat/kubernetes-talos-image-factory-override
Jul 8, 2026
Merged

feat(apps/kubernetes): overridable Talos image factory URL + in-sandbox e2e mirror#3244
Aleksei Sviridkin (lexfrei) merged 5 commits into
mainfrom
feat/kubernetes-talos-image-factory-override

Conversation

@myasnikovdaniil

@myasnikovdaniil myasnikovdaniil commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Problem

kubernetes-* e2e tests fail intermittently and each take 18–22 min. Root cause: every tenant worker VM imports its Talos OS disk directly from the public factory.talos.dev. That endpoint has no byte-range support (CDI falls back to a full copy-to-scratch) and its egress stream-resets / stalls mid-transfer from the CI runner, so a worker DataVolume import hangs past the chart's 12-minute node-join deadline → 0 tenant nodes join → test fails.

Seen on run 28924455578 (kubernetes-previous) and, on an unrelated branch, 28929095889 (kubernetes-latest) — so it's a shared external-dependency flake, not a per-PR bug.

Changes

1. Overridable image source (chart). Two new talos.* values, both defaulting to the public factory so rendered output is byte-identical and behaviour is unchanged:

  • talos.imageFactoryURL (default https://factory.talos.dev) — base URL for the worker OS disk image streamed in by CDI.
  • talos.installerRepository (default factory.talos.dev/installer) — OCI repository prefix for the in-guest talos-reconcile installer image.

This closes the README's Phase-2 "not currently overridable" gap for air-gapped / mirrored deployments.

2. In-sandbox e2e mirror. A best-effort talos-image-cache Deployment seeded once at install time with an aggressive curl --retry loop, then serving the image locally over HTTP with range support. Tenant Kubernetes CRs point spec.talos.imageFactoryURL at its Service via a shared helper that gates on the mirror being Available and falls back to the public factory otherwise — so the mirror can only help, never make CI worse. Deployed early so the seed overlaps the Cozystack install; readiness is resolved once and cached across bats files.

Verification

  • helm unittest — 173/173 pass (added override + trailing-slash cases).
  • helm template — default render byte-identical to pre-PR; override redirects both the disk URL and the installer image.
  • Regenerated values.schema.json, aggregated-API Go types, kubernetes-rd cozyrds schema and README via make generate (clean, no key-reorder churn).
  • Manifest YAML + shell syntax validated; CR heredoc renders valid YAML in both mirror and fallback cases.

Test plan

  • CI e2e: kubernetes-latest / kubernetes-previous import from the local mirror and join well within the node-join deadline. The mirror half can only be fully validated in a real CI run (busybox httpd / range serving / readiness gating); the fallback keeps it safe regardless.
feat(apps/kubernetes): the Talos worker OS image and installer are now overridable via `talos.imageFactoryURL` and `talos.installerRepository` (both default to the public factory.talos.dev), enabling air-gapped / mirrored deployments and an in-sandbox caching mirror for the e2e suite.

Refs #3231do not auto-close: this addresses the external-image-pull flake; the MachineDeployment.spec.replicas / LINSTOR-pressure concern in #3231 is a separate change.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features
    • Added configurable Talos worker image and installer settings (talos.imageFactoryURL, talos.installerRepository) for Kubernetes deployments.
    • Added support for using a local Talos image cache during end-to-end runs when available.
  • Bug Fixes
    • Reduced e2e flakiness by avoiding reliance on public Talos image downloads.
    • Normalized Talos factory URLs to handle trailing slashes correctly.
  • Documentation
    • Updated Kubernetes app docs to describe the new Talos options and Phase 1 behavior changes.
  • Tests
    • Added/expanded e2e and template rendering tests for the new Talos image URL behavior.

myasnikovdaniil and others added 2 commits July 8, 2026 18:11
…o overridable

Tenant worker VMs boot from a Talos raw disk image that CDI streams over
HTTP from the hardcoded public Talos Image Factory, and the in-guest
talos-reconcile Job pulls its installer image from the same hardcoded host.
Neither was overridable, so air-gapped / rate-limited / flaky-egress
environments could not point workers at a mirror (the README flagged this
as a Phase-2 gap), and CI had no way to avoid bulk-pulling the OS image
from the public internet per worker.

Add two chart values, both defaulting to the public factory so rendered
output is byte-identical and behaviour is unchanged:

- talos.imageFactoryURL (default https://factory.talos.dev) - base URL for
  the worker OS disk image streamed in by CDI.
- talos.installerRepository (default factory.talos.dev/installer) - OCI
  repository prefix for the in-guest installer image.

Regenerated values.schema.json, the aggregated-API Go types, the
kubernetes-rd cozyrds schema and the README via `make generate`; added
unittests for the override and trailing-slash trimming.

Refs #3231

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
The kubernetes-* e2e tests fail intermittently and run 18-22 min each
because every tenant worker VM imports its Talos OS disk directly from the
public factory.talos.dev: that endpoint has no byte-range support (forcing
CDI into a full copy-to-scratch) and its egress stream-resets / stalls
mid-transfer from the CI runner, so a worker DataVolume import hangs past
the chart's 12-minute node-join deadline and the test fails with zero
tenant nodes joined (observed on run 28924455578, and on the unrelated
branch in run 28929095889 - i.e. not specific to any one PR).

Deploy a best-effort in-sandbox mirror (hack/e2e-talos-image-cache.yaml)
during install: an initContainer fetches the image once with an aggressive
curl retry loop, then busybox httpd serves it locally with range support.
Tenant Kubernetes CRs point spec.talos.imageFactoryURL at its Service via
a shared helper (hack/e2e-apps/talos-image-cache.sh) that gates on the
mirror being Available and falls back to the public factory otherwise, so
the mirror can only help, never make CI worse. Deployed early so the seed
overlaps the Cozystack install; readiness is resolved once and cached
across bats files. Uses the overridable chart value added in the previous
commit.

Refs #3231

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
@github-actions github-actions Bot added size/L This PR changes 100-499 lines, ignoring generated files area/kubernetes Issues or PRs related to the tenant Kubernetes app kind/feature Categorizes issue or PR as related to a new feature labels Jul 8, 2026
@coderabbitai

coderabbitai Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

Adds Talos worker image and installer repository configuration across API, chart values, schemas, templates, and docs. Adds e2e Talos image-cache helpers, manifest, and test wiring to optionally source a mirrored worker image URL.

Changes

Talos image factory/installer configuration

Layer / File(s) Summary
API type, values schema, and defaults
api/apps/v1alpha1/kubernetes/types.go, packages/apps/kubernetes/values.yaml, packages/apps/kubernetes/values.schema.json, packages/system/kubernetes-rd/cozyrds/kubernetes.yaml
Adds ImageFactoryURL and InstallerRepository to the Talos API type, default values, JSON schema, and embedded ApplicationDefinition schema/ordering.
Template wiring and tests
packages/apps/kubernetes/templates/cluster.yaml, packages/apps/kubernetes/templates/talos/talos-reconcile-job.yaml, packages/apps/kubernetes/tests/cluster_test.yaml, packages/apps/kubernetes/README.md
Replaces hardcoded Talos factory URLs with configurable values in the worker image and installer Job templates, adds render tests, and documents the new parameters.
Talos image cache resolution helper
hack/e2e-apps/talos-image-cache.sh
Adds helper functions to resolve a cached Talos image factory URL and emit a YAML spec fragment when a mirror is available.
Talos image cache manifest and install test
hack/e2e-talos-image-cache.yaml, hack/e2e-install-cozystack.bats
Adds the Talos image-cache Service, Deployment, seed container, byte-range HTTP server, and a best-effort install test that applies the manifest.
e2e test wiring to use image cache
hack/e2e-apps/kubernetes-oidc-customconfig.bats, hack/e2e-apps/kubernetes-oidc-system.bats, hack/e2e-apps/run-kubernetes.sh
Sources the cache helper and injects the generated Talos image factory spec block into Kubernetes CR manifests in the e2e flows.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant BatsTest as e2e Bats Test
  participant CacheScript as talos-image-cache.sh
  participant K8sAPI as Kubernetes API
  participant KubernetesCR as Kubernetes CR

  BatsTest->>CacheScript: resolve_talos_image_factory_url()
  CacheScript->>K8sAPI: check talos-image-cache deployment
  alt deployment ready
    CacheScript->>K8sAPI: wait for rollout
    K8sAPI-->>CacheScript: rollout complete
    CacheScript-->>BatsTest: mirror URL
  else deployment missing or not ready
    CacheScript-->>BatsTest: empty string
  end
  BatsTest->>CacheScript: talos_image_factory_spec_block()
  CacheScript-->>BatsTest: YAML spec fragment
  BatsTest->>KubernetesCR: inject spec under `spec:`
  BatsTest->>K8sAPI: kubectl apply
Loading

Possibly related PRs

  • cozystack/cozystack#2931: Also changes Talos worker bootstrap configuration and Talos-related template wiring in the Kubernetes app.
  • cozystack/cozystack#3145: Also updates packages/apps/kubernetes/templates/talos/talos-reconcile-job.yaml and installer image construction.

Suggested labels: kind/api-change

Suggested reviewers: kvaps, lexfrei, lllamnyp

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main changes: configurable Talos image factory settings and an in-sandbox e2e mirror.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/kubernetes-talos-image-factory-override

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 Trivy (0.69.3)

Trivy execution failed: 2026-07-08T21:55:37Z FATAL Fatal error run error: fs scan error: scan error: scan failed: failed analysis: post analysis error: post analysis error: kubernetes scan error: fs filter error: fs filter error: walk error range error: stat .golangci.yml: no such file or directory: range error: stat .golangci.yml: no such file or directory


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@gemini-code-assist

Copy link
Copy Markdown
Contributor

Summary of Changes

Hello, 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 addresses intermittent failures in kubernetes-* e2e tests caused by flaky network access to the public Talos Image Factory. By introducing overridable configuration for the Talos OS image and installer repository, it enables support for air-gapped deployments and provides a mechanism for local caching. A new in-sandbox caching mirror has been added to the e2e test suite to stabilize worker node imports, ensuring that tests are more resilient to external network conditions while maintaining backward compatibility.

Highlights

  • Overridable Talos Image Sources: Added talos.imageFactoryURL and talos.installerRepository to the chart values, allowing users to override the default public factory URLs for air-gapped or mirrored environments.
  • In-Sandbox E2E Caching Mirror: Implemented a talos-image-cache Deployment in the e2e test suite that pre-fetches the Talos OS image and serves it locally, mitigating flaky egress issues during worker node imports.
  • Improved E2E Reliability: Updated e2e test scripts to automatically point worker DataVolume imports at the local mirror when available, falling back to the public factory otherwise to ensure test stability.
New Features

🧠 You can now enable Memory (public preview) 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 Assist

The 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 /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment Gemini (@gemini-code-assist) Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize the 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 .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. 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

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

@gemini-code-assist

Copy link
Copy Markdown
Contributor

Warning

Gemini encountered an error creating the review. You can try again by commenting /gemini review.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (2)
packages/apps/kubernetes/tests/cluster_test.yaml (1)

68-93: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Good test coverage for imageFactoryURL override and trailing-slash normalization.

The two new test cases correctly validate the configurable imageFactoryURL — the override test uses a fully-anchored regex, and the trailing-slash test cleverly verifies normalization by checking that talos/ + trimSuffix produces talos/image/ (not talos//image/).

One minor observation: the trailing-slash test (line 91) doesn't anchor the regex with $, while the override test (line 78) does. This is acceptable since the test's focus is specifically on slash normalization, but consider anchoring for consistency if you add more assertions later.

♻️ Optional: anchor the trailing-slash test regex for consistency
-          pattern: "^https://mirror\\.example\\.com/talos/image/[a-f0-9]+/"
+          pattern: "^https://mirror\\.example\\.com/talos/image/[a-f0-9]+/v[0-9.]+/openstack-amd64\\.raw\\.xz$"
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/apps/kubernetes/tests/cluster_test.yaml` around lines 68 - 93, The
trailing-slash normalization test for talos.imageFactoryURL is missing a fully
anchored regex, unlike the existing override test. Update the matchRegex pattern
in the trailing-slash case to anchor the expected URL consistently so the
assertion only passes for the intended http.source URL shape. Keep the change
limited to the cluster_test.yaml assertions around the imageFactoryURL cases.
hack/e2e-talos-image-cache.yaml (1)

84-88: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win

Add transfer stall detection to the curl seed command.

--retry-all-errors handles HTTP/2 stream resets (which produce errors), but a stalled download — data stops arriving without a reset — is not an error without a transfer timeout. The curl process would hang indefinitely, consuming the entire 12m rollout budget without retrying. Adding --speed-time/--speed-limit detects stalls and triggers retries, increasing the chance the cache is warm before the first tenant test needs it.

♻️ Proposed fix: add stall detection to curl
               curl --fail --location --show-error \
                    --retry 30 --retry-all-errors --retry-delay 15 \
                    --connect-timeout 30 \
+                   --speed-time 60 --speed-limit 1024 \
                    --output "$dest" \
                    "$url"
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@hack/e2e-talos-image-cache.yaml` around lines 84 - 88, The curl seed command
in the image cache setup can hang on stalled transfers because it only retries
errors and has no transfer-time stall detection. Update the curl invocation to
add stall detection using the existing seed/download command so that a slow or
frozen transfer times out and retries instead of consuming the rollout budget;
use the curl options alongside the current --retry/--connect-timeout flags in
the command that writes to "$dest" from "$url".
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@hack/e2e-talos-image-cache.yaml`:
- Around line 84-88: The curl seed command in the image cache setup can hang on
stalled transfers because it only retries errors and has no transfer-time stall
detection. Update the curl invocation to add stall detection using the existing
seed/download command so that a slow or frozen transfer times out and retries
instead of consuming the rollout budget; use the curl options alongside the
current --retry/--connect-timeout flags in the command that writes to "$dest"
from "$url".

In `@packages/apps/kubernetes/tests/cluster_test.yaml`:
- Around line 68-93: The trailing-slash normalization test for
talos.imageFactoryURL is missing a fully anchored regex, unlike the existing
override test. Update the matchRegex pattern in the trailing-slash case to
anchor the expected URL consistently so the assertion only passes for the
intended http.source URL shape. Keep the change limited to the cluster_test.yaml
assertions around the imageFactoryURL cases.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 66ec9a5f-f754-4a3d-ae39-cefcee7b1bb0

📥 Commits

Reviewing files that changed from the base of the PR and between 12c3cd0 and 47e7d22.

📒 Files selected for processing (14)
  • api/apps/v1alpha1/kubernetes/types.go
  • hack/e2e-apps/kubernetes-oidc-customconfig.bats
  • hack/e2e-apps/kubernetes-oidc-system.bats
  • hack/e2e-apps/run-kubernetes.sh
  • hack/e2e-apps/talos-image-cache.sh
  • hack/e2e-install-cozystack.bats
  • hack/e2e-talos-image-cache.yaml
  • packages/apps/kubernetes/README.md
  • packages/apps/kubernetes/templates/cluster.yaml
  • packages/apps/kubernetes/templates/talos/talos-reconcile-job.yaml
  • packages/apps/kubernetes/tests/cluster_test.yaml
  • packages/apps/kubernetes/values.schema.json
  • packages/apps/kubernetes/values.yaml
  • packages/system/kubernetes-rd/cozyrds/kubernetes.yaml

@IvanHunters IvanHunters left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict

NOT LGTM

The chart half (overridable talos.imageFactoryURL / talos.installerRepository) is correct and upgrade-safe, but the e2e mirror half — the part that is supposed to actually fix the flake — is non-functional: the serve container's busybox httpd applet does not exist in the pinned image, so the mirror never becomes Available and every run silently falls back to the public factory it was meant to replace.

Findings

[CRITICAL] hack/e2e-talos-image-cache.yaml:101busybox httpd applet is absent from the pinned image; the mirror serve container crash-loops and the whole caching layer is dead.

The serve container runs exec busybox httpd -f -v -p 8080 -h /data on docker.io/alpine/k8s:1.36.2@sha256:44ef4942…. I ran the exact command against that exact digest twice:

$ docker run --rm --entrypoint /bin/sh alpine/k8s:1.36.2@sha256:44ef4942… -ec 'exec busybox httpd -f -v -p 8080 -h /tmp'
httpd: applet not found
$ docker run --rm --entrypoint /bin/sh …@sha256:44ef4942… -ec 'busybox --list | grep -c "^httpd$"; command -v httpd'
0
(no standalone httpd binary either — command -v httpd exits 127)

This BusyBox build was compiled without the httpd applet, and there is no standalone httpd. busybox --list shows zero http* applets. The applet set is compile-time and identical across the arm64/amd64 variants of the same digest, so this is not arch-specific. Consequence chain: serve exits 127 immediately → readinessProbe tcpSocket: http never passes (port never opens) → Deployment never reaches Available → resolve_talos_image_factory_url() (hack/e2e-apps/talos-image-cache.sh:147) burns its full --timeout=12m on kubectl rollout status, then falls back to the public factory. Net effect: the entire second half of the PR does nothing except add a fixed ~12-minute wait to the first kubernetes-* test, while the flake the PR set out to fix remains fully live. The seed initContainer (curl) is fine — curl 8.20.0 is present and supports --retry-all-errors — but the emptyDir it fills is never served. Fix: use a static file server that actually exists in this image. alpine/k8s:1.36.2 ships python3, but python3 -m http.server does NOT support HTTP range requests (so it would defeat the range-serving rationale in the manifest comment at line 96). Use a range-capable server present/installable in the image (e.g. darkhttpd, thttpd, lighttpd, or apk add busybox-extras which provides httpd), and add a smoke check in the @test (curl -sf -r 0-0 http://… expecting 206) so a missing/incapable server fails the mirror deploy loudly instead of silently degrading to the 12-minute fallback.

[MINOR] hack/e2e-talos-image-cache.yaml:96 — range-support claim is unverified for whatever server ends up replacing busybox httpd.

The comment asserts "busybox httpd serves static files with HTTP range support, so a warm mirror also lets CDI use its faster range path". Two problems: (a) the server does not exist (see CRITICAL), and (b) the range-support property is exactly the load-bearing claim behind the whole mirror (the PR body states the public factory's lack of range support is the root cause), yet it is asserted, not tested. Whatever replaces the httpd line must be verified to answer Range: with 206 Partial Content and an Accept-Ranges: bytes header; otherwise the mirror reintroduces the same copy-to-scratch fallback and only removes the egress-flakiness half of the problem. Add the 206 assertion to the deploy @test.

Claim mismatches

[PARTIAL] "serving the image locally over HTTP with range support" / "busybox httpd serves static files with HTTP range support" — the chosen image cannot run busybox httpd at all (repro above), so neither the serving nor the range support materialises. The mirror half of the "Changes" section is not delivered by the current manifest.

[UNVERIFIABLE] "CI e2e: import from the local mirror and join well within the node-join deadline" — cannot be validated statically and, given the CRITICAL, the mirror will not serve in CI, so this checkbox would pass only via the public-factory fallback path (i.e. measuring the status quo, not the mirror). The "byte-identical default render" and "173/173 helm unittest" claims are [OK] — I re-ran helm unittest (173/173 pass, including the two new override/trailing-slash cases) and confirmed values.schema.json, the kubernetes-rd cozyrds openAPISchema, and the Go Talos struct are all regenerated and mutually consistent.

Operational risks

  • Fixed ~12-minute latency tax on every CI run: the dead mirror forces resolve_talos_image_factory_url to wait out the full rollout status --timeout=12m before falling back, on the first kubernetes-* test of the suite (hack/e2e-apps/talos-image-cache.sh:147). The PR's own goal is to reduce the 18–22 min per-test time; as written it adds wait instead. This alone should block merge until the serve container works.

Caveats

  • Phase 5b-A (existing-customer upgrade): chart-only change, no packages/core/platform touch, no migration required. The two new values are required + default, identical to the pre-existing schematicID/version shape, so tenant CRs that omit talos receive the defaults via Helm merge and render byte-identically to N−1 (templates/cluster.yaml:382, templates/talos/talos-reconcile-job.yaml:395 both fall back to the public factory string). No CRD field removal, no RBAC change, no image-digest bump. Verified by diffing old vs. new values.schema.json talos.required (["schematicID","version"]["imageFactoryURL","installerRepository","schematicID","version"], all four with defaults). No upgrade breakage.
  • Phase 5b-B (fresh install): defaults are the current public factory, so cold install behaviour is unchanged; air-gapped operators gain a real override (the stated feature). No new PackageSource / bundle / dependsOn / _cluster / _namespace keys introduced. Chart half is clean on both scenarios.
  • chart_lint render_error (index of untyped nil on .Values._cluster "cluster-domain" at talos-reconcile-job.yaml:487) is the known runtime-injected _cluster false-positive (populated by the cozystack-values Secret at runtime, nil under bare helm template); it is at line 487, pre-existing and unrelated to this PR's line-395 change. Not a defect.
  • Both SC2148 "missing shebang" shell_lint errors are false positives: talos-image-cache.sh and run-kubernetes.sh are sourced (.), not executed, matching the existing remediation-guard.sh convention. The new bats @test in e2e-install-cozystack.bats:36 is a real @test (runs under cozytest.sh) and is correctly guarded with || echo WARNING + return 0, so it stays best-effort and cannot fail the suite — that guarding is sound; the problem is purely that the thing it deploys does not work.
  • The mirror manifest lands in kube-system, not a tenant-* namespace, so PSS-restricted compliance does not apply; absence of securityContext is acceptable for a CI-only artifact.

Recommended follow-ups

  • Refs #3231 framing in the PR body is correct — keep the issue open. Given the CRITICAL, the flake is not actually addressed by the mirror; only the durable chart override lands. Do not let a green CI run (which would be green via fallback) be read as the mirror working.

…-image-factory-override

Signed-off-by: Aleksei Sviridkin <f@lex.la>

# Conflicts:
#	packages/system/kubernetes-rd/cozyrds/kubernetes.yaml
The talos-image-cache serve container ran `busybox httpd`, but the pinned alpine/k8s image ships no `httpd` applet and no standalone httpd, so the container exited 127 and crash-looped and the Deployment never became Available. The resolver then burned its full 12-minute rollout wait before falling back to the public factory — a fixed latency tax on the first kubernetes-* test while the flake the mirror targets stayed live.

Serve the seeded image with a small range-capable server on the image's bundled python3 instead (stock `python3 -m http.server` ignores Range headers and answers 200, not 206). It needs no extra package pull, so the mirror comes up deterministically and keeps CDI's fast range import. The seed curl now also aborts and retries a stalled transfer (--speed-time/--speed-limit), and the resolver verifies a 206 range response before pointing tenants at the mirror.

Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
…rtion

Match the full rendered worker-disk URL shape (version + artifact + end anchor) like the sibling override assertion, so the trailing-slash normalization case cannot pass on a partial match.

Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@hack/e2e-talos-image-cache.yaml`:
- Around line 167-188: Harden the range handling in send_head() by guarding the
Range header parsing with try/except and returning 416 for malformed values
instead of letting parsing errors escape. Also reset the per-request _remaining
state at the start of send_head() so a ranged HEAD response in the HTTP handler
does not carry leftover state into the next request on the same connection.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: dd48dd96-feae-42a7-bfd8-a5b15139d23b

📥 Commits

Reviewing files that changed from the base of the PR and between 47e7d22 and c32ac68.

📒 Files selected for processing (12)
  • api/apps/v1alpha1/kubernetes/types.go
  • hack/e2e-apps/kubernetes-oidc-customconfig.bats
  • hack/e2e-apps/kubernetes-oidc-system.bats
  • hack/e2e-apps/run-kubernetes.sh
  • hack/e2e-apps/talos-image-cache.sh
  • hack/e2e-talos-image-cache.yaml
  • packages/apps/kubernetes/README.md
  • packages/apps/kubernetes/templates/cluster.yaml
  • packages/apps/kubernetes/tests/cluster_test.yaml
  • packages/apps/kubernetes/values.schema.json
  • packages/apps/kubernetes/values.yaml
  • packages/system/kubernetes-rd/cozyrds/kubernetes.yaml
✅ Files skipped from review due to trivial changes (2)
  • packages/apps/kubernetes/README.md
  • packages/apps/kubernetes/tests/cluster_test.yaml
🚧 Files skipped from review as they are similar to previous changes (8)
  • packages/apps/kubernetes/values.yaml
  • packages/apps/kubernetes/templates/cluster.yaml
  • hack/e2e-apps/kubernetes-oidc-system.bats
  • api/apps/v1alpha1/kubernetes/types.go
  • hack/e2e-apps/run-kubernetes.sh
  • packages/apps/kubernetes/values.schema.json
  • packages/system/kubernetes-rd/cozyrds/kubernetes.yaml
  • hack/e2e-apps/kubernetes-oidc-customconfig.bats

Comment on lines +167 to +188
def send_head(self):
path = self.translate_path(self.path)
if os.path.isdir(path):
return super().send_head()
try:
f = open(path, "rb")
except OSError:
self.send_error(404)
return None
size = os.fstat(f.fileno()).st_size
ctype = self.guess_type(path)
rng = self.headers.get("Range", "")
if not rng.startswith("bytes="):
self.send_response(200)
self.send_header("Content-Type", ctype)
self.send_header("Content-Length", str(size))
self.send_header("Accept-Ranges", "bytes")
self.end_headers()
return f
start_s, _, end_s = rng[6:].split(",")[0].strip().partition("-")
start = int(start_s) if start_s else max(0, size - int(end_s))
end = int(end_s) if start_s and end_s else size - 1

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# After deploying the manifest, verify malformed and HEAD range requests don't poison keep-alive handling.
pod=$(kubectl -n kube-system get pod \
  -l app.kubernetes.io/name=talos-image-cache \
  --field-selector=status.phase=Running \
  -o jsonpath='{.items[0].metadata.name}')

kubectl -n kube-system exec "$pod" -c serve -- sh -ec '
  f=$(find /data -name "*.raw.xz" 2>/dev/null | head -n1)
  [ -n "$f" ]

  curl -sS -o /dev/null -w "malformed=%{http_code}\n" \
    -H "Range: bytes=abc-def" "http://127.0.0.1:8080/${f#/data/}"

  curl -sS -I -H "Range: bytes=0-0" "http://127.0.0.1:8080/${f#/data/}" >/dev/null

  curl -sS -o /dev/null -w "full=%{http_code} size=%{size_download}\n" \
    "http://127.0.0.1:8080/${f#/data/}"
'

Repository: cozystack/cozystack

Length of output: 243


🏁 Script executed:

#!/bin/bash
set -euo pipefail

git ls-files 'hack/e2e-talos-image-cache.yaml'
wc -l hack/e2e-talos-image-cache.yaml
sed -n '130,240p' hack/e2e-talos-image-cache.yaml

Repository: cozystack/cozystack

Length of output: 3746


Harden Range parsing and clear per-request state.
Wrap the Range parse in try/except and return 416 for malformed values; also reset _remaining at the start of send_head() so a ranged HEAD can’t leak state into the next request on the same keep-alive connection.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@hack/e2e-talos-image-cache.yaml` around lines 167 - 188, Harden the range
handling in send_head() by guarding the Range header parsing with try/except and
returning 416 for malformed values instead of letting parsing errors escape.
Also reset the per-request _remaining state at the start of send_head() so a
ranged HEAD response in the HTTP handler does not carry leftover state into the
next request on the same connection.

@lexfrei

Copy link
Copy Markdown
Contributor

Fixed the dead mirror plus the two nits, and updated the branch onto main.

Serve container (CRITICAL): confirmed — alpine/k8s:1.36.2 has no busybox httpd applet, so serve was crash-looping and the Deployment never went Available. Replaced it with a small range-capable server on the image's bundled python3 (a serve.py ConfigMap) — stock python3 -m http.server answers 200 to Range, so this adds the single-range 206 path. Verified against the exact pinned digest: Accept-Ranges: bytes, bytes=0-0206 with correct Content-Range, keep-alive, full-file byte match. The ~12-minute fallback tax is gone.

Range support (MINOR): now asserted — the resolver does an in-cluster curl -r 0-0 smoke check and only reports range-verified on a 206; a non-206 logs loudly and still uses the mirror rather than silently degrading.

Nits: --speed-time 60 --speed-limit 1024 on the seed curl so a frozen transfer aborts and retries; anchored the trailing-slash test to the full URL shape.

Also merged main to clear the conflict (only the generated cozyrds/kubernetes.yaml collided, regenerated from the merged schema, x-cozystack-options preserved). helm unittest 175/175.

@IvanHunters

Copy link
Copy Markdown
Collaborator

Verdict

LGTM with non-blocking notes

Clean, backwards-compatible additive change: two new overridable Talos fields wired through the templates with trimSuffix "/", generated artifacts (schema, README, -rd openAPISchema/keysOrder) all regenerated, and helm unittest green (175/175); the only substantive gap is asymmetric unit-test coverage.

Findings

[MINOR] packages/apps/kubernetes/tests/cluster_test.yaml:540installerRepository override has no unit-test assertion

The PR adds two overridable fields but only imageFactoryURL gets unit coverage (two new cases at cluster_test.yaml:540 and :553, including trailing-slash stripping). talos.installerRepository — which feeds talos-reconcile-job.yaml:420 ({{ $.Values.talos.installerRepository | trimSuffix "/" }}/{{ $.Values.talos.schematicID }}:{{ .talosVersion }}) — is exercised nowhere in tests/. Grep confirms zero hits in packages/apps/kubernetes/tests/. The two knobs share the same | trimSuffix "/" shape and the same air-gapped rationale; test them symmetrically. Add a cluster_test.yaml case asserting the reconcile Job's install.image honours an installerRepository override and strips a trailing slash, mirroring the imageFactoryURL cases.

Caveats

  • Phase 5b — existing-customer upgrade: no breaking change. Both new keys are added to values.schema.json talos.required[] AND carry a default; verified against main that schematicID/version were already required-with-default, so required-plus-default is the established pattern and the defaulter fills these before validation — existing tenant Kubernetes CRs that omit talos.* still admit. No migration script needed (migrations.targetVersion untouched, repo_invariants clean). Template change on cluster.yaml:98 renders byte-identical to the old hardcoded URL when the default applies (https://factory.talos.dev + trimSuffix "/" is a no-op); the installer reference on talos-reconcile-job.yaml:420 is likewise byte-identical under the default. No RBAC/CRD-field removal; the -rd openAPISchema and keysOrder were correctly regenerated to include both fields.
  • Phase 5b — fresh install: no new cold-start dependency, no PackageSource/bundle wiring change, no _cluster/_namespace key added. helm unittest passes 175/175 including the new cases. The chart_lint render_error (index of untyped nil on .Values._cluster) and the 8 missing_refs are the standard runtime-injected cozystack-values artefacts, not defects.
  • Security: talos.imageFactoryURL/installerRepository are tenant-settable, so a tenant can point the worker OS-image / installer source at an arbitrary host. This does not cross the existing trust boundary — the CDI importer fetches into the tenant's own namespace and the tenant already runs arbitrary guest-cluster workloads and VMs with egress governed by cozystack network contours; the field adds no new cross-tenant escalation. The egress boundary, not this field, remains the control.
  • The e2e mirror is genuinely best-effort: resolve_talos_image_factory_url emits an empty block (chart default = public factory) on every failure path, the install-time @test never fails the suite, and run-kubernetes.sh gates on rollout status --timeout=12m before pointing tenants at it. It lives entirely under hack/ (CI-only) and never ships in packages/, so it has zero production blast radius. The @test (not setup_file) choice for the mirror Deployment is correct and the author documents the cozytest.sh-only-runs-@test trap inline; sourcing the helper at bats file top-level matches existing precedent.
  • shell_lint flags SC2148 (no shebang) as an "error" on the new hack/e2e-apps/talos-image-cache.sh — false positive: it is a .-sourced library, and its siblings (remediation-guard.sh, run-kubernetes.sh) also carry no shebang by the same convention. The remaining shell-lint items are info/style (SC2086 quoting, SC1091 not-followed-source, SC2016) and pre-existing.
  • Image-mirror-routing note does not bite here: the docker.io/alpine/k8s:1.36.2@sha256:44ef49... reference in hack/e2e-talos-image-cache.yaml is digest-pinned and deliberately reuses an already-shipped, pre-pulled installer image; it lives in hack/ (a CI-only manifest), not in a shipped packages/*/templates/*, so the cozy-lib.image registry-helper requirement does not apply to it.

Recommended follow-ups

  • The README diff (packages/apps/kubernetes/README.md) correctly narrows the "air-gapped tenant workers" caveat to in-guest registry pulls (machine.registries.mirrors) as the remaining Phase 2 gap. No action needed on this PR; just flagging that the Phase 2 registries.mirrors restoration is still tracked separately and this PR does not close it.

@lexfrei Aleksei Sviridkin (lexfrei) merged commit 0814586 into main Jul 8, 2026
40 of 43 checks passed
@lexfrei Aleksei Sviridkin (lexfrei) deleted the feat/kubernetes-talos-image-factory-override branch July 8, 2026 22:24
myasnikovdaniil added a commit that referenced this pull request Jul 9, 2026
Fold the in-sandbox Talos image cache work into the Chainsaw layout:
the new shared helper relocates to hack/e2e-chainsaw/_lib/
talos-image-cache.sh and run-kubernetes.sh sources it from there (the
ouroboros DNS-check no-retry rework merges clean). The OIDC bats are
resolved as deleted again; their Chainsaw fixtures deliberately skip
the talos.imageFactoryURL injection — the helper only emits it when
the cache is up, and the zero-worker render-side tests instantiate no
DataVolume imports for it to redirect.

Assisted-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
myasnikovdaniil added a commit that referenced this pull request Jul 9, 2026
)

The in-sandbox Talos image mirror (#3244) is unreachable by the tenant
workers it serves, so the kubernetes-* e2e tests fail deterministically at
the 12m node-join wait instead of relieving the public-factory flake (#3231).

Root cause (from #3254 by @lexfrei, verified against the failing run's
crust-gather snapshot): tenant namespaces run a default-deny Cilium egress
(allow-internal/external-communication + the <tenant>-egress CCNP) that
permits the world entity and the tenant tree but NOT an arbitrary kube-system
Service, so a worker's CDI importer resolves talos-image-cache.kube-system.svc
yet its TCP connect to the ClusterIP is silently dropped and no disk imports.
#3244's gate never caught this because it probed the server over localhost
inside the serve pod, which passes regardless of cross-pod reachability.

Port of #3254 to the Chainsaw layout (hack/e2e-chainsaw/_lib/):
- ship a tightly-scoped CiliumClusterwideNetworkPolicy (importer pods in
  tenant-test -> mirror pod in kube-system), applied at point-of-use once
  Cilium CRDs exist; excluded from the pre-Cilium install apply via yq.
- replace the readiness probe with a throwaway pod in tenant-test labelled
  cdi.kubevirt.io=importer that curls the ClusterIP with a range request and
  must get 206 - the exact egress path a real importer faces; tenants use the
  mirror only when that end-to-end check passes, else fall back to the public
  factory, so it can only help, never make CI worse.
- unit test pinning the manifest split, probe/policy label agreement, the
  strict 206 gate, and the --overrides JSON builder.

Also adds (a2) DataVolume import-stage diagnostics to the node-join failure
block in run-kubernetes.sh (importer logs + a ClusterIP re-probe via
talos_image_cache_diagnose) so the import-stage failure mode is legible.

Assisted-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
IvanHunters added a commit that referenced this pull request Jul 9, 2026
…workers (#3254)

## What this PR does

The in-sandbox Talos image cache added in #3244 is unreachable by the
tenant workers it was meant to serve, so it makes the
`kubernetes-latest` / `kubernetes-previous` e2e tests fail
deterministically instead of relieving the intermittent public-factory
flake it targets (#3231). Every branch off current `main` inherits this
— for example #3240 has failed its tenant-Kubernetes e2e repeatedly for
this reason alone, despite being unrelated (it only touches host boot
assets).

Root cause: tenant namespaces run under a default-deny Cilium egress
installed by the tenant chart. A worker's CDI importer is allowed to
reach the outside world (so the public factory works) and kube-dns (so
the name resolves), but nothing permits egress to an arbitrary
`kube-system` Service. The importer therefore resolves
`talos-image-cache.kube-system.svc` yet its TCP connect to the ClusterIP
is silently dropped (`dial tcp <clusterip>:80: i/o timeout`), the
DataVolume import never starts, the worker VM stays `DataVolumeError`,
and no tenant node joins within the 12-minute deadline. #3244's
readiness gate never caught this because it probed the server over
`localhost` inside the serve Pod, which passes regardless of cross-Pod
reachability.

Two changes:

- Ship a tightly-scoped `CiliumClusterwideNetworkPolicy` that lets
importer Pods in `tenant-test` egress to the mirror Pod in
`kube-system`. Cilium unions allow rules, so this only adds a hole;
scoping it to a namespace that is already default-deny egress means it
never flips another namespace's posture, and the mirror Pod needs no
ingress rule because no ingress policy selects it. The policy is applied
at point-of-use (not with the rest of the mirror manifest, which is
applied before Cilium's CRDs exist).
- Replace the localhost range-probe with one that faces the exact same
network path as a real importer: a throwaway Pod in the tenant
namespace, labelled `cdi.kubevirt.io=importer`, that fetches the seeded
image from the Service ClusterIP with a Range request and must get a
`206`. Tenants are pointed at the mirror only when that end-to-end check
passes; otherwise the harness falls back to the public factory. This
restores the invariant #3244 intended — the mirror can only help, never
make CI worse.

Not a retry/timeout bump: it fixes the real network path and adds a
genuine reachability gate.

### Screenshots

N/A — CI-only change, no user-facing surface.

### Release note

```release-note
test(e2e): make the in-sandbox Talos image cache reachable by tenant Kubernetes workers, fixing deterministic kubernetes-* e2e failures (#3231)
```


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

* **New Features**
* Updated Talos image cache e2e selection to run a tenant-scoped
reachability probe to the local mirror and require HTTP byte-range
support (`206`) before using it.
* If the probe fails or does not confirm byte-range behavior, the flow
now reliably falls back to the public Talos image source.
* Added CI-only tenant egress controls to allow importer Pods to reach
the mirror Service.
* **Documentation**
* Expanded e2e guidance to clarify the mirror is best-effort and that
fallback happens at probe time.
* **Tests**
* Added a new Bats suite covering manifest splitting, policy wiring,
probe overrides, and strict `206` gating/fallback scenarios.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/kubernetes Issues or PRs related to the tenant Kubernetes app kind/feature Categorizes issue or PR as related to a new feature size/L This PR changes 100-499 lines, ignoring generated files

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants