andygrove opened a new issue, #2302: URL: https://github.com/apache/datafusion-ballista/issues/2302
Follow-ups from #2244, which added the kind backend (`K8sCluster`) and the baseline scenario. None of these block the baseline, but the first one shapes how the #2029 scenarios get written, so it is worth doing before that work starts. ### 1. Supervise the scheduler port-forward `spawn_port_forward` in `chaos-testing/src/k8s.rs` launches `kubectl port-forward svc/ballista-scheduler` once, sends both streams to `/dev/null`, and never checks on it again. That single process carries everything the harness does from outside the cluster: the `df://` client connection, the `/api/executors` poll in `registered_executors`, and the query results coming back through the scheduler's flight proxy. Two problems come out of that: - Even though it targets a Service, `kubectl` resolves that Service to one pod when the forward starts and does not re-resolve. Any scenario that restarts or reschedules the scheduler pod leaves a permanently dead forward, and the harness has no way to notice. Executor kills are fine, so the currently planned scenarios may not trip this, but a scheduler-side HA scenario will. - Long-lived forwards also drop on their own from apiserver hiccups or idle timeouts, which is a general flake source once the suite runs longer than one baseline query. Worth adding a supervisor that notices the child has exited and re-establishes the forward, or switching to a NodePort so there is no relay process at all. Either way, capturing the forward's stderr instead of discarding it would help, since today a dead forward surfaces as an opaque connection refused with nothing to go on. Folding it into `dump_diagnostics` would cover that. ### 2. Guard the fixture directory before clearing it `K8sCluster::start` calls `clear_dir_contents` on whatever `CHAOS_FIXTURE_DIR` points at, with no check on the shape of the path. The default (`$HOME/.ballista-chaos-fixtures`) is safe and the clearing itself is the right call, since a stale fixture from an older build leaking into a run is a genuinely confusing failure. But an operator typo or an inherited env var from another tool turns this into a recursive delete of something that matters. A cheap check that the directory is empty or contains only files the harness recognises would close it. ### 3. Optional: cache the in-Docker build in CI The job runs green in about 12 to 14 minutes, so this is a nice-to-have rather than a problem. The chaos image compiles the binaries inside the build on purpose, so the image always matches the container architecture and the harness works on Apple Silicon. Keeping that is the right call and this issue is not proposing changing it. The consequence is that CI cannot reuse the host `target/` dir that `Swatinem/rust-cache` keeps warm, and the `--mount=type=cache` mounts in the Dockerfile live in the local BuildKit builder, which is empty on every fresh runner. Anyone picking this up should know the obvious fix does not work. Adding `cache-to: type=gha` caches layers, not BuildKit cache mounts, so the `RUN cargo build` step would still start cold whenever the source changes. Making it actually pay off means either a cargo-chef style split so the dependency build lands in its own rarely-changing layer, or the buildkit-cache-dance action, which is third party and would have to clear the ASF allowlist. Given the allowlist trouble this workflow already hit, and that 14 minutes is in line with the rest of the matrix, closing this as not worth it is a reasonable outcome. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
