Skip to content

Commit 2fac0bc

Browse files
committed
Pod logs: a container that hasn't started is an answer, not a red error
The pod Logs tab showed ImagePullBackOff, CreateContainerConfigError and ContainerCreating as a Radar failure: the snapshot returned HTTP 500 and the live stream ended on a red "Failed to open log stream". - The snapshot returns 404 with error_code container_not_started and a message built from the pod's status. - The stream stays open with a grey note, retries every 2s, starts streaming when the container does, and sends a keepalive every 15s so a proxy doesn't drop it during a long image pull. - REST, MCP and Helm hook evidence share one "is there an earlier run" check (podlogs.EarlierRun). - LogsViewer is keyed by pod, locks Previous and the range while the stream opens, and keeps showing a stream error above lines already loaded.
1 parent c2aecfe commit 2fac0bc

9 files changed

Lines changed: 499 additions & 154 deletions

File tree

‎internal/helm/hook_evidence.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -347,7 +347,7 @@ func collectHookLogs(ctx context.Context, client kubernetes.Interface, namespace
347347
if len(logs) >= maxHookEvidenceLogs {
348348
break
349349
}
350-
if podlogs.ContainerRestartCount(pod, container) > 0 {
350+
if hadEarlierRun, _ := podlogs.EarlierRun(pod, container); hadEarlierRun {
351351
if log, ok := fetchHookLog(ctx, client, namespace, pod.Name, container, true); ok {
352352
logs = append(logs, log)
353353
}

‎internal/mcp/tools_workloads.go‎

Lines changed: 4 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import (
1919
"k8s.io/client-go/dynamic"
2020

2121
"github.com/skyhook-io/radar/internal/k8s"
22+
"github.com/skyhook-io/radar/internal/podlogs"
2223
aicontext "github.com/skyhook-io/radar/pkg/ai/context"
2324
"github.com/skyhook-io/radar/pkg/k8score"
2425
"github.com/skyhook-io/radar/pkg/rollouts"
@@ -618,20 +619,8 @@ type podLogEntry struct {
618619
// zero restart count with no last termination covers both an apiserver
619620
// "not found" response and an empty successful previous-log stream.
620621
func expectedPreviousLogAbsence(pod *corev1.Pod, container string) bool {
621-
if pod == nil {
622-
return false
623-
}
624-
for _, statuses := range [][]corev1.ContainerStatus{
625-
pod.Status.ContainerStatuses,
626-
pod.Status.InitContainerStatuses,
627-
} {
628-
matching := filterContainerStatuses(statuses, container)
629-
if len(matching) == 0 {
630-
continue
631-
}
632-
return matching[0].RestartCount == 0 && matching[0].LastTerminationState.Terminated == nil
633-
}
634-
return false
622+
had, known := podlogs.EarlierRun(pod, container)
623+
return known && !had
635624
}
636625

637626
// fetchPodLogs fans out kubectl-logs requests across the given pods x containers.
@@ -676,8 +665,7 @@ func fetchPodLogs(ctx context.Context, pods []*corev1.Pod, namespace, containerF
676665
if err != nil {
677666
log.Printf("[mcp] Failed to get logs for %s/%s: %v", podName, containerName, err)
678667
entry.Error = fmt.Sprintf("failed to get logs: %v", err)
679-
entry.previousLogNotFound = previous && apierrors.IsBadRequest(err) &&
680-
strings.Contains(err.Error(), fmt.Sprintf("previous terminated container %q in pod %q not found", containerName, podName))
668+
entry.previousLogNotFound = previous && podlogs.IsNoPreviousRun(err)
681669
mu.Lock()
682670
allLogs = append(allLogs, entry)
683671
mu.Unlock()

‎internal/podlogs/pods.go‎

Lines changed: 0 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -48,25 +48,6 @@ func IsPodReady(pod *corev1.Pod) bool {
4848
return true
4949
}
5050

51-
// ContainerRestartCount reports how many times the named container (regular or
52-
// init) has restarted, or 0 when the pod has no status for it.
53-
func ContainerRestartCount(pod *corev1.Pod, name string) int32 {
54-
if pod == nil {
55-
return 0
56-
}
57-
for _, status := range pod.Status.InitContainerStatuses {
58-
if status.Name == name {
59-
return status.RestartCount
60-
}
61-
}
62-
for _, status := range pod.Status.ContainerStatuses {
63-
if status.Name == name {
64-
return status.RestartCount
65-
}
66-
}
67-
return 0
68-
}
69-
7051
// ContainerTerminated reports whether the named container's current run ended.
7152
func ContainerTerminated(pod *corev1.Pod, name string) bool {
7253
for _, statuses := range [][]corev1.ContainerStatus{pod.Status.ContainerStatuses, pod.Status.InitContainerStatuses, pod.Status.EphemeralContainerStatuses} {

‎internal/podlogs/runs.go‎

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,78 @@
1+
package podlogs
2+
3+
import (
4+
"strings"
5+
6+
corev1 "k8s.io/api/core/v1"
7+
apierrors "k8s.io/apimachinery/pkg/api/errors"
8+
)
9+
10+
// EarlierRun reports whether the pod's status records a run of the container
11+
// before the current one. known is false when the pod has no status for the
12+
// container yet, so a caller cannot tell either way.
13+
//
14+
// The restart count and the last termination are both checked: either one is
15+
// evidence of an earlier run, and the apiserver's "not found" answer for a
16+
// previous-run read cannot tell a container that never restarted from one
17+
// whose earlier run the node has since dropped.
18+
func EarlierRun(pod *corev1.Pod, container string) (had, known bool) {
19+
status, ok := containerStatus(pod, container)
20+
if !ok {
21+
return false, false
22+
}
23+
return status.RestartCount > 0 || status.LastTerminationState.Terminated != nil, true
24+
}
25+
26+
// IsNoPreviousRun matches the apiserver's answer to a previous-run read when it
27+
// has no earlier run to give. It comes as a 400, so any other failure that
28+
// mentions the phrase is left alone.
29+
func IsNoPreviousRun(err error) bool {
30+
return apierrors.IsBadRequest(err) && strings.Contains(err.Error(), "previous terminated container")
31+
}
32+
33+
const waitingToStart = "is waiting to start"
34+
35+
// IsContainerNotStarted matches the apiserver's answer to a log read for a
36+
// container that has not run yet: ContainerCreating, an image that cannot be
37+
// pulled, a missing ConfigMap or Secret. A container that ran before and is now
38+
// waiting to restart still has logs, so it does not get this answer.
39+
func IsContainerNotStarted(err error) bool {
40+
return apierrors.IsBadRequest(err) && strings.Contains(err.Error(), waitingToStart)
41+
}
42+
43+
// NotStartedMessage says why a container that has not started has no logs.
44+
// The reason comes from the pod's status when Radar has it, since that carries
45+
// the kubelet's detail (the image it could not pull, the ConfigMap it could not
46+
// find), and otherwise from the apiserver's answer.
47+
func NotStartedMessage(pod *corev1.Pod, container string, err error) string {
48+
const lead = "This container has not started yet, so it has no logs."
49+
reason := ""
50+
if status, ok := containerStatus(pod, container); ok && status.State.Waiting != nil && status.State.Waiting.Reason != "" {
51+
reason = status.State.Waiting.Reason
52+
if msg := strings.TrimSpace(status.State.Waiting.Message); msg != "" {
53+
reason += ": " + msg
54+
}
55+
} else if err != nil {
56+
if _, after, found := strings.Cut(err.Error(), waitingToStart+": "); found {
57+
reason = strings.TrimSpace(after)
58+
}
59+
}
60+
if reason == "" {
61+
return lead
62+
}
63+
return lead + " Kubernetes reports " + strings.TrimSuffix(reason, ".") + "."
64+
}
65+
66+
func containerStatus(pod *corev1.Pod, name string) (corev1.ContainerStatus, bool) {
67+
if pod == nil {
68+
return corev1.ContainerStatus{}, false
69+
}
70+
for _, statuses := range [][]corev1.ContainerStatus{pod.Status.InitContainerStatuses, pod.Status.ContainerStatuses} {
71+
for _, status := range statuses {
72+
if status.Name == name {
73+
return status, true
74+
}
75+
}
76+
}
77+
return corev1.ContainerStatus{}, false
78+
}

‎internal/podlogs/runs_test.go‎

Lines changed: 124 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,124 @@
1+
package podlogs
2+
3+
import (
4+
"context"
5+
"encoding/json"
6+
"errors"
7+
"net/http"
8+
"net/http/httptest"
9+
"testing"
10+
11+
corev1 "k8s.io/api/core/v1"
12+
"k8s.io/client-go/kubernetes"
13+
"k8s.io/client-go/rest"
14+
15+
"github.com/skyhook-io/radar/pkg/k8score"
16+
)
17+
18+
// readAnswer returns the error a log read gets when the apiserver answers with
19+
// the given 400 message, the way it relays the kubelet's refusal.
20+
func readAnswer(t *testing.T, message string) error {
21+
t.Helper()
22+
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
23+
w.Header().Set("Content-Type", "application/json")
24+
w.WriteHeader(http.StatusBadRequest)
25+
_ = json.NewEncoder(w).Encode(map[string]any{"kind": "Status", "apiVersion": "v1", "status": "Failure", "message": message, "reason": "BadRequest", "code": 400})
26+
}))
27+
t.Cleanup(srv.Close)
28+
client, err := kubernetes.NewForConfig(&rest.Config{Host: srv.URL})
29+
if err != nil {
30+
t.Fatal(err)
31+
}
32+
_, err = k8score.GetContainerLogs(context.Background(), client, "default", "api-0", "app", k8score.LogOptions{})
33+
return err
34+
}
35+
36+
func TestIsNoPreviousRun(t *testing.T) {
37+
if !IsNoPreviousRun(readAnswer(t, `previous terminated container "app" in pod "api-0" not found`)) {
38+
t.Error("the apiserver's no-earlier-run answer should be recognised")
39+
}
40+
if IsNoPreviousRun(errors.New(`previous terminated container "app" in pod "api-0" not found`)) {
41+
t.Error("only the apiserver's 400 answer should be recognised, not any error with the same words")
42+
}
43+
if IsNoPreviousRun(errors.New("connection refused")) || IsNoPreviousRun(nil) {
44+
t.Error("an unrelated failure or no error is not a missing earlier run")
45+
}
46+
}
47+
48+
// The messages are the kubelet's, captured from a kind cluster.
49+
func TestIsContainerNotStarted(t *testing.T) {
50+
for _, message := range []string{
51+
`container "app" in pod "api-0" is waiting to start: ContainerCreating`,
52+
`container "app" in pod "api-0" is waiting to start: image can't be pulled`,
53+
`container "app" in pod "api-0" is waiting to start: trying and failing to pull image`,
54+
`container "app" in pod "api-0" is waiting to start: CreateContainerConfigError`,
55+
`container "app" in pod "api-0" is waiting to start - no logs yet`,
56+
} {
57+
if !IsContainerNotStarted(readAnswer(t, message)) {
58+
t.Errorf("not recognised: %s", message)
59+
}
60+
}
61+
if IsContainerNotStarted(readAnswer(t, `previous terminated container "app" in pod "api-0" not found`)) {
62+
t.Error("a missing earlier run is a different answer")
63+
}
64+
if IsContainerNotStarted(errors.New(`container "app" in pod "api-0" is waiting to start: ContainerCreating`)) {
65+
t.Error("only the apiserver's 400 answer should be recognised")
66+
}
67+
if IsContainerNotStarted(nil) {
68+
t.Error("no error is not a container that has not started")
69+
}
70+
}
71+
72+
func TestEarlierRun(t *testing.T) {
73+
terminated := corev1.ContainerState{Terminated: &corev1.ContainerStateTerminated{ExitCode: 1}}
74+
for _, tc := range []struct {
75+
name string
76+
pod *corev1.Pod
77+
had, known bool
78+
}{
79+
{"never restarted", &corev1.Pod{Status: corev1.PodStatus{ContainerStatuses: []corev1.ContainerStatus{{Name: "app"}}}}, false, true},
80+
{"current run ended is not an earlier run", &corev1.Pod{Status: corev1.PodStatus{ContainerStatuses: []corev1.ContainerStatus{{Name: "app", State: terminated}}}}, false, true},
81+
{"restart count", &corev1.Pod{Status: corev1.PodStatus{ContainerStatuses: []corev1.ContainerStatus{{Name: "app", RestartCount: 1}}}}, true, true},
82+
{"last termination without a restart count", &corev1.Pod{Status: corev1.PodStatus{ContainerStatuses: []corev1.ContainerStatus{{Name: "app", LastTerminationState: terminated}}}}, true, true},
83+
{"init container", &corev1.Pod{Status: corev1.PodStatus{InitContainerStatuses: []corev1.ContainerStatus{{Name: "app", RestartCount: 2}}}}, true, true},
84+
{"no status yet", &corev1.Pod{}, false, false},
85+
{"no pod", nil, false, false},
86+
} {
87+
t.Run(tc.name, func(t *testing.T) {
88+
had, known := EarlierRun(tc.pod, "app")
89+
if had != tc.had || known != tc.known {
90+
t.Fatalf("EarlierRun = (%t, %t), want (%t, %t)", had, known, tc.had, tc.known)
91+
}
92+
})
93+
}
94+
}
95+
96+
func TestNotStartedMessage(t *testing.T) {
97+
waiting := func(reason, message string) *corev1.Pod {
98+
return &corev1.Pod{Status: corev1.PodStatus{ContainerStatuses: []corev1.ContainerStatus{{
99+
Name: "app", State: corev1.ContainerState{Waiting: &corev1.ContainerStateWaiting{Reason: reason, Message: message}},
100+
}}}}
101+
}
102+
answer := errors.New(`failed to stream logs for default/api-0/app: container "app" in pod "api-0" is waiting to start: trying and failing to pull image`)
103+
for _, tc := range []struct {
104+
name string
105+
pod *corev1.Pod
106+
err error
107+
want string
108+
}{
109+
{"status reason and message", waiting("CreateContainerConfigError", `configmap "missing-config" not found`), answer,
110+
`This container has not started yet, so it has no logs. Kubernetes reports CreateContainerConfigError: configmap "missing-config" not found.`},
111+
{"status reason only", waiting("ContainerCreating", ""), answer,
112+
"This container has not started yet, so it has no logs. Kubernetes reports ContainerCreating."},
113+
{"no status falls back to the apiserver's answer", nil, answer,
114+
"This container has not started yet, so it has no logs. Kubernetes reports trying and failing to pull image."},
115+
{"nothing to add", nil, errors.New(`container "app" in pod "api-0" is waiting to start - no logs yet`),
116+
"This container has not started yet, so it has no logs."},
117+
} {
118+
t.Run(tc.name, func(t *testing.T) {
119+
if got := NotStartedMessage(tc.pod, "app", tc.err); got != tc.want {
120+
t.Fatalf("got %q\nwant %q", got, tc.want)
121+
}
122+
})
123+
}
124+
}

0 commit comments

Comments
 (0)