Fix ephemeral runner job exit and cleanup (#37)

* fix: run ephemeral github runners once

* fix: drain accepted runner delete responses

Author: intel352

cmd/github-actions-runner-job/main.go

@@ -313,7 +313,7 @@ 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) runnerID() int64 {

cmd/github-actions-runner-job/main_test.go

@@ -64,7 +64,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")
+	writeExecutable(t, filepath.Join(runnerDir, "run.sh"), "#!/bin/sh\nprintf '%s\\n' \"$@\" > \"$GITHUB_ACTIONS_RUNNER_JOB_TEST_DIR/run.args\"\nprintf 'runner executed\\n' > \"$GITHUB_ACTIONS_RUNNER_JOB_TEST_DIR/run.log\"\n")
 	workspace := t.TempDir()
 	t.Chdir(workspace)
 	t.Setenv("COMPUTE_GITHUB_RUNNER_PROVIDER_URL", sidecar.URL)
@@ -115,6 +115,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"`
 	}

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 {

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