From 50df0c044fbb41be0c544e06c0d2ffb3a4de0786 Mon Sep 17 00:00:00 2001 From: Max Knee Date: Tue, 23 Jun 2026 14:52:03 -0400 Subject: [PATCH 1/4] Surface pod container errors in EphemeralRunner status and GitHub Actions UI MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When runner pods fail due to container-level issues (ImagePullBackOff, CrashLoopBackOff, OOMKilled, etc.), the error was previously invisible — pod.Status.Message is empty for these cases, and GitHub Actions only shows a generic timeout. This change extracts container waiting/terminated state errors and surfaces them in three ways: 1. EphemeralRunner.Status.Message now contains the specific error 2. Kubernetes Events are emitted on the EphemeralRunner resource 3. Optionally, a GitHub Check Run annotation is created (requires checks:write permission and wiring GitHubAnnotator in main) Co-Authored-By: Claude Opus 4.6 (1M context) --- controllers/actions.github.com/constants.go | 17 ++ .../ephemeralrunner_controller.go | 49 ++++- .../actions.github.com/github_annotations.go | 70 +++++++ .../github_annotations_test.go | 69 +++++++ .../github_checkrun_annotator.go | 149 ++++++++++++++ .../github_checkrun_annotator_test.go | 61 ++++++ controllers/actions.github.com/pod_errors.go | 94 +++++++++ .../actions.github.com/pod_errors_test.go | 185 ++++++++++++++++++ 8 files changed, 690 insertions(+), 4 deletions(-) create mode 100644 controllers/actions.github.com/github_annotations.go create mode 100644 controllers/actions.github.com/github_annotations_test.go create mode 100644 controllers/actions.github.com/github_checkrun_annotator.go create mode 100644 controllers/actions.github.com/github_checkrun_annotator_test.go create mode 100644 controllers/actions.github.com/pod_errors.go create mode 100644 controllers/actions.github.com/pod_errors_test.go diff --git a/controllers/actions.github.com/constants.go b/controllers/actions.github.com/constants.go index 588c95beb7..c534f1d71c 100644 --- a/controllers/actions.github.com/constants.go +++ b/controllers/actions.github.com/constants.go @@ -83,3 +83,20 @@ const ( ReasonTooManyPodFailures = "TooManyPodFailures" ReasonInvalidPodFailure = "InvalidPod" ) + +// Container waiting state reasons that indicate unrecoverable pod errors. +// These are surfaced in EphemeralRunner.Status.Message for easier diagnosis. +var terminalContainerWaitingReasons = map[string]string{ + "ImagePullBackOff": "ImagePull", + "ErrImagePull": "ImagePull", + "InvalidImageName": "ImagePull", + "RegistryUnavailable": "ImagePull", + "CreateContainerConfigError": "ContainerConfig", + "CrashLoopBackOff": "ContainerCrash", + "RunContainerError": "ContainerCrash", +} + +// Container terminated reasons that indicate unrecoverable pod errors. +var terminalContainerTerminatedReasons = map[string]string{ + "OOMKilled": "ResourceLimit", +} diff --git a/controllers/actions.github.com/ephemeralrunner_controller.go b/controllers/actions.github.com/ephemeralrunner_controller.go index 64b98e2745..ceed90f1d8 100644 --- a/controllers/actions.github.com/ephemeralrunner_controller.go +++ b/controllers/actions.github.com/ephemeralrunner_controller.go @@ -32,6 +32,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" + "k8s.io/client-go/tools/events" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" @@ -46,8 +47,10 @@ const ( // EphemeralRunnerReconciler reconciles a EphemeralRunner object type EphemeralRunnerReconciler struct { client.Client - Log logr.Logger - Scheme *runtime.Scheme + Log logr.Logger + Scheme *runtime.Scheme + Recorder events.EventRecorder + GitHubAnnotator GitHubAnnotator ResourceBuilder } @@ -232,6 +235,13 @@ func (r *EphemeralRunnerReconciler) Reconcile(ctx context.Context, req ctrl.Requ if len(ephemeralRunner.Status.Failures) > maxFailures { log.Info(fmt.Sprintf("EphemeralRunner has failed more than %d times. Deleting ephemeral runner so it can be re-created", maxFailures)) + msg := fmt.Sprintf("Runner pod failed %d times and will not be retried", maxFailures) + if ephemeralRunner.Status.Message != "" { + msg = fmt.Sprintf("%s. Last error: %s", msg, ephemeralRunner.Status.Message) + } + r.Recorder.Eventf(&ephemeralRunner, nil, corev1.EventTypeWarning, ReasonTooManyPodFailures, "MaxRetriesExceeded", msg) + annotateRunnerFailure(ctx, r.GitHubAnnotator, &ephemeralRunner, log) + if err := r.Delete(ctx, &ephemeralRunner); err != nil { log.Error(fmt.Errorf("failed to delete ephemeral runner after %d failures: %w", maxFailures, err), "Failed to delete ephemeral runner") return ctrl.Result{}, err @@ -365,11 +375,28 @@ func (r *EphemeralRunnerReconciler) Reconcile(ctx context.Context, req ctrl.Requ return ctrl.Result{}, r.deleteEphemeralRunnerOrPod(ctx, &ephemeralRunner, pod, log) case cs == nil: - // starting, no container state yet + // Runner container status not yet available. Check if any container is in a + // terminal error state (e.g. ImagePullBackOff) that prevents the pod from progressing. + if containerErrors := extractPodContainerErrors(pod); len(containerErrors) > 0 { + errMsg := formatPodContainerErrors(containerErrors) + log.Info("Pod has container errors while waiting for runner container", "errors", errMsg) + r.Recorder.Eventf(&ephemeralRunner, nil, corev1.EventTypeWarning, "ContainerError", "PodStartupFailure", errMsg) + return ctrl.Result{}, r.deleteEphemeralRunnerOrPod(ctx, &ephemeralRunner, pod, log) + } log.Info("Waiting for runner container status to be available") return ctrl.Result{}, nil - case cs.State.Terminated == nil: // container is not terminated and pod phase is not failed, so runner is still running + case cs.State.Terminated == nil: + // Container is not terminated and pod phase is not failed. Check for waiting errors + // (e.g. CrashLoopBackOff on the runner container itself). + if cs.State.Waiting != nil { + if _, isTerminal := terminalContainerWaitingReasons[cs.State.Waiting.Reason]; isTerminal { + errMsg := fmt.Sprintf("container %q: %s: %s", cs.Name, cs.State.Waiting.Reason, cs.State.Waiting.Message) + log.Info("Runner container is in terminal waiting state", "reason", cs.State.Waiting.Reason) + r.Recorder.Eventf(&ephemeralRunner, nil, corev1.EventTypeWarning, "ContainerError", "RunnerContainerWaiting", errMsg) + return ctrl.Result{}, r.deleteEphemeralRunnerOrPod(ctx, &ephemeralRunner, pod, log) + } + } log.Info("Runner container is still running; updating ephemeral runner status") if err := r.updateRunStatusFromPod(ctx, &ephemeralRunner, pod, log); err != nil { log.Info("Failed to update ephemeral runner status. Requeue to not miss this event") @@ -634,6 +661,17 @@ func (r *EphemeralRunnerReconciler) deletePodAsFailed(ctx context.Context, ephem ephemeralRunner.Status.Reason = pod.Status.Reason ephemeralRunner.Status.Message = pod.Status.Message + // When pod-level reason/message are empty, extract details from container statuses. + // This surfaces errors like ImagePullBackOff that only appear at the container level. + if ephemeralRunner.Status.Message == "" { + if containerErrors := extractPodContainerErrors(pod); len(containerErrors) > 0 { + ephemeralRunner.Status.Message = formatPodContainerErrors(containerErrors) + if ephemeralRunner.Status.Reason == "" { + ephemeralRunner.Status.Reason = containerErrors[0].Category + } + } + } + if err := r.Status().Patch(ctx, ephemeralRunner, client.MergeFrom(original)); err != nil { return fmt.Errorf("failed to update ephemeral runner status with failure count: %w", err) } @@ -859,6 +897,9 @@ func (r *EphemeralRunnerReconciler) deleteRunnerFromService(ctx context.Context, // SetupWithManager sets up the controller with the Manager. func (r *EphemeralRunnerReconciler) SetupWithManager(mgr ctrl.Manager, opts ...Option) error { r.ResourceBuilder.setSchemeIfUnset(r.Scheme) + if r.Recorder == nil { + r.Recorder = mgr.GetEventRecorder("ephemeral-runner-controller") + } return builderWithOptions( ctrl.NewControllerManagedBy(mgr). diff --git a/controllers/actions.github.com/github_annotations.go b/controllers/actions.github.com/github_annotations.go new file mode 100644 index 0000000000..a8ddf6a016 --- /dev/null +++ b/controllers/actions.github.com/github_annotations.go @@ -0,0 +1,70 @@ +package actionsgithubcom + +import ( + "context" + "fmt" + + v1alpha1 "github.com/actions/actions-runner-controller/apis/actions.github.com/v1alpha1" + "github.com/actions/actions-runner-controller/github/actions" + "github.com/go-logr/logr" +) + +// GitHubAnnotator creates Check Run annotations on GitHub to surface pod errors in the Actions UI. +// Implementations must handle authentication and API calls to GitHub. +type GitHubAnnotator interface { + CreateErrorAnnotation(ctx context.Context, opts ErrorAnnotationOpts) error +} + +// ErrorAnnotationOpts contains the information needed to create a GitHub error annotation. +type ErrorAnnotationOpts struct { + Owner string + Repository string + RunnerName string + Namespace string + Message string + Reason string + WorkflowRunID int64 + JobID string +} + +// annotateRunnerFailure attempts to create a GitHub Check Run annotation for a failed runner. +// It is a best-effort operation — errors are logged but do not block the reconciliation. +func annotateRunnerFailure(ctx context.Context, annotator GitHubAnnotator, ephemeralRunner *v1alpha1.EphemeralRunner, log logr.Logger) { + if annotator == nil { + return + } + + owner, repo, err := parseOwnerRepo(ephemeralRunner.Spec.GitHubConfigURL) + if err != nil { + log.Info("Cannot annotate GitHub: failed to parse config URL", "error", err) + return + } + + opts := ErrorAnnotationOpts{ + Owner: owner, + Repository: repo, + RunnerName: ephemeralRunner.Name, + Namespace: ephemeralRunner.Namespace, + Message: ephemeralRunner.Status.Message, + Reason: ephemeralRunner.Status.Reason, + WorkflowRunID: ephemeralRunner.Status.WorkflowRunID, + JobID: ephemeralRunner.Status.JobID, + } + + if err := annotator.CreateErrorAnnotation(ctx, opts); err != nil { + log.Error(err, "Failed to create GitHub error annotation (best-effort)") + } +} + +func parseOwnerRepo(githubConfigURL string) (string, string, error) { + ghConfig, err := actions.ParseGitHubConfigFromURL(githubConfigURL) + if err != nil { + return "", "", fmt.Errorf("failed to parse GitHub config URL: %w", err) + } + + if ghConfig.Organization == "" { + return "", "", fmt.Errorf("cannot determine owner from config URL %q", githubConfigURL) + } + + return ghConfig.Organization, ghConfig.Repository, nil +} diff --git a/controllers/actions.github.com/github_annotations_test.go b/controllers/actions.github.com/github_annotations_test.go new file mode 100644 index 0000000000..dc8bed5db6 --- /dev/null +++ b/controllers/actions.github.com/github_annotations_test.go @@ -0,0 +1,69 @@ +package actionsgithubcom + +import ( + "context" + "testing" + + v1alpha1 "github.com/actions/actions-runner-controller/apis/actions.github.com/v1alpha1" + "github.com/go-logr/logr" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestParseOwnerRepo_RepoLevel(t *testing.T) { + owner, repo, err := parseOwnerRepo("https://github.com/my-org/my-repo") + require.NoError(t, err) + assert.Equal(t, "my-org", owner) + assert.Equal(t, "my-repo", repo) +} + +func TestParseOwnerRepo_OrgLevel(t *testing.T) { + owner, repo, err := parseOwnerRepo("https://github.com/my-org") + require.NoError(t, err) + assert.Equal(t, "my-org", owner) + assert.Equal(t, "", repo) +} + +func TestParseOwnerRepo_InvalidURL(t *testing.T) { + // URL with no path resolves to an empty organization, which we reject + _, _, err := parseOwnerRepo("https://github.com") + assert.Error(t, err) +} + +type fakeAnnotator struct { + called bool + opts ErrorAnnotationOpts + err error +} + +func (f *fakeAnnotator) CreateErrorAnnotation(_ context.Context, opts ErrorAnnotationOpts) error { + f.called = true + f.opts = opts + return f.err +} + +func TestAnnotateRunnerFailure_CallsAnnotator(t *testing.T) { + annotator := &fakeAnnotator{} + runner := &v1alpha1.EphemeralRunner{} + runner.Name = "test-runner" + runner.Namespace = "test-ns" + runner.Spec.GitHubConfigURL = "https://github.com/my-org/my-repo" + runner.Status.Message = "container \"runner\": ImagePullBackOff: image not found" + runner.Status.Reason = "ImagePull" + + annotateRunnerFailure(context.Background(), annotator, runner, logr.Discard()) + + assert.True(t, annotator.called) + assert.Equal(t, "my-org", annotator.opts.Owner) + assert.Equal(t, "my-repo", annotator.opts.Repository) + assert.Equal(t, "test-runner", annotator.opts.RunnerName) + assert.Contains(t, annotator.opts.Message, "ImagePullBackOff") +} + +func TestAnnotateRunnerFailure_NilAnnotator(t *testing.T) { + runner := &v1alpha1.EphemeralRunner{} + runner.Spec.GitHubConfigURL = "https://github.com/my-org/my-repo" + + // Should not panic + annotateRunnerFailure(context.Background(), nil, runner, logr.Discard()) +} diff --git a/controllers/actions.github.com/github_checkrun_annotator.go b/controllers/actions.github.com/github_checkrun_annotator.go new file mode 100644 index 0000000000..b29df0862e --- /dev/null +++ b/controllers/actions.github.com/github_checkrun_annotator.go @@ -0,0 +1,149 @@ +package actionsgithubcom + +import ( + "context" + "fmt" + "net/http" + "strconv" + + "github.com/actions/actions-runner-controller/apis/actions.github.com/v1alpha1/appconfig" + "github.com/actions/actions-runner-controller/github/actions" + "github.com/bradleyfalzon/ghinstallation/v2" + "github.com/go-logr/logr" + "github.com/google/go-github/v52/github" + "golang.org/x/oauth2" +) + +const checkRunName = "runner-pod-failure" + +// CheckRunAnnotator implements GitHubAnnotator by creating GitHub Check Runs +// using the go-github client library. +type CheckRunAnnotator struct { + Log logr.Logger +} + +func (a *CheckRunAnnotator) CreateErrorAnnotation(ctx context.Context, opts ErrorAnnotationOpts) error { + if opts.Repository == "" { + return fmt.Errorf("cannot create check run annotation: repository is empty (org-level runners not supported)") + } + + return fmt.Errorf("check run annotation requires a GitHub client; use NewCheckRunAnnotatorWithConfig instead") +} + +// CheckRunAnnotatorWithCredentials implements GitHubAnnotator with credential resolution. +// It creates a go-github client per call using the provided AppConfig. +type CheckRunAnnotatorWithCredentials struct { + Log logr.Logger + AppConfig *appconfig.AppConfig + ConfigURL string +} + +func (a *CheckRunAnnotatorWithCredentials) CreateErrorAnnotation(ctx context.Context, opts ErrorAnnotationOpts) error { + if opts.Repository == "" { + a.Log.Info("Skipping GitHub annotation: no repository (org-level runner)", "owner", opts.Owner) + return nil + } + + client, err := a.newGitHubClient() + if err != nil { + return fmt.Errorf("failed to create GitHub client for annotations: %w", err) + } + + summary := fmt.Sprintf( + "Runner pod `%s/%s` failed to start after %s retries.\n\n**Error:** %s", + opts.Namespace, + opts.RunnerName, + "multiple", + opts.Message, + ) + + title := fmt.Sprintf("Runner startup failure: %s", opts.Reason) + + status := "completed" + conclusion := "failure" + + checkRun := &github.CreateCheckRunOptions{ + Name: checkRunName, + Status: &status, + Conclusion: &conclusion, + Output: &github.CheckRunOutput{ + Title: &title, + Summary: &summary, + }, + } + + // If we have a specific workflow run, try to get its head SHA for the check run + if opts.WorkflowRunID > 0 { + run, _, err := client.Actions.GetWorkflowRunByID(ctx, opts.Owner, opts.Repository, opts.WorkflowRunID) + if err == nil && run.GetHeadSHA() != "" { + checkRun.HeadSHA = run.GetHeadSHA() + } else { + a.Log.Info("Could not resolve head SHA from workflow run, using default branch", "workflowRunID", opts.WorkflowRunID) + } + } + + // If no head SHA resolved, we cannot create a check run (it's required) + if checkRun.HeadSHA == "" { + a.Log.Info("No head SHA available for check run, skipping annotation") + return nil + } + + _, _, err = client.Checks.CreateCheckRun(ctx, opts.Owner, opts.Repository, *checkRun) + if err != nil { + return fmt.Errorf("failed to create check run: %w", err) + } + + a.Log.Info("Created GitHub check run annotation", "owner", opts.Owner, "repo", opts.Repository, "runner", opts.RunnerName) + return nil +} + +func (a *CheckRunAnnotatorWithCredentials) newGitHubClient() (*github.Client, error) { + ghConfig, err := actions.ParseGitHubConfigFromURL(a.ConfigURL) + if err != nil { + return nil, fmt.Errorf("failed to parse GitHub config URL: %w", err) + } + + apiURL := ghConfig.GitHubAPIURL("") + + if a.AppConfig.Token != "" { + ts := oauth2.StaticTokenSource(&oauth2.Token{AccessToken: a.AppConfig.Token}) + httpClient := oauth2.NewClient(context.Background(), ts) + if apiURL.Host == "api.github.com" { + return github.NewClient(httpClient), nil + } + client, err := github.NewEnterpriseClient(apiURL.String(), apiURL.String(), httpClient) + if err != nil { + return nil, fmt.Errorf("failed to create enterprise client with PAT: %w", err) + } + return client, nil + } + + appID, err := strconv.ParseInt(a.AppConfig.AppID, 10, 64) + if err != nil { + return nil, fmt.Errorf("failed to parse app ID: %w", err) + } + + transport, err := ghinstallation.New( + http.DefaultTransport, + appID, + a.AppConfig.AppInstallationID, + []byte(a.AppConfig.AppPrivateKey), + ) + if err != nil { + return nil, fmt.Errorf("failed to create GitHub App transport: %w", err) + } + + if apiURL.Host != "api.github.com" { + transport.BaseURL = apiURL.String() + } + + httpClient := &http.Client{Transport: transport} + if apiURL.Host == "api.github.com" { + return github.NewClient(httpClient), nil + } + client, err := github.NewEnterpriseClient(apiURL.String(), apiURL.String(), httpClient) + if err != nil { + return nil, fmt.Errorf("failed to create enterprise client with GitHub App: %w", err) + } + return client, nil +} diff --git a/controllers/actions.github.com/github_checkrun_annotator_test.go b/controllers/actions.github.com/github_checkrun_annotator_test.go new file mode 100644 index 0000000000..8c023bf8b8 --- /dev/null +++ b/controllers/actions.github.com/github_checkrun_annotator_test.go @@ -0,0 +1,61 @@ +package actionsgithubcom + +import ( + "context" + "testing" + + "github.com/actions/actions-runner-controller/apis/actions.github.com/v1alpha1/appconfig" + "github.com/go-logr/logr" + "github.com/stretchr/testify/assert" +) + +func TestCheckRunAnnotatorWithCredentials_SkipsOrgLevel(t *testing.T) { + annotator := &CheckRunAnnotatorWithCredentials{ + Log: logr.Discard(), + AppConfig: &appconfig.AppConfig{Token: "fake-token"}, + ConfigURL: "https://github.com/my-org", + } + + err := annotator.CreateErrorAnnotation(context.Background(), ErrorAnnotationOpts{ + Owner: "my-org", + Repository: "", // org-level, no repo + RunnerName: "test-runner", + Message: "ImagePullBackOff", + Reason: "ImagePull", + }) + + assert.NoError(t, err) +} + +func TestCheckRunAnnotatorWithCredentials_SkipsNoHeadSHA(t *testing.T) { + annotator := &CheckRunAnnotatorWithCredentials{ + Log: logr.Discard(), + AppConfig: &appconfig.AppConfig{Token: "fake-token"}, + ConfigURL: "https://github.com/my-org/my-repo", + } + + // No WorkflowRunID means no head SHA can be resolved, so annotation is skipped + err := annotator.CreateErrorAnnotation(context.Background(), ErrorAnnotationOpts{ + Owner: "my-org", + Repository: "my-repo", + RunnerName: "test-runner", + Message: "ImagePullBackOff: image not found", + Reason: "ImagePull", + WorkflowRunID: 0, + }) + + assert.NoError(t, err) +} + +func TestCheckRunAnnotator_RejectsEmptyRepo(t *testing.T) { + annotator := &CheckRunAnnotator{Log: logr.Discard()} + + err := annotator.CreateErrorAnnotation(context.Background(), ErrorAnnotationOpts{ + Owner: "my-org", + Repository: "", + RunnerName: "test-runner", + Message: "ImagePullBackOff", + }) + + assert.Error(t, err) +} diff --git a/controllers/actions.github.com/pod_errors.go b/controllers/actions.github.com/pod_errors.go new file mode 100644 index 0000000000..e245118141 --- /dev/null +++ b/controllers/actions.github.com/pod_errors.go @@ -0,0 +1,94 @@ +package actionsgithubcom + +import ( + "fmt" + "strings" + + corev1 "k8s.io/api/core/v1" +) + +// PodContainerError represents a detected container-level error from a pod's status. +type PodContainerError struct { + ContainerName string + Reason string + Message string + Category string + IsInit bool +} + +func (e *PodContainerError) String() string { + prefix := "container" + if e.IsInit { + prefix = "init-container" + } + if e.Message != "" { + return fmt.Sprintf("%s %q: %s: %s", prefix, e.ContainerName, e.Reason, e.Message) + } + return fmt.Sprintf("%s %q: %s", prefix, e.ContainerName, e.Reason) +} + +// extractPodContainerErrors inspects pod container statuses for terminal error states. +// It checks both waiting and terminated states across init containers and regular containers. +// Returns nil if no terminal errors are detected. +func extractPodContainerErrors(pod *corev1.Pod) []PodContainerError { + var errors []PodContainerError + + for i := range pod.Status.InitContainerStatuses { + cs := &pod.Status.InitContainerStatuses[i] + if err := checkContainerStatus(cs, true); err != nil { + errors = append(errors, *err) + } + } + + for i := range pod.Status.ContainerStatuses { + cs := &pod.Status.ContainerStatuses[i] + if err := checkContainerStatus(cs, false); err != nil { + errors = append(errors, *err) + } + } + + return errors +} + +func checkContainerStatus(cs *corev1.ContainerStatus, isInit bool) *PodContainerError { + if cs.State.Waiting != nil { + if category, ok := terminalContainerWaitingReasons[cs.State.Waiting.Reason]; ok { + return &PodContainerError{ + ContainerName: cs.Name, + Reason: cs.State.Waiting.Reason, + Message: cs.State.Waiting.Message, + Category: category, + IsInit: isInit, + } + } + } + + if cs.State.Terminated != nil { + if category, ok := terminalContainerTerminatedReasons[cs.State.Terminated.Reason]; ok { + return &PodContainerError{ + ContainerName: cs.Name, + Reason: cs.State.Terminated.Reason, + Message: cs.State.Terminated.Message, + Category: category, + IsInit: isInit, + } + } + } + + return nil +} + +// formatPodContainerErrors produces a human-readable summary of all detected container errors. +func formatPodContainerErrors(errors []PodContainerError) string { + if len(errors) == 0 { + return "" + } + if len(errors) == 1 { + return errors[0].String() + } + parts := make([]string, len(errors)) + for i, e := range errors { + parts[i] = e.String() + } + return strings.Join(parts, "; ") +} diff --git a/controllers/actions.github.com/pod_errors_test.go b/controllers/actions.github.com/pod_errors_test.go new file mode 100644 index 0000000000..309c23005e --- /dev/null +++ b/controllers/actions.github.com/pod_errors_test.go @@ -0,0 +1,185 @@ +package actionsgithubcom + +import ( + "testing" + + "github.com/stretchr/testify/assert" + corev1 "k8s.io/api/core/v1" +) + +func TestExtractPodContainerErrors_ImagePullBackOff(t *testing.T) { + pod := &corev1.Pod{ + Status: corev1.PodStatus{ + ContainerStatuses: []corev1.ContainerStatus{ + { + Name: "runner", + State: corev1.ContainerState{ + Waiting: &corev1.ContainerStateWaiting{ + Reason: "ImagePullBackOff", + Message: "Back-off pulling image \"ghcr.io/org/runner:v3\"", + }, + }, + }, + }, + }, + } + + errors := extractPodContainerErrors(pod) + assert.Len(t, errors, 1) + assert.Equal(t, "runner", errors[0].ContainerName) + assert.Equal(t, "ImagePullBackOff", errors[0].Reason) + assert.Equal(t, "ImagePull", errors[0].Category) + assert.False(t, errors[0].IsInit) + assert.Contains(t, errors[0].String(), "ghcr.io/org/runner:v3") +} + +func TestExtractPodContainerErrors_InitContainerError(t *testing.T) { + pod := &corev1.Pod{ + Status: corev1.PodStatus{ + InitContainerStatuses: []corev1.ContainerStatus{ + { + Name: "setup", + State: corev1.ContainerState{ + Waiting: &corev1.ContainerStateWaiting{ + Reason: "ErrImagePull", + Message: "unauthorized: authentication required", + }, + }, + }, + }, + ContainerStatuses: []corev1.ContainerStatus{ + { + Name: "runner", + State: corev1.ContainerState{ + Waiting: &corev1.ContainerStateWaiting{ + Reason: "PodInitializing", + Message: "", + }, + }, + }, + }, + }, + } + + errors := extractPodContainerErrors(pod) + assert.Len(t, errors, 1) + assert.Equal(t, "setup", errors[0].ContainerName) + assert.True(t, errors[0].IsInit) + assert.Equal(t, "ImagePull", errors[0].Category) +} + +func TestExtractPodContainerErrors_OOMKilled(t *testing.T) { + pod := &corev1.Pod{ + Status: corev1.PodStatus{ + ContainerStatuses: []corev1.ContainerStatus{ + { + Name: "runner", + State: corev1.ContainerState{ + Terminated: &corev1.ContainerStateTerminated{ + Reason: "OOMKilled", + Message: "", + ExitCode: 137, + }, + }, + }, + }, + }, + } + + errors := extractPodContainerErrors(pod) + assert.Len(t, errors, 1) + assert.Equal(t, "OOMKilled", errors[0].Reason) + assert.Equal(t, "ResourceLimit", errors[0].Category) +} + +func TestExtractPodContainerErrors_NoErrors(t *testing.T) { + pod := &corev1.Pod{ + Status: corev1.PodStatus{ + ContainerStatuses: []corev1.ContainerStatus{ + { + Name: "runner", + State: corev1.ContainerState{ + Running: &corev1.ContainerStateRunning{}, + }, + }, + }, + }, + } + + errors := extractPodContainerErrors(pod) + assert.Nil(t, errors) +} + +func TestExtractPodContainerErrors_NonTerminalWaiting(t *testing.T) { + pod := &corev1.Pod{ + Status: corev1.PodStatus{ + ContainerStatuses: []corev1.ContainerStatus{ + { + Name: "runner", + State: corev1.ContainerState{ + Waiting: &corev1.ContainerStateWaiting{ + Reason: "ContainerCreating", + }, + }, + }, + }, + }, + } + + errors := extractPodContainerErrors(pod) + assert.Nil(t, errors) +} + +func TestExtractPodContainerErrors_MultipleErrors(t *testing.T) { + pod := &corev1.Pod{ + Status: corev1.PodStatus{ + InitContainerStatuses: []corev1.ContainerStatus{ + { + Name: "init-creds", + State: corev1.ContainerState{ + Waiting: &corev1.ContainerStateWaiting{ + Reason: "ImagePullBackOff", + Message: "pull access denied", + }, + }, + }, + }, + ContainerStatuses: []corev1.ContainerStatus{ + { + Name: "runner", + State: corev1.ContainerState{ + Waiting: &corev1.ContainerStateWaiting{ + Reason: "CreateContainerConfigError", + Message: "secret not found", + }, + }, + }, + }, + }, + } + + errors := extractPodContainerErrors(pod) + assert.Len(t, errors, 2) + + formatted := formatPodContainerErrors(errors) + assert.Contains(t, formatted, "init-container") + assert.Contains(t, formatted, "ImagePullBackOff") + assert.Contains(t, formatted, "CreateContainerConfigError") +} + +func TestFormatPodContainerErrors_Empty(t *testing.T) { + assert.Equal(t, "", formatPodContainerErrors(nil)) +} + +func TestFormatPodContainerErrors_Single(t *testing.T) { + errors := []PodContainerError{ + { + ContainerName: "runner", + Reason: "ImagePullBackOff", + Message: "image not found", + Category: "ImagePull", + }, + } + result := formatPodContainerErrors(errors) + assert.Equal(t, `container "runner": ImagePullBackOff: image not found`, result) +} From 367e05b1ed8dcd5c4041c56c24cd4f251620b010 Mon Sep 17 00:00:00 2001 From: Max Knee Date: Tue, 23 Jun 2026 14:56:47 -0400 Subject: [PATCH 2/4] Wire up DynamicCheckRunAnnotator in main.go behind --enable-github-error-annotations flag Adds a CLI flag to opt into GitHub Check Run annotations when runner pods fail. The DynamicCheckRunAnnotator resolves credentials per-runner via the existing SecretResolver, so it works with multi-org/repo deployments using different GitHub Apps or PATs. Usage: pass --enable-github-error-annotations=true to the controller. Requires checks:write permission on the GitHub App or PAT. Co-Authored-By: Claude Opus 4.6 (1M context) --- .../actions.github.com/github_annotations.go | 19 +++-- .../github_checkrun_annotator.go | 80 +++++++++++++------ main.go | 15 +++- 3 files changed, 80 insertions(+), 34 deletions(-) diff --git a/controllers/actions.github.com/github_annotations.go b/controllers/actions.github.com/github_annotations.go index a8ddf6a016..ca8aca400c 100644 --- a/controllers/actions.github.com/github_annotations.go +++ b/controllers/actions.github.com/github_annotations.go @@ -17,14 +17,18 @@ type GitHubAnnotator interface { // ErrorAnnotationOpts contains the information needed to create a GitHub error annotation. type ErrorAnnotationOpts struct { - Owner string - Repository string - RunnerName string - Namespace string - Message string - Reason string + Owner string + Repository string + RunnerName string + Namespace string + Message string + Reason string WorkflowRunID int64 - JobID string + JobID string + + // runner is the EphemeralRunner object, used internally by DynamicCheckRunAnnotator + // to resolve credentials via SecretResolver. + runner *v1alpha1.EphemeralRunner } // annotateRunnerFailure attempts to create a GitHub Check Run annotation for a failed runner. @@ -49,6 +53,7 @@ func annotateRunnerFailure(ctx context.Context, annotator GitHubAnnotator, ephem Reason: ephemeralRunner.Status.Reason, WorkflowRunID: ephemeralRunner.Status.WorkflowRunID, JobID: ephemeralRunner.Status.JobID, + runner: ephemeralRunner, } if err := annotator.CreateErrorAnnotation(ctx, opts); err != nil { diff --git a/controllers/actions.github.com/github_checkrun_annotator.go b/controllers/actions.github.com/github_checkrun_annotator.go index b29df0862e..e6ab753abb 100644 --- a/controllers/actions.github.com/github_checkrun_annotator.go +++ b/controllers/actions.github.com/github_checkrun_annotator.go @@ -16,8 +16,7 @@ import ( const checkRunName = "runner-pod-failure" -// CheckRunAnnotator implements GitHubAnnotator by creating GitHub Check Runs -// using the go-github client library. +// CheckRunAnnotator is a basic implementation that rejects calls without credentials. type CheckRunAnnotator struct { Log logr.Logger } @@ -27,11 +26,11 @@ func (a *CheckRunAnnotator) CreateErrorAnnotation(ctx context.Context, opts Erro return fmt.Errorf("cannot create check run annotation: repository is empty (org-level runners not supported)") } - return fmt.Errorf("check run annotation requires a GitHub client; use NewCheckRunAnnotatorWithConfig instead") + return fmt.Errorf("check run annotation requires credentials; use CheckRunAnnotatorWithCredentials or DynamicCheckRunAnnotator") } -// CheckRunAnnotatorWithCredentials implements GitHubAnnotator with credential resolution. -// It creates a go-github client per call using the provided AppConfig. +// CheckRunAnnotatorWithCredentials implements GitHubAnnotator with static credentials. +// Useful for testing or single-repo deployments. type CheckRunAnnotatorWithCredentials struct { Log logr.Logger AppConfig *appconfig.AppConfig @@ -44,25 +43,58 @@ func (a *CheckRunAnnotatorWithCredentials) CreateErrorAnnotation(ctx context.Con return nil } - client, err := a.newGitHubClient() + client, err := newGitHubClientFromConfig(a.AppConfig, a.ConfigURL) if err != nil { return fmt.Errorf("failed to create GitHub client for annotations: %w", err) } + return createCheckRun(ctx, client, opts, a.Log) +} + +// DynamicCheckRunAnnotator implements GitHubAnnotator by resolving credentials +// dynamically per-runner via the SecretResolver. This is the production implementation +// used when wired up in main.go. +type DynamicCheckRunAnnotator struct { + Log logr.Logger + SecretResolver SecretResolver +} + +func (a *DynamicCheckRunAnnotator) CreateErrorAnnotation(ctx context.Context, opts ErrorAnnotationOpts) error { + if opts.Repository == "" { + a.Log.Info("Skipping GitHub annotation: no repository (org-level runner)", "owner", opts.Owner) + return nil + } + + if opts.runner == nil { + return fmt.Errorf("runner object is required for dynamic credential resolution") + } + + appConfig, err := a.SecretResolver.GetAppConfig(ctx, opts.runner) + if err != nil { + return fmt.Errorf("failed to resolve credentials for annotation: %w", err) + } + + client, err := newGitHubClientFromConfig(appConfig, opts.runner.GitHubConfigUrl()) + if err != nil { + return fmt.Errorf("failed to create GitHub client: %w", err) + } + + return createCheckRun(ctx, client, opts, a.Log) +} + +func createCheckRun(ctx context.Context, client *github.Client, opts ErrorAnnotationOpts, log logr.Logger) error { summary := fmt.Sprintf( - "Runner pod `%s/%s` failed to start after %s retries.\n\n**Error:** %s", + "Runner pod `%s/%s` failed to start after multiple retries.\n\n**Error:** %s", opts.Namespace, opts.RunnerName, - "multiple", opts.Message, ) title := fmt.Sprintf("Runner startup failure: %s", opts.Reason) - status := "completed" conclusion := "failure" - checkRun := &github.CreateCheckRunOptions{ + checkRunOpts := &github.CreateCheckRunOptions{ Name: checkRunName, Status: &status, Conclusion: &conclusion, @@ -72,41 +104,39 @@ func (a *CheckRunAnnotatorWithCredentials) CreateErrorAnnotation(ctx context.Con }, } - // If we have a specific workflow run, try to get its head SHA for the check run if opts.WorkflowRunID > 0 { run, _, err := client.Actions.GetWorkflowRunByID(ctx, opts.Owner, opts.Repository, opts.WorkflowRunID) if err == nil && run.GetHeadSHA() != "" { - checkRun.HeadSHA = run.GetHeadSHA() + checkRunOpts.HeadSHA = run.GetHeadSHA() } else { - a.Log.Info("Could not resolve head SHA from workflow run, using default branch", "workflowRunID", opts.WorkflowRunID) + log.Info("Could not resolve head SHA from workflow run", "workflowRunID", opts.WorkflowRunID) } } - // If no head SHA resolved, we cannot create a check run (it's required) - if checkRun.HeadSHA == "" { - a.Log.Info("No head SHA available for check run, skipping annotation") + if checkRunOpts.HeadSHA == "" { + log.Info("No head SHA available for check run, skipping annotation") return nil } - _, _, err = client.Checks.CreateCheckRun(ctx, opts.Owner, opts.Repository, *checkRun) + _, _, err := client.Checks.CreateCheckRun(ctx, opts.Owner, opts.Repository, *checkRunOpts) if err != nil { return fmt.Errorf("failed to create check run: %w", err) } - a.Log.Info("Created GitHub check run annotation", "owner", opts.Owner, "repo", opts.Repository, "runner", opts.RunnerName) + log.Info("Created GitHub check run annotation", "owner", opts.Owner, "repo", opts.Repository, "runner", opts.RunnerName) return nil } -func (a *CheckRunAnnotatorWithCredentials) newGitHubClient() (*github.Client, error) { - ghConfig, err := actions.ParseGitHubConfigFromURL(a.ConfigURL) +func newGitHubClientFromConfig(appConfig *appconfig.AppConfig, configURL string) (*github.Client, error) { + ghConfig, err := actions.ParseGitHubConfigFromURL(configURL) if err != nil { return nil, fmt.Errorf("failed to parse GitHub config URL: %w", err) } apiURL := ghConfig.GitHubAPIURL("") - if a.AppConfig.Token != "" { - ts := oauth2.StaticTokenSource(&oauth2.Token{AccessToken: a.AppConfig.Token}) + if appConfig.Token != "" { + ts := oauth2.StaticTokenSource(&oauth2.Token{AccessToken: appConfig.Token}) httpClient := oauth2.NewClient(context.Background(), ts) if apiURL.Host == "api.github.com" { return github.NewClient(httpClient), nil @@ -118,7 +148,7 @@ func (a *CheckRunAnnotatorWithCredentials) newGitHubClient() (*github.Client, er return client, nil } - appID, err := strconv.ParseInt(a.AppConfig.AppID, 10, 64) + appID, err := strconv.ParseInt(appConfig.AppID, 10, 64) if err != nil { return nil, fmt.Errorf("failed to parse app ID: %w", err) } @@ -126,8 +156,8 @@ func (a *CheckRunAnnotatorWithCredentials) newGitHubClient() (*github.Client, er transport, err := ghinstallation.New( http.DefaultTransport, appID, - a.AppConfig.AppInstallationID, - []byte(a.AppConfig.AppPrivateKey), + appConfig.AppInstallationID, + []byte(appConfig.AppPrivateKey), ) if err != nil { return nil, fmt.Errorf("failed to create GitHub App transport: %w", err) diff --git a/main.go b/main.go index 80c047e63f..35e7eb2003 100644 --- a/main.go +++ b/main.go @@ -116,6 +116,8 @@ func main() { k8sClientRateLimiterBurst int workqueueRateLimiter string + + enableGitHubErrorAnnotations bool ) var c github.Config err = envconfig.Process("github", &c) @@ -163,6 +165,7 @@ func main() { flag.IntVar(&k8sClientRateLimiterQPS, "k8s-client-rate-limiter-qps", 20, "The QPS value of the K8s client rate limiter.") flag.IntVar(&k8sClientRateLimiterBurst, "k8s-client-rate-limiter-burst", 30, "The burst value of the K8s client rate limiter.") flag.StringVar(&workqueueRateLimiter, "workqueue-rate-limiter", "", `The workqueue rate limiter to use. Valid values are "bucket_rate_limiter" (default) and "typed_rate_limiter" (per-item only, no global token bucket).`) + flag.BoolVar(&enableGitHubErrorAnnotations, "enable-github-error-annotations", false, "Enable creating GitHub Check Run annotations when runner pods fail with unrecoverable errors. Requires checks:write permission on the GitHub App or PAT.") flag.Parse() runnerPodDefaults.RunnerImagePullSecrets = runnerImagePullSecrets @@ -336,12 +339,20 @@ func main() { } runnerOpts := append(controllerOpts, actionsgithubcom.WithMaxConcurrentReconciles(opts.RunnerMaxConcurrentReconciles)) - if err = (&actionsgithubcom.EphemeralRunnerReconciler{ + ephemeralRunnerReconciler := &actionsgithubcom.EphemeralRunnerReconciler{ Client: mgr.GetClient(), Log: log.WithName("EphemeralRunner").WithValues("version", build.Version), Scheme: mgr.GetScheme(), ResourceBuilder: rb, - }).SetupWithManager(mgr, runnerOpts...); err != nil { + } + if enableGitHubErrorAnnotations { + log.Info("GitHub error annotations enabled — runner pod failures will create Check Run annotations") + ephemeralRunnerReconciler.GitHubAnnotator = &actionsgithubcom.DynamicCheckRunAnnotator{ + Log: log.WithName("GitHubAnnotator"), + SecretResolver: secretResolver, + } + } + if err = ephemeralRunnerReconciler.SetupWithManager(mgr, runnerOpts...); err != nil { log.Error(err, "unable to create controller", "controller", "EphemeralRunner") os.Exit(1) } From 2ebee9d7c6c470d4558e93fc97ae86c2037eb589 Mon Sep 17 00:00:00 2001 From: Max Knee Date: Tue, 23 Jun 2026 15:32:17 -0400 Subject: [PATCH 3/4] Include error in log when workflow run HEAD SHA resolution fails Addresses review feedback: log the actual error from GetWorkflowRunByID to help diagnose auth/scoping issues when the annotation is skipped. Co-Authored-By: Claude Opus 4.6 (1M context) --- controllers/actions.github.com/github_checkrun_annotator.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/controllers/actions.github.com/github_checkrun_annotator.go b/controllers/actions.github.com/github_checkrun_annotator.go index e6ab753abb..5349c28e4a 100644 --- a/controllers/actions.github.com/github_checkrun_annotator.go +++ b/controllers/actions.github.com/github_checkrun_annotator.go @@ -106,10 +106,10 @@ func createCheckRun(ctx context.Context, client *github.Client, opts ErrorAnnota if opts.WorkflowRunID > 0 { run, _, err := client.Actions.GetWorkflowRunByID(ctx, opts.Owner, opts.Repository, opts.WorkflowRunID) - if err == nil && run.GetHeadSHA() != "" { + if err != nil { + log.Info("Could not resolve head SHA from workflow run", "workflowRunID", opts.WorkflowRunID, "error", err) + } else if run.GetHeadSHA() != "" { checkRunOpts.HeadSHA = run.GetHeadSHA() - } else { - log.Info("Could not resolve head SHA from workflow run", "workflowRunID", opts.WorkflowRunID) } } From 4bdf7e216bc38327829bd7ccd2dcc556a9cce9db Mon Sep 17 00:00:00 2001 From: Max Knee Date: Tue, 23 Jun 2026 15:39:49 -0400 Subject: [PATCH 4/4] Regenerate mocks for GitHubAnnotator interface CI requires mocks to be up-to-date with interfaces. Run `go tool mockery` to include the new GitHubAnnotator mock. Co-Authored-By: Claude Opus 4.6 (1M context) --- controllers/actions.github.com/mocks_test.go | 84 ++++++++++++++++++++ 1 file changed, 84 insertions(+) diff --git a/controllers/actions.github.com/mocks_test.go b/controllers/actions.github.com/mocks_test.go index 7b6f8ca3ef..adce50d7eb 100644 --- a/controllers/actions.github.com/mocks_test.go +++ b/controllers/actions.github.com/mocks_test.go @@ -13,6 +13,90 @@ import ( mock "github.com/stretchr/testify/mock" ) +// NewMockGitHubAnnotator creates a new instance of MockGitHubAnnotator. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations. +// The first argument is typically a *testing.T value. +func NewMockGitHubAnnotator(t interface { + mock.TestingT + Cleanup(func()) +}) *MockGitHubAnnotator { + mock := &MockGitHubAnnotator{} + mock.Mock.Test(t) + + t.Cleanup(func() { mock.AssertExpectations(t) }) + + return mock +} + +// MockGitHubAnnotator is an autogenerated mock type for the GitHubAnnotator type +type MockGitHubAnnotator struct { + mock.Mock +} + +type MockGitHubAnnotator_Expecter struct { + mock *mock.Mock +} + +func (_m *MockGitHubAnnotator) EXPECT() *MockGitHubAnnotator_Expecter { + return &MockGitHubAnnotator_Expecter{mock: &_m.Mock} +} + +// CreateErrorAnnotation provides a mock function for the type MockGitHubAnnotator +func (_mock *MockGitHubAnnotator) CreateErrorAnnotation(ctx context.Context, opts ErrorAnnotationOpts) error { + ret := _mock.Called(ctx, opts) + + if len(ret) == 0 { + panic("no return value specified for CreateErrorAnnotation") + } + + var r0 error + if returnFunc, ok := ret.Get(0).(func(context.Context, ErrorAnnotationOpts) error); ok { + r0 = returnFunc(ctx, opts) + } else { + r0 = ret.Error(0) + } + return r0 +} + +// MockGitHubAnnotator_CreateErrorAnnotation_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'CreateErrorAnnotation' +type MockGitHubAnnotator_CreateErrorAnnotation_Call struct { + *mock.Call +} + +// CreateErrorAnnotation is a helper method to define mock.On call +// - ctx context.Context +// - opts ErrorAnnotationOpts +func (_e *MockGitHubAnnotator_Expecter) CreateErrorAnnotation(ctx interface{}, opts interface{}) *MockGitHubAnnotator_CreateErrorAnnotation_Call { + return &MockGitHubAnnotator_CreateErrorAnnotation_Call{Call: _e.mock.On("CreateErrorAnnotation", ctx, opts)} +} + +func (_c *MockGitHubAnnotator_CreateErrorAnnotation_Call) Run(run func(ctx context.Context, opts ErrorAnnotationOpts)) *MockGitHubAnnotator_CreateErrorAnnotation_Call { + _c.Call.Run(func(args mock.Arguments) { + var arg0 context.Context + if args[0] != nil { + arg0 = args[0].(context.Context) + } + var arg1 ErrorAnnotationOpts + if args[1] != nil { + arg1 = args[1].(ErrorAnnotationOpts) + } + run( + arg0, + arg1, + ) + }) + return _c +} + +func (_c *MockGitHubAnnotator_CreateErrorAnnotation_Call) Return(err error) *MockGitHubAnnotator_CreateErrorAnnotation_Call { + _c.Call.Return(err) + return _c +} + +func (_c *MockGitHubAnnotator_CreateErrorAnnotation_Call) RunAndReturn(run func(ctx context.Context, opts ErrorAnnotationOpts) error) *MockGitHubAnnotator_CreateErrorAnnotation_Call { + _c.Call.Return(run) + return _c +} + // NewMockSecretResolver creates a new instance of MockSecretResolver. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations. // The first argument is typically a *testing.T value. func NewMockSecretResolver(t interface {