fix(ingress-nginx): re-pin to the segfault-free v1.11.5 rebuild - #4012
Conversation
The v1.11.5 tag in cozystack/ingress-nginx-with-protobuf-exporter now points at the merge of its #7, which builds lua-protobuf with -fno-strict-aliasing. The digest pinned here was the build before that, whose pb.so makes nginx's cache manager and cache loader children segfault continuously: tens per second, indefinitely. The workers are unaffected, so the pod stays Ready with no restarts and nothing surfaces until the master itself segfaults, which is how this reached main unnoticed. Verified against the published image rather than a local build. The one-line repro, luajit -e 'require "protoc"', exits 0 where it exited 139, and a 25 second nginx run on a generated ingress-nginx config produces no signal 11 where the pinned build produced 105. The exporter is re-pinned alongside it because one tag builds both images, so the rebuild published a new exporter digest too. Its source did not change. release-1.5 and release-1.6 pin v1.11.2 and were never affected. Signed-off-by: Myasnikov Daniil <[email protected]>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan provides up to 8 included reviews per hour; 5 remain after this review. 📝 WalkthroughWalkthroughThe ingress-nginx values update the pinned image digests for the controller and protobuf-exporter containers. Existing image tags and configuration remain unchanged. ChangesIngress image updates
Estimated code review effort: 1 (Trivial) | ~3 minutes Merge Risk: ⚪ Minimal · up to The PR re-pins ingress-nginx and its exporter to rebuilt images intended to eliminate the controller child-process crashes. No actionable merge-blocking risk remains beyond normal checks and review. Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Full details: Docstring CoverageExplanation No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0 files. (1 skipped: 1 unsupported.) ✨ Finishing Touches🧪 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 |
|
Created backport PR for
Please cherry-pick the changes locally and resolve any conflicts. git fetch origin backport-4012-to-release-1.6
git worktree add --checkout .worktree/backport-4012-to-release-1.6 backport-4012-to-release-1.6
cd .worktree/backport-4012-to-release-1.6
git reset --hard HEAD^
git cherry-pick -x 127cc2ce9ab1d42536c6c2977fb68b19da28cc2c
git push --force-with-lease |
|
Not needed on release-1.6, so closing #4017 and dropping backport label from here. release-1.6 pins controller v1.11.2 ( Other reason is this is 4th commit of a chain of 4 ( |
Re-pins the ingress controller to the rebuilt `v1.11.5`. The digest pinned before this segfaults. `pb.c` in lua-protobuf violates strict aliasing and gcc 14 at `-O2` acts on it, so `pb.so` in the previously pinned build makes nginx `cache manager` and `cache loader` children segfault continuously, tens per second and it never stops. Workers are untouched, so the pod stays `Ready` with `restarts=0` and nothing surfaces until master itself dies, which is how it got here unnoticed. Fixed in cozystack/ingress-nginx-with-protobuf-exporter#7, tag `v1.11.5` re-pointed at that merge and both images rebuilt. Verified against the published image, not a local build: * `luajit -e 'require "protoc"'` exits 0, was 139. * 25 second nginx run on a real generated config: 0 `signal 11`, was 105. 0 emerg, all four lua modules load. * `luarocks` and `gcc` are gone from the runtime image now, so `apk` no longer upgrades musl underneath the nginx the base image already built. The exporter is re-pinned too, because one tag builds both images and the rebuild published a new exporter digest. Its source did not change. `release-1.5` and `release-1.6` pin v1.11.2 and were never affected, so nothing to backport. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Chores** * Updated container image versions for the ingress controller and protobuf exporter to newer verified builds. * Existing image tags and configuration remain unchanged. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
Replaces QEMU substrate with Talos containers on both lanes that gate a merge. srv1-srv3 run as containers, so tenant worker sits at L2 instead of L3. Measured in CI on commits where both substrates ran the same tree: | | qemu | container | |---|---|---| | `kubernetes-latest` / `-previous` | 1143-2040s, red on every vgif-absent runner | 466-809s, green in both strata | | whole job | | 24-49 min faster | Almost all of it is in Chainsaw. Install takes about half an hour either way. Node-join soft-red gate goes with it. It was a prosthetic for nested virt: a worker that registers in minutes elsewhere could miss any deadline this test can afford, and #3513 established the discriminator is host `kvm_amd` `vgif`, not anything about the product. Remove the nesting level and there is nothing left to tolerate, so `hack/e2e-node-join-soft-red.sh`, the marker and the `soft_red` outputs are gone from all four workflows. ### What moves off the per-PR path DRBD, and with it `replicated` StorageClass, its `Immediate` binding mode, tenant StorageClass propagation (applies only to remotely-accessible classes), and cozystack talos node image with its extensions. None of it is lost - `nightly.yaml` and `e2e-tag.yaml` still run the suite on QEMU, so all four keep nightly and release-candidate coverage. Live migration is not on that list because nothing tests it today anyway. One more belongs on the list and was missing from it: the nocloud disk build. `build-talos` no longer runs `make -C packages/core/talos talos-nocloud`, so `images/talos/profiles/nocloud.yaml` is compiled by `nightly.yaml` and by the tag build's `make assets` rather than by anything a PR runs, and `promote-rc.yaml` still hard-fails a promotion whose rc release has no `nocloud-amd64.raw.xz`. Nothing in that chain breaks, because the tag build supplies the asset as before. What a PR no longer catches is narrow: it still compiles `installer.yaml` through the same imager, and the two profiles are generated by the same script and differ only in `platform`, `kind`, `imageOptions` and `outFormat`. ### Product fixes Four, all found by measurement while building the lane, each on its own commit: The linstor `drbd.enabled` gate. Without it drbd-logger sidecar exits, satellite is never Ready, piraeus registers zero nodes and every PVC hangs Pending behind a cluster that looks healthy. linstor `--strict-topology`. Without it external-provisioner passes every topology segment as `requisite`, so a node-pinned volume can be provisioned away from its pod and the pod is then unschedulable forever with nothing erroring anywhere. kubevirt-cdi importer resources are configurable now. Default restates CDI's own values exactly so production behaviour does not move, and the lane raises the ceiling through its own Package. vm-disk binds standalone disks immediately. A VMDisk on `local` silently never populated, while the same disk on `replicated` did. ### Where the run stands The lane has gone green twice, most recently on `cc70cef7b` where `E2E (in-tree)` took 2h3m against its 215m cap with 45 Chainsaw tests passing. The measurements quoted elsewhere in this description come from the first of those runs, on a commit that has since been rebased away: the two tenant-Kubernetes suites came in at 569.72s (`kubernetes-latest`) and 470.19s (`kubernetes-previous`) against a 67m operation. Both runs answer the four earlier reds, all of which were the lua-protobuf segfault from #4012 once `vminstance`/`vmdisk` were fixed on the branch. ghcr.io mirror removal is carried here too and is the same change as #4007, whichever lands first makes the other a no-op. ### What review changed `drbd.enabled=false` now refuses to render together with `talos.enabled=false`. It removed the logger sidecar only, while the two DRBD initContainers piraeus contributes unconditionally are deleted by the Talos configuration alone, so that combination produced a satellite that can never reach Ready. `isp-full-generic` sets `talos.enabled=false`, so it was reachable and not theoretical. vm-disk keeps unconditional immediate binding, a standalone disk has no consumer to wait for. What it costs on a node-pinned class is in the chart README now, and the annotation is pinned per source type and on the existing-DataVolume path where `helm upgrade` retrofits it. The rest is the harness holding its own promises: HelmRelease gate reads are bounded and its minimum-count half is pinned by a test that fails without it, two Chainsaw ops sit above the deadlines they contain, the fallback-StorageClass render restores `replicated` when it fails, container lane compose project and pool files are per-checkout, and three comments that said the opposite of the code now say what is true (QEMU is still wired into nightly and e2e-tag, the lane replaces two Packages and not one, the argument-count assertion cannot see upstream drift). ### Release note ```release-note fix(linstor): CSI provisioning is constrained to the node the scheduler chose (`--strict-topology`), and the DRBD sidecar can be switched off with `drbd.enabled` on substrates whose kernel cannot run DRBD. On an existing cluster the new default re-templates `spec.csiController.podTemplate`, so piraeus rolls `linstor-csi-controller` once during the upgrade. `drbd.enabled=false` requires `talos.enabled=true` and the chart now refuses the other combination at render time, because only the Talos satellite configuration removes the DRBD initContainers. fix(kubevirt-cdi): CDI worker pod resources are configurable through `importerResources`, which reaches the importer, the uploader and the host-assisted cloner. The default restates CDI's own built-in values exactly, so an install that sets nothing behaves as before. fix(vm-disk): every disk requests immediate binding, not only `upload` sources, so a standalone disk on a WaitForFirstConsumer StorageClass is populated instead of waiting for a consumer that may never arrive. On a node-pinned class such as `local` the volume then binds where CDI's worker pod was scheduled rather than where the VM will run; see the chart README before choosing such a class for VM disks. The default `replicated` is unaffected. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added container-based end-to-end testing with local storage support. * Added optional backup-access preflight checks for database backup examples. * Added configurable CDI worker resources and LINSTOR storage settings. * Added configurable immediate storage binding for VM disks. * Added stronger readiness checks, diagnostics, and parallel test execution. * **Bug Fixes** * Improved cleanup failure reporting, capacity validation, and missing-image detection. * Improved FoundationDB health verification and storage placement behavior. * **Removed** * Removed GHCR mirror support and soft-red node-join tolerance. * Removed standalone OIDC render test suites. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
Re-pins the ingress controller to the rebuilt
v1.11.5. The digest pinned before this segfaults.pb.cin lua-protobuf violates strict aliasing and gcc 14 at-O2acts on it, sopb.soin the previously pinned build makes nginxcache managerandcache loaderchildren segfault continuously, tens per second and it never stops. Workers are untouched, so the pod staysReadywithrestarts=0and nothing surfaces until master itself dies, which is how it got here unnoticed. Fixed in cozystack/ingress-nginx-with-protobuf-exporter#7, tagv1.11.5re-pointed at that merge and both images rebuilt.Verified against the published image, not a local build:
luajit -e 'require "protoc"'exits 0, was 139.signal 11, was 105. 0 emerg, all four lua modules load.luarocksandgccare gone from the runtime image now, soapkno longer upgrades musl underneath the nginx the base image already built.The exporter is re-pinned too, because one tag builds both images and the rebuild published a new exporter digest. Its source did not change.
release-1.5andrelease-1.6pin v1.11.2 and were never affected, so nothing to backport.Summary by CodeRabbit