Add end-to-end testing infra #3599

DrJosh9000 · 2025-11-27T06:45:41Z

Description

Adds a new package with some test helper code, and new pipeline yml.

So far it "end-to-end tests" the same agent version that runs the test, and does not inspect the job logs. These will be the next steps.

Context

PB-698

Changes

Similar to agent-stack-k8s integration test setup.

Testing

Tests have run locally (with go test ./...). Buildkite employees may check this if the pipeline has run automatically.
Code is formatted (with go tool gofumpt -extra -w .)

Disclosures / Credits

I did not use AI tools at all

moskyb · 2025-12-02T03:23:29Z

internal/e2e/basic_test.go

+	if err := agent.Process.Signal(syscall.SIGTERM); err != nil {
+		t.Errorf("agent.Process.Signal(%d) = %v", syscall.SIGTERM, err)
+	}
+	if err := agent.Wait(); err != nil {
+		t.Errorf("agent.Wait() = %v", err)
+	}


this should get handled by the test completing, right? when the test finishes, its context gets cancelled, killing the process?

The agent is killed, so on the BK side it becomes lost. The queue can't be deleted because an agent is "still running".

Maybe this should go in a cleanup function registered in startAgent ? Thoughts?

Ugh, it's complicated by the ordering of cleanups and context cancellation. I've changed it to a deferred function that sends SIGQUIT.

Maybe this should go in a cleanup function registered in startAgent ? Thoughts?

yeah, i think that's a good call — there's a bit of nuance there that could be easy to miss, but much harder if there's a default cleanup function.

so i'm sure i have it right, SIGTERM is the one we don't catch, right?

We catch SIGINT, SIGTERM, SIGQUIT, and SIGHUP. We're not allowed to catch SIGKILL. The difference between SIGQUIT and SIGTERM is that we proceed directly to cancelling jobs with SIGQUIT, but that only happens for the second SIGTERM.

agent/clicommand/agent_start.go

Lines 1385 to 1456 in c1039ae

func (ps *poolSignals) handle(ctx context.Context) chan os.Signal {

signals := make(chan os.Signal, 1)

signal.Notify(

signals,

os.Interrupt,

syscall.SIGHUP,

syscall.SIGTERM,

syscall.SIGINT,

syscall.SIGQUIT,

)

go ps.handleLoop(ctx, signals)

return signals

}

func (ps *poolSignals) handleLoop(ctx context.Context, signals chan os.Signal) {

_, setStatus, done := status.AddSimpleItem(ctx, "Handle Pool Signals")

defer done()

setStatus("⏳ Waiting for a signal")

interruptCount := 0

if ps.skipGraceful {

interruptCount = 1

}

ungracefulStop := func() {

// We shouldn't block the signal handler loop either by waiting

// for the jobs to cancel or by waiting for the cancel grace

// period to expire.

go ps.pool.StopUngracefully() // one last chance to stop

go func() {

// Assuming cancelling jobs takes the full cancel grace period,

// allow 1 second to send agent disconnects.

time.Sleep(ps.cancelGracePeriod + 1*time.Second)

// We get here if the main goroutine hasn't returned yet.

ps.log.Info("Timed out waiting for agents to exit; exiting immediately with status 1")

os.Exit(1)

}()

}

for sig := range signals {

ps.log.Debug("Received signal `%v`", sig)

setStatus(fmt.Sprintf("Received signal `%v`", sig))

switch sig {

case syscall.SIGQUIT:

ungracefulStop()

case syscall.SIGTERM, syscall.SIGINT:

interruptCount++

switch interruptCount {

case 1:

ps.log.Info("Received CTRL-C, send again to forcefully kill the agent(s)")

ps.pool.StopGracefully()

case 2:

ps.log.Info("Forcefully stopping running jobs and stopping the agent(s) in %v", ps.cancelGracePeriod)

if !ps.skipGraceful {

ps.log.Info("Press Ctrl-C one more time to exit immediately without disconnecting - note that agents will be considered lost!")

}

ungracefulStop()

case 3:

ps.log.Info("Exiting immediately with status 1")

os.Exit(1)

}

default:

ps.log.Debug("Ignoring signal `%s`", sig.String())

}

}

}

