diff --git a/cmd/github-actions-runner-job/main.go b/cmd/github-actions-runner-job/main.go index b74e2e4..e0dd7f3 100644 --- a/cmd/github-actions-runner-job/main.go +++ b/cmd/github-actions-runner-job/main.go @@ -240,35 +240,49 @@ func (d *runnerDriver) RunGitHubJob(ctx context.Context, mode internal.Ephemeral if mode == "" { mode = internal.EphemeralRunnerJobModeAttachToQueued } + var dispatchRef string + var dispatchInputs map[string]string if mode == internal.EphemeralRunnerJobModeDispatchThenWait { if strings.TrimSpace(d.req.Workflow) == "" || strings.TrimSpace(d.req.Repository) == "" { return internal.EphemeralRunnerJobResult{}, fmt.Errorf("repository and workflow are required for %s", mode) } - ref := strings.TrimSpace(d.req.Ref) - if ref == "" { - ref = "main" + dispatchRef = strings.TrimSpace(d.req.Ref) + if dispatchRef == "" { + dispatchRef = "main" } inputs, err := d.workflowDispatchInputs(spec) if err != nil { return internal.EphemeralRunnerJobResult{}, err } - if err := d.sidecar.dispatchWorkflow(ctx, d.req.Repository, d.req.Workflow, ref, inputs); err != nil { - return internal.EphemeralRunnerJobResult{}, err - } + dispatchInputs = inputs } if err := d.configureRunner(ctx, spec, token.Token); err != nil { return internal.EphemeralRunnerJobResult{}, err } - runnerID := d.runnerID() + result := internal.EphemeralRunnerJobResult{ + RunnerID: d.runnerID(), + RunnerName: spec.RunnerName, + Labels: append([]string(nil), spec.Labels...), + } + if mode == internal.EphemeralRunnerJobModeDispatchThenWait { + runner, err := d.startRunner(ctx) + if err != nil { + return result, err + } + if err := d.sidecar.dispatchWorkflow(ctx, d.req.Repository, d.req.Workflow, dispatchRef, dispatchInputs); err != nil { + runner.cancel() + _ = runner.wait() + return result, err + } + if err := runner.wait(); err != nil { + return result, err + } + return result, nil + } if err := d.runRunner(ctx); err != nil { - return internal.EphemeralRunnerJobResult{}, err + return result, err } - return internal.EphemeralRunnerJobResult{ - RunnerID: runnerID, - RunnerName: spec.RunnerName, - Labels: append([]string(nil), spec.Labels...), - CleanupStatus: "removed", - }, nil + return result, nil } func (d *runnerDriver) workflowDispatchInputs(spec internal.EphemeralRunnerJobSpec) (map[string]string, error) { @@ -313,7 +327,11 @@ func (d *runnerDriver) configureRunner(ctx context.Context, spec internal.Epheme } func (d *runnerDriver) runRunner(ctx context.Context) error { - return runCommand(ctx, filepath.Join(d.runnerDir, "run.sh"), d.runnerDir) + return runCommand(ctx, filepath.Join(d.runnerDir, "run.sh"), d.runnerDir, "--once") +} + +func (d *runnerDriver) startRunner(ctx context.Context) (*runningCommand, error) { + return startCommand(ctx, filepath.Join(d.runnerDir, "run.sh"), d.runnerDir, "--once") } func (d *runnerDriver) runnerID() int64 { @@ -341,6 +359,37 @@ func runCommand(ctx context.Context, path, dir string, args ...string) error { return nil } +type runningCommand struct { + path string + cancel context.CancelFunc + cmd *exec.Cmd + output bytes.Buffer +} + +func startCommand(ctx context.Context, path, dir string, args ...string) (*runningCommand, error) { + runCtx, cancel := context.WithCancel(ctx) + cmd := exec.CommandContext(runCtx, path, args...) + cmd.Dir = dir + cmd.Env = os.Environ() + running := &runningCommand{path: path, cancel: cancel, cmd: cmd} + cmd.Stdout = &running.output + cmd.Stderr = &running.output + if err := cmd.Start(); err != nil { + cancel() + return nil, fmt.Errorf("%s start failed: %w", filepath.Base(path), err) + } + return running, nil +} + +func (r *runningCommand) wait() error { + err := r.cmd.Wait() + r.cancel() + if err != nil { + return fmt.Errorf("%s failed: %w: %s", filepath.Base(r.path), err, strings.TrimSpace(r.output.String())) + } + return nil +} + func writeProofArtifact(result internal.EphemeralRunnerJobResult) error { data, err := json.MarshalIndent(result, "", " ") if err != nil { diff --git a/cmd/github-actions-runner-job/main_test.go b/cmd/github-actions-runner-job/main_test.go index 22ca13c..948781f 100644 --- a/cmd/github-actions-runner-job/main_test.go +++ b/cmd/github-actions-runner-job/main_test.go @@ -9,10 +9,12 @@ import ( "path/filepath" "strings" "testing" + "time" ) func TestT915CommandRunsDynamicProviderEnvelopeThroughSidecarAndRunner(t *testing.T) { var tokenCalls, dispatchCalls, deleteCalls int + workspace := t.TempDir() sidecar := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { if got := r.Header.Get("Authorization"); got != "Bearer provider-token" { t.Fatalf("authorization = %q", got) @@ -24,6 +26,17 @@ func TestT915CommandRunsDynamicProviderEnvelopeThroughSidecarAndRunner(t *testin _, _ = w.Write([]byte(`{"token":"runner-registration-token","expires_at":"2026-06-26T22:00:00Z"}`)) case r.Method == http.MethodPost && r.URL.Path == "/v1/actions/repos/GoCodeAlone/workflow-compute/workflows/dogfood.yml/dispatches": dispatchCalls++ + runStarted := filepath.Join(workspace, "run.started") + for range 20 { + if _, err := os.Stat(runStarted); err == nil { + break + } + time.Sleep(50 * time.Millisecond) + } + if _, err := os.Stat(runStarted); err != nil { + http.Error(w, "runner listener was not started before dispatch", http.StatusConflict) + return + } var body struct { Ref string `json:"ref"` Inputs map[string]string `json:"inputs"` @@ -52,6 +65,9 @@ func TestT915CommandRunsDynamicProviderEnvelopeThroughSidecarAndRunner(t *testin t.Fatalf("dispatch inputs must normalize caller keys, found %q in %#v", forbidden, body.Inputs) } } + if err := os.WriteFile(filepath.Join(workspace, "dispatch.seen"), []byte("1\n"), 0o600); err != nil { + t.Fatalf("write dispatch marker: %v", err) + } w.WriteHeader(http.StatusNoContent) case r.Method == http.MethodDelete && r.URL.Path == "/v1/actions/orgs/GoCodeAlone/runners/42": deleteCalls++ @@ -64,8 +80,7 @@ func TestT915CommandRunsDynamicProviderEnvelopeThroughSidecarAndRunner(t *testin runnerDir := t.TempDir() writeExecutable(t, filepath.Join(runnerDir, "config.sh"), "#!/bin/sh\nprintf '%s\\n' \"$@\" > \"$GITHUB_ACTIONS_RUNNER_JOB_TEST_DIR/config.args\"\nprintf '{\"agentId\":42}\\n' > .runner\n") - writeExecutable(t, filepath.Join(runnerDir, "run.sh"), "#!/bin/sh\nprintf 'runner executed\\n' > \"$GITHUB_ACTIONS_RUNNER_JOB_TEST_DIR/run.log\"\n") - workspace := t.TempDir() + writeExecutable(t, filepath.Join(runnerDir, "run.sh"), "#!/bin/sh\nprintf '%s\\n' \"$@\" > \"$GITHUB_ACTIONS_RUNNER_JOB_TEST_DIR/run.args\"\ntouch \"$GITHUB_ACTIONS_RUNNER_JOB_TEST_DIR/run.started\"\nwhile [ ! -f \"$GITHUB_ACTIONS_RUNNER_JOB_TEST_DIR/dispatch.seen\" ]; do sleep 0.1; done\nprintf 'runner executed\\n' > \"$GITHUB_ACTIONS_RUNNER_JOB_TEST_DIR/run.log\"\n") t.Chdir(workspace) t.Setenv("COMPUTE_GITHUB_RUNNER_PROVIDER_URL", sidecar.URL) t.Setenv("COMPUTE_GITHUB_RUNNER_PROVIDER_TOKEN", "provider-token") @@ -115,6 +130,9 @@ func TestT915CommandRunsDynamicProviderEnvelopeThroughSidecarAndRunner(t *testin if got := readFile(t, filepath.Join(workspace, "run.log")); !strings.Contains(got, "runner executed") { t.Fatalf("run script did not execute: %q", got) } + if got := readFile(t, filepath.Join(workspace, "run.args")); strings.TrimSpace(got) != "--once" { + t.Fatalf("run args: got %q want --once", got) + } var result struct { Artifacts []string `json:"artifacts"` } diff --git a/internal/module_runner_provider.go b/internal/module_runner_provider.go index 1577783..3321b87 100644 --- a/internal/module_runner_provider.go +++ b/internal/module_runner_provider.go @@ -90,7 +90,7 @@ func (c *httpGitHubRunnerClient) RemoveRunner(ctx context.Context, owner, repo s return errors.New("runner_id must be positive") } endpoint := fmt.Sprintf("%s/repos/%s/%s/actions/runners/%d", c.baseURL, url.PathEscape(owner), url.PathEscape(repo), runnerID) - return c.do(ctx, http.MethodDelete, endpoint, nil, token, http.StatusNoContent, nil) + return c.doRunnerDelete(ctx, endpoint, token) } func (c *httpGitHubRunnerClient) OrgRegistrationToken(ctx context.Context, organization, token string) (GitHubRunnerRegistrationToken, error) { @@ -110,7 +110,7 @@ func (c *httpGitHubRunnerClient) RemoveOrgRunner(ctx context.Context, organizati return errors.New("runner_id must be positive") } endpoint := fmt.Sprintf("%s/orgs/%s/actions/runners/%d", c.baseURL, url.PathEscape(organization), runnerID) - return c.do(ctx, http.MethodDelete, endpoint, nil, token, http.StatusNoContent, nil) + return c.doRunnerDelete(ctx, endpoint, token) } func (c *httpGitHubRunnerClient) PreflightOrg(ctx context.Context, req GitHubRunnerProviderPreflightRequest, token string) (GitHubRunnerProviderPreflight, error) { @@ -190,6 +190,15 @@ func (c *httpGitHubRunnerClient) do(ctx context.Context, method, endpoint string } func (c *httpGitHubRunnerClient) doRaw(ctx context.Context, method, endpoint string, body any, token string, wantStatus int, out any) (http.Header, error) { + return c.doRawAllowed(ctx, method, endpoint, body, token, []int{wantStatus}, out) +} + +func (c *httpGitHubRunnerClient) doRunnerDelete(ctx context.Context, endpoint, token string) error { + _, err := c.doRawAllowed(ctx, http.MethodDelete, endpoint, nil, token, []int{http.StatusNoContent, http.StatusNotFound}, nil) + return err +} + +func (c *httpGitHubRunnerClient) doRawAllowed(ctx context.Context, method, endpoint string, body any, token string, wantStatuses []int, out any) (http.Header, error) { if strings.TrimSpace(token) == "" { return nil, errors.New("github token is required") } @@ -221,11 +230,19 @@ func (c *httpGitHubRunnerClient) doRaw(ctx context.Context, method, endpoint str return nil, fmt.Errorf("github runner request failed: %w", err) } defer resp.Body.Close() - if resp.StatusCode != wantStatus { + ok := false + for _, status := range wantStatuses { + if resp.StatusCode == status { + ok = true + break + } + } + if !ok { limited, _ := io.ReadAll(io.LimitReader(resp.Body, 2048)) return nil, fmt.Errorf("github runner request returned %s: %s", resp.Status, strings.TrimSpace(string(limited))) } if out == nil { + _, _ = io.Copy(io.Discard, resp.Body) return resp.Header, nil } if err := json.NewDecoder(resp.Body).Decode(out); err != nil { diff --git a/internal/runner_provider_test.go b/internal/runner_provider_test.go index 9972987..762d7c3 100644 --- a/internal/runner_provider_test.go +++ b/internal/runner_provider_test.go @@ -133,6 +133,61 @@ func TestT915_GitHubRunnerClientDispatchesWorkflow(t *testing.T) { } } +func TestT915_GitHubRunnerClientTreatsMissingRunnerAsRemoved(t *testing.T) { + var paths []string + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method != http.MethodDelete { + t.Fatalf("method: got %q want DELETE", r.Method) + } + paths = append(paths, r.URL.Path) + http.Error(w, `{"message":"Not Found"}`, http.StatusNotFound) + })) + defer server.Close() + + client := &httpGitHubRunnerClient{baseURL: server.URL, httpClient: server.Client()} + if err := client.RemoveRunner(context.Background(), "GoCodeAlone", "workflow-compute", 42, "github-token"); err != nil { + t.Fatalf("remove repo runner should ignore already-missing runner: %v", err) + } + if err := client.RemoveOrgRunner(context.Background(), "GoCodeAlone", 43, "github-token"); err != nil { + t.Fatalf("remove org runner should ignore already-missing runner: %v", err) + } + want := []string{ + "/repos/GoCodeAlone/workflow-compute/actions/runners/42", + "/orgs/GoCodeAlone/actions/runners/43", + } + if strings.Join(paths, "\n") != strings.Join(want, "\n") { + t.Fatalf("delete paths:\ngot:\n%s\nwant:\n%s", strings.Join(paths, "\n"), strings.Join(want, "\n")) + } +} + +func TestT915_GitHubRunnerClientDrainsAcceptedDeleteResponseBody(t *testing.T) { + body := &trackingReadCloser{reader: strings.NewReader(`{"message":"Not Found"}`)} + client := &httpGitHubRunnerClient{ + baseURL: "https://api.github.invalid", + httpClient: &http.Client{Transport: roundTripFunc(func(r *http.Request) (*http.Response, error) { + if r.Method != http.MethodDelete { + t.Fatalf("method: got %q want DELETE", r.Method) + } + return &http.Response{ + StatusCode: http.StatusNotFound, + Status: "404 Not Found", + Header: make(http.Header), + Body: body, + Request: r, + }, nil + })}, + } + if err := client.RemoveOrgRunner(context.Background(), "GoCodeAlone", 43, "github-token"); err != nil { + t.Fatalf("remove org runner: %v", err) + } + if !body.read { + t.Fatal("accepted delete response body was not drained") + } + if !body.closed { + t.Fatal("accepted delete response body was not closed") + } +} + func TestT41_GitHubRunnerProviderModuleRejectsUnknownConfig(t *testing.T) { _, err := newGitHubRunnerProviderModule("provider", map[string]any{ "token": "github-token", @@ -542,6 +597,31 @@ type fakeRunnerClient struct { dispatchedInputs map[string]string } +type roundTripFunc func(*http.Request) (*http.Response, error) + +func (f roundTripFunc) RoundTrip(r *http.Request) (*http.Response, error) { + return f(r) +} + +type trackingReadCloser struct { + reader *strings.Reader + read bool + closed bool +} + +func (b *trackingReadCloser) Read(p []byte) (int, error) { + n, err := b.reader.Read(p) + if n > 0 || err == io.EOF { + b.read = true + } + return n, err +} + +func (b *trackingReadCloser) Close() error { + b.closed = true + return nil +} + func (f *fakeRunnerClient) RegistrationToken(_ context.Context, owner, repo, _ string) (GitHubRunnerRegistrationToken, error) { f.registrationRepository = owner + "/" + repo return f.token, nil