mirror of
https://github.com/cozystack/cozystack.git
synced 2026-11-08 21:14:56 +09:00
Aleksei Sviridkin
dda339f351
## What this PR does The tenant Kubernetes e2e suites decide once whether worker image pulls go to the in-cluster Talos mirror or to the public Image Factory, and that decision came from a plain `kubectl get deploy talos-image-cache`. A plain `get` exits non-zero for a connection refused, an Unauthorized and a real absence alike, so any blip during that one call read as "no mirror", and the answer went straight into the decision file every later tenant test reads. One unlucky moment sent every worker in the run to the public factory over live egress, which is the dependency the mirror exists to remove, and nothing in the log said so. The run went ahead against the flaky thing and the failures read as environment noise. The lookup now asks with `--ignore-not-found -o name`. A real absence answers exit 0 with no output, which leaves a non-zero exit meaning only that the question failed, and the helper re-asks a couple of times before giving up. Only the empty answer turns the suite toward the public factory. An unanswered lookup warns, falls back for its own caller alone, and is deliberately not cached, so the next tenant test asks again instead of inheriting a guess. The warning has to be backed by something, and kubectl's stderr alone is not enough. When the wall-clock bound kills an attempt, neither the bound nor the SIGTERMed kubectl writes a word, and that stall is precisely the failure the re-ask was written against, so a warning that sent the reader to kubectl's message would be pointing at an empty stretch of log exactly when the lookup mattered. The lookup therefore names the last attempt's exit status before giving up, in the vocabulary the diagnostic reads already use for 124 and 137, and kubectl's own text stays on stderr for the cases where there is any. Each attempt is bounded twice, and the two bounds are deliberately not equal: `timeout -k 5 40` around `--request-timeout=30s`. kubectl defaults the request timeout to 0, so a connection that establishes and then stalls has no deadline at all, and the flag on its own is not a wall-clock bound either, because kubectl retries discovery several times before it gives up. Re-asking an unbounded call three times would have multiplied a hang instead of riding out a blip. The ten seconds between the two are load-bearing, and I only found out why by measuring rather than reasoning. Driven against a stub apiserver the three failure modes are not alike: a refused port answers in under a second and names the refusal, one that completes TCP but stalls mid-TLS hits kubectl's own handshake deadline and still produces text despite being killed, and a blackholed address (SYN dropped, no RST, which is what a wedged node or a netpol looks like) produces nothing at all until some deadline expires. That last mode is the one the headroom buys back. With both bounds at the same value the expiry that arrives first is always the wall-clock kill, so the blackhole case ends as a bare exit 124; with the wrapper ten seconds above, the request deadline ends the attempt and kubectl says what happened, and the wrapper stays as the backstop for a client that overruns its own deadline. Controlled A/B, same target and same inner bound, only the outer differs: 30 over 30 gives exit 124 and zero bytes, 40 over 30 gives exit 1 at the thirty second mark and a message naming the cause. The cost is ten more seconds per attempt on a path where the apiserver is already unreachable, inside a 50m op. Worth stating separately, because it is a different question with a different answer: before this change the inner flag was not doing anything at all. `--request-timeout=30s` sat under a `timeout` of the same 30 seconds, and across all three measured modes it never once fired. The refused case never reached it, the TLS stall was ended by kubectl's own shorter handshake deadline, and the blackhole was ended by the wall-clock kill landing at the same instant. So the flag was decoration, and the comment above it claimed otherwise. That is a separate defect from the lost diagnostics, and it is fixed by the same separation rather than by adding anything. Where `timeout` is not installed the read runs unwrapped instead of through it. Its exit 127 would otherwise count as an attempt that did not answer, three times over, so a missing local binary would pin the whole suite to the public factory while every message blamed the apiserver. `talos_image_cache_diagnose` asks the same question on the node-join failure path, and #3676 landed its own three-way gate there while this branch was open. The two stay separate rather than sharing this helper. That one runs under a phase budget where re-asking spends what the tenant snapshot behind it needs, so it answers on a single attempt; this one runs on the happy path, where a re-ask costs a suite nothing and buys back the mirror. This branch leaves that gate as it was merged. Unit tests put a throwaway `kubectl` and `timeout` on PATH and drive all three outcomes, the re-ask, the caching rule, the `timeout`-absent path and the killed-attempt path where nothing but the lookup itself can explain the failure: `hack/cozytest.sh hack/talos-image-cache_test.bats`. The stubs are files rather than shell functions because an external `timeout` binary execs a real file and would never see a shell override. The re-ask is a retry loop, which `docs/agents/e2e-testing.md` restricts. It falls under the exemption there rather than the prohibition: what is retried is a network read with no product or test logic under it, and the deterministic steps the rule protects are untouched. This relates to #3668 and closes the first door only. The decision is cached once at the end of the function regardless of which inner check produced the negative answer, and inside the "mirror is present" branch the `rollout status` wait and the three reads in `_talos_image_cache_reachable_from_tenant` still conflate "the mirror does not work" with "the probe could not be run", with the same suite-wide effect one layer down. The issue's own status comment says it stays open after this change, so there is no closing keyword here. Three weaknesses this PR does not remove, named because each is a real thing a later reader will hit. The `absent` case in `resolve_talos_image_factory_url` is reached by `else` fall-through rather than by matching the token, so a fourth state added to the helper later would land in the branch that turns the suite toward the public factory rather than in the safe one; a test pins the `present` token against exactly that drift, but the shape stays fall-through. The lookup's own `30` and `5` are literals while the diagnostic reads next door run off a validated, overridable pair, and that is deliberate rather than an oversight: those knobs are named and documented for the dump budget, so wiring the lookup to them would let a test that turns the dumps down silently change which image factory a whole suite pulls from. And `_TALOS_IMAGE_CACHE_QUERY_TRIES` is the one knob in the file without that validation. A non-numeric value spins the loop forever rather than failing: the comparison sits in an `if` condition, so `set -e` is exempt, the branch is simply never taken, and the job burns to its ceiling with no output. Nothing outside the tests sets it, which is why it is here and not in the diff. Worth recording for whoever does fix it: the obvious remedy, reusing this file's own `_talos_image_cache_seconds`, does not work where the knob is assigned, because the validator is defined a hundred lines further down and the assignment runs at source time. The call resolves to nothing, the substitution yields an empty string, and an empty bound spins the same loop. A real fix has to move the definition up or validate inside the function at call time. ### Screenshots Not a UI change. ### Downstream repositories Walked the trigger map against the diff. It touches `hack/e2e-chainsaw/_lib/talos-image-cache.sh` and `hack/talos-image-cache_test.bats` and nothing else: no file under `hack/` is moved or renamed, no make target changes behaviour, and no package, chart, CRD or values key is involved. The `cozystack/ccp` entry is the closest one and it does not fire. - [x] No downstream repository is affected by this change ### Release note ```release-note fix(e2e): a single failed apiserver lookup no longer sends a whole e2e run to the public Talos Image Factory ```
Cozystack
Cozystack is a free platform and framework for building clouds.
Cozystack is a CNCF Sandbox Level Project that was originally built and sponsored by Ænix.
With Cozystack, you can transform a bunch of servers into an intelligent system with a simple REST API for spawning Kubernetes clusters, Database-as-a-Service, virtual machines, load balancers, HTTP caching services, and other services with ease.
Use Cozystack to build your own cloud or provide a cost-effective development environment.
Use-Cases
-
Using Cozystack to build a public cloud
You can use Cozystack as a backend for a public cloud -
Using Cozystack to build a private cloud
You can use Cozystack as a platform to build a private cloud powered by Infrastructure-as-Code approach -
Using Cozystack as a Kubernetes distribution
You can use Cozystack as a Kubernetes distribution for Bare Metal
Documentation
The documentation is located on the cozystack.io website.
Read the Getting Started section for a quick start.
If you encounter any difficulties, start with the troubleshooting guide and work your way through the process that we've outlined.
Versioning
Versioning adheres to the Semantic Versioning principles.
A full list of the available releases is available in the GitHub repository's Release section.
Contributions
Contributions are highly appreciated and very welcomed!
In case of bugs, please check if the issue has already been opened by checking the GitHub Issues section. If it isn't, you can open a new one. A detailed report will help us replicate it, assess it, and work on a fix.
You can express your intention to on the fix on your own. Commits are used to generate the changelog, and their author will be referenced in it.
If you have a concrete feature request, open an issue. For a cross-cutting or architectural change, open a design proposal in the community repo. For questions or early ideas, use GitHub Discussions. See CONTRIBUTING.md for the full routing.
Community
You are welcome to join our Telegram group and come to our weekly community meetings. Add them to your Google Calendar or iCal for convenience.
License
Cozystack is licensed under Apache 2.0.
The code is provided as-is with no warranties.
Commercial Support
A list of companies providing commercial support for this project can be found on official site.