zhming0

No blocker, looks good to me 🚀

zhming0 · 2025-12-02T04:31:33Z

.buildkite/docker-compose.yml

+    build:
+      context: .
+      dockerfile: Dockerfile-e2e


Do we need build when the dockerfile only contain a single line of FROM ...?

The Dockerfile mainly exists so that dependabot has something it can easily update. It might work with docker-compose.yml now, I haven't checked lately.

zhming0 · 2025-12-02T04:34:59Z

.buildkite/pipeline.e2e.yml

+  CI_E2E_TESTS_TARGET_ORG: agent-e2e-testing
+  CI_E2E_TESTS_TARGET_CLUSTER: e1773a06-b43c-4b77-84e5-5e088a737f6a


Would it be good to log a task in linear to remove these two?

Remove and replace with what? Hard-code in the tests?

I am thinking replacing with automatically inspecting agent cluster token and find the org and cluster, like we did in the k8s stack

Oh ok. Sure.

zhming0 · 2025-12-02T04:46:43Z

internal/e2e/testcase.go

+		state, _, err := tc.bkClient.Builds.Get(ctx, targetOrg, tc.pipeline.Slug, strconv.Itoa(build.Number), &buildkite.BuildGetOptions{})
+		if err != nil {
+			return "", err
+		}
+		switch state.State {
+		case "passed", "failed", "canceled", "canceling":
+			return state.State, nil
+
+		case "scheduled", "running":


Without looking into how bk client work, this is a bit confusing? Why is there a "scheduled" state on build? I thought it's a job thing 🤔 .

I copied this part with modifications from agent-stack-k8s. It seems there is a scheduled build state: https://github.com/buildkite/agent-stack-k8s/blob/eab654d3dda6574d1aa3fe9ba67c1dcb17987e8d/internal/integration/api/generated.go#L358-L359

zhming0 · 2025-12-02T04:51:18Z

internal/e2e/testcase.go

+	// The agent should be cancelled automatically by t.Context.
+	// But cancellation SIGKILLs the agent, so it doesn't disconnect cleanly.
+	// It is eventually marked lost on the backend, but not for a few minutes,
+	// which is long enough to block queue cleanup.
+	// This cleanup function SIGQUITs it, which forces an ungraceful
+	// but clean exit (jobs are cancelled but it has time to disconnect).
+	// This cleanup function has to happen *within* the test (use defer!),
+	// because the test context will otherwise kill the agent!
+	return cmd, func() {


zhming0 · 2025-12-02T05:00:36Z

internal/e2e/testcase.go

+	}, extraArgs...)
+	tc.Logf("Starting agent with args: %q", args)
+
+	cmd := exec.CommandContext(tc.Context(), agentPath, args...)


[totally not blocking] I wonder if we could intercept tc.Context()'s cancelation and change that to SIGTERM to the agent command? It would save the defer on the caller side.

I have a similar idea that might work... please stand by...

Now it replaces the command's Cancel func, and uses WaitDelay to ensure it is killed later. No special cleanup or defer needed!

DrJosh9000 marked this pull request as draft November 27, 2025 06:47

DrJosh9000 force-pushed the pb-968-create-a-testing-framework branch 5 times, most recently from e009661 to f09915e Compare December 1, 2025 06:35

DrJosh9000 changed the title ~~Add e2e testing package in internal~~ Add e2e testing infra Dec 1, 2025

DrJosh9000 changed the title ~~Add e2e testing infra~~ Add end-to-end testing infra Dec 1, 2025

DrJosh9000 force-pushed the pb-968-create-a-testing-framework branch 22 times, most recently from 45be616 to ff2c367 Compare December 2, 2025 03:06

DrJosh9000 force-pushed the pb-968-create-a-testing-framework branch 3 times, most recently from c339a38 to 49b4919 Compare December 2, 2025 03:10

DrJosh9000 marked this pull request as ready for review December 2, 2025 03:13

