From 3903e2ebf905a1907b58f2b52cd928359978fc2b Mon Sep 17 00:00:00 2001 From: Benny Zlotnik Date: Sun, 9 Aug 2026 16:46:10 +0300 Subject: [PATCH 01/24] fix(e2e): pin nip.io baseDomain in /etc/hosts Host-side clients (jmp, curl) resolve .jumpstarter..nip.io against a public resolver on every invocation. A slow or rate-limited lookup burns the client's entire connect budget and surfaces as "Timeout connecting to grpc....:8082", which reads like a server fault. The IP is already embedded in the nip.io name, so serve the same answer locally. Mirrors the existing dex /etc/hosts handling. Non-nip.io base domains are left alone. --- e2e/setup-e2e.sh | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/e2e/setup-e2e.sh b/e2e/setup-e2e.sh index 702cefaef..272edaaae 100755 --- a/e2e/setup-e2e.sh +++ b/e2e/setup-e2e.sh @@ -275,6 +275,35 @@ deploy_controller() { # shellcheck source=lib/install.sh source "$SCRIPT_DIR/lib/install.sh" +# Pin the ingress hostnames in /etc/hosts so that host-side clients (jmp, curl) +# never depend on public DNS. +# +# baseDomain is a nip.io wildcard name of the form jumpstarter..nip.io, so +# every jmp invocation would otherwise resolve .jumpstarter..nip.io +# against a public resolver. A slow or rate-limited lookup burns the client's +# whole connect budget and surfaces as "Timeout connecting to grpc....:8082". +# The IP is already embedded in the name, so we can serve the same answer +# locally. Non-nip.io base domains are left alone. +pin_basedomain_hosts_entries() { + local basedomain="$1" + local ip + + ip=$(echo "${basedomain}" | sed -nE 's/^.*\.([0-9]+\.[0-9]+\.[0-9]+\.[0-9]+)\.nip\.io$/\1/p') + if [ -z "${ip}" ]; then + log_info "baseDomain ${basedomain} is not a nip.io name, leaving DNS resolution alone" + return 0 + fi + + if grep -q "grpc.${basedomain}" /etc/hosts 2>/dev/null; then + log_info "✓ ${basedomain} entries already in /etc/hosts" + return 0 + fi + + log_warn "About to add ${basedomain} entries to /etc/hosts (requires sudo)" + echo "${ip} ${basedomain} grpc.${basedomain} router.${basedomain} login.${basedomain}" | sudo tee -a /etc/hosts + log_info "✓ Pinned ${basedomain} to ${ip} in /etc/hosts" +} + # Step 6: Setup test environment setup_test_environment() { log_info "Setting up test environment..." @@ -290,6 +319,7 @@ setup_test_environment() { log_error "Failed to get baseDomain from Jumpstarter CR in namespace ${JS_NAMESPACE}" exit 1 fi + pin_basedomain_hosts_entries "${BASEDOMAIN}" export ENDPOINT="grpc.${BASEDOMAIN}:8082" export LOGIN_ENDPOINT="login.${BASEDOMAIN}:8086" log_info "Controller endpoint: $ENDPOINT" From 2fda3f7393c95351a40eee6e73b8d8bc9ccf07fd Mon Sep 17 00:00:00 2001 From: Benny Zlotnik Date: Sun, 9 Aug 2026 16:46:10 +0300 Subject: [PATCH 02/24] fix(e2e): don't treat kubectl stderr as query output RunCmd folds stderr into the returned string. For a value polled inside Eventually that is wrong: `-o jsonpath={.items[0].metadata.name}` against an empty list exits non-zero and prints "array index out of bounds", so Eventually(...).ShouldNot(BeEmpty()) accepts the error text as a result and stops waiting on the first attempt. A five-minute wait satisfied itself in 122ms, and the captured error string was then passed to `kubectl wait exporters.jumpstarter.dev/`, turning a clean RBAC denial into an unreadable failure. Add KubectlQuery, which returns "" when kubectl fails, and use it for the polled queries. Guard WaitForExporter against being handed a non-name. --- e2e/test/exporterset_qemu_test.go | 21 ++++++++------------- e2e/test/utils.go | 28 ++++++++++++++++++++++++++-- 2 files changed, 34 insertions(+), 15 deletions(-) diff --git a/e2e/test/exporterset_qemu_test.go b/e2e/test/exporterset_qemu_test.go index eaecb1a46..93118acbf 100644 --- a/e2e/test/exporterset_qemu_test.go +++ b/e2e/test/exporterset_qemu_test.go @@ -117,11 +117,10 @@ var _ = Describe("ExporterSet QEMU E2E Tests", Label("exporterset-qemu"), Ordere By("waiting for ExporterSet to create an exporter") var exporterName string Eventually(func() string { - out, _ := Kubectl("-n", ns, "get", "exporter", + exporterName = KubectlQuery("-n", ns, "get", "exporter", "-l", guest.Selector, "-o", "jsonpath={.items[0].metadata.name}") - exporterName = out - return out + return exporterName }, 5*time.Minute, 5*time.Second).ShouldNot(BeEmpty()) By(fmt.Sprintf("waiting for exporter %s Online/Registered/Available", exporterName)) @@ -129,15 +128,13 @@ var _ = Describe("ExporterSet QEMU E2E Tests", Label("exporterset-qemu"), Ordere By("waiting for Pod Ready") Eventually(func() string { - out, _ := Kubectl("-n", ns, "get", "pod", exporterName, + return KubectlQuery("-n", ns, "get", "pod", exporterName, "-o", "jsonpath={.status.phase}") - return out }, 5*time.Minute, 5*time.Second).Should(Equal("Running")) Eventually(func() string { - out, _ := Kubectl("-n", ns, "get", "pod", exporterName, + return KubectlQuery("-n", ns, "get", "pod", exporterName, "-o", "jsonpath={.status.containerStatuses[*].ready}") - return out }, 5*time.Minute, 5*time.Second).Should(ContainSubstring("true")) By(fmt.Sprintf("verifying runtime image provides %s", guest.QemuBinary)) @@ -151,14 +148,13 @@ var _ = Describe("ExporterSet QEMU E2E Tests", Label("exporterset-qemu"), Ordere It("leases, flashes Alpine, and boots to a console login marker", func() { By("waiting for a Running pod so we can read shared volume SizeLimit") Eventually(func() string { - out, _ := Kubectl("-n", ns, "get", "pod", + return KubectlQuery("-n", ns, "get", "pod", "-l", guest.Selector, "--field-selector=status.phase=Running", "-o", "jsonpath={.items[0].metadata.name}") - return out }, 2*time.Minute, 5*time.Second).ShouldNot(BeEmpty()) - sizeLimit, _ := Kubectl("-n", ns, "get", "pod", + sizeLimit := KubectlQuery("-n", ns, "get", "pod", "-l", guest.Selector, "--field-selector=status.phase=Running", "-o", "jsonpath={.items[0].spec.volumes[?(@.name==\"shared\")].emptyDir.sizeLimit}") @@ -262,11 +258,10 @@ j qemu power off By("waiting for the replacement exporter to become Available") var exporterName string Eventually(func() string { - out, _ := Kubectl("-n", ns, "get", "exporter", + exporterName = KubectlQuery("-n", ns, "get", "exporter", "-l", guest.Selector, "-o", "jsonpath={.items[0].metadata.name}") - exporterName = out - return out + return exporterName }, 2*time.Minute, 5*time.Second).ShouldNot(BeEmpty()) WaitForExporter(exporterName) diff --git a/e2e/test/utils.go b/e2e/test/utils.go index 4beefae5b..74d4f5462 100644 --- a/e2e/test/utils.go +++ b/e2e/test/utils.go @@ -24,6 +24,7 @@ import ( "os" "os/exec" "path/filepath" + "regexp" "strconv" "strings" "sync" @@ -335,6 +336,23 @@ func MustKubectl(args ...string) string { return out } +// KubectlQuery runs a kubectl query and returns its stdout, or "" if kubectl +// failed. Use it for values polled inside Eventually. +// +// Kubectl folds stderr into the returned string, which is wrong for a polled +// query: `-o jsonpath={.items[0].metadata.name}` against an empty list exits +// non-zero and prints "array index out of bounds", so a poll written as +// Eventually(...).ShouldNot(BeEmpty()) accepts that error text as a result and +// stops waiting on the very first attempt. Returning "" keeps the poll running +// until the resource actually appears. +func KubectlQuery(args ...string) string { + stdout, _, err := RunCmdSplit("kubectl", args...) + if err != nil { + return "" + } + return stdout +} + // ReadYAMLField reads a top-level field from a YAML file and returns its // string value. For scalar values the string representation is returned; // for nested structures the re-marshalled YAML is returned. @@ -689,9 +707,16 @@ func (pt *ProcessTracker) IsProcessRunning() bool { // --- Exporter wait helpers --- +// validExporterName matches a Kubernetes resource name, so that a caller +// passing a captured kubectl error string produces a clear failure here rather +// than an unreadable `kubectl wait exporters.jumpstarter.dev/error: ...`. +var validExporterName = regexp.MustCompile(`^[a-z0-9]([-.a-z0-9]*[a-z0-9])?$`) + // WaitForExporter waits for an exporter to become Online, Registered, and Available. func WaitForExporter(name string) { ns := Namespace() + ExpectWithOffset(1, validExporterName.MatchString(name)).To(BeTrue(), + "WaitForExporter called with an invalid exporter name %q", name) exporterRef := fmt.Sprintf("exporters.jumpstarter.dev/%s", name) // Brief delay to avoid catching pre-disconnect state @@ -703,9 +728,8 @@ func WaitForExporter(name string) { // Poll until exporterStatus is Available Eventually(func() string { - out, _ := Kubectl("-n", ns, "get", exporterRef, + return KubectlQuery("-n", ns, "get", exporterRef, "-o", "jsonpath={.status.exporterStatus}") - return out }, defaultWaitTimeout, exporterPollPeriod).Should(Equal("Available"), "timed out waiting for %s to reach Available status", name) } From 74a44ff41dfecde66190127196228b60010a4b46 Mon Sep 17 00:00:00 2001 From: Benny Zlotnik Date: Sun, 9 Aug 2026 16:46:26 +0300 Subject: [PATCH 03/24] ci(e2e): retry a failed spec once before failing the job The e2e suite talks to a real cluster over the network, so a spec can fail for reasons unrelated to the code under test. Honour E2E_FLAKE_ATTEMPTS (default 1 locally, 2 in CI) so a single infrastructure hiccup does not fail an unrelated PR. Retried specs are still reported as flaky in the ginkgo summary, so genuine instability stays visible. --- .github/workflows/e2e.yaml | 4 ++++ e2e/lib/common.sh | 9 ++++++++- 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/.github/workflows/e2e.yaml b/.github/workflows/e2e.yaml index f7e2ccf46..8a9aebd3c 100644 --- a/.github/workflows/e2e.yaml +++ b/.github/workflows/e2e.yaml @@ -10,6 +10,10 @@ permissions: env: CONTAINER_TOOL: docker + # Retry a failed spec once before failing the job. Retries are reported as + # flakes in the ginkgo summary, so this hides nothing; it only stops a single + # infrastructure hiccup from failing an unrelated PR. + E2E_FLAKE_ATTEMPTS: "2" jobs: changes: diff --git a/e2e/lib/common.sh b/e2e/lib/common.sh index 9ee3b6a42..e6f50eeab 100644 --- a/e2e/lib/common.sh +++ b/e2e/lib/common.sh @@ -79,7 +79,14 @@ run_ginkgo() { timeout="60m" fi - local flags=(-v --show-node-events --trace --timeout "${timeout}") + # Retry a failed spec instead of failing the whole suite. The e2e suite talks + # to a real cluster over the network, so a spec can fail for reasons that have + # nothing to do with the code under test (a slow DNS answer, a pod scheduled + # late, a router connection dropped). A retried spec is still reported as + # flaky in the summary, so genuine instability stays visible. + local flake_attempts="${E2E_FLAKE_ATTEMPTS:-1}" + + local flags=(-v --show-node-events --trace --timeout "${timeout}" --flake-attempts "${flake_attempts}") if [ -n "$label_filter" ]; then flags+=(--label-filter "$label_filter") fi From ad447fc080be2d0262b59ea9e405575ae7f64811 Mon Sep 17 00:00:00 2001 From: Benny Zlotnik Date: Sun, 9 Aug 2026 16:46:26 +0300 Subject: [PATCH 04/24] test(e2e): continue after a failure in independent suites A failing spec in an Ordered container skips every spec after it, so a run reports one problem instead of all of them. Add ContinueOnFailure to the six suites whose specs re-establish their own state in AfterEach. The compat suites and exporterset-qemu are genuinely sequential (a "creates resources" spec feeds later ones), so they keep the current behaviour. --- e2e/test/auth_logging_test.go | 2 +- e2e/test/direct_listener_test.go | 2 +- e2e/test/dut_network_test.go | 2 +- e2e/test/e2e_test.go | 2 +- e2e/test/exit_on_lease_end_test.go | 2 +- e2e/test/hooks_test.go | 2 +- 6 files changed, 6 insertions(+), 6 deletions(-) diff --git a/e2e/test/auth_logging_test.go b/e2e/test/auth_logging_test.go index 26c96c794..1cd8969e6 100644 --- a/e2e/test/auth_logging_test.go +++ b/e2e/test/auth_logging_test.go @@ -35,7 +35,7 @@ import ( // with legacy (controller-issued) tokens so the token lives in a local config // file where the test can corrupt it. It is intentionally NOT labelled for the // compat suites — old controller images do not have auth-failure logging. -var _ = Describe("Auth Failure Logging E2E Tests", Label("auth-logging"), Ordered, func() { +var _ = Describe("Auth Failure Logging E2E Tests", Label("auth-logging"), Ordered, ContinueOnFailure, func() { const ( clientName = "test-client-authlog" exporterName = "test-exporter-authlog" diff --git a/e2e/test/direct_listener_test.go b/e2e/test/direct_listener_test.go index 0b301c7a8..7f082e485 100644 --- a/e2e/test/direct_listener_test.go +++ b/e2e/test/direct_listener_test.go @@ -26,7 +26,7 @@ import ( . "github.com/onsi/gomega" //nolint:revive ) -var _ = Describe("Direct Listener E2E Tests", Label("direct-listener"), Ordered, func() { +var _ = Describe("Direct Listener E2E Tests", Label("direct-listener"), Ordered, ContinueOnFailure, func() { var ( tracker *ProcessTracker listenerPort = 19090 diff --git a/e2e/test/dut_network_test.go b/e2e/test/dut_network_test.go index 1b3fe5cda..e7f9a5c3b 100644 --- a/e2e/test/dut_network_test.go +++ b/e2e/test/dut_network_test.go @@ -51,7 +51,7 @@ func sudoArgs(args ...string) (string, []string) { return args[0], args[1:] } -var _ = Describe("DUT Network E2E Tests", Label("dut-network"), Ordered, func() { +var _ = Describe("DUT Network E2E Tests", Label("dut-network"), Ordered, ContinueOnFailure, func() { var ( tracker *ProcessTracker listenerPort = 19091 diff --git a/e2e/test/e2e_test.go b/e2e/test/e2e_test.go index ddb4a8f38..b16bc393e 100644 --- a/e2e/test/e2e_test.go +++ b/e2e/test/e2e_test.go @@ -26,7 +26,7 @@ import ( . "github.com/onsi/gomega" //nolint:revive ) -var _ = Describe("Core E2E Tests", Label("core"), Ordered, func() { +var _ = Describe("Core E2E Tests", Label("core"), Ordered, ContinueOnFailure, func() { var tracker *ProcessTracker BeforeAll(func() { diff --git a/e2e/test/exit_on_lease_end_test.go b/e2e/test/exit_on_lease_end_test.go index 9a0aa914f..219e703dd 100644 --- a/e2e/test/exit_on_lease_end_test.go +++ b/e2e/test/exit_on_lease_end_test.go @@ -24,7 +24,7 @@ import ( . "github.com/onsi/gomega" //nolint:revive ) -var _ = Describe("Exit On Lease End E2E Tests", Label("exit-on-lease-end"), Ordered, func() { +var _ = Describe("Exit On Lease End E2E Tests", Label("exit-on-lease-end"), Ordered, ContinueOnFailure, func() { var ( tracker *ProcessTracker exporterConfigPath string diff --git a/e2e/test/hooks_test.go b/e2e/test/hooks_test.go index 66340d831..c1cf51f32 100644 --- a/e2e/test/hooks_test.go +++ b/e2e/test/hooks_test.go @@ -26,7 +26,7 @@ import ( . "github.com/onsi/gomega" //nolint:revive ) -var _ = Describe("Hooks E2E Tests", Label("hooks"), Ordered, func() { +var _ = Describe("Hooks E2E Tests", Label("hooks"), Ordered, ContinueOnFailure, func() { var ( tracker *ProcessTracker exporterConfigPath string From 82778eefb1f46f08207d7f2c5af0d371b228ae16 Mon Sep 17 00:00:00 2001 From: Benny Zlotnik Date: Sun, 9 Aug 2026 16:46:26 +0300 Subject: [PATCH 05/24] fix(grpc): separate DNS resolution timeout from connect timeout MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit fail_after(timeout) wrapped both resolution and connection, so a slow resolver consumed the whole budget and reported "Timeout connecting to :" with an empty cause list — blaming the server for a name that was never resolved. Give resolution its own budget and its own message, and name the resolved IPs in the connect error. --- .../jumpstarter/jumpstarter/common/grpc.py | 44 +++++++++++-------- 1 file changed, 26 insertions(+), 18 deletions(-) diff --git a/python/packages/jumpstarter/jumpstarter/common/grpc.py b/python/packages/jumpstarter/jumpstarter/common/grpc.py index cf2f6690c..19290892a 100644 --- a/python/packages/jumpstarter/jumpstarter/common/grpc.py +++ b/python/packages/jumpstarter/jumpstarter/common/grpc.py @@ -57,24 +57,31 @@ async def _ssl_channel_credentials_insecure(target: str, timeout: float) -> grpc except ValueError as e: raise ConfigurationError(f"Failed parsing {target}") from e + ssl_context = ssl.create_default_context() + ssl_context.check_hostname = False + ssl_context.verify_mode = ssl.CERT_NONE + + # Resolve all IP addresses for the hostname. + # + # Resolution gets its own budget, separate from the connect budget below. A + # slow or rate-limited resolver would otherwise consume the whole timeout and + # surface as "Timeout connecting to :" with no per-IP errors, + # pointing at the server when the name was never resolved in the first place. + loop = asyncio.get_running_loop() try: with fail_after(timeout): - ssl_context = ssl.create_default_context() - ssl_context.check_hostname = False - ssl_context.verify_mode = ssl.CERT_NONE - - # Resolve all IP addresses for the hostname - loop = asyncio.get_running_loop() - addr_info = await loop.getaddrinfo( - parsed.hostname, port, family=socket.AF_UNSPEC, type=socket.SOCK_STREAM - ) - - # Log resolved IPs - resolved_ips = [sockaddr[0] for _, _, _, _, sockaddr in addr_info] - logger.debug( - f"Resolved {parsed.hostname} to {len(resolved_ips)} IP(s): {', '.join(resolved_ips)}" - ) + addr_info = await loop.getaddrinfo(parsed.hostname, port, family=socket.AF_UNSPEC, type=socket.SOCK_STREAM) + except socket.gaierror as e: + raise ConnectionError(f"Failed resolving {parsed.hostname}") from e + except TimeoutError as e: + raise ConnectionError(f"Timeout resolving {parsed.hostname} after {timeout}s") from e + + # Log resolved IPs + resolved_ips = [sockaddr[0] for _, _, _, _, sockaddr in addr_info] + logger.debug(f"Resolved {parsed.hostname} to {len(resolved_ips)} IP(s): {', '.join(resolved_ips)}") + try: + with fail_after(timeout): # Try all IPs in parallel - race for first success # Wrap tasks to include IP info with results/exceptions async def try_with_ip(ip_address: str): @@ -121,10 +128,11 @@ async def try_with_ip(ip_address: str): for task in tasks: if not task.done(): task.cancel() - except socket.gaierror as e: - raise ConnectionError(f"Failed resolving {parsed.hostname}") from e except TimeoutError as e: - raise ConnectionError(f"Timeout connecting to {parsed.hostname}:{port}") from e + raise ConnectionError( + f"Timeout connecting to {parsed.hostname}:{port} after {timeout}s " + f"(resolved to {', '.join(resolved_ips)})" + ) from e async def ssl_channel_credentials(target: str, tls_config, timeout=5): From beb50b7e163975ca8f5cd21923080635723f41f5 Mon Sep 17 00:00:00 2001 From: Benny Zlotnik Date: Sun, 9 Aug 2026 16:46:26 +0300 Subject: [PATCH 06/24] test(e2e): accept every beforeLease hook failure outcome MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The client and the exporter race: the exporter ends the lease as soon as the hook fails, so which message the client prints depends on how far it got first. If the lease was torn down before the client's first RPC, the client legitimately reports the exporter as unreachable — an outcome the specs did not accept, causing spurious failures. Extract a shared beforeLeaseFailureOutput covering all the outcomes. --- e2e/test/hooks_test.go | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/e2e/test/hooks_test.go b/e2e/test/hooks_test.go index c1cf51f32..5b6389ff9 100644 --- a/e2e/test/hooks_test.go +++ b/e2e/test/hooks_test.go @@ -127,6 +127,20 @@ var _ = Describe("Hooks E2E Tests", Label("hooks"), Ordered, ContinueOnFailure, // ==================================================================== // Group B: beforeLease Failure Modes // ==================================================================== + + // beforeLeaseFailureOutput matches every client-visible outcome of a failing + // beforeLease hook. + // + // The client and the exporter race here: the exporter ends the lease the + // moment the hook fails, so which message the client prints depends on how + // far it got first. It may have seen the hook's own output, the shutdown + // notice, a dropped connection, or — if the exporter tore the lease down + // before the client's very first RPC — nothing at all, in which case the + // client reports the exporter as unreachable. All of these mean the hook + // failed and the lease ended, which is what these specs assert. + const beforeLeaseFailureOutput = `(beforeLease hook fail|Exporter shutting down|Connection to exporter lost|` + + `did not respond to initial status check|unreachable after)` + Context("Group B: beforeLease Failure Modes", func() { It("B1: beforeLease onFailure=warn allows shell to proceed", func() { startHooksExporter("exporter-hooks-before-fail-warn.yaml") @@ -146,7 +160,7 @@ var _ = Describe("Hooks E2E Tests", Label("hooks"), Ordered, ContinueOnFailure, "--retry-timeout", "0", "--selector", "example.com/board=hooks", "j", "power", "on") Expect(err).To(HaveOccurred()) - Expect(out).To(MatchRegexp(`(beforeLease hook fail|Exporter shutting down|Connection to exporter lost)`)) + Expect(out).To(MatchRegexp(beforeLeaseFailureOutput)) WaitForExporter("test-exporter-hooks") }) @@ -159,7 +173,7 @@ var _ = Describe("Hooks E2E Tests", Label("hooks"), Ordered, ContinueOnFailure, "--retry-timeout", "0", "--selector", "example.com/board=hooks", "j", "power", "on") Expect(err).To(HaveOccurred()) - Expect(out).To(MatchRegexp(`(beforeLease hook fail|Connection to exporter lost)`)) + Expect(out).To(MatchRegexp(beforeLeaseFailureOutput)) // The exporter should release the lease and return to Available WaitForExporter("test-exporter-hooks") @@ -170,7 +184,7 @@ var _ = Describe("Hooks E2E Tests", Label("hooks"), Ordered, ContinueOnFailure, "--retry-timeout", "0", "--selector", "example.com/board=hooks", "j", "power", "on") Expect(err2).To(HaveOccurred()) - Expect(out2).To(MatchRegexp(`(beforeLease hook fail|Connection to exporter lost)`)) + Expect(out2).To(MatchRegexp(beforeLeaseFailureOutput)) // Exporter should recover again WaitForExporter("test-exporter-hooks") @@ -196,7 +210,7 @@ var _ = Describe("Hooks E2E Tests", Label("hooks"), Ordered, ContinueOnFailure, "--retry-timeout", "0", "--selector", "example.com/board=hooks", "j", "power", "on") Expect(err).To(HaveOccurred()) - Expect(out).To(MatchRegexp(`(beforeLease hook fail|Exporter shutting down|Connection to exporter lost)`)) + Expect(out).To(MatchRegexp(beforeLeaseFailureOutput)) // Exporter process should have exited (allow extra time on slower runners like ARM) Eventually(func() bool { From 5343f588784354cbd516edbefd717bcdde6eba04 Mon Sep 17 00:00:00 2001 From: Benny Zlotnik Date: Sun, 9 Aug 2026 16:55:04 +0300 Subject: [PATCH 07/24] fix(shell): stop waiting when the exporter reports itself OFFLINE MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A beforeLease hook with onFailure=exit reports BEFORE_LEASE_HOOK_FAILED and immediately overwrites it with OFFLINE before tearing the session down. The client waits on [LEASE_READY, BEFORE_LEASE_HOOK_FAILED], so whether it notices is a race against two back-to-back status writes: win it and the shell fails in ~5s, lose it and the hook-failed status is already gone and nothing else it waits for will ever arrive, so it sits out the full 300s timeout. In CI this cost 300s per affected spec, nondeterministically — the same green run had B5 at 25s on arm64 and 306s on amd64, with C2 at 306s on both. Together those two specs were 41% of a 25-minute suite. Treat OFFLINE as the terminal outcome it is. The session's status starts at AVAILABLE, so OFFLINE only ever appears as a deliberate shutdown transition and cannot be observed as an initial value. The resulting error carries the exporter's own message ("Exporter shutting down ..."). This leaves the indefinite UNAVAILABLE retry from #606 untouched, so an exporter restarting under a live lease still does not kill the session. --- .../jumpstarter-cli/jumpstarter_cli/shell.py | 21 +++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/python/packages/jumpstarter-cli/jumpstarter_cli/shell.py b/python/packages/jumpstarter-cli/jumpstarter_cli/shell.py index 4de573b39..895036c85 100644 --- a/python/packages/jumpstarter-cli/jumpstarter_cli/shell.py +++ b/python/packages/jumpstarter-cli/jumpstarter_cli/shell.py @@ -338,11 +338,28 @@ async def _run_shell_with_lease_async(lease, exporter_logs, config, command, can logger.info("Waiting for beforeLease hook to complete...") # Wait for LEASE_READY or hook failure using background monitor + # OFFLINE is a terminal outcome here, not just a + # transient blip. A beforeLease hook with + # onFailure=exit reports BEFORE_LEASE_HOOK_FAILED + # and immediately overwrites it with OFFLINE before + # tearing the session down. If our poll lands after + # that overwrite, the hook-failed status is gone and + # nothing else we are waiting for will ever arrive, + # so without OFFLINE in the target list we sit here + # for the full timeout. result = await monitor.wait_for_any_of( - [ExporterStatus.LEASE_READY, ExporterStatus.BEFORE_LEASE_HOOK_FAILED], timeout=300.0 + [ + ExporterStatus.LEASE_READY, + ExporterStatus.BEFORE_LEASE_HOOK_FAILED, + ExporterStatus.OFFLINE, + ], + timeout=300.0, ) - if result == ExporterStatus.BEFORE_LEASE_HOOK_FAILED: + if result in ( + ExporterStatus.BEFORE_LEASE_HOOK_FAILED, + ExporterStatus.OFFLINE, + ): reason = monitor.status_message or "beforeLease hook failed" raise ExporterOfflineError(reason) elif result is None: From 0bcb0ec2c998cdb33212b27fb8a094161d20e209 Mon Sep 17 00:00:00 2001 From: Benny Zlotnik Date: Sun, 9 Aug 2026 17:03:10 +0300 Subject: [PATCH 08/24] test(e2e): create pagination fixtures with one kubectl apply The two pagination specs each spawned ten jmp processes to create their fixtures, and the exporter spec spawned ten more to delete them. The resources only exist so the client has something to page through, so build a multi-doc manifest and apply it in a single call, and delete the exporters by label selector. Adds MustKubectlApply for piping a manifest to `kubectl apply -f -`. --- e2e/test/e2e_test.go | 43 +++++++++++++++++++++++++++++++++---------- e2e/test/utils.go | 15 +++++++++++++++ 2 files changed, 48 insertions(+), 10 deletions(-) diff --git a/e2e/test/e2e_test.go b/e2e/test/e2e_test.go index b16bc393e..ef5856c87 100644 --- a/e2e/test/e2e_test.go +++ b/e2e/test/e2e_test.go @@ -428,10 +428,25 @@ var _ = Describe("Core E2E Tests", Label("core"), Ordered, ContinueOnFailure, fu WaitForExporters("test-exporter-oidc", "test-exporter-sa", "test-exporter-legacy") MustJmp("config", "client", "use", "test-client-oidc") + // As with the exporter pagination spec, the leases are fixtures for + // the client's pagination, so create them in a single apply. + var manifest strings.Builder for i := 1; i <= 10; i++ { - out, err := Jmp("create", "lease", "--selector", "example.com/board=oidc", "--duration", "1d") - Expect(err).NotTo(HaveOccurred(), out) + fmt.Fprintf(&manifest, `--- +apiVersion: jumpstarter.dev/v1alpha1 +kind: Lease +metadata: + name: pagination-lease-%d +spec: + clientRef: + name: test-client-oidc + duration: 24h + selector: + matchLabels: + example.com/board: oidc +`, i) } + MustKubectlApply(manifest.String()) out, err := Jmp("get", "leases", "--page-size", "5", "-o", "name") Expect(err).NotTo(HaveOccurred(), out) @@ -445,23 +460,31 @@ var _ = Describe("Core E2E Tests", Label("core"), Ordered, ContinueOnFailure, fu WaitForExporters("test-exporter-oidc", "test-exporter-sa", "test-exporter-legacy") MustJmp("config", "client", "use", "test-client-oidc") - ns := Namespace() + // The exporters are fixtures for the client's pagination, so + // create them in a single apply rather than one jmp process each. + var manifest strings.Builder for i := 1; i <= 10; i++ { name := fmt.Sprintf("pagination-exp-%d", i) - out, err := Jmp("admin", "create", "exporter", "-n", ns, name, - "--nointeractive", "-l", "pagination=true", - "--oidc-username", fmt.Sprintf("dex:%s", name)) - Expect(err).NotTo(HaveOccurred(), out) + fmt.Fprintf(&manifest, `--- +apiVersion: jumpstarter.dev/v1alpha1 +kind: Exporter +metadata: + name: %s + labels: + pagination: "true" +spec: + username: dex:%s +`, name, name) } + MustKubectlApply(manifest.String()) out, err := Jmp("get", "exporters", "--selector", "pagination=true", "--page-size", "5", "-o", "name") Expect(err).NotTo(HaveOccurred(), out) lines := strings.Split(strings.TrimSpace(out), "\n") Expect(lines).To(HaveLen(10)) - for i := 1; i <= 10; i++ { - MustJmp("admin", "delete", "exporter", "--namespace", ns, fmt.Sprintf("pagination-exp-%d", i), "--delete") - } + MustKubectl("-n", Namespace(), "delete", "exporters.jumpstarter.dev", + "-l", "pagination=true", "--wait=false") }) It("lease listing shows expires at and remaining columns", func() { diff --git a/e2e/test/utils.go b/e2e/test/utils.go index 74d4f5462..cc46199bb 100644 --- a/e2e/test/utils.go +++ b/e2e/test/utils.go @@ -336,6 +336,21 @@ func MustKubectl(args ...string) string { return out } +// MustKubectlApply pipes a manifest to `kubectl apply -f -` and fails the test +// on error. Use it to create a batch of fixture resources in one call; the jmp +// admin CLI creates them one process at a time, which is far slower than the +// test needs when the resources are only there to be listed. +func MustKubectlApply(manifest string) string { + cmd := exec.Command("kubectl", "-n", Namespace(), "apply", "-f", "-") + cmd.Stdin = strings.NewReader(manifest) + var out bytes.Buffer + cmd.Stdout = &out + cmd.Stderr = &out + err := cmd.Run() + ExpectWithOffset(1, err).NotTo(HaveOccurred(), "kubectl apply failed: %s", out.String()) + return strings.TrimSpace(out.String()) +} + // KubectlQuery runs a kubectl query and returns its stdout, or "" if kubectl // failed. Use it for values polled inside Eventually. // From 64a0630b1cf2456d78bb3199a22c3939c8453284 Mon Sep 17 00:00:00 2001 From: Benny Zlotnik Date: Sun, 9 Aug 2026 17:04:16 +0300 Subject: [PATCH 09/24] test(e2e): reuse the hooks exporter when the config is unchanged Every hooks spec stopped the exporter, slept a second, rewrote the config and waited for it to come back, even when the overlay it needed was the one already running. Track the running overlay and skip the restart when it matches; exit-mode specs clear it because they leave the exporter deliberately dead. --- e2e/test/hooks_test.go | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/e2e/test/hooks_test.go b/e2e/test/hooks_test.go index 5b6389ff9..ce9cc6711 100644 --- a/e2e/test/hooks_test.go +++ b/e2e/test/hooks_test.go @@ -30,6 +30,9 @@ var _ = Describe("Hooks E2E Tests", Label("hooks"), Ordered, ContinueOnFailure, var ( tracker *ProcessTracker exporterConfigPath string + // runningConfig is the overlay the exporter currently running in loop + // mode was started with, or "" when no reusable exporter is running. + runningConfig string ) exporterOverlay := func(configFile string) string { @@ -38,7 +41,18 @@ var _ = Describe("Hooks E2E Tests", Label("hooks"), Ordered, ContinueOnFailure, // startHooksExporter stops the previous exporter, applies the config overlay, // and starts the exporter in a restart loop. + // + // Restarting costs several seconds per spec, so an exporter already running + // in loop mode with the same overlay is reused. The specs leave no state + // behind that outlives a lease, so the only thing that has to match is the + // config. Exit-mode specs clear runningConfig because they deliberately + // leave the exporter dead. startHooksExporter := func(configFile string) { + if runningConfig == configFile { + WaitForExporter("test-exporter-hooks") + return + } + tracker.StopAll() time.Sleep(time.Second) @@ -47,12 +61,14 @@ var _ = Describe("Hooks E2E Tests", Label("hooks"), Ordered, ContinueOnFailure, tracker.StartExporterLoop("test-exporter-hooks") WaitForExporter("test-exporter-hooks") + runningConfig = configFile } // startHooksExporterSingle starts without a restart loop (for exit-mode tests). startHooksExporterSingle := func(configFile string) { tracker.StopAll() time.Sleep(time.Second) + runningConfig = "" ClearHooksConfig(exporterConfigPath) MergeExporterConfig(exporterConfigPath, exporterOverlay(configFile)) From 9c200ad359e43b939e770fd23c15eb5055711f38 Mon Sep 17 00:00:00 2001 From: Benny Zlotnik Date: Sun, 9 Aug 2026 17:06:28 +0300 Subject: [PATCH 10/24] test(e2e): allow running containers in parallel behind E2E_PROCS Everything runs in one ginkgo process today, so the suite costs the sum of its containers. Add opt-in `--procs` via E2E_PROCS and mark the containers that cannot share a runner as Serial: core and the compat suites write the shared client config, dut-network owns host networking, and exporterset-qemu saturates the CPU under TCG. Default is unchanged (one process), so this only takes effect where it is asked for. --- e2e/lib/common.sh | 9 +++++++++ e2e/test/compat_old_client_test.go | 3 ++- e2e/test/compat_old_controller_test.go | 3 ++- e2e/test/dut_network_test.go | 4 +++- e2e/test/e2e_test.go | 5 ++++- e2e/test/exporterset_qemu_test.go | 4 +++- 6 files changed, 23 insertions(+), 5 deletions(-) diff --git a/e2e/lib/common.sh b/e2e/lib/common.sh index e6f50eeab..6a15a7f70 100644 --- a/e2e/lib/common.sh +++ b/e2e/lib/common.sh @@ -86,7 +86,16 @@ run_ginkgo() { # flaky in the summary, so genuine instability stays visible. local flake_attempts="${E2E_FLAKE_ATTEMPTS:-1}" + # Run top-level containers concurrently when asked. Off by default: the + # suite shares one cluster and one runner, so more processes is not free. + # Containers that touch host-global state or the shared client config are + # marked Serial and still run one at a time, after the parallel ones. + local procs="${E2E_PROCS:-1}" + local flags=(-v --show-node-events --trace --timeout "${timeout}" --flake-attempts "${flake_attempts}") + if [ "${procs}" -gt 1 ]; then + flags+=(--procs "${procs}") + fi if [ -n "$label_filter" ]; then flags+=(--label-filter "$label_filter") fi diff --git a/e2e/test/compat_old_client_test.go b/e2e/test/compat_old_client_test.go index 5aaf902a8..f3586c18f 100644 --- a/e2e/test/compat_old_client_test.go +++ b/e2e/test/compat_old_client_test.go @@ -26,7 +26,8 @@ import ( . "github.com/onsi/gomega" //nolint:revive ) -var _ = Describe("Compat: Old Client E2E Tests", Label("compat", "old-client"), Ordered, func() { +// Serial: installs an older client into the shared environment. +var _ = Describe("Compat: Old Client E2E Tests", Label("compat", "old-client"), Ordered, Serial, func() { var ( tracker *ProcessTracker ns string diff --git a/e2e/test/compat_old_controller_test.go b/e2e/test/compat_old_controller_test.go index f3ce3bc8e..3c10271cc 100644 --- a/e2e/test/compat_old_controller_test.go +++ b/e2e/test/compat_old_controller_test.go @@ -26,7 +26,8 @@ import ( . "github.com/onsi/gomega" //nolint:revive ) -var _ = Describe("Compat: Old Controller E2E Tests", Label("compat", "old-controller"), Ordered, func() { +// Serial: replaces the running controller and switches the active client. +var _ = Describe("Compat: Old Controller E2E Tests", Label("compat", "old-controller"), Ordered, Serial, func() { var ( tracker *ProcessTracker ns string diff --git a/e2e/test/dut_network_test.go b/e2e/test/dut_network_test.go index e7f9a5c3b..4cb426da4 100644 --- a/e2e/test/dut_network_test.go +++ b/e2e/test/dut_network_test.go @@ -51,7 +51,9 @@ func sudoArgs(args ...string) (string, []string) { return args[0], args[1:] } -var _ = Describe("DUT Network E2E Tests", Label("dut-network"), Ordered, ContinueOnFailure, func() { +// Serial: builds veth pairs, bridges and nftables rules in the host network +// namespace, and drives dnsmasq. There is only one host to share. +var _ = Describe("DUT Network E2E Tests", Label("dut-network"), Ordered, ContinueOnFailure, Serial, func() { var ( tracker *ProcessTracker listenerPort = 19091 diff --git a/e2e/test/e2e_test.go b/e2e/test/e2e_test.go index ef5856c87..2c46922cb 100644 --- a/e2e/test/e2e_test.go +++ b/e2e/test/e2e_test.go @@ -26,7 +26,10 @@ import ( . "github.com/onsi/gomega" //nolint:revive ) -var _ = Describe("Core E2E Tests", Label("core"), Ordered, ContinueOnFailure, func() { +// Serial: these specs select the active client with `jmp config client use`, +// which writes the shared client config, so they cannot run alongside other +// containers that read it. +var _ = Describe("Core E2E Tests", Label("core"), Ordered, ContinueOnFailure, Serial, func() { var tracker *ProcessTracker BeforeAll(func() { diff --git a/e2e/test/exporterset_qemu_test.go b/e2e/test/exporterset_qemu_test.go index 93118acbf..14f9595f1 100644 --- a/e2e/test/exporterset_qemu_test.go +++ b/e2e/test/exporterset_qemu_test.go @@ -62,7 +62,9 @@ func loadQemuGuestArch() qemuGuestArch { } } -var _ = Describe("ExporterSet QEMU E2E Tests", Label("exporterset-qemu"), Ordered, func() { +// Serial: boots VMs under TCG emulation, which will starve every other spec +// on the runner if it shares the CPU. +var _ = Describe("ExporterSet QEMU E2E Tests", Label("exporterset-qemu"), Ordered, Serial, func() { var ( ns string manifest string From 74eaff33a935a134362fc36acfb0c32da2617d5d Mon Sep 17 00:00:00 2001 From: Benny Zlotnik Date: Sun, 9 Aug 2026 17:42:55 +0300 Subject: [PATCH 11/24] test(grpc): cover the insecure TLS resolve and connect paths diff-cover flagged the split timeout change as uncovered. Exercise the four outcomes it distinguishes: a reachable IP, an unresolvable name, a resolver that stalls past the budget, and a server that never completes the handshake. --- .../jumpstarter/common/grpc_test.py | 96 ++++++++++++++++++- 1 file changed, 94 insertions(+), 2 deletions(-) diff --git a/python/packages/jumpstarter/jumpstarter/common/grpc_test.py b/python/packages/jumpstarter/jumpstarter/common/grpc_test.py index d7625d980..d736a6edb 100644 --- a/python/packages/jumpstarter/jumpstarter/common/grpc_test.py +++ b/python/packages/jumpstarter/jumpstarter/common/grpc_test.py @@ -1,4 +1,11 @@ -from jumpstarter.common.grpc import _override_default_grpc_options +import asyncio +import socket +from unittest.mock import patch + +import pytest + +from jumpstarter.common.exceptions import ConnectionError +from jumpstarter.common.grpc import _override_default_grpc_options, _ssl_channel_credentials_insecure def test_default_options_preserve_existing_defaults(): @@ -7,8 +14,93 @@ def test_default_options_preserve_existing_defaults(): assert options["grpc.keepalive_time_ms"] == 20000 - def test_user_options_override_defaults(): user_options = {"grpc.keepalive_time_ms": 50000} options = dict(_override_default_grpc_options(user_options)) assert options["grpc.keepalive_time_ms"] == 50000 + + +def _addr_info(*ips): + return [(socket.AF_INET, socket.SOCK_STREAM, 6, "", (ip, 443)) for ip in ips] + + +class _LoopWithFakeResolver: + """Delegates to the real loop, which anyio still needs, and fakes only + getaddrinfo.""" + + def __init__(self, loop, getaddrinfo): + self._loop = loop + self.getaddrinfo = getaddrinfo + + def __getattr__(self, name): + return getattr(self._loop, name) + + +def _patch_resolver(getaddrinfo): + def fake_get_running_loop(): + return _LoopWithFakeResolver(asyncio.events.get_running_loop(), getaddrinfo) + + return patch("asyncio.get_running_loop", fake_get_running_loop) + + +class TestSslChannelCredentialsInsecure: + """Resolution and connection are timed separately, so the error a user + sees points at the step that actually stalled.""" + + @pytest.mark.asyncio + async def test_returns_credentials_from_first_reachable_ip(self): + async def getaddrinfo(*_args, **_kwargs): + return _addr_info("192.0.2.1", "192.0.2.2") + + with _patch_resolver(getaddrinfo): + with patch( + "jumpstarter.common.grpc._try_connect_and_extract_cert", + return_value=b"-----BEGIN CERTIFICATE-----\n", + ): + credentials = await _ssl_channel_credentials_insecure("example.com:443", timeout=5) + + assert credentials is not None + + @pytest.mark.asyncio + async def test_resolution_failure_names_the_host(self): + async def getaddrinfo(*_args, **_kwargs): + raise socket.gaierror("Name or service not known") + + with _patch_resolver(getaddrinfo): + with pytest.raises(ConnectionError, match="Failed resolving example.com"): + await _ssl_channel_credentials_insecure("example.com:443", timeout=5) + + @pytest.mark.asyncio + async def test_slow_resolver_is_reported_as_a_resolution_timeout(self): + async def getaddrinfo(*_args, **_kwargs): + await asyncio.sleep(10) + + with _patch_resolver(getaddrinfo): + with pytest.raises(ConnectionError, match="Timeout resolving example.com"): + await _ssl_channel_credentials_insecure("example.com:443", timeout=0.05) + + @pytest.mark.asyncio + async def test_connect_timeout_reports_the_resolved_ips(self): + async def getaddrinfo(*_args, **_kwargs): + return _addr_info("192.0.2.1") + + async def never_connects(*_args, **_kwargs): + await asyncio.sleep(10) + + with _patch_resolver(getaddrinfo): + with patch("jumpstarter.common.grpc._try_connect_and_extract_cert", never_connects): + with pytest.raises(ConnectionError, match="Timeout connecting to example.com:443"): + await _ssl_channel_credentials_insecure("example.com:443", timeout=0.05) + + @pytest.mark.asyncio + async def test_all_ips_failing_lists_the_errors(self): + async def getaddrinfo(*_args, **_kwargs): + return _addr_info("192.0.2.1", "192.0.2.2") + + async def refused(*_args, **_kwargs): + raise OSError("connection refused") + + with _patch_resolver(getaddrinfo): + with patch("jumpstarter.common.grpc._try_connect_and_extract_cert", refused): + with pytest.raises(ConnectionError, match="all IPs exhausted"): + await _ssl_channel_credentials_insecure("example.com:443", timeout=5) From b559af2041ee7ed7c7d93edc4179492fceab0dad Mon Sep 17 00:00:00 2001 From: Benny Zlotnik Date: Sun, 9 Aug 2026 17:57:41 +0300 Subject: [PATCH 12/24] fix(shell): stop waiting when the exporter goes offline after the lease Same race as the beforeLease hook, on the other end of the session: an afterLease hook with onFailure=exit reports AFTER_LEASE_HOOK_FAILED and overwrites it with OFFLINE in the next breath, so a poll landing after the overwrite never sees a status the client is waiting for. The poll loop retries UNAVAILABLE indefinitely by design, so nothing else breaks the wait either and the client sits for the full 300s. This is what kept C2 at 306s in CI after B5 dropped to 6s. --- .../jumpstarter-cli/jumpstarter_cli/shell.py | 22 +++++++++++++++-- .../jumpstarter_cli/shell_test.py | 24 +++++++++++++++++++ 2 files changed, 44 insertions(+), 2 deletions(-) diff --git a/python/packages/jumpstarter-cli/jumpstarter_cli/shell.py b/python/packages/jumpstarter-cli/jumpstarter_cli/shell.py index 895036c85..2f2fb4159 100644 --- a/python/packages/jumpstarter-cli/jumpstarter_cli/shell.py +++ b/python/packages/jumpstarter-cli/jumpstarter_cli/shell.py @@ -452,8 +452,23 @@ async def _run_shell_with_lease_async(lease, exporter_logs, config, command, can if success: # Wait for hook to complete using background monitor # This allows afterLease logs to be displayed in real-time + # OFFLINE is terminal here for the + # same reason it is when waiting on + # the beforeLease hook: an + # afterLease hook with + # onFailure=exit reports + # AFTER_LEASE_HOOK_FAILED and + # overwrites it with OFFLINE in the + # next breath, so a poll landing + # after the overwrite would wait out + # the full timeout for a status the + # exporter has already moved past. result = await monitor.wait_for_any_of( - [ExporterStatus.AVAILABLE, ExporterStatus.AFTER_LEASE_HOOK_FAILED], + [ + ExporterStatus.AVAILABLE, + ExporterStatus.AFTER_LEASE_HOOK_FAILED, + ExporterStatus.OFFLINE, + ], timeout=300.0, ) if result == ExporterStatus.AVAILABLE: @@ -465,7 +480,10 @@ async def _run_shell_with_lease_async(lease, exporter_logs, config, command, can click.style(f"Warning: {warning_text}", fg="yellow", bold=True) ) logger.info("afterLease hook completed") - elif result == ExporterStatus.AFTER_LEASE_HOOK_FAILED: + elif result in ( + ExporterStatus.AFTER_LEASE_HOOK_FAILED, + ExporterStatus.OFFLINE, + ): reason = monitor.status_message or "afterLease hook failed" raise ExporterOfflineError(reason) elif monitor.connection_lost: diff --git a/python/packages/jumpstarter-cli/jumpstarter_cli/shell_test.py b/python/packages/jumpstarter-cli/jumpstarter_cli/shell_test.py index 8edb6067f..e9964ccf9 100644 --- a/python/packages/jumpstarter-cli/jumpstarter_cli/shell_test.py +++ b/python/packages/jumpstarter-cli/jumpstarter_cli/shell_test.py @@ -909,6 +909,30 @@ async def fake_client_from_path(*_a, **_kw): assert exit_code == 42 client.end_session_async.assert_not_called() + async def test_offline_during_after_lease_hook_fails_fast(self): + """An afterLease hook with onFailure=exit reports AFTER_LEASE_HOOK_FAILED + and overwrites it with OFFLINE before shutting down, so a client that + only watches for the failure status waits out the full timeout.""" + monitor = _FakeStatusMonitor(statuses=[ExporterStatus.LEASE_READY, ExporterStatus.OFFLINE]) + client = _build_fake_client( + monitor, + get_status_return=ExporterStatus.LEASE_READY, + end_session_return=True, + ) + lease = _make_shell_lease(release=True, lease_ended=False) + cancel_scope = Mock(cancel_called=False) + + @asynccontextmanager + async def fake_client_from_path(*_a, **_kw): + yield client + + with ( + patch("jumpstarter_cli.shell.client_from_path", side_effect=fake_client_from_path), + patch("jumpstarter_cli.shell._run_shell_only", return_value=0), + ): + with pytest.raises(ExporterOfflineError): + await _run_shell_with_lease_async(lease, False, None, (), cancel_scope) + async def test_calls_end_session_when_lease_not_ended(self): monitor = _FakeStatusMonitor(statuses=[ExporterStatus.LEASE_READY, ExporterStatus.AVAILABLE]) client = _build_fake_client( From a811ac63062cf70ef9e06d941649042631c51d3d Mon Sep 17 00:00:00 2001 From: Benny Zlotnik Date: Sun, 9 Aug 2026 19:13:07 +0300 Subject: [PATCH 13/24] test(e2e): stop exporters gracefully and wait on real conditions `jmp run` forks and the child calls setsid(), so the PID the tracker holds is the parent. StopAll sent it SIGKILL, which left the exporter itself orphaned with its controller registration intact: the controller then had to time out the heartbeat before the exporter counted as gone, and the pkill fallback reaped the orphan without it ever unregistering. That is the 73s WaitForExporterOffline in the exit_on_lease_end AfterEach. SIGTERM instead, with SIGKILL only as a fallback, so the parent forwards to the child's process group and the child reports OFFLINE and unregisters. The grace period is an upper bound, not a fixed cost. That in turn removes the reason for the flat 2s sleep in WaitForExporter, which cost ~100s across the suite. It was guarding two stale reads, and both have exact conditions available now. After a lease ends, exporterStatus can still read Available from before the lease started, so the poll also requires status.leaseRef to be empty; the controller derives it from the active non-ended leases and the Exporter is reconciled on lease changes through the owner reference, so it clears as soon as the release is processed. Status and lease come from a single query so they cannot be read from different revisions. After a restart the conditions still describe the process that went away, which no status check can detect, so the hooks helper waits for the exporter to go offline where it used to sleep a second. WaitForExporterOffline moved off Kubectl for the same class of reason: an empty result is one of its accepted answers and Kubectl folds stderr into the output, so a failed query was indistinguishable from "offline" and would have ended the wait on the first attempt. Harmless while it sat in an AfterEach; not harmless now that a restart depends on it. Also drop the ExporterSet QEMU polls from 5s. Those conditions are reached by a controller reacting to an event, so the poll period is almost entirely overshoot once the condition holds. --- e2e/test/exporterset_qemu_test.go | 25 ++++-- e2e/test/hooks_test.go | 7 +- e2e/test/utils.go | 123 ++++++++++++++++++++++++------ 3 files changed, 122 insertions(+), 33 deletions(-) diff --git a/e2e/test/exporterset_qemu_test.go b/e2e/test/exporterset_qemu_test.go index 14f9595f1..36975de64 100644 --- a/e2e/test/exporterset_qemu_test.go +++ b/e2e/test/exporterset_qemu_test.go @@ -29,6 +29,17 @@ import ( const exporterSetQemuClientName = "test-client-exporterset-qemu" +// Poll periods for the waits in this file. The conditions here are reached by a +// controller reacting to an event rather than by anything on a fixed schedule, +// so the poll period is almost entirely overshoot once the condition holds. +const ( + // qemuPollPeriod is for waits that run a single kubectl query per attempt. + qemuPollPeriod = time.Second + // qemuComposePollPeriod is for waits that run several queries per attempt, + // where the attempt itself already costs a good fraction of a second. + qemuComposePollPeriod = 2 * time.Second +) + // qemuGuestArch holds native ExporterSet QEMU e2e identifiers for the host. type qemuGuestArch struct { Arch string @@ -123,7 +134,7 @@ var _ = Describe("ExporterSet QEMU E2E Tests", Label("exporterset-qemu"), Ordere "-l", guest.Selector, "-o", "jsonpath={.items[0].metadata.name}") return exporterName - }, 5*time.Minute, 5*time.Second).ShouldNot(BeEmpty()) + }, 5*time.Minute, qemuPollPeriod).ShouldNot(BeEmpty()) By(fmt.Sprintf("waiting for exporter %s Online/Registered/Available", exporterName)) WaitForExporter(exporterName) @@ -132,12 +143,12 @@ var _ = Describe("ExporterSet QEMU E2E Tests", Label("exporterset-qemu"), Ordere Eventually(func() string { return KubectlQuery("-n", ns, "get", "pod", exporterName, "-o", "jsonpath={.status.phase}") - }, 5*time.Minute, 5*time.Second).Should(Equal("Running")) + }, 5*time.Minute, qemuPollPeriod).Should(Equal("Running")) Eventually(func() string { return KubectlQuery("-n", ns, "get", "pod", exporterName, "-o", "jsonpath={.status.containerStatuses[*].ready}") - }, 5*time.Minute, 5*time.Second).Should(ContainSubstring("true")) + }, 5*time.Minute, qemuPollPeriod).Should(ContainSubstring("true")) By(fmt.Sprintf("verifying runtime image provides %s", guest.QemuBinary)) // fedora-minimal has no `which`; use a shell builtin. @@ -154,7 +165,7 @@ var _ = Describe("ExporterSet QEMU E2E Tests", Label("exporterset-qemu"), Ordere "-l", guest.Selector, "--field-selector=status.phase=Running", "-o", "jsonpath={.items[0].metadata.name}") - }, 2*time.Minute, 5*time.Second).ShouldNot(BeEmpty()) + }, 2*time.Minute, qemuPollPeriod).ShouldNot(BeEmpty()) sizeLimit := KubectlQuery("-n", ns, "get", "pod", "-l", guest.Selector, @@ -202,7 +213,7 @@ var _ = Describe("ExporterSet QEMU E2E Tests", Label("exporterset-qemu"), Ordere g.Expect(err).NotTo(HaveOccurred()) g.Expect(uid).NotTo(BeEmpty()) oldUID = uid - }, 2*time.Minute, 5*time.Second).Should(Succeed()) + }, 2*time.Minute, qemuComposePollPeriod).Should(Succeed()) By(fmt.Sprintf("power on, assert %s is running, then power off", guest.QemuBinary)) // One lease: start QEMU via the runtime sidecar, confirm the expected @@ -255,7 +266,7 @@ j qemu power off g.Expect(err).NotTo(HaveOccurred()) g.Expect(strings.Fields(strings.TrimSpace(exporters))).To(HaveLen(1), "expected exactly one Exporter after recycle, got %q", exporters) - }, 5*time.Minute, 5*time.Second).Should(Succeed()) + }, 5*time.Minute, qemuComposePollPeriod).Should(Succeed()) By("waiting for the replacement exporter to become Available") var exporterName string @@ -264,7 +275,7 @@ j qemu power off "-l", guest.Selector, "-o", "jsonpath={.items[0].metadata.name}") return exporterName - }, 2*time.Minute, 5*time.Second).ShouldNot(BeEmpty()) + }, 2*time.Minute, qemuPollPeriod).ShouldNot(BeEmpty()) WaitForExporter(exporterName) By("verifying the replacement still responds to qemu power on/off") diff --git a/e2e/test/hooks_test.go b/e2e/test/hooks_test.go index ce9cc6711..5bd96b1f1 100644 --- a/e2e/test/hooks_test.go +++ b/e2e/test/hooks_test.go @@ -54,7 +54,10 @@ var _ = Describe("Hooks E2E Tests", Label("hooks"), Ordered, ContinueOnFailure, } tracker.StopAll() - time.Sleep(time.Second) + // The controller must observe the old exporter leave before we wait for + // the new one; otherwise WaitForExporter is satisfied by the conditions + // the dead process left behind. + WaitForExporterOffline("test-exporter-hooks") ClearHooksConfig(exporterConfigPath) MergeExporterConfig(exporterConfigPath, exporterOverlay(configFile)) @@ -67,7 +70,7 @@ var _ = Describe("Hooks E2E Tests", Label("hooks"), Ordered, ContinueOnFailure, // startHooksExporterSingle starts without a restart loop (for exit-mode tests). startHooksExporterSingle := func(configFile string) { tracker.StopAll() - time.Sleep(time.Second) + WaitForExporterOffline("test-exporter-hooks") runningConfig = "" ClearHooksConfig(exporterConfigPath) diff --git a/e2e/test/utils.go b/e2e/test/utils.go index cc46199bb..9a006f20c 100644 --- a/e2e/test/utils.go +++ b/e2e/test/utils.go @@ -40,9 +40,18 @@ const ( defaultNamespace = "jumpstarter-lab" defaultWaitTimeout = 5 * time.Minute exporterPollPeriod = 500 * time.Millisecond - exporterPostDelay = 2 * time.Second exporterProcessWait = 2 * time.Second + // stopGracePeriod is how long StopAll gives a SIGTERMed exporter to + // unregister before falling back to SIGKILL. It matches the exporter's own + // unregistration timeout (exporter.py, _unregister_with_controller). It is + // an upper bound, not a fixed cost: StopAll polls and returns as soon as + // the process is gone, which is well under a second in the normal case. + stopGracePeriod = 10 * time.Second + // stopKillTimeout is how long StopAll waits after the SIGKILL fallback. + stopKillTimeout = 10 * time.Second + stopPollPeriod = 50 * time.Millisecond + // DexIssuer is the in-cluster Dex OIDC issuer used by e2e login helpers. DexIssuer = "https://dex.dex.svc.cluster.local:5556" ) @@ -675,8 +684,40 @@ func AnyPIDAlive(pids []int) bool { return false } -// StopAll cancels all restart loops, kills all tracked processes, waits +// signalPIDs best-effort sends sig to each of the given PIDs. +func signalPIDs(pids []int, sig syscall.Signal) { + for _, pid := range pids { + proc, err := os.FindProcess(pid) + if err != nil { + continue + } + _ = proc.Signal(sig) + } +} + +// waitPIDsGone reports whether all the given PIDs have exited within timeout. +func waitPIDsGone(pids []int, timeout time.Duration) bool { + deadline := time.Now().Add(timeout) + for time.Now().Before(deadline) { + if !AnyPIDAlive(pids) { + return true + } + time.Sleep(stopPollPeriod) + } + return !AnyPIDAlive(pids) +} + +// StopAll cancels all restart loops, terminates all tracked processes, waits // until those PIDs are gone, then clears the tracker and kills orphans. +// +// Termination is SIGTERM first, SIGKILL only as a fallback. `jmp run` forks and +// the child calls setsid(), so the tracked PID is the parent: a SIGKILL there +// leaves the exporter itself orphaned with its controller registration intact, +// and the controller then has to time out the heartbeat before the exporter +// counts as gone. On SIGTERM the parent forwards the signal to the child's +// process group, the child reports OFFLINE and unregisters, and the parent +// exits only once it has reaped the child — so the parent being gone means the +// exporter really has left the controller. func (pt *ProcessTracker) StopAll() { // Cancel all restart-loop goroutines first for _, cancel := range pt.cancels { @@ -685,21 +726,25 @@ func (pt *ProcessTracker) StopAll() { pt.cancels = nil pids := pt.TrackedPIDs() + signalPIDs(pids, syscall.SIGTERM) + + // Reap in the background. A zombie still answers signal 0, so a child that + // nobody waited on would look alive for the whole grace period. Processes + // started through StartExporter* already have a Wait goroutine; the extra + // waiter just loses the race and gets an error, which is harmless. for _, pid := range pids { - proc, err := os.FindProcess(pid) - if err != nil { - continue + if proc, err := os.FindProcess(pid); err == nil { + go func() { _, _ = proc.Wait() }() } - _ = proc.Signal(syscall.SIGKILL) - _, _ = proc.Wait() } // Wait until tracked PIDs are actually gone before clearing the list, // so callers that snapshot PIDs (or poll IsProcessRunning) observe a // real termination rather than an emptied tracker. - deadline := time.Now().Add(10 * time.Second) - for time.Now().Before(deadline) && AnyPIDAlive(pids) { - time.Sleep(50 * time.Millisecond) + if !waitPIDsGone(pids, stopGracePeriod) { + GinkgoWriter.Printf("Exporter PIDs %v did not exit on SIGTERM, sending SIGKILL\n", pids) + signalPIDs(pids, syscall.SIGKILL) + waitPIDsGone(pids, stopKillTimeout) } pt.pids = nil @@ -727,26 +772,47 @@ func (pt *ProcessTracker) IsProcessRunning() bool { // than an unreadable `kubectl wait exporters.jumpstarter.dev/error: ...`. var validExporterName = regexp.MustCompile(`^[a-z0-9]([-.a-z0-9]*[a-z0-9])?$`) -// WaitForExporter waits for an exporter to become Online, Registered, and Available. +// exporterFree is the value exporterState returns for an exporter that is +// Available with no lease outstanding. +const exporterFree = "Available|" + +// exporterState reads an exporter's status and its outstanding lease in a +// single query, so the two can never be read from different revisions. +func exporterState(ns, exporterRef string) string { + return KubectlQuery("-n", ns, "get", exporterRef, + "-o", "jsonpath={.status.exporterStatus}|{.status.leaseRef.name}") +} + +// WaitForExporter waits for an exporter to become Online, Registered, and +// Available with no lease outstanding. +// +// Waiting for the lease to clear is what makes this safe to call right after a +// `jmp shell` returns. The controller has not necessarily processed the release +// yet at that point, so exporterStatus can still read Available from before the +// lease ever started, and a wait that only looked at the status would be +// satisfied by that stale value. status.leaseRef is derived from the active, +// non-ended leases (exporter_controller.go, reconcileStatusLeaseRef), so it +// only empties once the release has actually been reconciled. +// +// A caller that just stopped an exporter has the same problem one step earlier +// — the conditions still describe the process that went away — and must wait +// for WaitForExporterOffline before calling this. func WaitForExporter(name string) { ns := Namespace() ExpectWithOffset(1, validExporterName.MatchString(name)).To(BeTrue(), "WaitForExporter called with an invalid exporter name %q", name) exporterRef := fmt.Sprintf("exporters.jumpstarter.dev/%s", name) - // Brief delay to avoid catching pre-disconnect state - time.Sleep(exporterPostDelay) - // Wait for Online + Registered conditions MustRunCmd("kubectl", "-n", ns, "wait", "--timeout", "5m", "--for=condition=Online", "--for=condition=Registered", exporterRef) - // Poll until exporterStatus is Available - Eventually(func() string { - return KubectlQuery("-n", ns, "get", exporterRef, - "-o", "jsonpath={.status.exporterStatus}") - }, defaultWaitTimeout, exporterPollPeriod).Should(Equal("Available"), - "timed out waiting for %s to reach Available status", name) + // Poll until the exporter is Available and holds no lease + EventuallyWithOffset(1, func() string { + return exporterState(ns, exporterRef) + }, defaultWaitTimeout, exporterPollPeriod).Should(Equal(exporterFree), + "timed out waiting for %s to be Available with no outstanding lease "+ + "(reported as |)", name) } // WaitForExporters waits for multiple exporters in parallel. @@ -763,16 +829,25 @@ func WaitForExporters(names ...string) { wg.Wait() } -// WaitForExporterOffline waits for an exporter to go offline. +// WaitForExporterOffline waits for an exporter to stop reporting itself Online. +// +// The query goes through RunCmdSplit rather than Kubectl because an empty +// result is one of the accepted answers (the Online condition may not be set +// yet) and Kubectl folds stderr into its output: a failed query would otherwise +// be indistinguishable from "the exporter is offline" and end the wait on the +// first attempt. func WaitForExporterOffline(name string) { ns := Namespace() exporterRef := fmt.Sprintf("exporters.jumpstarter.dev/%s", name) - Eventually(func() bool { - out, _ := Kubectl("-n", ns, "get", exporterRef, + EventuallyWithOffset(1, func() bool { + out, _, err := RunCmdSplit("kubectl", "-n", ns, "get", exporterRef, "-o", `jsonpath={.status.conditions[?(@.type=="Online")].status}`) + if err != nil { + return false + } return out == "False" || out == "Unknown" || out == "" - }, 200*time.Second, time.Second).Should(BeTrue(), + }, 200*time.Second, exporterPollPeriod).Should(BeTrue(), "timed out waiting for %s to go offline", name) } From 47e74f96e5646cade5e9040025e903e28947d009 Mon Sep 17 00:00:00 2001 From: Benny Zlotnik Date: Sun, 9 Aug 2026 20:17:17 +0300 Subject: [PATCH 14/24] ci: restore the go module cache setup-go has cache enabled by default, but every job logged Restore cache failed: Dependencies file is not found in /home/runner/work/jumpstarter/jumpstarter. Supported file pattern: go.mod The action looks for a dependency file next to the working directory to build its cache key, and there is no go.mod at the repo root -- the modules are controller/ and e2e/test/. So the cache never restored and never saved, in all nine setup-go usages across four workflows. Point cache-dependency-path at the go.sum files that do exist. In the e2e job this covers the ~36s spent re-downloading and rebuilding kind and grpcurl on every run. --- .github/workflows/build-images.yaml | 1 + .github/workflows/e2e.yaml | 6 ++++++ .github/workflows/lint.yaml | 1 + .github/workflows/release-operator-installer.yaml | 1 + 4 files changed, 9 insertions(+) diff --git a/.github/workflows/build-images.yaml b/.github/workflows/build-images.yaml index 73ed93bc1..6f27c79bd 100644 --- a/.github/workflows/build-images.yaml +++ b/.github/workflows/build-images.yaml @@ -135,6 +135,7 @@ jobs: uses: actions/setup-go@b7ad1dad31e06c5925ef5d2fc7ad053ef454303e # v7 with: go-version-file: .go-version + cache-dependency-path: "**/go.sum" - name: Build operator installer manifest if: ${{ matrix.generate_installer && steps.check.outputs.skip != 'true' }} diff --git a/.github/workflows/e2e.yaml b/.github/workflows/e2e.yaml index 8a9aebd3c..15f14cc2b 100644 --- a/.github/workflows/e2e.yaml +++ b/.github/workflows/e2e.yaml @@ -72,6 +72,7 @@ jobs: uses: actions/setup-go@b7ad1dad31e06c5925ef5d2fc7ad053ef454303e # v7 with: go-version-file: .go-version + cache-dependency-path: "**/go.sum" - name: Cache controller image id: cache @@ -111,6 +112,7 @@ jobs: uses: actions/setup-go@b7ad1dad31e06c5925ef5d2fc7ad053ef454303e # v7 with: go-version-file: .go-version + cache-dependency-path: "**/go.sum" - name: Cache operator artifacts id: cache @@ -155,6 +157,7 @@ jobs: uses: actions/setup-go@b7ad1dad31e06c5925ef5d2fc7ad053ef454303e # v7 with: go-version-file: .go-version + cache-dependency-path: "**/go.sum" - name: Cache exporterset-controller image id: cache @@ -314,6 +317,7 @@ jobs: uses: actions/setup-go@b7ad1dad31e06c5925ef5d2fc7ad053ef454303e # v7 with: go-version-file: .go-version + cache-dependency-path: "**/go.sum" - name: Load e2e artifacts uses: ./.github/actions/load-e2e-artifacts @@ -378,6 +382,7 @@ jobs: uses: actions/setup-go@b7ad1dad31e06c5925ef5d2fc7ad053ef454303e # v7 with: go-version-file: .go-version + cache-dependency-path: "**/go.sum" - name: Setup compat environment (old controller v0.8.1) run: make e2e-compat-setup COMPAT_SCENARIO=old-controller @@ -420,6 +425,7 @@ jobs: uses: actions/setup-go@b7ad1dad31e06c5925ef5d2fc7ad053ef454303e # v7 with: go-version-file: .go-version + cache-dependency-path: "**/go.sum" - name: Load e2e artifacts uses: ./.github/actions/load-e2e-artifacts diff --git a/.github/workflows/lint.yaml b/.github/workflows/lint.yaml index 79b0a1a91..83270e72a 100644 --- a/.github/workflows/lint.yaml +++ b/.github/workflows/lint.yaml @@ -55,6 +55,7 @@ jobs: uses: actions/setup-go@b7ad1dad31e06c5925ef5d2fc7ad053ef454303e # v7 with: go-version-file: .go-version + cache-dependency-path: "**/go.sum" - name: Run go linter working-directory: controller diff --git a/.github/workflows/release-operator-installer.yaml b/.github/workflows/release-operator-installer.yaml index 1cef2616d..3f93341ff 100644 --- a/.github/workflows/release-operator-installer.yaml +++ b/.github/workflows/release-operator-installer.yaml @@ -20,6 +20,7 @@ jobs: uses: actions/setup-go@b7ad1dad31e06c5925ef5d2fc7ad053ef454303e # v7 with: go-version-file: .go-version + cache-dependency-path: "**/go.sum" - name: Build operator installer manifest env: From ec5548d830560cafd6f4b93b1106bbdfe4dd9b72 Mon Sep 17 00:00:00 2001 From: Benny Zlotnik Date: Sun, 9 Aug 2026 20:17:51 +0300 Subject: [PATCH 15/24] test(e2e): scope the orphan exporter sweep to this tracker StopAll ended with a global `pkill -9 -f "jmp run --exporter"`. Two problems, both blocking `ginkgo --procs`: - The pattern is not scoped to the caller, so one ginkgo process's cleanup would reap every other process's exporters. - Being a substring match, "jmp run --exporter" also matches "jmp run --exporter-config", so it reached the direct-listener and dut-network exporters it was never meant to touch. The sweep is still needed: `jmp run` forks (run.py:215) and the child calls setsid() (run.py:255), so the SIGKILL fallback on the tracked parent orphans the child. The child does not re-exec, so it carries the parent's argv and can be matched precisely. Record the flag/value pair each process was started with and sweep /proc for whole-argv matches against only those. This kills the same orphans without touching anything the tracker did not start. --- e2e/test/utils.go | 92 +++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 86 insertions(+), 6 deletions(-) diff --git a/e2e/test/utils.go b/e2e/test/utils.go index 9a006f20c..3414bd46d 100644 --- a/e2e/test/utils.go +++ b/e2e/test/utils.go @@ -25,6 +25,7 @@ import ( "os/exec" "path/filepath" "regexp" + "slices" "strconv" "strings" "sync" @@ -457,13 +458,34 @@ func (lb *logBuffer) Close() { } } +// procSpec identifies an exporter by the flag/value pair it was started with, +// e.g. {"--exporter", "hooks-exporter"} or {"--exporter-config", "/tmp/x.yaml"}. +// `jmp run` forks and the child calls setsid() without re-execing, so the child +// carries the same argv as the tracked parent and can be found by matching it. +type procSpec struct { + flag string + value string +} + // ProcessTracker manages background exporter processes. type ProcessTracker struct { pids []int + specs []procSpec logs map[string]*logBuffer cancels []context.CancelFunc } +// track records a started process and the argv identity that finds its forked +// child, so StopAll can sweep an orphan without touching exporters belonging to +// another ginkgo process. +func (pt *ProcessTracker) track(pid int, flag, value string) { + pt.pids = append(pt.pids, pid) + spec := procSpec{flag: flag, value: value} + if !slices.Contains(pt.specs, spec) { + pt.specs = append(pt.specs, spec) + } +} + // NewProcessTracker creates a new ProcessTracker. func NewProcessTracker() *ProcessTracker { return &ProcessTracker{ @@ -524,7 +546,7 @@ func (pt *ProcessTracker) StartExporterLoop(exporterName string, jmpBin ...strin // Track the PID under the parent lock-free path; this is safe // because StopAll first cancels the context so this goroutine // will not spawn new processes concurrently. - pt.pids = append(pt.pids, pid) + pt.track(pid, "--exporter", exporterName) if restartCount > 0 { GinkgoWriter.Printf("Restarted exporter %s (PID %d, restart #%d)\n", exporterName, pid, restartCount) @@ -552,7 +574,7 @@ func (pt *ProcessTracker) StartExporterSingle(exporterName string) *exec.Cmd { cmd.SysProcAttr = &syscall.SysProcAttr{Setpgid: true} err := cmd.Start() ExpectWithOffset(1, err).NotTo(HaveOccurred(), "failed to start exporter %s", exporterName) - pt.pids = append(pt.pids, cmd.Process.Pid) + pt.track(cmd.Process.Pid, "--exporter", exporterName) GinkgoWriter.Printf("Started exporter %s (PID %d)\n", exporterName, cmd.Process.Pid) // Reap the child process in the background so it doesn't become a zombie. @@ -577,7 +599,7 @@ func (pt *ProcessTracker) StartExporterWithConfig(name, configPath string) *exec err := cmd.Start() ExpectWithOffset(1, err).NotTo(HaveOccurred(), "failed to start exporter with config %s", configPath) - pt.pids = append(pt.pids, cmd.Process.Pid) + pt.track(cmd.Process.Pid, "--exporter-config", configPath) GinkgoWriter.Printf("Started exporter %s (PID %d) with config %s\n", name, cmd.Process.Pid, configPath) // Reap the child process in the background so it doesn't become a zombie. @@ -608,7 +630,7 @@ func (pt *ProcessTracker) StartDirectExporter(configFile string, port int, passp err := cmd.Start() ExpectWithOffset(1, err).NotTo(HaveOccurred(), "failed to start direct exporter with config %s", configFile) - pt.pids = append(pt.pids, cmd.Process.Pid) + pt.track(cmd.Process.Pid, "--exporter-config", configFile) GinkgoWriter.Printf("Started direct exporter (PID %d) on port %d\n", cmd.Process.Pid, port) return cmd, stderrBuf } @@ -748,8 +770,66 @@ func (pt *ProcessTracker) StopAll() { } pt.pids = nil - // Kill orphaned jmp exporter processes - _ = exec.Command("pkill", "-9", "-f", "jmp run --exporter").Run() + pt.sweepOrphans() +} + +// sweepOrphans SIGKILLs any surviving `jmp run` process that matches one of the +// argv identities this tracker started. It is a safety net for the SIGKILL +// fallback above: killing the parent orphans the forked child, which keeps the +// parent's argv. +// +// This is deliberately not a `pkill -f "jmp run --exporter"`. That pattern is +// global, so under `ginkgo --procs` one process's cleanup would reap every other +// process's exporters, and being a substring match it also matched the +// `--exporter-config` runs it was never meant to touch. Matching whole argv +// elements against the specs this tracker recorded avoids both. +func (pt *ProcessTracker) sweepOrphans() { + if len(pt.specs) == 0 { + return + } + + entries, err := os.ReadDir("/proc") + if err != nil { + return + } + + self := os.Getpid() + for _, entry := range entries { + pid, err := strconv.Atoi(entry.Name()) + if err != nil || pid == self { + continue + } + + raw, err := os.ReadFile(filepath.Join("/proc", entry.Name(), "cmdline")) + if err != nil { + continue // process exited, or not ours to read + } + argv := strings.Split(strings.TrimSuffix(string(raw), "\x00"), "\x00") + if !pt.argvMatches(argv) { + continue + } + + GinkgoWriter.Printf("Killing orphaned exporter process %d (%s)\n", pid, strings.Join(argv, " ")) + if proc, err := os.FindProcess(pid); err == nil { + _ = proc.Signal(syscall.SIGKILL) + } + } +} + +// argvMatches reports whether argv is a `jmp run` invocation carrying one of the +// tracked flag/value pairs as adjacent, whole arguments. +func (pt *ProcessTracker) argvMatches(argv []string) bool { + if len(argv) < 3 || !slices.Contains(argv, "run") { + return false + } + for _, spec := range pt.specs { + for i := 0; i < len(argv)-1; i++ { + if argv[i] == spec.flag && argv[i+1] == spec.value { + return true + } + } + } + return false } // Cleanup stops all processes and closes log files. From b09cd1b329a29b9985fa5bbf69963e97dd28a82c Mon Sep 17 00:00:00 2001 From: Benny Zlotnik Date: Sun, 9 Aug 2026 20:18:06 +0300 Subject: [PATCH 16/24] test(e2e): run the core suite in parallel with the other containers The core suite was Serial for one reason: it selected the active client with `jmp config client use`. That writes `current-client` into the single shared config.yaml (user.py:52), which is process-global state, so core could not run alongside any container that reads it. Being both the largest non-Serial container and Serial, it made E2E_PROCS buy nothing. Name the client explicitly with --client at each jmp invocation instead, matching what the rest of the file already does for the legacy client. `jmp login` needs no such change: it writes per-client files under clients/.yaml (client.py:115), not the shared config. Nothing else in the parallel set collides: exporter names, lease selectors (oidc/sa/legacy vs hooks, exit-on-lease-end, authlog) and the direct listener's port are all distinct, and every lease deletion is client-scoped. compat_old_controller still uses `config client use`, but it is Serial and runs alone after the parallel batch. With that, turn on E2E_PROCS=2. --- .github/workflows/e2e.yaml | 4 +++ e2e/test/e2e_test.go | 74 +++++++++++++++++++++----------------- 2 files changed, 45 insertions(+), 33 deletions(-) diff --git a/.github/workflows/e2e.yaml b/.github/workflows/e2e.yaml index 15f14cc2b..dea32fac2 100644 --- a/.github/workflows/e2e.yaml +++ b/.github/workflows/e2e.yaml @@ -14,6 +14,10 @@ env: # flakes in the ginkgo summary, so this hides nothing; it only stops a single # infrastructure hiccup from failing an unrelated PR. E2E_FLAKE_ATTEMPTS: "2" + # Run the non-Serial containers two at a time. The runner has 4 CPUs and the + # suite spends most of its time waiting on the cluster, so a second process + # overlaps the two long containers (hooks, core) rather than competing for CPU. + E2E_PROCS: "2" jobs: changes: diff --git a/e2e/test/e2e_test.go b/e2e/test/e2e_test.go index 2c46922cb..bb4b80ee2 100644 --- a/e2e/test/e2e_test.go +++ b/e2e/test/e2e_test.go @@ -26,10 +26,11 @@ import ( . "github.com/onsi/gomega" //nolint:revive ) -// Serial: these specs select the active client with `jmp config client use`, -// which writes the shared client config, so they cannot run alongside other -// containers that read it. -var _ = Describe("Core E2E Tests", Label("core"), Ordered, ContinueOnFailure, Serial, func() { +// Every jmp invocation here names its client explicitly with --client rather +// than selecting one with `jmp config client use`. That call writes the shared +// client config, which is process-global state: it would make these specs +// unsafe to run alongside any other container that reads it. +var _ = Describe("Core E2E Tests", Label("core"), Ordered, ContinueOnFailure, func() { var tracker *ProcessTracker BeforeAll(func() { @@ -364,55 +365,63 @@ var _ = Describe("Core E2E Tests", Label("core"), Ordered, ContinueOnFailure, Se Context("Lease operations", func() { It("can operate on leases", func() { WaitForExporters("test-exporter-oidc", "test-exporter-sa", "test-exporter-legacy") - MustJmp("config", "client", "use", "test-client-oidc") - MustJmp("create", "lease", "--selector", "example.com/board=oidc", "--duration", "1d") - MustJmp("get", "leases") - MustJmp("get", "exporters") + MustJmp("create", "lease", "--client", "test-client-oidc", + "--selector", "example.com/board=oidc", "--duration", "1d") + MustJmp("get", "leases", "--client", "test-client-oidc") + MustJmp("get", "exporters", "--client", "test-client-oidc") // Verify label selector filtering (regression test for #36) - out, err := Jmp("get", "leases", "--selector", "example.com/board=oidc", "-o", "yaml") + out, err := Jmp("get", "leases", "--client", "test-client-oidc", + "--selector", "example.com/board=oidc", "-o", "yaml") Expect(err).NotTo(HaveOccurred(), out) Expect(out).To(ContainSubstring("example.com/board=oidc")) - out, err = Jmp("get", "leases", "--selector", "example.com/board=doesnotexist") + out, err = Jmp("get", "leases", "--client", "test-client-oidc", + "--selector", "example.com/board=doesnotexist") Expect(err).NotTo(HaveOccurred(), out) Expect(out).To(Equal("No resources found.")) // Test complex selectors with matchExpressions - MustJmp("create", "lease", "--selector", "example.com/board=sa,!nonexistent", "--duration", "1d") + MustJmp("create", "lease", "--client", "test-client-oidc", + "--selector", "example.com/board=sa,!nonexistent", "--duration", "1d") - out, err = Jmp("get", "leases", "--selector", "example.com/board=sa", "-o", "yaml") + out, err = Jmp("get", "leases", "--client", "test-client-oidc", + "--selector", "example.com/board=sa", "-o", "yaml") Expect(err).NotTo(HaveOccurred(), out) Expect(out).To(ContainSubstring("example.com/board=sa")) - out, err = Jmp("get", "leases", "--selector", "!nonexistent", "-o", "yaml") + out, err = Jmp("get", "leases", "--client", "test-client-oidc", + "--selector", "!nonexistent", "-o", "yaml") Expect(err).NotTo(HaveOccurred(), out) Expect(out).To(ContainSubstring("!nonexistent")) - out, err = Jmp("get", "leases", "--selector", "example.com/board=sa,!production", "-o", "yaml") + out, err = Jmp("get", "leases", "--client", "test-client-oidc", + "--selector", "example.com/board=sa,!production", "-o", "yaml") Expect(err).NotTo(HaveOccurred(), out) Expect(out).To(ContainSubstring("example.com/board=sa")) - out, err = Jmp("get", "leases", "--selector", "example.com/board=sa,!example.com/board") + out, err = Jmp("get", "leases", "--client", "test-client-oidc", + "--selector", "example.com/board=sa,!example.com/board") Expect(err).NotTo(HaveOccurred(), out) Expect(out).To(Equal("No resources found.")) - out, err = Jmp("get", "leases", "--selector", "example.com/board=sa,!nonexistent,region=us") + out, err = Jmp("get", "leases", "--client", "test-client-oidc", + "--selector", "example.com/board=sa,!nonexistent,region=us") Expect(err).NotTo(HaveOccurred(), out) Expect(out).To(Equal("No resources found.")) - MustJmp("delete", "leases", "--all") + MustJmp("delete", "leases", "--client", "test-client-oidc", "--all") }) It("can create a lease with context metadata", func() { WaitForExporters("test-exporter-oidc", "test-exporter-sa", "test-exporter-legacy") - MustJmp("config", "client", "use", "test-client-oidc") DeferCleanup(func() { - MustJmp("delete", "leases", "--all") + MustJmp("delete", "leases", "--client", "test-client-oidc", "--all") }) out := MustJmp("create", "lease", + "--client", "test-client-oidc", "--selector", "example.com/board=oidc", "--duration", "1d", "--context", "build_id=nightly-42", @@ -429,7 +438,6 @@ var _ = Describe("Core E2E Tests", Label("core"), Ordered, ContinueOnFailure, Se It("paginated lease listing returns all leases", func() { WaitForExporters("test-exporter-oidc", "test-exporter-sa", "test-exporter-legacy") - MustJmp("config", "client", "use", "test-client-oidc") // As with the exporter pagination spec, the leases are fixtures for // the client's pagination, so create them in a single apply. @@ -451,17 +459,17 @@ spec: } MustKubectlApply(manifest.String()) - out, err := Jmp("get", "leases", "--page-size", "5", "-o", "name") + out, err := Jmp("get", "leases", "--client", "test-client-oidc", + "--page-size", "5", "-o", "name") Expect(err).NotTo(HaveOccurred(), out) lines := strings.Split(strings.TrimSpace(out), "\n") Expect(lines).To(HaveLen(10)) - MustJmp("delete", "leases", "--all") + MustJmp("delete", "leases", "--client", "test-client-oidc", "--all") }) It("paginated exporter listing returns all exporters", func() { WaitForExporters("test-exporter-oidc", "test-exporter-sa", "test-exporter-legacy") - MustJmp("config", "client", "use", "test-client-oidc") // The exporters are fixtures for the client's pagination, so // create them in a single apply rather than one jmp process each. @@ -481,7 +489,8 @@ spec: } MustKubectlApply(manifest.String()) - out, err := Jmp("get", "exporters", "--selector", "pagination=true", "--page-size", "5", "-o", "name") + out, err := Jmp("get", "exporters", "--client", "test-client-oidc", + "--selector", "pagination=true", "--page-size", "5", "-o", "name") Expect(err).NotTo(HaveOccurred(), out) lines := strings.Split(strings.TrimSpace(out), "\n") Expect(lines).To(HaveLen(10)) @@ -492,24 +501,22 @@ spec: It("lease listing shows expires at and remaining columns", func() { WaitForExporters("test-exporter-oidc", "test-exporter-sa", "test-exporter-legacy") - MustJmp("config", "client", "use", "test-client-oidc") - - MustJmp("create", "lease", "--selector", "example.com/board=oidc", "--duration", "1d") + MustJmp("create", "lease", "--client", "test-client-oidc", + "--selector", "example.com/board=oidc", "--duration", "1d") out, err := RunCmdWithEnv(map[string]string{"COLUMNS": "200"}, - "jmp", "get", "leases") + "jmp", "get", "leases", "--client", "test-client-oidc") Expect(err).NotTo(HaveOccurred(), out) Expect(out).To(ContainSubstring("EXPIRES AT")) Expect(out).To(ContainSubstring("REMAINING")) - MustJmp("delete", "leases", "--all") + MustJmp("delete", "leases", "--client", "test-client-oidc", "--all") }) It("can transfer lease to another client", func() { WaitForExporters("test-exporter-oidc", "test-exporter-sa", "test-exporter-legacy") - MustJmp("config", "client", "use", "test-client-oidc") - - out := MustJmp("create", "lease", "--selector", "example.com/board=oidc", + out := MustJmp("create", "lease", "--client", "test-client-oidc", + "--selector", "example.com/board=oidc", "--duration", "1d", "-o", "yaml") // Parse the lease YAML to extract the lease name. @@ -525,7 +532,8 @@ spec: MustKubectl("-n", ns, "wait", "--timeout", "60s", "--for=condition=Ready", fmt.Sprintf("leases.jumpstarter.dev/%s", leaseName)) - out, err := Jmp("update", "lease", leaseName, "--to-client", "test-client-legacy", "-o", "yaml") + out, err := Jmp("update", "lease", leaseName, "--client", "test-client-oidc", + "--to-client", "test-client-legacy", "-o", "yaml") Expect(err).NotTo(HaveOccurred(), out) Expect(out).To(ContainSubstring("test-client-legacy")) From 8830bff7c07e4970b6039e1a4def4c0beeb4f2df Mon Sep 17 00:00:00 2001 From: Benny Zlotnik Date: Sun, 9 Aug 2026 21:17:09 +0300 Subject: [PATCH 17/24] ci(e2e): cache controller tools and load kind images in parallel MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cache controller/bin (kind, kustomize, grpcurl) across runs so the Makefile's go-install-tool guard skips compilation on cache hit. Load all container images into the kind cluster concurrently — each kind load is I/O bound so overlapping them cuts wall-clock to roughly the cost of the single largest image. --- .github/workflows/e2e.yaml | 30 ++++++++++++++++++++++ controller/hack/deploy_with_operator.sh | 33 +++++++++++++++++++------ 2 files changed, 55 insertions(+), 8 deletions(-) diff --git a/.github/workflows/e2e.yaml b/.github/workflows/e2e.yaml index dea32fac2..8bb530057 100644 --- a/.github/workflows/e2e.yaml +++ b/.github/workflows/e2e.yaml @@ -339,6 +339,16 @@ jobs: sudo modprobe "$mod" 2>/dev/null || true done + - name: Cache controller tools + uses: actions/cache@55cc8345863c7cc4c66a329aec7e433d2d1c52a9 # v6 + with: + path: controller/bin + key: controller-tools-${{ matrix.arch }}-kind${{ env.KIND_VERSION }}-kustomize${{ env.KUSTOMIZE_VERSION }}-grpcurl${{ env.GRPCURL_VERSION }} + env: + KIND_VERSION: v0.27.0 + KUSTOMIZE_VERSION: v5.4.1 + GRPCURL_VERSION: v1.9.2 + - name: Setup e2e test environment run: make e2e-setup env: @@ -388,6 +398,16 @@ jobs: go-version-file: .go-version cache-dependency-path: "**/go.sum" + - name: Cache controller tools + uses: actions/cache@55cc8345863c7cc4c66a329aec7e433d2d1c52a9 # v6 + with: + path: controller/bin + key: controller-tools-amd64-kind${{ env.KIND_VERSION }}-kustomize${{ env.KUSTOMIZE_VERSION }}-grpcurl${{ env.GRPCURL_VERSION }} + env: + KIND_VERSION: v0.27.0 + KUSTOMIZE_VERSION: v5.4.1 + GRPCURL_VERSION: v1.9.2 + - name: Setup compat environment (old controller v0.8.1) run: make e2e-compat-setup COMPAT_SCENARIO=old-controller env: @@ -436,6 +456,16 @@ jobs: with: arch: amd64 + - name: Cache controller tools + uses: actions/cache@55cc8345863c7cc4c66a329aec7e433d2d1c52a9 # v6 + with: + path: controller/bin + key: controller-tools-amd64-kind${{ env.KIND_VERSION }}-kustomize${{ env.KUSTOMIZE_VERSION }}-grpcurl${{ env.GRPCURL_VERSION }} + env: + KIND_VERSION: v0.27.0 + KUSTOMIZE_VERSION: v5.4.1 + GRPCURL_VERSION: v1.9.2 + - name: Setup compat environment (old client v0.7.4) run: make e2e-compat-setup COMPAT_SCENARIO=old-client env: diff --git a/controller/hack/deploy_with_operator.sh b/controller/hack/deploy_with_operator.sh index 308737b6f..b8ec7a281 100755 --- a/controller/hack/deploy_with_operator.sh +++ b/controller/hack/deploy_with_operator.sh @@ -29,23 +29,40 @@ if [ "${USE_CERTMANAGER}" = "true" ]; then fi fi -# load the container images into the cluster -load_image "${IMG}" -load_image "${OPERATOR_IMG}" -load_image "${EXPORTER_SET_CONTROLLER_IMG}" -# Exporter + QEMU runtime images are required for ExporterSet QEMU e2e / samples. -# Missing images are skipped so plain controller deploys still work. +# Load container images into the cluster in parallel. Each `kind load` is I/O +# bound (piping a tarball into the node's containerd), so overlapping them cuts +# wall-clock to roughly the cost of the single largest image. +_load_pids=() +_load_failed=0 + +load_image "${IMG}" & +_load_pids+=($!) +load_image "${OPERATOR_IMG}" & +_load_pids+=($!) +load_image "${EXPORTER_SET_CONTROLLER_IMG}" & +_load_pids+=($!) + if container_image_exists "${EXPORTER_IMG}"; then - load_image "${EXPORTER_IMG}" + load_image "${EXPORTER_IMG}" & + _load_pids+=($!) else echo -e "${YELLOW}Skipping load of exporter image (not present locally): ${EXPORTER_IMG}${NC}" fi if container_image_exists "${QEMU_RUNTIME_IMG}"; then - load_image "${QEMU_RUNTIME_IMG}" + load_image "${QEMU_RUNTIME_IMG}" & + _load_pids+=($!) else echo -e "${YELLOW}Skipping load of qemu-runtime image (not present locally): ${QEMU_RUNTIME_IMG}${NC}" fi +for pid in "${_load_pids[@]}"; do + wait "${pid}" || _load_failed=1 +done +if [ "${_load_failed}" -eq 1 ]; then + echo -e "${RED}One or more images failed to load${NC}" + exit 1 +fi + # Deploy the operator echo -e "${GREEN}Deploying Jumpstarter operator ...${NC}" kubectl apply -f deploy/operator/dist/install.yaml From eec87c65c0b8ff781e3f2144d01edddc81438db3 Mon Sep 17 00:00:00 2001 From: Benny Zlotnik Date: Mon, 10 Aug 2026 08:32:10 +0300 Subject: [PATCH 18/24] try to reduce python test runtime Signed-off-by: Benny Zlotnik --- .github/workflows/python-tests.yaml | 30 +++++++++++++++++-- python/Makefile | 9 +++++- .../jumpstarter-driver-adb/pyproject.toml | 1 - .../pyproject.toml | 1 - .../jumpstarter-driver-ble/pyproject.toml | 1 - .../jumpstarter-driver-doip/pyproject.toml | 1 - .../pyproject.toml | 2 +- .../jumpstarter-driver-esp32/pyproject.toml | 1 - .../pyproject.toml | 1 - .../pyproject.toml | 1 - .../jumpstarter-driver-http/pyproject.toml | 1 - .../pyproject.toml | 1 - .../pyproject.toml | 1 - .../jumpstarter-driver-obd/pyproject.toml | 1 - .../jumpstarter-driver-pi-pico/pyproject.toml | 1 - .../pyproject.toml | 1 - .../jumpstarter-driver-renode/pyproject.toml | 1 - .../jumpstarter-driver-ridesx/pyproject.toml | 1 - .../jumpstarter-driver-shell/pyproject.toml | 1 - .../jumpstarter-driver-someip/pyproject.toml | 1 - .../pyproject.toml | 1 - .../pyproject.toml | 1 - .../jumpstarter-driver-ssh/pyproject.toml | 1 - .../pyproject.toml | 1 - .../jumpstarter-driver-tasmota/pyproject.toml | 1 - .../jumpstarter-driver-tftp/pyproject.toml | 1 - .../jumpstarter-driver-tmt/pyproject.toml | 1 - .../jumpstarter-driver-uds-can/pyproject.toml | 1 - .../pyproject.toml | 1 - .../jumpstarter-driver-uds/pyproject.toml | 1 - .../jumpstarter-driver-vnc/pyproject.toml | 1 - .../jumpstarter-driver-xcp/pyproject.toml | 1 - .../jumpstarter-driver-yepkit/pyproject.toml | 1 - python/pyproject.toml | 2 +- 34 files changed, 37 insertions(+), 36 deletions(-) diff --git a/.github/workflows/python-tests.yaml b/.github/workflows/python-tests.yaml index bcb50e9b6..44a54b0af 100644 --- a/.github/workflows/python-tests.yaml +++ b/.github/workflows/python-tests.yaml @@ -145,7 +145,7 @@ jobs: uses: actions/cache@55cc8345863c7cc4c66a329aec7e433d2d1c52a9 # v6 with: path: python/packages/jumpstarter-driver-qemu/images - key: fedora-cloud-43-1.6 + key: fedora-cloud-43-1.6-preconfigured-v1 - name: Download Fedora Cloud images if: steps.cache-fedora-cloud-images.outputs.cache-hit != 'true' @@ -155,12 +155,36 @@ jobs: "https://iad.mirror.rackspace.com/fedora/releases/43/Cloud/${arch}/images/Fedora-Cloud-Base-Generic-43-1.6.${arch}.qcow2" done + - name: Pre-configure Fedora Cloud images for fast boot + if: runner.os == 'Linux' && steps.cache-fedora-cloud-images.outputs.cache-hit != 'true' + run: | + sudo apt-get install -y libguestfs-tools + arch=x86_64 + img="python/packages/jumpstarter-driver-qemu/images/Fedora-Cloud-Base-Generic-43-1.6.${arch}.qcow2" + sudo virt-customize -a "$img" \ + --hostname demo \ + --run-command 'useradd -m -s /bin/bash jumpstarter' \ + --password jumpstarter:password:password \ + --run-command 'echo "jumpstarter ALL=(ALL) NOPASSWD:ALL" > /etc/sudoers.d/jumpstarter' \ + --run-command 'systemctl disable cloud-init cloud-init-local cloud-config cloud-final' \ + --run-command 'rm -rf /var/lib/cloud' \ + --selinux-relabel + - name: Run pytest working-directory: python env: - PYTEST_ADDOPTS: "--cov-report=xml --log-level=CRITICAL --log-cli-level=CRITICAL" + PYTEST_ADDOPTS: "--log-level=CRITICAL --log-cli-level=CRITICAL" run: | - make test -j4 LOGS_DIR=${{ runner.temp }}/test-logs + changed_pkgs=$(git diff --name-only --relative origin/${{ github.base_ref || 'main' }}...HEAD -- packages/ \ + | sed -n 's|^packages/\([^/]*\)/.*|\1|p' | sort -u | tr '\n' ' ') + echo "Coverage-enabled packages: ${changed_pkgs:-all (fallback)}" + if [ -z "$changed_pkgs" ]; then + # Fallback: workflow_dispatch or merge_group without a base ref + export PYTEST_ADDOPTS="--cov=. --cov-report=xml $PYTEST_ADDOPTS" + make test -j8 LOGS_DIR=${{ runner.temp }}/test-logs + else + make test -j8 LOGS_DIR=${{ runner.temp }}/test-logs COV_PACKAGES="$changed_pkgs" + fi - name: Upload test logs if: always() diff --git a/python/Makefile b/python/Makefile index 1d00eda3f..ff917a9cf 100644 --- a/python/Makefile +++ b/python/Makefile @@ -39,7 +39,14 @@ pkg-test-%: packages/% @mkdir -p $(LOGS_DIR) @rm -f $(LOGS_DIR)/$*.failed @bash -c 'set -o pipefail; \ - PYTHONUNBUFFERED=1 uv run --isolated --directory $< pytest 2>&1 | tee $(LOGS_DIR)/$*.log; \ + cov_addopts=""; \ + if [ -n "$(COV_PACKAGES)" ]; then \ + if echo " $(COV_PACKAGES) " | grep -q " $* "; then \ + cov_addopts="--cov=. --cov-report=xml"; \ + fi; \ + fi; \ + PYTHONUNBUFFERED=1 PYTEST_ADDOPTS="$$cov_addopts $${PYTEST_ADDOPTS:-}" \ + uv run --isolated --directory $< pytest 2>&1 | tee $(LOGS_DIR)/$*.log; \ rc=$$?; \ if [ $$rc -ne 0 ] && [ $$rc -ne 5 ]; then \ touch $(LOGS_DIR)/$*.failed; \ diff --git a/python/packages/jumpstarter-driver-adb/pyproject.toml b/python/packages/jumpstarter-driver-adb/pyproject.toml index 23764530e..4af9a3185 100644 --- a/python/packages/jumpstarter-driver-adb/pyproject.toml +++ b/python/packages/jumpstarter-driver-adb/pyproject.toml @@ -29,7 +29,6 @@ Homepage = "https://jumpstarter.dev" source_archive = "https://github.com/jumpstarter-dev/repo/archive/{commit_hash}.zip" [tool.pytest.ini_options] -addopts = "--cov --cov-report=html --cov-report=xml" log_cli = true log_cli_level = "INFO" testpaths = ["jumpstarter_driver_adb"] diff --git a/python/packages/jumpstarter-driver-androidemulator/pyproject.toml b/python/packages/jumpstarter-driver-androidemulator/pyproject.toml index f8cbfa319..9e39aa648 100644 --- a/python/packages/jumpstarter-driver-androidemulator/pyproject.toml +++ b/python/packages/jumpstarter-driver-androidemulator/pyproject.toml @@ -31,7 +31,6 @@ Homepage = "https://jumpstarter.dev" source_archive = "https://github.com/jumpstarter-dev/repo/archive/{commit_hash}.zip" [tool.pytest.ini_options] -addopts = "--cov --cov-report=html --cov-report=xml" log_cli = true log_cli_level = "INFO" testpaths = ["jumpstarter_driver_androidemulator"] diff --git a/python/packages/jumpstarter-driver-ble/pyproject.toml b/python/packages/jumpstarter-driver-ble/pyproject.toml index f06c65377..bfedc0e8b 100644 --- a/python/packages/jumpstarter-driver-ble/pyproject.toml +++ b/python/packages/jumpstarter-driver-ble/pyproject.toml @@ -26,7 +26,6 @@ Homepage = "https://jumpstarter.dev" source_archive = "https://github.com/jumpstarter-dev/repo/archive/{commit_hash}.zip" [tool.pytest.ini_options] -addopts = "--cov --cov-report=html --cov-report=xml" log_cli = true log_cli_level = "INFO" testpaths = ["jumpstarter_driver_ble"] diff --git a/python/packages/jumpstarter-driver-doip/pyproject.toml b/python/packages/jumpstarter-driver-doip/pyproject.toml index ef02705c3..0dc811fa4 100644 --- a/python/packages/jumpstarter-driver-doip/pyproject.toml +++ b/python/packages/jumpstarter-driver-doip/pyproject.toml @@ -31,7 +31,6 @@ source = "vcs" raw-options = { 'root' = '../../../' } [tool.pytest.ini_options] -addopts = "--cov --cov-report=html --cov-report=xml" log_cli = true log_cli_level = "INFO" testpaths = ["jumpstarter_driver_doip"] diff --git a/python/packages/jumpstarter-driver-dut-network/pyproject.toml b/python/packages/jumpstarter-driver-dut-network/pyproject.toml index 296e8dd2e..635e92cb7 100644 --- a/python/packages/jumpstarter-driver-dut-network/pyproject.toml +++ b/python/packages/jumpstarter-driver-dut-network/pyproject.toml @@ -25,7 +25,7 @@ Homepage = "https://jumpstarter.dev" source_archive = "https://github.com/jumpstarter-dev/jumpstarter/archive/{commit_hash}.zip" [tool.pytest.ini_options] -addopts = "--cov --cov-report=html --cov-report=xml -rs" +addopts = "-rs" log_cli = true log_cli_level = "INFO" testpaths = ["jumpstarter_driver_dut_network"] diff --git a/python/packages/jumpstarter-driver-esp32/pyproject.toml b/python/packages/jumpstarter-driver-esp32/pyproject.toml index 129379428..9af8c8d91 100644 --- a/python/packages/jumpstarter-driver-esp32/pyproject.toml +++ b/python/packages/jumpstarter-driver-esp32/pyproject.toml @@ -29,7 +29,6 @@ Homepage = "https://jumpstarter.dev" source_archive = "https://github.com/jumpstarter-dev/repo/archive/{commit_hash}.zip" [tool.pytest.ini_options] -addopts = "--cov --cov-report=html --cov-report=xml" log_cli = true log_cli_level = "INFO" testpaths = ["jumpstarter_driver_esp32"] diff --git a/python/packages/jumpstarter-driver-flashers/pyproject.toml b/python/packages/jumpstarter-driver-flashers/pyproject.toml index fabccdfa1..95496e697 100644 --- a/python/packages/jumpstarter-driver-flashers/pyproject.toml +++ b/python/packages/jumpstarter-driver-flashers/pyproject.toml @@ -34,7 +34,6 @@ Homepage = "https://jumpstarter.dev" source_archive = "https://github.com/jumpstarter-dev/repo/archive/{commit_hash}.zip" [tool.pytest.ini_options] -addopts = "--cov --cov-report=html --cov-report=xml" log_cli = true log_cli_level = "INFO" testpaths = ["jumpstarter_driver_flashers"] diff --git a/python/packages/jumpstarter-driver-http-power/pyproject.toml b/python/packages/jumpstarter-driver-http-power/pyproject.toml index f6c5dba94..62bcfd321 100644 --- a/python/packages/jumpstarter-driver-http-power/pyproject.toml +++ b/python/packages/jumpstarter-driver-http-power/pyproject.toml @@ -26,7 +26,6 @@ Homepage = "https://jumpstarter.dev" source_archive = "https://github.com/jumpstarter-dev/repo/archive/{commit_hash}.zip" [tool.pytest.ini_options] -addopts = "--cov --cov-report=html --cov-report=xml" log_cli = true log_cli_level = "INFO" testpaths = ["jumpstarter_driver_http_power"] diff --git a/python/packages/jumpstarter-driver-http/pyproject.toml b/python/packages/jumpstarter-driver-http/pyproject.toml index 0ab72a83a..181b936de 100644 --- a/python/packages/jumpstarter-driver-http/pyproject.toml +++ b/python/packages/jumpstarter-driver-http/pyproject.toml @@ -26,7 +26,6 @@ Homepage = "https://jumpstarter.dev" source_archive = "https://github.com/jumpstarter-dev/repo/archive/{commit_hash}.zip" [tool.pytest.ini_options] -addopts = "--cov=jumpstarter_driver_http --cov-report=html --cov-report=xml" asyncio_mode = "strict" asyncio_default_fixture_loop_scope = "function" testpaths = ["jumpstarter_driver_http"] diff --git a/python/packages/jumpstarter-driver-mitmproxy/pyproject.toml b/python/packages/jumpstarter-driver-mitmproxy/pyproject.toml index 654851ac5..4984c64d8 100644 --- a/python/packages/jumpstarter-driver-mitmproxy/pyproject.toml +++ b/python/packages/jumpstarter-driver-mitmproxy/pyproject.toml @@ -12,7 +12,6 @@ dependencies = ["jumpstarter", "mitmproxy>=10.0"] MitmproxyDriver = "jumpstarter_driver_mitmproxy.driver:MitmproxyDriver" [tool.pytest.ini_options] -addopts = "--cov --cov-report=html --cov-report=xml" log_cli = true log_cli_level = "INFO" testpaths = [ diff --git a/python/packages/jumpstarter-driver-noyito-relay/pyproject.toml b/python/packages/jumpstarter-driver-noyito-relay/pyproject.toml index 13ceaf5d8..e360531ea 100644 --- a/python/packages/jumpstarter-driver-noyito-relay/pyproject.toml +++ b/python/packages/jumpstarter-driver-noyito-relay/pyproject.toml @@ -28,7 +28,6 @@ Homepage = "https://jumpstarter.dev" source_archive = "https://github.com/jumpstarter-dev/repo/archive/{commit_hash}.zip" [tool.pytest.ini_options] -addopts = "--cov --cov-report=html --cov-report=xml" log_cli = true log_cli_level = "INFO" testpaths = ["jumpstarter_driver_noyito_relay"] diff --git a/python/packages/jumpstarter-driver-obd/pyproject.toml b/python/packages/jumpstarter-driver-obd/pyproject.toml index c35f48cc1..644154e30 100644 --- a/python/packages/jumpstarter-driver-obd/pyproject.toml +++ b/python/packages/jumpstarter-driver-obd/pyproject.toml @@ -25,7 +25,6 @@ Homepage = "https://jumpstarter.dev" source_archive = "https://github.com/jumpstarter-dev/repo/archive/{commit_hash}.zip" [tool.pytest.ini_options] -addopts = "--cov --cov-report=html --cov-report=xml" log_cli = true log_cli_level = "INFO" testpaths = ["jumpstarter_driver_obd"] diff --git a/python/packages/jumpstarter-driver-pi-pico/pyproject.toml b/python/packages/jumpstarter-driver-pi-pico/pyproject.toml index 346d72f01..d948115c1 100644 --- a/python/packages/jumpstarter-driver-pi-pico/pyproject.toml +++ b/python/packages/jumpstarter-driver-pi-pico/pyproject.toml @@ -28,7 +28,6 @@ Homepage = "https://jumpstarter.dev" source_archive = "https://github.com/jumpstarter-dev/repo/archive/{commit_hash}.zip" [tool.pytest.ini_options] -addopts = "--cov --cov-report=html --cov-report=xml" log_cli = true log_cli_level = "INFO" testpaths = ["jumpstarter_driver_pi_pico"] diff --git a/python/packages/jumpstarter-driver-probe-rs/pyproject.toml b/python/packages/jumpstarter-driver-probe-rs/pyproject.toml index b48f913cc..0080cc9e3 100644 --- a/python/packages/jumpstarter-driver-probe-rs/pyproject.toml +++ b/python/packages/jumpstarter-driver-probe-rs/pyproject.toml @@ -25,7 +25,6 @@ Homepage = "https://jumpstarter.dev" source_archive = "https://github.com/jumpstarter-dev/repo/archive/{commit_hash}.zip" [tool.pytest.ini_options] -addopts = "--cov --cov-report=html --cov-report=xml" log_cli = true log_cli_level = "INFO" testpaths = ["jumpstarter_driver_probe_rs"] diff --git a/python/packages/jumpstarter-driver-renode/pyproject.toml b/python/packages/jumpstarter-driver-renode/pyproject.toml index a11fa74d7..2fa2deb68 100644 --- a/python/packages/jumpstarter-driver-renode/pyproject.toml +++ b/python/packages/jumpstarter-driver-renode/pyproject.toml @@ -31,7 +31,6 @@ Homepage = "https://jumpstarter.dev" source_archive = "https://github.com/jumpstarter-dev/repo/archive/{commit_hash}.zip" [tool.pytest.ini_options] -addopts = "--cov --cov-report=html --cov-report=xml" log_cli = true log_cli_level = "INFO" testpaths = ["jumpstarter_driver_renode"] diff --git a/python/packages/jumpstarter-driver-ridesx/pyproject.toml b/python/packages/jumpstarter-driver-ridesx/pyproject.toml index 1bda732f3..bc63009a0 100644 --- a/python/packages/jumpstarter-driver-ridesx/pyproject.toml +++ b/python/packages/jumpstarter-driver-ridesx/pyproject.toml @@ -27,7 +27,6 @@ Homepage = "https://jumpstarter.dev" source_archive = "https://github.com/jumpstarter-dev/repo/archive/{commit_hash}.zip" [tool.pytest.ini_options] -addopts = "--cov --cov-report=html --cov-report=xml" log_cli = true log_cli_level = "INFO" testpaths = ["jumpstarter_driver_ridesx"] diff --git a/python/packages/jumpstarter-driver-shell/pyproject.toml b/python/packages/jumpstarter-driver-shell/pyproject.toml index a866cfc57..69ce3aa22 100644 --- a/python/packages/jumpstarter-driver-shell/pyproject.toml +++ b/python/packages/jumpstarter-driver-shell/pyproject.toml @@ -11,7 +11,6 @@ dependencies = ["anyio>=4.10.0", "jumpstarter", "click>=8.1.8"] [project.entry-points."jumpstarter.drivers"] Shell = "jumpstarter_driver_shell.driver:Shell" [tool.pytest.ini_options] -addopts = "--cov --cov-report=html --cov-report=xml" log_cli = true log_cli_level = "INFO" testpaths = ["jumpstarter_driver_shell"] diff --git a/python/packages/jumpstarter-driver-someip/pyproject.toml b/python/packages/jumpstarter-driver-someip/pyproject.toml index 22f5b1f32..6ad6f9f0f 100644 --- a/python/packages/jumpstarter-driver-someip/pyproject.toml +++ b/python/packages/jumpstarter-driver-someip/pyproject.toml @@ -31,7 +31,6 @@ source = "vcs" raw-options = { 'root' = '../../../' } [tool.pytest.ini_options] -addopts = "--cov --cov-report=html --cov-report=xml" log_cli = true log_cli_level = "INFO" testpaths = ["jumpstarter_driver_someip"] diff --git a/python/packages/jumpstarter-driver-ssh-mitm/pyproject.toml b/python/packages/jumpstarter-driver-ssh-mitm/pyproject.toml index 4d4ca9f2b..b9f73d2b8 100644 --- a/python/packages/jumpstarter-driver-ssh-mitm/pyproject.toml +++ b/python/packages/jumpstarter-driver-ssh-mitm/pyproject.toml @@ -27,7 +27,6 @@ Homepage = "https://jumpstarter.dev" source_archive = "https://github.com/jumpstarter-dev/repo/archive/{commit_hash}.zip" [tool.pytest.ini_options] -addopts = "--cov --cov-report=html --cov-report=xml" log_cli = true log_cli_level = "INFO" testpaths = ["jumpstarter_driver_ssh_mitm"] diff --git a/python/packages/jumpstarter-driver-ssh-mount/pyproject.toml b/python/packages/jumpstarter-driver-ssh-mount/pyproject.toml index c2264bfde..bb44537e2 100644 --- a/python/packages/jumpstarter-driver-ssh-mount/pyproject.toml +++ b/python/packages/jumpstarter-driver-ssh-mount/pyproject.toml @@ -28,7 +28,6 @@ Homepage = "https://jumpstarter.dev" source_archive = "https://github.com/jumpstarter-dev/repo/archive/{commit_hash}.zip" [tool.pytest.ini_options] -addopts = "--cov --cov-report=html --cov-report=xml" log_cli = true log_cli_level = "INFO" testpaths = ["jumpstarter_driver_ssh_mount"] diff --git a/python/packages/jumpstarter-driver-ssh/pyproject.toml b/python/packages/jumpstarter-driver-ssh/pyproject.toml index c8d95d3fa..0e62eb104 100644 --- a/python/packages/jumpstarter-driver-ssh/pyproject.toml +++ b/python/packages/jumpstarter-driver-ssh/pyproject.toml @@ -28,7 +28,6 @@ Homepage = "https://jumpstarter.dev" source_archive = "https://github.com/jumpstarter-dev/repo/archive/{commit_hash}.zip" [tool.pytest.ini_options] -addopts = "--cov --cov-report=html --cov-report=xml" log_cli = true log_cli_level = "INFO" testpaths = ["jumpstarter_driver_ssh"] diff --git a/python/packages/jumpstarter-driver-stlink-msd/pyproject.toml b/python/packages/jumpstarter-driver-stlink-msd/pyproject.toml index e3f07cb0d..bab37735d 100644 --- a/python/packages/jumpstarter-driver-stlink-msd/pyproject.toml +++ b/python/packages/jumpstarter-driver-stlink-msd/pyproject.toml @@ -27,7 +27,6 @@ Homepage = "https://jumpstarter.dev" source_archive = "https://github.com/jumpstarter-dev/repo/archive/{commit_hash}.zip" [tool.pytest.ini_options] -addopts = "--cov --cov-report=html --cov-report=xml" log_cli = true log_cli_level = "INFO" testpaths = ["jumpstarter_driver_stlink_msd"] diff --git a/python/packages/jumpstarter-driver-tasmota/pyproject.toml b/python/packages/jumpstarter-driver-tasmota/pyproject.toml index 696d957ca..82300aa7f 100644 --- a/python/packages/jumpstarter-driver-tasmota/pyproject.toml +++ b/python/packages/jumpstarter-driver-tasmota/pyproject.toml @@ -26,7 +26,6 @@ Homepage = "https://jumpstarter.dev" source_archive = "https://github.com/jumpstarter-dev/repo/archive/{commit_hash}.zip" [tool.pytest.ini_options] -addopts = "--cov --cov-report=html --cov-report=xml" log_cli = true log_cli_level = "INFO" testpaths = ["jumpstarter_driver_tasmota"] diff --git a/python/packages/jumpstarter-driver-tftp/pyproject.toml b/python/packages/jumpstarter-driver-tftp/pyproject.toml index 552ea68ff..ba19831f5 100644 --- a/python/packages/jumpstarter-driver-tftp/pyproject.toml +++ b/python/packages/jumpstarter-driver-tftp/pyproject.toml @@ -38,7 +38,6 @@ log_cli = true log_cli_level = "INFO" testpaths = ["jumpstarter_driver_tftp"] asyncio_mode = "auto" -addopts = "--cov --cov-report=html --cov-report=xml" [tool.coverage.run] source = ["."] diff --git a/python/packages/jumpstarter-driver-tmt/pyproject.toml b/python/packages/jumpstarter-driver-tmt/pyproject.toml index 200b4e84b..c94e7573c 100644 --- a/python/packages/jumpstarter-driver-tmt/pyproject.toml +++ b/python/packages/jumpstarter-driver-tmt/pyproject.toml @@ -27,7 +27,6 @@ Homepage = "https://jumpstarter.dev" source_archive = "https://github.com/jumpstarter-dev/repo/archive/{commit_hash}.zip" [tool.pytest.ini_options] -addopts = "" log_cli = true log_cli_level = "INFO" testpaths = ["jumpstarter_driver_tmt"] diff --git a/python/packages/jumpstarter-driver-uds-can/pyproject.toml b/python/packages/jumpstarter-driver-uds-can/pyproject.toml index 4e111ee7b..8c82e90ae 100644 --- a/python/packages/jumpstarter-driver-uds-can/pyproject.toml +++ b/python/packages/jumpstarter-driver-uds-can/pyproject.toml @@ -35,7 +35,6 @@ source = "vcs" raw-options = { 'root' = '../../../' } [tool.pytest.ini_options] -addopts = "--cov --cov-report=html --cov-report=xml" log_cli = true log_cli_level = "INFO" testpaths = ["jumpstarter_driver_uds_can"] diff --git a/python/packages/jumpstarter-driver-uds-doip/pyproject.toml b/python/packages/jumpstarter-driver-uds-doip/pyproject.toml index 1c0ddb826..2b40cbcf3 100644 --- a/python/packages/jumpstarter-driver-uds-doip/pyproject.toml +++ b/python/packages/jumpstarter-driver-uds-doip/pyproject.toml @@ -33,7 +33,6 @@ source = "vcs" raw-options = { 'root' = '../../../' } [tool.pytest.ini_options] -addopts = "--cov --cov-report=html --cov-report=xml" log_cli = true log_cli_level = "INFO" testpaths = ["jumpstarter_driver_uds_doip"] diff --git a/python/packages/jumpstarter-driver-uds/pyproject.toml b/python/packages/jumpstarter-driver-uds/pyproject.toml index 3ecc8ad0e..ec7beb34e 100644 --- a/python/packages/jumpstarter-driver-uds/pyproject.toml +++ b/python/packages/jumpstarter-driver-uds/pyproject.toml @@ -28,7 +28,6 @@ source = "vcs" raw-options = { 'root' = '../../../' } [tool.pytest.ini_options] -addopts = "--cov --cov-report=html --cov-report=xml" log_cli = true log_cli_level = "INFO" testpaths = ["jumpstarter_driver_uds"] diff --git a/python/packages/jumpstarter-driver-vnc/pyproject.toml b/python/packages/jumpstarter-driver-vnc/pyproject.toml index 93aabbcdc..2e84d2690 100644 --- a/python/packages/jumpstarter-driver-vnc/pyproject.toml +++ b/python/packages/jumpstarter-driver-vnc/pyproject.toml @@ -28,7 +28,6 @@ Homepage = "https://jumpstarter.dev" source_archive = "https://github.com/jumpstarter-dev/jumpstarter/archive/{commit_hash}.zip" [tool.pytest.ini_options] -addopts = "--cov --cov-report=html --cov-report=xml" log_cli = true log_cli_level = "INFO" testpaths = ["jumpstarter_driver_vnc"] diff --git a/python/packages/jumpstarter-driver-xcp/pyproject.toml b/python/packages/jumpstarter-driver-xcp/pyproject.toml index d2fdcba5a..b75bff278 100644 --- a/python/packages/jumpstarter-driver-xcp/pyproject.toml +++ b/python/packages/jumpstarter-driver-xcp/pyproject.toml @@ -25,7 +25,6 @@ Homepage = "https://jumpstarter.dev" source_archive = "https://github.com/jumpstarter-dev/repo/archive/{commit_hash}.zip" [tool.pytest.ini_options] -addopts = "--cov --cov-report=html --cov-report=xml" log_cli = true log_cli_level = "INFO" testpaths = ["jumpstarter_driver_xcp"] diff --git a/python/packages/jumpstarter-driver-yepkit/pyproject.toml b/python/packages/jumpstarter-driver-yepkit/pyproject.toml index a90ae2df7..cd165cafa 100644 --- a/python/packages/jumpstarter-driver-yepkit/pyproject.toml +++ b/python/packages/jumpstarter-driver-yepkit/pyproject.toml @@ -26,7 +26,6 @@ Homepage = "https://jumpstarter.dev" source_archive = "https://github.com/jumpstarter-dev/repo/archive/{commit_hash}.zip" [tool.pytest.ini_options] -addopts = "--cov --cov-report=html --cov-report=xml" log_cli = true log_cli_level = "INFO" testpaths = ["jumpstarter_driver_yepkit"] diff --git a/python/pyproject.toml b/python/pyproject.toml index f0f1dcef4..f61726a3b 100644 --- a/python/pyproject.toml +++ b/python/pyproject.toml @@ -110,7 +110,7 @@ omit = ["conftest.py", "test_*.py", "*_test.py", "*_pb2.py", "*_pb2_grpc.py"] skip_empty = true [tool.pytest.ini_options] -addopts = "--capture=no --doctest-modules --cov=. --cov-report=html --cov-report=xml" +addopts = "--capture=no" [tool.hatch.version] source = "vcs" From c6e4db80f3fb31d0c233a697cceb14bbf10a112c Mon Sep 17 00:00:00 2001 From: Benny Zlotnik Date: Mon, 10 Aug 2026 08:41:50 +0300 Subject: [PATCH 19/24] fix(ci): add --no-network to virt-customize passt (usermode network stack) fails on GH Actions runners. None of the image pre-configuration steps need network access. Co-authored-by: Cursor --- .github/workflows/python-tests.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/python-tests.yaml b/.github/workflows/python-tests.yaml index 44a54b0af..69ea890cd 100644 --- a/.github/workflows/python-tests.yaml +++ b/.github/workflows/python-tests.yaml @@ -161,7 +161,7 @@ jobs: sudo apt-get install -y libguestfs-tools arch=x86_64 img="python/packages/jumpstarter-driver-qemu/images/Fedora-Cloud-Base-Generic-43-1.6.${arch}.qcow2" - sudo virt-customize -a "$img" \ + sudo virt-customize --no-network -a "$img" \ --hostname demo \ --run-command 'useradd -m -s /bin/bash jumpstarter' \ --password jumpstarter:password:password \ From 9fc908897e9ed2308c384cb863c1bcd1182d2048 Mon Sep 17 00:00:00 2001 From: Benny Zlotnik Date: Mon, 10 Aug 2026 09:05:09 +0300 Subject: [PATCH 20/24] perf(mitmproxy): share proxy process across tests in a class Class-scoped fixtures keep mitmdump running across tests within each class. Mocks are cleared and reconfigured between tests via hot-reload instead of full process restart (~3s savings per test). Co-authored-by: Cursor --- .../driver_integration_test.py | 776 ++++++++---------- 1 file changed, 328 insertions(+), 448 deletions(-) diff --git a/python/packages/jumpstarter-driver-mitmproxy/jumpstarter_driver_mitmproxy/driver_integration_test.py b/python/packages/jumpstarter-driver-mitmproxy/jumpstarter_driver_mitmproxy/driver_integration_test.py index 89ee10ae4..67a824945 100644 --- a/python/packages/jumpstarter-driver-mitmproxy/jumpstarter_driver_mitmproxy/driver_integration_test.py +++ b/python/packages/jumpstarter-driver-mitmproxy/jumpstarter_driver_mitmproxy/driver_integration_test.py @@ -72,19 +72,20 @@ def _is_mitmdump_available() -> bool: ) -@pytest.fixture +@pytest.fixture(scope="class") def proxy_port(): return _free_port() -@pytest.fixture +@pytest.fixture(scope="class") def web_port(): return _free_port() -@pytest.fixture -def client(tmp_path, proxy_port, web_port): +@pytest.fixture(scope="class") +def client(tmp_path_factory, proxy_port, web_port): """Create a MitmproxyDriver wrapped in Jumpstarter's local serve harness.""" + tmp_path = tmp_path_factory.mktemp("mitmproxy") instance = MitmproxyDriver( listen={"host": "127.0.0.1", "port": proxy_port}, web={"host": "127.0.0.1", "port": web_port}, @@ -159,169 +160,134 @@ def test_start_passthrough_mode(self, client, proxy_port): class TestMockEndpoints: """Mock configuration + real HTTP requests through the proxy.""" + @pytest.fixture(autouse=True) + def _proxy_lifecycle(self, client, proxy_port): + """Start the proxy once for the class, clear mocks between tests.""" + client.clear_mocks() + if not client.is_running(): + client.start("mock") + assert _wait_for_port("127.0.0.1", proxy_port) + yield + def test_simple_mock_response(self, client, proxy_port): - _start_mock_with_endpoints(client, proxy_port, [ - ("GET", "/api/v1/status", { - "body": {"id": "test-001", "online": True}, - }), - ]) + client.set_mock("GET", "/api/v1/status", body={"id": "test-001", "online": True}) + time.sleep(0.3) - try: - response = requests.get( - "http://example.com/api/v1/status", - proxies={"http": f"http://127.0.0.1:{proxy_port}"}, - timeout=10, - ) - assert response.status_code == 200 - data = response.json() - assert data["id"] == "test-001" - assert data["online"] is True - finally: - client.stop() + response = requests.get( + "http://example.com/api/v1/status", + proxies={"http": f"http://127.0.0.1:{proxy_port}"}, + timeout=10, + ) + assert response.status_code == 200 + data = response.json() + assert data["id"] == "test-001" + assert data["online"] is True def test_multiple_mock_endpoints(self, client, proxy_port): - _start_mock_with_endpoints(client, proxy_port, [ - ("GET", "/api/v1/health", {"body": {"ok": True}}), - ("POST", "/api/v1/telemetry", { - "status": 202, "body": {"accepted": True}, - }), - ]) + client.set_mock("GET", "/api/v1/health", body={"ok": True}) + client.set_mock("POST", "/api/v1/telemetry", status=202, body={"accepted": True}) + time.sleep(0.3) - try: - proxies = {"http": f"http://127.0.0.1:{proxy_port}"} + proxies = {"http": f"http://127.0.0.1:{proxy_port}"} - resp_get = requests.get( - "http://example.com/api/v1/health", - proxies=proxies, timeout=10, - ) - assert resp_get.status_code == 200 - assert resp_get.json()["ok"] is True + resp_get = requests.get( + "http://example.com/api/v1/health", + proxies=proxies, timeout=10, + ) + assert resp_get.status_code == 200 + assert resp_get.json()["ok"] is True - resp_post = requests.post( - "http://example.com/api/v1/telemetry", - proxies=proxies, timeout=10, - ) - assert resp_post.status_code == 202 - assert resp_post.json()["accepted"] is True - finally: - client.stop() + resp_post = requests.post( + "http://example.com/api/v1/telemetry", + proxies=proxies, timeout=10, + ) + assert resp_post.status_code == 202 + assert resp_post.json()["accepted"] is True def test_mock_error_status_codes(self, client, proxy_port): - _start_mock_with_endpoints(client, proxy_port, [ - ("GET", "/api/v1/missing", { - "status": 404, "body": {"error": "not found"}, - }), - ("GET", "/api/v1/broken", { - "status": 500, "body": {"error": "internal error"}, - }), - ]) + client.set_mock("GET", "/api/v1/missing", status=404, body={"error": "not found"}) + client.set_mock("GET", "/api/v1/broken", status=500, body={"error": "internal error"}) + time.sleep(0.3) - try: - proxies = {"http": f"http://127.0.0.1:{proxy_port}"} + proxies = {"http": f"http://127.0.0.1:{proxy_port}"} - resp_404 = requests.get( - "http://example.com/api/v1/missing", - proxies=proxies, timeout=10, - ) - assert resp_404.status_code == 404 - assert resp_404.json()["error"] == "not found" + resp_404 = requests.get( + "http://example.com/api/v1/missing", + proxies=proxies, timeout=10, + ) + assert resp_404.status_code == 404 + assert resp_404.json()["error"] == "not found" - resp_500 = requests.get( - "http://example.com/api/v1/broken", - proxies=proxies, timeout=10, - ) - assert resp_500.status_code == 500 - assert resp_500.json()["error"] == "internal error" - finally: - client.stop() + resp_500 = requests.get( + "http://example.com/api/v1/broken", + proxies=proxies, timeout=10, + ) + assert resp_500.status_code == 500 + assert resp_500.json()["error"] == "internal error" def test_clear_mocks(self, client, proxy_port): client.set_mock("GET", "/a", body={"x": 1}) client.set_mock("GET", "/b", body={"x": 2}) - client.start("mock") - try: - result = client.clear_mocks() - assert "Cleared 2" in result + result = client.clear_mocks() + assert "Cleared 2" in result - mocks = client.list_mocks() - assert len(mocks) == 0 - finally: - client.stop() + mocks = client.list_mocks() + assert len(mocks) == 0 def test_remove_single_mock(self, client, proxy_port): client.set_mock("GET", "/keep", body={"x": 1}) client.set_mock("GET", "/remove", body={"x": 2}) - client.start("mock") - try: - client.remove_mock("GET", "/remove") + client.remove_mock("GET", "/remove") - mocks = client.list_mocks() - assert "GET /keep" in mocks - assert "GET /remove" not in mocks - finally: - client.stop() + mocks = client.list_mocks() + assert "GET /keep" in mocks + assert "GET /remove" not in mocks def test_context_manager_mock_endpoint(self, client, proxy_port): - _start_mock_with_endpoints(client, proxy_port, [ - ("GET", "/api/v1/base", {"body": {"base": True}}), - ]) + client.set_mock("GET", "/api/v1/base", body={"base": True}) + time.sleep(0.3) - try: - proxies = {"http": f"http://127.0.0.1:{proxy_port}"} + proxies = {"http": f"http://127.0.0.1:{proxy_port}"} + + resp_base = requests.get( + "http://example.com/api/v1/base", + proxies=proxies, timeout=10, + ) + assert resp_base.status_code == 200 + assert resp_base.json()["base"] is True - # Verify base mock works - resp_base = requests.get( - "http://example.com/api/v1/base", + with client.mock_endpoint( + "GET", "/api/v1/temp", + body={"temporary": True}, + ): + time.sleep(1) + response = requests.get( + "http://example.com/api/v1/temp", proxies=proxies, timeout=10, ) - assert resp_base.status_code == 200 - assert resp_base.json()["base"] is True - - # Use mock_endpoint context manager to add a temporary mock - with client.mock_endpoint( - "GET", "/api/v1/temp", - body={"temporary": True}, - ): - # Allow addon to detect config change - time.sleep(1) - response = requests.get( - "http://example.com/api/v1/temp", - proxies=proxies, timeout=10, - ) - assert response.status_code == 200 - assert response.json()["temporary"] is True - - # After exiting the context manager, mock should be removed - mocks = client.list_mocks() - assert "GET /api/v1/temp" not in mocks - finally: - client.stop() + assert response.status_code == 200 + assert response.json()["temporary"] is True + + mocks = client.list_mocks() + assert "GET /api/v1/temp" not in mocks def test_hot_reload_mocks(self, client, proxy_port): """Verify that mocks added after start are picked up via hot-reload.""" - client.start("mock") - assert _wait_for_port("127.0.0.1", proxy_port) - - try: - # Set mock after proxy is already running - client.set_mock( - "GET", "/api/v1/hotreload", - body={"reloaded": True}, - ) - # Give the addon time to detect the file change on next request - time.sleep(1) + client.set_mock( + "GET", "/api/v1/hotreload", + body={"reloaded": True}, + ) + time.sleep(0.3) - response = requests.get( - "http://example.com/api/v1/hotreload", - proxies={"http": f"http://127.0.0.1:{proxy_port}"}, - timeout=10, - ) - assert response.status_code == 200 - assert response.json()["reloaded"] is True - finally: - client.stop() + response = requests.get( + "http://example.com/api/v1/hotreload", + proxies={"http": f"http://127.0.0.1:{proxy_port}"}, + timeout=10, + ) + assert response.status_code == 200 + assert response.json()["reloaded"] is True class TestPassthrough: @@ -359,190 +325,134 @@ def test_passthrough_to_local_server(self, client, proxy_port, upstream): class TestRequestCapture: """End-to-end tests for request capture via the proxy.""" + @pytest.fixture(autouse=True) + def _proxy_lifecycle(self, client, proxy_port): + """Start the proxy once for the class, clear state between tests.""" + client.clear_mocks() + client.set_mock("GET", "/api/v1/status", body={"id": "test-001", "online": True}) + client.set_mock("GET", "/api/v1/health", body={"ok": True}) + client.set_mock("GET", "/api/v1/delayed", body={"ok": True}) + client.set_mock("GET", "/api/v1/first", body={"n": 1}) + client.set_mock("GET", "/api/v1/second", body={"n": 2}) + client.set_mock("GET", "/api/v1/third", body={"n": 3}) + if not client.is_running(): + client.start("mock") + assert _wait_for_port("127.0.0.1", proxy_port) + time.sleep(0.3) + client.clear_captured_requests() + yield + def test_captured_requests_appear(self, client, proxy_port): - _start_mock_with_endpoints(client, proxy_port, [ - ("GET", "/api/v1/status", { - "body": {"id": "test-001", "online": True}, - }), - ]) + requests.get( + "http://example.com/api/v1/status", + proxies={"http": f"http://127.0.0.1:{proxy_port}"}, + timeout=10, + ) + result = client.wait_for_request("GET", "/api/v1/status", 5.0) + assert result["method"] == "GET" + assert result["path"] == "/api/v1/status" + assert result["response_status"] == 200 + assert result["was_mocked"] is True - try: - client.clear_captured_requests() + def test_clear_captured_requests(self, client, proxy_port): + requests.get( + "http://example.com/api/v1/health", + proxies={"http": f"http://127.0.0.1:{proxy_port}"}, + timeout=10, + ) + client.wait_for_request("GET", "/api/v1/health", 5.0) - requests.get( - "http://example.com/api/v1/status", - proxies={"http": f"http://127.0.0.1:{proxy_port}"}, - timeout=10, - ) - # Wait for the capture event to arrive - result = client.wait_for_request("GET", "/api/v1/status", 5.0) - assert result["method"] == "GET" - assert result["path"] == "/api/v1/status" - assert result["response_status"] == 200 - assert result["was_mocked"] is True - finally: - client.stop() + result = client.clear_captured_requests() + assert "Cleared" in result - def test_clear_captured_requests(self, client, proxy_port): - _start_mock_with_endpoints(client, proxy_port, [ - ("GET", "/api/v1/health", {"body": {"ok": True}}), - ]) + captured = client.get_captured_requests() + assert len(captured) == 0 - try: + def test_wait_for_request(self, client, proxy_port): + def delayed_request(): + time.sleep(1) requests.get( - "http://example.com/api/v1/health", + "http://example.com/api/v1/delayed", proxies={"http": f"http://127.0.0.1:{proxy_port}"}, timeout=10, ) - # Wait for capture - client.wait_for_request("GET", "/api/v1/health", 5.0) - result = client.clear_captured_requests() - assert "Cleared" in result + t = threading.Thread(target=delayed_request) + t.start() - captured = client.get_captured_requests() - assert len(captured) == 0 - finally: - client.stop() - - def test_wait_for_request(self, client, proxy_port): - _start_mock_with_endpoints(client, proxy_port, [ - ("GET", "/api/v1/delayed", {"body": {"ok": True}}), - ]) + result = client.wait_for_request("GET", "/api/v1/delayed", 10.0) + assert result["method"] == "GET" + assert result["path"] == "/api/v1/delayed" - try: - client.clear_captured_requests() - - # Send request after a short delay in background - def delayed_request(): - time.sleep(1) - requests.get( - "http://example.com/api/v1/delayed", - proxies={"http": f"http://127.0.0.1:{proxy_port}"}, - timeout=10, - ) - - t = threading.Thread(target=delayed_request) - t.start() - - result = client.wait_for_request("GET", "/api/v1/delayed", 10.0) - assert result["method"] == "GET" - assert result["path"] == "/api/v1/delayed" - - t.join(timeout=5) - finally: - client.stop() + t.join(timeout=5) def test_wait_for_request_timeout(self, client, proxy_port): - _start_mock_with_endpoints(client, proxy_port, [ - ("GET", "/api/v1/status", {"body": {"ok": True}}), - ]) - - try: - client.clear_captured_requests() - with pytest.raises(TimeoutError): - client.wait_for_request("GET", "/api/nonexistent", 1.0) - finally: - client.stop() + with pytest.raises(TimeoutError): + client.wait_for_request("GET", "/api/nonexistent", 1.0) def test_capture_context_manager(self, client, proxy_port): - _start_mock_with_endpoints(client, proxy_port, [ - ("GET", "/api/v1/status", { - "body": {"id": "test-001"}, - }), - ]) - - try: - with client.capture() as cap: - requests.get( - "http://example.com/api/v1/status", - proxies={"http": f"http://127.0.0.1:{proxy_port}"}, - timeout=10, - ) - cap.wait_for_request("GET", "/api/v1/status", 5.0) - - # After exit, snapshot is frozen - assert len(cap.requests) >= 1 - assert cap.requests[0]["method"] == "GET" - finally: - client.stop() - - def test_assert_request_made(self, client, proxy_port): - _start_mock_with_endpoints(client, proxy_port, [ - ("GET", "/api/v1/health", {"body": {"ok": True}}), - ]) - - try: - client.clear_captured_requests() - + with client.capture() as cap: requests.get( - "http://example.com/api/v1/health", + "http://example.com/api/v1/status", proxies={"http": f"http://127.0.0.1:{proxy_port}"}, timeout=10, ) - # Wait for capture to arrive - client.wait_for_request("GET", "/api/v1/health", 5.0) + cap.wait_for_request("GET", "/api/v1/status", 5.0) - # Should pass - result = client.assert_request_made("GET", "/api/v1/health") - assert result["method"] == "GET" + assert len(cap.requests) >= 1 + assert cap.requests[0]["method"] == "GET" - # Should fail - with pytest.raises(AssertionError, match="not captured"): - client.assert_request_made("POST", "/api/v1/missing") - finally: - client.stop() + def test_assert_request_made(self, client, proxy_port): + requests.get( + "http://example.com/api/v1/health", + proxies={"http": f"http://127.0.0.1:{proxy_port}"}, + timeout=10, + ) + client.wait_for_request("GET", "/api/v1/health", 5.0) - def test_multiple_requests_captured_in_order(self, client, proxy_port): - _start_mock_with_endpoints(client, proxy_port, [ - ("GET", "/api/v1/first", {"body": {"n": 1}}), - ("GET", "/api/v1/second", {"body": {"n": 2}}), - ("GET", "/api/v1/third", {"body": {"n": 3}}), - ]) + result = client.assert_request_made("GET", "/api/v1/health") + assert result["method"] == "GET" - try: - client.clear_captured_requests() - proxies = {"http": f"http://127.0.0.1:{proxy_port}"} + with pytest.raises(AssertionError, match="not captured"): + client.assert_request_made("POST", "/api/v1/missing") - requests.get( - "http://example.com/api/v1/first", - proxies=proxies, timeout=10, - ) - requests.get( - "http://example.com/api/v1/second", - proxies=proxies, timeout=10, - ) - requests.get( - "http://example.com/api/v1/third", - proxies=proxies, timeout=10, - ) + def test_multiple_requests_captured_in_order(self, client, proxy_port): + proxies = {"http": f"http://127.0.0.1:{proxy_port}"} - # Wait for the last request to be captured - client.wait_for_request("GET", "/api/v1/third", 5.0) + requests.get("http://example.com/api/v1/first", proxies=proxies, timeout=10) + requests.get("http://example.com/api/v1/second", proxies=proxies, timeout=10) + requests.get("http://example.com/api/v1/third", proxies=proxies, timeout=10) - captured = client.get_captured_requests() - assert len(captured) >= 3 + client.wait_for_request("GET", "/api/v1/third", 5.0) - paths = [r["path"] for r in captured] - assert "/api/v1/first" in paths - assert "/api/v1/second" in paths - assert "/api/v1/third" in paths + captured = client.get_captured_requests() + assert len(captured) >= 3 - # Verify ordering: first should appear before second, - # second before third - idx_first = paths.index("/api/v1/first") - idx_second = paths.index("/api/v1/second") - idx_third = paths.index("/api/v1/third") - assert idx_first < idx_second < idx_third - finally: - client.stop() + paths = [r["path"] for r in captured] + assert "/api/v1/first" in paths + assert "/api/v1/second" in paths + assert "/api/v1/third" in paths + + idx_first = paths.index("/api/v1/first") + idx_second = paths.index("/api/v1/second") + idx_third = paths.index("/api/v1/third") + assert idx_first < idx_second < idx_third class TestConditionalMocks: """Conditional mock rules with real HTTP requests through the proxy.""" + @pytest.fixture(autouse=True) + def _proxy_lifecycle(self, client, proxy_port): + """Start the proxy once for the class, clear mocks between tests.""" + client.clear_mocks() + if not client.is_running(): + client.start("mock") + assert _wait_for_port("127.0.0.1", proxy_port) + yield + def test_conditional_body_json_match(self, client, proxy_port): - """POST with matching JSON body → 200, non-matching → 401.""" + """POST with matching JSON body -> 200, non-matching -> 401.""" client.set_mock_conditional("POST", "/api/auth", [ { "match": {"body_json": {"username": "admin", @@ -552,34 +462,28 @@ def test_conditional_body_json_match(self, client, proxy_port): }, {"status": 401, "body": {"error": "unauthorized"}}, ]) - client.start("mock") - assert _wait_for_port("127.0.0.1", proxy_port) + time.sleep(0.3) - try: - proxies = {"http": f"http://127.0.0.1:{proxy_port}"} + proxies = {"http": f"http://127.0.0.1:{proxy_port}"} - # Matching credentials → 200 - resp_ok = requests.post( - "http://example.com/api/auth", - json={"username": "admin", "password": "secret"}, - proxies=proxies, timeout=10, - ) - assert resp_ok.status_code == 200 - assert resp_ok.json()["token"] == "mock-token-001" + resp_ok = requests.post( + "http://example.com/api/auth", + json={"username": "admin", "password": "secret"}, + proxies=proxies, timeout=10, + ) + assert resp_ok.status_code == 200 + assert resp_ok.json()["token"] == "mock-token-001" - # Wrong credentials → 401 (fallback) - resp_fail = requests.post( - "http://example.com/api/auth", - json={"username": "hacker", "password": "wrong"}, - proxies=proxies, timeout=10, - ) - assert resp_fail.status_code == 401 - assert resp_fail.json()["error"] == "unauthorized" - finally: - client.stop() + resp_fail = requests.post( + "http://example.com/api/auth", + json={"username": "hacker", "password": "wrong"}, + proxies=proxies, timeout=10, + ) + assert resp_fail.status_code == 401 + assert resp_fail.json()["error"] == "unauthorized" def test_conditional_header_match(self, client, proxy_port): - """GET with matching header → 200, without → 401.""" + """GET with matching header -> 200, without -> 401.""" client.set_mock_conditional("GET", "/api/data", [ { "match": {"headers": {"Authorization": "Bearer tok123"}}, @@ -588,32 +492,26 @@ def test_conditional_header_match(self, client, proxy_port): }, {"status": 401, "body": {"error": "unauthorized"}}, ]) - client.start("mock") - assert _wait_for_port("127.0.0.1", proxy_port) + time.sleep(0.3) - try: - proxies = {"http": f"http://127.0.0.1:{proxy_port}"} + proxies = {"http": f"http://127.0.0.1:{proxy_port}"} - # With correct auth header → 200 - resp_ok = requests.get( - "http://example.com/api/data", - headers={"Authorization": "Bearer tok123"}, - proxies=proxies, timeout=10, - ) - assert resp_ok.status_code == 200 - assert resp_ok.json()["items"] == [1, 2, 3] + resp_ok = requests.get( + "http://example.com/api/data", + headers={"Authorization": "Bearer tok123"}, + proxies=proxies, timeout=10, + ) + assert resp_ok.status_code == 200 + assert resp_ok.json()["items"] == [1, 2, 3] - # Without auth header → 401 - resp_fail = requests.get( - "http://example.com/api/data", - proxies=proxies, timeout=10, - ) - assert resp_fail.status_code == 401 - finally: - client.stop() + resp_fail = requests.get( + "http://example.com/api/data", + proxies=proxies, timeout=10, + ) + assert resp_fail.status_code == 401 def test_conditional_query_match(self, client, proxy_port): - """GET with matching query param → 200, without → default.""" + """GET with matching query param -> 200, without -> default.""" client.set_mock_conditional("GET", "/api/search", [ { "match": {"query": {"q": "hello"}}, @@ -622,29 +520,23 @@ def test_conditional_query_match(self, client, proxy_port): }, {"status": 200, "body": {"results": []}}, ]) - client.start("mock") - assert _wait_for_port("127.0.0.1", proxy_port) + time.sleep(0.3) - try: - proxies = {"http": f"http://127.0.0.1:{proxy_port}"} + proxies = {"http": f"http://127.0.0.1:{proxy_port}"} - # Matching query param - resp_match = requests.get( - "http://example.com/api/search?q=hello", - proxies=proxies, timeout=10, - ) - assert resp_match.status_code == 200 - assert resp_match.json()["results"] == ["hello world"] + resp_match = requests.get( + "http://example.com/api/search?q=hello", + proxies=proxies, timeout=10, + ) + assert resp_match.status_code == 200 + assert resp_match.json()["results"] == ["hello world"] - # No query param → fallback - resp_default = requests.get( - "http://example.com/api/search", - proxies=proxies, timeout=10, - ) - assert resp_default.status_code == 200 - assert resp_default.json()["results"] == [] - finally: - client.stop() + resp_default = requests.get( + "http://example.com/api/search", + proxies=proxies, timeout=10, + ) + assert resp_default.status_code == 200 + assert resp_default.json()["results"] == [] def test_conditional_with_template(self, client, proxy_port): """Rule containing body_template with dynamic expressions.""" @@ -659,57 +551,56 @@ def test_conditional_with_template(self, client, proxy_port): }, {"status": 200, "body": {"mode": "static"}}, ]) - client.start("mock") - assert _wait_for_port("127.0.0.1", proxy_port) + time.sleep(0.3) - try: - proxies = {"http": f"http://127.0.0.1:{proxy_port}"} + proxies = {"http": f"http://127.0.0.1:{proxy_port}"} - # With dynamic header → template response - resp = requests.get( - "http://example.com/api/echo", - headers={"X-Mode": "dynamic"}, - proxies=proxies, timeout=10, - ) - assert resp.status_code == 200 - data = resp.json() - assert data["mode"] == "dynamic" - assert "/api/echo" in data["path"] - - # Without header → static fallback - resp_static = requests.get( - "http://example.com/api/echo", - proxies=proxies, timeout=10, - ) - assert resp_static.json()["mode"] == "static" - finally: - client.stop() + resp = requests.get( + "http://example.com/api/echo", + headers={"X-Mode": "dynamic"}, + proxies=proxies, timeout=10, + ) + assert resp.status_code == 200 + data = resp.json() + assert data["mode"] == "dynamic" + assert "/api/echo" in data["path"] + + resp_static = requests.get( + "http://example.com/api/echo", + proxies=proxies, timeout=10, + ) + assert resp_static.json()["mode"] == "static" class TestEnhancedTemplates: """Tests for enhanced template expressions with real HTTP requests.""" + @pytest.fixture(autouse=True) + def _proxy_lifecycle(self, client, proxy_port): + """Start the proxy once for the class, clear mocks between tests.""" + client.clear_mocks() + if not client.is_running(): + client.start("mock") + assert _wait_for_port("127.0.0.1", proxy_port) + yield + def test_request_body_json_in_template(self, client, proxy_port): """Echo a JSON field from request body via template.""" client.set_mock_template( "POST", "/api/echo", template={"echoed_name": "{{request_body_json(name)}}"}, ) - client.start("mock") - assert _wait_for_port("127.0.0.1", proxy_port) + time.sleep(0.3) - try: - proxies = {"http": f"http://127.0.0.1:{proxy_port}"} + proxies = {"http": f"http://127.0.0.1:{proxy_port}"} - resp = requests.post( - "http://example.com/api/echo", - json={"name": "Alice", "age": 30}, - proxies=proxies, timeout=10, - ) - assert resp.status_code == 200 - assert resp.json()["echoed_name"] == "Alice" - finally: - client.stop() + resp = requests.post( + "http://example.com/api/echo", + json={"name": "Alice", "age": 30}, + proxies=proxies, timeout=10, + ) + assert resp.status_code == 200 + assert resp.json()["echoed_name"] == "Alice" def test_request_query_in_template(self, client, proxy_port): """Echo a query param in the response via template.""" @@ -717,20 +608,16 @@ def test_request_query_in_template(self, client, proxy_port): "GET", "/api/greet", template={"greeting": "Hello, {{request_query(name)}}!"}, ) - client.start("mock") - assert _wait_for_port("127.0.0.1", proxy_port) + time.sleep(0.3) - try: - proxies = {"http": f"http://127.0.0.1:{proxy_port}"} + proxies = {"http": f"http://127.0.0.1:{proxy_port}"} - resp = requests.get( - "http://example.com/api/greet?name=Bob", - proxies=proxies, timeout=10, - ) - assert resp.status_code == 200 - assert resp.json()["greeting"] == "Hello, Bob!" - finally: - client.stop() + resp = requests.get( + "http://example.com/api/greet?name=Bob", + proxies=proxies, timeout=10, + ) + assert resp.status_code == 200 + assert resp.json()["greeting"] == "Hello, Bob!" def test_state_in_template(self, client, proxy_port): """Set state then read it via {{state(key)}} in template.""" @@ -739,28 +626,32 @@ def test_state_in_template(self, client, proxy_port): "GET", "/api/whoami", template={"user": "{{state(current_user)}}"}, ) - client.start("mock") - assert _wait_for_port("127.0.0.1", proxy_port) + time.sleep(0.3) - try: - proxies = {"http": f"http://127.0.0.1:{proxy_port}"} + proxies = {"http": f"http://127.0.0.1:{proxy_port}"} - resp = requests.get( - "http://example.com/api/whoami", - proxies=proxies, timeout=10, - ) - assert resp.status_code == 200 - assert resp.json()["user"] == "Alice" - finally: - client.stop() + resp = requests.get( + "http://example.com/api/whoami", + proxies=proxies, timeout=10, + ) + assert resp.status_code == 200 + assert resp.json()["user"] == "Alice" class TestAuthScenario: """Full auth token flow using conditional rules.""" + @pytest.fixture(autouse=True) + def _proxy_lifecycle(self, client, proxy_port): + """Start the proxy once for the class, clear mocks between tests.""" + client.clear_mocks() + if not client.is_running(): + client.start("mock") + assert _wait_for_port("127.0.0.1", proxy_port) + yield + def test_auth_token_flow(self, client, proxy_port): - """Login with credentials → get token → use token for data.""" - # Auth endpoint: correct creds → token, else 401 + """Login with credentials -> get token -> use token for data.""" client.set_mock_conditional("POST", "/api/auth", [ { "match": {"body_json": {"username": "admin", @@ -770,8 +661,6 @@ def test_auth_token_flow(self, client, proxy_port): }, {"status": 401, "body": {"error": "unauthorized"}}, ]) - - # Data endpoint: valid token → data, else 401 client.set_mock_conditional("GET", "/api/data", [ { "match": {"headers": { @@ -782,45 +671,36 @@ def test_auth_token_flow(self, client, proxy_port): }, {"status": 401, "body": {"error": "unauthorized"}}, ]) + time.sleep(0.3) - client.start("mock") - assert _wait_for_port("127.0.0.1", proxy_port) - - try: - proxies = {"http": f"http://127.0.0.1:{proxy_port}"} + proxies = {"http": f"http://127.0.0.1:{proxy_port}"} - # Step 1: Login with correct credentials - login_resp = requests.post( - "http://example.com/api/auth", - json={"username": "admin", "password": "secret"}, - proxies=proxies, timeout=10, - ) - assert login_resp.status_code == 200 - token = login_resp.json()["token"] - assert token == "mock-token-001" - - # Step 2: Access data with token - data_resp = requests.get( - "http://example.com/api/data", - headers={"Authorization": f"Bearer {token}"}, - proxies=proxies, timeout=10, - ) - assert data_resp.status_code == 200 - assert data_resp.json()["items"] == [1, 2, 3] + login_resp = requests.post( + "http://example.com/api/auth", + json={"username": "admin", "password": "secret"}, + proxies=proxies, timeout=10, + ) + assert login_resp.status_code == 200 + token = login_resp.json()["token"] + assert token == "mock-token-001" + + data_resp = requests.get( + "http://example.com/api/data", + headers={"Authorization": f"Bearer {token}"}, + proxies=proxies, timeout=10, + ) + assert data_resp.status_code == 200 + assert data_resp.json()["items"] == [1, 2, 3] - # Step 3: Access data without token → 401 - unauth_resp = requests.get( - "http://example.com/api/data", - proxies=proxies, timeout=10, - ) - assert unauth_resp.status_code == 401 + unauth_resp = requests.get( + "http://example.com/api/data", + proxies=proxies, timeout=10, + ) + assert unauth_resp.status_code == 401 - # Step 4: Login with wrong credentials → 401 - bad_login = requests.post( - "http://example.com/api/auth", - json={"username": "hacker", "password": "nope"}, - proxies=proxies, timeout=10, - ) - assert bad_login.status_code == 401 - finally: - client.stop() + bad_login = requests.post( + "http://example.com/api/auth", + json={"username": "hacker", "password": "nope"}, + proxies=proxies, timeout=10, + ) + assert bad_login.status_code == 401 From 8ee5c11e42d4a961a5d78af4e812ac4a39c14a68 Mon Sep 17 00:00:00 2001 From: Benny Zlotnik Date: Mon, 10 Aug 2026 09:23:59 +0300 Subject: [PATCH 21/24] perf(ci): parallelize e2e operator build prerequisites docker-build, docker-build-exporter-set-controller, build-operator, and cluster creation are independent targets. Running with -j3 lets them overlap, saving ~2-3 minutes of sequential container builds. Co-authored-by: Cursor --- .github/workflows/controller-kind.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/controller-kind.yaml b/.github/workflows/controller-kind.yaml index d5bfff854..2a9868501 100644 --- a/.github/workflows/controller-kind.yaml +++ b/.github/workflows/controller-kind.yaml @@ -29,4 +29,4 @@ jobs: - name: Run operator e2e test working-directory: controller - run: make test-operator-e2e + run: make test-operator-e2e -j3 --output-sync=target From 85a28081666ee4351899b818f694962df6568a24 Mon Sep 17 00:00:00 2001 From: Benny Zlotnik Date: Mon, 10 Aug 2026 09:31:09 +0300 Subject: [PATCH 22/24] perf(ci): compile Go on host with cached modules instead of inside containers Adds a CI-optimized build path that: - Compiles Go binaries directly on the runner (with setup-go module cache) - Packages them into minimal runtime-only containers (no Go toolchain) - Eliminates pulling the ~1.5GB go-toolset image for each build - Eliminates re-downloading modules inside containers without cache New targets: docker-build-ci, build-operator-ci, deploy-operator-ci, test-operator-e2e-ci. The CI workflow now uses setup-go with cache and the -ci targets. Expected savings: 3-5 minutes on the e2e job (module download + compile now benefits from GitHub Actions Go cache across runs). Co-authored-by: Cursor --- .github/workflows/controller-kind.yaml | 8 +++++++- controller/Containerfile.ci | 5 +++++ .../Containerfile.exporter-set-controller.ci | 5 +++++ controller/Containerfile.operator.ci | 5 +++++ controller/Makefile | 20 +++++++++++++++++++ controller/deploy/operator/Makefile | 5 +++++ 6 files changed, 47 insertions(+), 1 deletion(-) create mode 100644 controller/Containerfile.ci create mode 100644 controller/Containerfile.exporter-set-controller.ci create mode 100644 controller/Containerfile.operator.ci diff --git a/.github/workflows/controller-kind.yaml b/.github/workflows/controller-kind.yaml index 2a9868501..91ec1a2ea 100644 --- a/.github/workflows/controller-kind.yaml +++ b/.github/workflows/controller-kind.yaml @@ -27,6 +27,12 @@ jobs: with: fetch-depth: 0 + - name: Set up Go + uses: actions/setup-go@d35c59abb061a4a6fb18e82ac0862c26744d6ab5 # v5 + with: + go-version-file: controller/go.mod + cache-dependency-path: controller/go.sum + - name: Run operator e2e test working-directory: controller - run: make test-operator-e2e -j3 --output-sync=target + run: make test-operator-e2e-ci -j3 --output-sync=target diff --git a/controller/Containerfile.ci b/controller/Containerfile.ci new file mode 100644 index 000000000..5ade38d28 --- /dev/null +++ b/controller/Containerfile.ci @@ -0,0 +1,5 @@ +FROM registry.access.redhat.com/ubi9/ubi-micro:9.8-1784702951@sha256:b1e86b97028b8fcfb6d85f997c39e6b6b67496163ef8d80d243220a4918e8bef +WORKDIR / +COPY manager router ./ +USER 65532:65532 +ENTRYPOINT ["/manager"] diff --git a/controller/Containerfile.exporter-set-controller.ci b/controller/Containerfile.exporter-set-controller.ci new file mode 100644 index 000000000..6b008e415 --- /dev/null +++ b/controller/Containerfile.exporter-set-controller.ci @@ -0,0 +1,5 @@ +FROM registry.access.redhat.com/ubi9/ubi-micro:9.8-1784702951@sha256:b1e86b97028b8fcfb6d85f997c39e6b6b67496163ef8d80d243220a4918e8bef +WORKDIR / +COPY exporter-set-controller . +USER 65532:65532 +ENTRYPOINT ["/exporter-set-controller"] diff --git a/controller/Containerfile.operator.ci b/controller/Containerfile.operator.ci new file mode 100644 index 000000000..ec4db8983 --- /dev/null +++ b/controller/Containerfile.operator.ci @@ -0,0 +1,5 @@ +FROM registry.access.redhat.com/ubi9/ubi-micro:9.8-1784702951@sha256:b1e86b97028b8fcfb6d85f997c39e6b6b67496163ef8d80d243220a4918e8bef +WORKDIR / +COPY manager . +USER 65532:65532 +ENTRYPOINT ["/manager"] diff --git a/controller/Makefile b/controller/Makefile index 1fbd0ef9a..4e8b013d6 100644 --- a/controller/Makefile +++ b/controller/Makefile @@ -110,6 +110,10 @@ lint-fix: golangci-lint ## Run golangci-lint linter and perform fixes build-operator: make -C deploy/operator build-installer docker-build +.PHONY: build-operator-ci +build-operator-ci: + make -C deploy/operator build-installer docker-build-ci + .PHONY: build build: manifests generate fmt vet ## Build manager binary. go build -ldflags "$(LDFLAGS)" -o bin/manager cmd/main.go @@ -135,6 +139,14 @@ docker-build: ## Build docker image with the manager. --build-arg BUILD_DATE=$(BUILD_DATE) \ -t ${IMG} -f Containerfile . +.PHONY: docker-build-ci +docker-build-ci: ## Build docker images from pre-compiled host binaries (fast CI path). + CGO_ENABLED=0 go build -ldflags "$(LDFLAGS)" -o bin/manager cmd/main.go + CGO_ENABLED=0 go build -ldflags "$(LDFLAGS)" -o bin/router cmd/router/main.go + CGO_ENABLED=0 go build -ldflags "$(LDFLAGS)" -o bin/exporter-set-controller cmd/exporter-set-controller/main.go + $(CONTAINER_TOOL) build -t ${IMG} -f Containerfile.ci bin/ + $(CONTAINER_TOOL) build -t ${EXPORTER_SET_CONTROLLER_IMG} -f Containerfile.exporter-set-controller.ci bin/ + .PHONY: docker-build-exporter-set-controller docker-build-exporter-set-controller: ## Build docker image for the exporter-set-controller. $(CONTAINER_TOOL) build \ @@ -211,9 +223,17 @@ endif deploy-operator: docker-build docker-build-exporter-set-controller build-operator cluster grpcurl ## Deploy only the operator (without Jumpstarter CR) NETWORKING_MODE=ingress DEPLOY_JUMPSTARTER=false ./hack/deploy_with_operator.sh +.PHONY: deploy-operator-ci +deploy-operator-ci: docker-build-ci build-operator-ci cluster grpcurl ## CI-optimized: host-compiled binaries, no multi-stage container builds. + NETWORKING_MODE=ingress DEPLOY_JUMPSTARTER=false ./hack/deploy_with_operator.sh + .PHONY: test-operator-e2e test-operator-e2e: grpcurl deploy-operator make -C deploy/operator test-e2e + +.PHONY: test-operator-e2e-ci +test-operator-e2e-ci: grpcurl deploy-operator-ci ## CI-optimized e2e test (host-compiled Go, cached modules). + make -C deploy/operator test-e2e .PHONY: operator-logs operator-logs: kubectl logs -n jumpstarter-operator-system -l app.kubernetes.io/name=jumpstarter-operator -f diff --git a/controller/deploy/operator/Makefile b/controller/deploy/operator/Makefile index 89fef9e58..be13fd22b 100644 --- a/controller/deploy/operator/Makefile +++ b/controller/deploy/operator/Makefile @@ -177,6 +177,11 @@ docker-build: ## Build docker image with the manager. --build-arg BUILD_DATE=$(BUILD_DATE) \ -t ${IMG} ../../ -f ../../Containerfile.operator +.PHONY: docker-build-ci +docker-build-ci: ## CI-optimized: host-compiled binary, no multi-stage build. + CGO_ENABLED=0 go build -ldflags "$(LDFLAGS)" -o bin/manager cmd/main.go + $(CONTAINER_TOOL) build -t ${IMG} -f ../../Containerfile.operator.ci bin/ + .PHONY: docker-push docker-push: ## Push docker image with the manager. $(CONTAINER_TOOL) push ${IMG} From 23e89f8ebb57b77ad9d7df82ddf8a98d392b301e Mon Sep 17 00:00:00 2001 From: Benny Zlotnik Date: Mon, 10 Aug 2026 10:00:10 +0300 Subject: [PATCH 23/24] fix(ci): fix QEMU SSH failure and test-report race condition Two bugs: 1. virt-customize disabled cloud-init but didn't generate SSH host keys or enable password auth, so `qemu.shell()` (Fabric/SSH) got "Error reading SSH protocol banner". Fix: ssh-keygen -A + PasswordAuthentication yes + enable sshd. 2. test-report ran before all tests finished due to missing Make dependency. With -j8, Make started test-report in parallel with pkg-test-all because test-report had no prerequisites. It printed "All package tests passed" 23 seconds before the QEMU test actually failed. Fix: test-report now depends on pkg-test-all. Bump image cache key to v2 to pick up the SSH changes. Co-authored-by: Cursor --- .github/workflows/controller-kind.yaml | 2 +- .github/workflows/python-tests.yaml | 5 ++++- python/Makefile | 2 +- 3 files changed, 6 insertions(+), 3 deletions(-) diff --git a/.github/workflows/controller-kind.yaml b/.github/workflows/controller-kind.yaml index 91ec1a2ea..b1d1b3928 100644 --- a/.github/workflows/controller-kind.yaml +++ b/.github/workflows/controller-kind.yaml @@ -35,4 +35,4 @@ jobs: - name: Run operator e2e test working-directory: controller - run: make test-operator-e2e-ci -j3 --output-sync=target + run: make test-operator-e2e-ci -j --output-sync=target diff --git a/.github/workflows/python-tests.yaml b/.github/workflows/python-tests.yaml index 69ea890cd..a568c44e4 100644 --- a/.github/workflows/python-tests.yaml +++ b/.github/workflows/python-tests.yaml @@ -145,7 +145,7 @@ jobs: uses: actions/cache@55cc8345863c7cc4c66a329aec7e433d2d1c52a9 # v6 with: path: python/packages/jumpstarter-driver-qemu/images - key: fedora-cloud-43-1.6-preconfigured-v1 + key: fedora-cloud-43-1.6-preconfigured-v2 - name: Download Fedora Cloud images if: steps.cache-fedora-cloud-images.outputs.cache-hit != 'true' @@ -168,6 +168,9 @@ jobs: --run-command 'echo "jumpstarter ALL=(ALL) NOPASSWD:ALL" > /etc/sudoers.d/jumpstarter' \ --run-command 'systemctl disable cloud-init cloud-init-local cloud-config cloud-final' \ --run-command 'rm -rf /var/lib/cloud' \ + --run-command 'ssh-keygen -A' \ + --run-command 'sed -i "s/^#*PasswordAuthentication.*/PasswordAuthentication yes/" /etc/ssh/sshd_config' \ + --run-command 'systemctl enable sshd' \ --selinux-relabel - name: Run pytest diff --git a/python/Makefile b/python/Makefile index ff917a9cf..4d927d2d6 100644 --- a/python/Makefile +++ b/python/Makefile @@ -63,7 +63,7 @@ pkg-ty-%: packages/% pkg-test-all: sync $(addprefix pkg-test-,$(PKG_TARGETS)) ifdef LOGS_DIR -test-report: +test-report: pkg-test-all @failed=0; \ for f in $(LOGS_DIR)/*.failed; do \ [ -f "$$f" ] || continue; \ From 1ac04ee97699910c06d5d71f64d595ce55827886 Mon Sep 17 00:00:00 2001 From: Benny Zlotnik Date: Mon, 10 Aug 2026 10:09:41 +0300 Subject: [PATCH 24/24] perf(ci): remove unnecessary disk cleanup + consolidate apt installs - Remove gha-cleanup step from python-tests: the runner has 88GB free, Python tests need ~5GB. The cleanup was wasting ~1.5 minutes. - Merge 3 separate apt-get update + install steps into one: saves ~15-20s of redundant package index downloads. - Apply setup-go + host-compiled CI targets to deploy-kind job too (was only on e2e-test-operator). Saves ~3-4 minutes there. Co-authored-by: Cursor --- .github/workflows/controller-kind.yaml | 8 +++++++- .github/workflows/python-tests.yaml | 21 ++------------------- controller/Makefile | 8 ++++++++ 3 files changed, 17 insertions(+), 20 deletions(-) diff --git a/.github/workflows/controller-kind.yaml b/.github/workflows/controller-kind.yaml index b1d1b3928..42b87e20f 100644 --- a/.github/workflows/controller-kind.yaml +++ b/.github/workflows/controller-kind.yaml @@ -15,9 +15,15 @@ jobs: with: fetch-depth: 0 + - name: Set up Go + uses: actions/setup-go@d35c59abb061a4a6fb18e82ac0862c26744d6ab5 # v5 + with: + go-version-file: controller/go.mod + cache-dependency-path: controller/go.sum + - name: Run make deploy working-directory: controller - run: make deploy + run: make deploy-ci -j --output-sync=target e2e-test-operator: runs-on: ubuntu-latest diff --git a/.github/workflows/python-tests.yaml b/.github/workflows/python-tests.yaml index a568c44e4..cf3a6d232 100644 --- a/.github/workflows/python-tests.yaml +++ b/.github/workflows/python-tests.yaml @@ -66,12 +66,6 @@ jobs: with: fetch-depth: 0 - - name: Free up runner disk space - uses: mathio/gha-cleanup@aca3d43a05bf564f5fc69acb929a65b01797f22b # v1.1.2 - with: - remove-browsers: true - verbose: true - - id: uv run: echo "version=$(cat .uv-version)" >> "$GITHUB_OUTPUT" - name: Install uv @@ -96,19 +90,8 @@ jobs: sudo chmod 0666 /dev/kvm /dev/vhost-vsock /dev/vhost-net sudo apt-get update - sudo apt-get install -y qemu-system-arm qemu-system-x86 rpm2cpio cpio - - - name: Install libgpiod-dev (Linux) - if: runner.os == 'Linux' - run: | - sudo apt-get update - sudo apt-get install -y libgpiod-dev liblgpio-dev - - - name: Install nftables and dnsmasq (Linux) - if: runner.os == 'Linux' - run: | - sudo apt-get update - sudo apt-get install -y nftables dnsmasq-base isc-dhcp-client dnsutils + sudo apt-get install -y qemu-system-arm qemu-system-x86 rpm2cpio cpio \ + libgpiod-dev liblgpio-dev nftables dnsmasq-base isc-dhcp-client dnsutils - name: Load kernel modules for DUT network tests (Linux) if: runner.os == 'Linux' diff --git a/controller/Makefile b/controller/Makefile index 4e8b013d6..d12b4d910 100644 --- a/controller/Makefile +++ b/controller/Makefile @@ -218,6 +218,14 @@ ifeq ($(SKIP_BUILD),) endif ./hack/deploy_with_operator.sh +.PHONY: deploy-ci +deploy-ci: cluster grpcurl ## CI-optimized deploy: host-compiled binaries, no multi-stage container builds. +ifeq ($(SKIP_BUILD),) + $(MAKE) docker-build-ci + $(MAKE) build-operator-ci +endif + ./hack/deploy_with_operator.sh + .PHONY: deploy-operator deploy-operator: docker-build docker-build-exporter-set-controller build-operator cluster grpcurl ## Deploy only the operator (without Jumpstarter CR)