DrJosh9000 requested a review from a team December 2, 2025 03:16

moskyb reviewed Dec 2, 2025

View reviewed changes

DrJosh9000 force-pushed the pb-968-create-a-testing-framework branch 2 times, most recently from dd01ab9 to 7d13f5b Compare December 2, 2025 04:12

zhming0 approved these changes Dec 2, 2025

View reviewed changes

zhming0 reviewed Dec 2, 2025

View reviewed changes

DrJosh9000 force-pushed the pb-968-create-a-testing-framework branch from 7d13f5b to 3b6f49d Compare December 2, 2025 05:21

DrJosh9000 added 2 commits December 2, 2025 16:23

Add e2e internal package

a764635

Add e2e pipeline

4167772

DrJosh9000 force-pushed the pb-968-create-a-testing-framework branch from 3b6f49d to 4167772 Compare December 2, 2025 05:23

DrJosh9000 merged commit c9ca995 into main Dec 2, 2025
2 checks passed

DrJosh9000 deleted the pb-968-create-a-testing-framework branch December 2, 2025 05:30

moskyb mentioned this pull request Dec 5, 2025

Bump version + changelog for v3.114.1 #3612

Merged

	func (ps *poolSignals) handle(ctx context.Context) chan os.Signal {
	signals := make(chan os.Signal, 1)
	signal.Notify(
	signals,
	os.Interrupt,
	syscall.SIGHUP,
	syscall.SIGTERM,
	syscall.SIGINT,
	syscall.SIGQUIT,
	)

	go ps.handleLoop(ctx, signals)
	return signals
	}

	func (ps *poolSignals) handleLoop(ctx context.Context, signals chan os.Signal) {
	_, setStatus, done := status.AddSimpleItem(ctx, "Handle Pool Signals")
	defer done()
	setStatus("⏳ Waiting for a signal")

	interruptCount := 0
	if ps.skipGraceful {
	interruptCount = 1
	}

	ungracefulStop := func() {
	// We shouldn't block the signal handler loop either by waiting
	// for the jobs to cancel or by waiting for the cancel grace
	// period to expire.
	go ps.pool.StopUngracefully() // one last chance to stop
	go func() {
	// Assuming cancelling jobs takes the full cancel grace period,
	// allow 1 second to send agent disconnects.
	time.Sleep(ps.cancelGracePeriod + 1*time.Second)
	// We get here if the main goroutine hasn't returned yet.
	ps.log.Info("Timed out waiting for agents to exit; exiting immediately with status 1")
	os.Exit(1)
	}()
	}

	for sig := range signals {
	ps.log.Debug("Received signal `%v`", sig)
	setStatus(fmt.Sprintf("Received signal `%v`", sig))

	switch sig {
	case syscall.SIGQUIT:
	ungracefulStop()

	case syscall.SIGTERM, syscall.SIGINT:
	interruptCount++
	switch interruptCount {
	case 1:
	ps.log.Info("Received CTRL-C, send again to forcefully kill the agent(s)")
	ps.pool.StopGracefully()

	case 2:
	ps.log.Info("Forcefully stopping running jobs and stopping the agent(s) in %v", ps.cancelGracePeriod)
	if !ps.skipGraceful {
	ps.log.Info("Press Ctrl-C one more time to exit immediately without disconnecting - note that agents will be considered lost!")
	}
	ungracefulStop()

	case 3:
	ps.log.Info("Exiting immediately with status 1")
	os.Exit(1)
	}

	default:
	ps.log.Debug("Ignoring signal `%s`", sig.String())
	}
	}
	}

		CI_E2E_TESTS_TARGET_ORG: agent-e2e-testing
		CI_E2E_TESTS_TARGET_CLUSTER: e1773a06-b43c-4b77-84e5-5e088a737f6a

Add end-to-end testing infra #3599

Add end-to-end testing infra #3599

Uh oh!

Conversation

DrJosh9000 commented Nov 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Context

Changes

Testing

Disclosures / Credits

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zhming0 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

DrJosh9000 commented Nov 27, 2025 •

edited

Loading