Skip to content

Commit a3526bd

Browse files
authored
fix: populate LogContext from live JobSet pods for clustered tasks (#7464)
* fix: logs Signed-off-by: M. Adil Fayyaz <62440954+AdilFayyaz@users.noreply.github.com> * fix: review Signed-off-by: M. Adil Fayyaz <62440954+AdilFayyaz@users.noreply.github.com> * fix Signed-off-by: M. Adil Fayyaz <62440954+AdilFayyaz@users.noreply.github.com> * fix docstrings Signed-off-by: M. Adil Fayyaz <62440954+AdilFayyaz@users.noreply.github.com> * fix Signed-off-by: M. Adil Fayyaz <62440954+AdilFayyaz@users.noreply.github.com> --------- Signed-off-by: M. Adil Fayyaz <62440954+AdilFayyaz@users.noreply.github.com>
1 parent dee9c75 commit a3526bd

4 files changed

Lines changed: 201 additions & 14 deletions

File tree

flyteplugins/go/tasks/plugins/k8s/clustered/build.go

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import (
1313
flyteerr "github.com/flyteorg/flyte/v2/flyteplugins/go/tasks/errors"
1414
pluginsCore "github.com/flyteorg/flyte/v2/flyteplugins/go/tasks/pluginmachinery/core"
1515
"github.com/flyteorg/flyte/v2/flyteplugins/go/tasks/pluginmachinery/flytek8s"
16+
"github.com/flyteorg/flyte/v2/flyteplugins/go/tasks/pluginmachinery/flytek8s/config"
1617
"github.com/flyteorg/flyte/v2/flyteplugins/go/tasks/pluginmachinery/utils"
1718
clusteredpb "github.com/flyteorg/flyte/v2/gen/go/flyteidl2/plugins"
1819
)
@@ -45,6 +46,18 @@ func (clusteredResourceHandler) BuildResource(ctx context.Context, taskCtx plugi
4546

4647
podSpec = applyInterconnect(ctx, spec.GetInterconnect(), podSpec)
4748

49+
// Propagate the node-execution labels/annotations onto the pod template. The plugin
50+
// manager's addObjectMetadata only stamps these (incl. execution-id/node-id) on the
51+
// top-level JobSet, and the JobSet controller does not copy arbitrary parent labels
52+
// down to child pods. Without this, child pods lack execution-id/node-id and the
53+
// node-execution-scoped K8sReader.List in getLogContext returns nothing, so no
54+
// LogContext reaches the UI. Mirrors ray's buildWorkerPodTemplate.
55+
cfg := config.GetK8sPluginConfig()
56+
objectMeta.Labels = utils.UnionMaps(cfg.DefaultLabels, objectMeta.Labels,
57+
utils.CopyMap(taskCtx.TaskExecutionMetadata().GetLabels()))
58+
objectMeta.Annotations = utils.UnionMaps(cfg.DefaultAnnotations, objectMeta.Annotations,
59+
utils.CopyMap(taskCtx.TaskExecutionMetadata().GetAnnotations()))
60+
4861
// The SDK is responsible for setting container.Command to the entrypoint module
4962
// (python -m flyte.distributed._entrypoint) at serde time. The plugin stays
5063
// module-path-agnostic so SDK renames do not require a backend release.

flyteplugins/go/tasks/plugins/k8s/clustered/clustered_test.go

Lines changed: 131 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,9 @@ import (
1212
"k8s.io/apimachinery/pkg/api/resource"
1313
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
1414
k8sscheme "k8s.io/client-go/kubernetes/scheme"
15-
jobsetv1alpha2 "sigs.k8s.io/jobset/api/jobset/v1alpha2"
15+
"sigs.k8s.io/controller-runtime/pkg/client"
1616
"sigs.k8s.io/controller-runtime/pkg/client/fake"
17+
jobsetv1alpha2 "sigs.k8s.io/jobset/api/jobset/v1alpha2"
1718

1819
pluginsCore "github.com/flyteorg/flyte/v2/flyteplugins/go/tasks/pluginmachinery/core"
1920
coreMocks "github.com/flyteorg/flyte/v2/flyteplugins/go/tasks/pluginmachinery/core/mocks"
@@ -99,8 +100,8 @@ func dummyTaskCtx(taskTemplate *core.TaskTemplate) *coreMocks.TaskExecutionConte
99100
meta := &coreMocks.TaskExecutionMetadata{}
100101
meta.EXPECT().GetTaskExecutionID().Return(tID)
101102
meta.EXPECT().GetNamespace().Return(testNS)
102-
meta.EXPECT().GetAnnotations().Return(map[string]string{})
103-
meta.EXPECT().GetLabels().Return(map[string]string{})
103+
meta.EXPECT().GetAnnotations().Return(map[string]string{"flyte.org/test-annotation": "av"})
104+
meta.EXPECT().GetLabels().Return(map[string]string{"execution-id": "my-exec", "node-id": "n1"})
104105
meta.EXPECT().GetOwnerReference().Return(metav1.OwnerReference{Kind: "node", Name: "n1"})
105106
meta.EXPECT().IsInterruptible().Return(false)
106107
meta.EXPECT().GetOverrides().Return(overrides)
@@ -154,6 +155,14 @@ func TestBuildResource_HappyPath(t *testing.T) {
154155
assert.Equal(t, int32(4), *jobSpec.Completions)
155156
assert.Equal(t, batchv1.IndexedCompletion, *jobSpec.CompletionMode)
156157
assert.Equal(t, int32(0), *jobSpec.BackoffLimit)
158+
159+
// The node-execution labels/annotations must be propagated onto the pod template so
160+
// JobSet child pods carry execution-id/node-id; otherwise the node-execution-scoped
161+
// K8sReader.List in getLogContext returns nothing and no logs reach the UI.
162+
podMeta := jobSpec.Template.ObjectMeta
163+
assert.Equal(t, "my-exec", podMeta.Labels["execution-id"])
164+
assert.Equal(t, "n1", podMeta.Labels["node-id"])
165+
assert.Equal(t, "av", podMeta.Annotations["flyte.org/test-annotation"])
157166
}
158167

159168
func TestBuildResource_PrimaryContainerPreserved(t *testing.T) {
@@ -320,13 +329,21 @@ func makeJobSet(condType jobsetv1alpha2.JobSetConditionType, status metav1.Condi
320329
return js
321330
}
322331

323-
func dummyPluginCtx(taskTemplate *core.TaskTemplate) *k8smocks.PluginContext {
332+
// emptyK8sReader returns a fake client with no objects, for tests that don't
333+
// exercise pod inspection (getLogContext just yields an empty pod list -> nil LogContext).
334+
func emptyK8sReader() client.Reader {
335+
return fake.NewClientBuilder().WithScheme(k8sscheme.Scheme).Build()
336+
}
337+
338+
func dummyPluginCtx(taskTemplate *core.TaskTemplate, k8sReader client.Reader) *k8smocks.PluginContext {
324339
pCtx := &k8smocks.PluginContext{}
325340

326341
taskReader := &coreMocks.TaskReader{}
327342
taskReader.EXPECT().Read(mock.Anything).Return(taskTemplate, nil)
328343
pCtx.EXPECT().TaskReader().Return(taskReader)
329344

345+
pCtx.EXPECT().K8sReader().Return(k8sReader)
346+
330347
tID := &coreMocks.TaskExecutionID{}
331348
tID.EXPECT().GetID().Return(&core.TaskExecutionIdentifier{
332349
NodeExecutionId: &core.NodeExecutionIdentifier{
@@ -352,7 +369,7 @@ func TestGetTaskPhase_Initializing(t *testing.T) {
352369
js := makeJobSet("", "", suspend)
353370

354371
spec := &clusteredpb.ClusteredTaskSpec{Replicas: 2, NprocPerNode: 1}
355-
pCtx := dummyPluginCtx(buildTaskTemplate(spec))
372+
pCtx := dummyPluginCtx(buildTaskTemplate(spec), emptyK8sReader())
356373

357374
handler := clusteredResourceHandler{}
358375
phase, err := handler.GetTaskPhase(context.Background(), pCtx, js)
@@ -364,7 +381,7 @@ func TestGetTaskPhase_Success(t *testing.T) {
364381
js := makeJobSet(jobsetv1alpha2.JobSetCompleted, metav1.ConditionTrue, false)
365382

366383
spec := &clusteredpb.ClusteredTaskSpec{Replicas: 2, NprocPerNode: 1}
367-
pCtx := dummyPluginCtx(buildTaskTemplate(spec))
384+
pCtx := dummyPluginCtx(buildTaskTemplate(spec), emptyK8sReader())
368385

369386
handler := clusteredResourceHandler{}
370387
phase, err := handler.GetTaskPhase(context.Background(), pCtx, js)
@@ -376,7 +393,7 @@ func TestGetTaskPhase_Failure(t *testing.T) {
376393
js := makeJobSet(jobsetv1alpha2.JobSetFailed, metav1.ConditionTrue, false)
377394

378395
spec := &clusteredpb.ClusteredTaskSpec{Replicas: 2, NprocPerNode: 1}
379-
pCtx := dummyPluginCtx(buildTaskTemplate(spec))
396+
pCtx := dummyPluginCtx(buildTaskTemplate(spec), emptyK8sReader())
380397

381398
handler := clusteredResourceHandler{}
382399
phase, err := handler.GetTaskPhase(context.Background(), pCtx, js)
@@ -397,7 +414,7 @@ func TestGetTaskPhase_Running(t *testing.T) {
397414
}
398415

399416
spec := &clusteredpb.ClusteredTaskSpec{Replicas: 2, NprocPerNode: 1}
400-
pCtx := dummyPluginCtx(buildTaskTemplate(spec))
417+
pCtx := dummyPluginCtx(buildTaskTemplate(spec), emptyK8sReader())
401418

402419
handler := clusteredResourceHandler{}
403420
phase, err := handler.GetTaskPhase(context.Background(), pCtx, js)
@@ -424,7 +441,7 @@ func TestGetTaskPhase_FastFail_NoJobsFailed(t *testing.T) {
424441
}
425442

426443
spec := &clusteredpb.ClusteredTaskSpec{Replicas: 2, NprocPerNode: 1}
427-
pCtx := dummyPluginCtx(buildTaskTemplate(spec))
444+
pCtx := dummyPluginCtx(buildTaskTemplate(spec), emptyK8sReader())
428445

429446
handler := clusteredResourceHandler{}
430447
phase, err := handler.GetTaskPhase(context.Background(), pCtx, js)
@@ -442,7 +459,7 @@ func TestGetTaskPhase_MaintenanceRetry_FlagFalse(t *testing.T) {
442459
NprocPerNode: 1,
443460
FailurePolicy: &clusteredpb.ClusterFailurePolicy{RestartOnHostMaintenance: false},
444461
}
445-
pCtx := dummyPluginCtx(buildTaskTemplate(spec))
462+
pCtx := dummyPluginCtx(buildTaskTemplate(spec), emptyK8sReader())
446463

447464
handler := clusteredResourceHandler{}
448465
phase, err := handler.GetTaskPhase(context.Background(), pCtx, js)
@@ -484,8 +501,7 @@ func TestGetTaskPhase_FastFail_Worker0Failed(t *testing.T) {
484501
fakeClient := fake.NewClientBuilder().WithScheme(k8sscheme.Scheme).WithObjects(pod).Build()
485502

486503
spec := &clusteredpb.ClusteredTaskSpec{Replicas: 2, NprocPerNode: 1}
487-
pCtx := dummyPluginCtx(buildTaskTemplate(spec))
488-
pCtx.EXPECT().K8sReader().Return(fakeClient)
504+
pCtx := dummyPluginCtx(buildTaskTemplate(spec), fakeClient)
489505

490506
handler := clusteredResourceHandler{}
491507
phase, err := handler.GetTaskPhase(context.Background(), pCtx, js)
@@ -517,8 +533,7 @@ func TestGetTaskPhase_MaintenanceRetry_SystemFailure(t *testing.T) {
517533
NprocPerNode: 1,
518534
FailurePolicy: &clusteredpb.ClusterFailurePolicy{RestartOnHostMaintenance: true},
519535
}
520-
pCtx := dummyPluginCtx(buildTaskTemplate(spec))
521-
pCtx.EXPECT().K8sReader().Return(fakeClient)
536+
pCtx := dummyPluginCtx(buildTaskTemplate(spec), fakeClient)
522537

523538
handler := clusteredResourceHandler{}
524539
phase, err := handler.GetTaskPhase(context.Background(), pCtx, js)
@@ -527,6 +542,108 @@ func TestGetTaskPhase_MaintenanceRetry_SystemFailure(t *testing.T) {
527542
assert.Equal(t, core.ExecutionError_SYSTEM, phase.Err().GetKind())
528543
}
529544

545+
func TestGetTaskPhase_LogContext(t *testing.T) {
546+
const primaryContainer = "primary"
547+
const sidecarContainer = "sidecar"
548+
549+
// mkPod builds a realistic JobSet child pod: a primary container plus a sidecar,
550+
// with matching container statuses so BuildPodLogContext produces real container
551+
// contexts. Pending pods carry no statuses.
552+
mkPod := func(name string, phase corev1.PodPhase) *corev1.Pod {
553+
pod := &corev1.Pod{
554+
ObjectMeta: metav1.ObjectMeta{
555+
Name: name,
556+
Namespace: testNS,
557+
},
558+
Spec: corev1.PodSpec{
559+
Containers: []corev1.Container{{Name: primaryContainer}, {Name: sidecarContainer}},
560+
},
561+
Status: corev1.PodStatus{Phase: phase},
562+
}
563+
if phase == corev1.PodRunning {
564+
running := corev1.ContainerState{Running: &corev1.ContainerStateRunning{StartedAt: metav1.NewTime(time.Now())}}
565+
pod.Status.ContainerStatuses = []corev1.ContainerStatus{
566+
{Name: primaryContainer, State: running},
567+
{Name: sidecarContainer, State: running},
568+
}
569+
}
570+
return pod
571+
}
572+
573+
// jobSet annotates the authoritative primary container name at build time.
574+
makeRunningJobSet := func() *jobsetv1alpha2.JobSet {
575+
js := makeJobSet("", "", false)
576+
js.Annotations = map[string]string{primaryContainerAnnotation: primaryContainer}
577+
js.Status.Conditions = []metav1.Condition{
578+
{Type: "SomeActiveCondition", Status: metav1.ConditionTrue, LastTransitionTime: metav1.NewTime(time.Now())},
579+
}
580+
return js
581+
}
582+
583+
// Real JobSet pods carry a random suffix after the "<jobset>-workers-<job>-<idx>" stem.
584+
rank0 := rank0PodName(testJobName) + "-x1y2z"
585+
rank1 := testJobName + "-workers-0-1-a9b8c"
586+
rank2 := testJobName + "-workers-0-2-pppp"
587+
588+
t.Run("primary pod and container resolved from live pods", func(t *testing.T) {
589+
js := makeRunningJobSet()
590+
fakeClient := fake.NewClientBuilder().WithScheme(k8sscheme.Scheme).
591+
WithObjects(
592+
mkPod(rank0, corev1.PodRunning),
593+
mkPod(rank1, corev1.PodRunning),
594+
mkPod(rank2, corev1.PodPending),
595+
).Build()
596+
597+
spec := &clusteredpb.ClusteredTaskSpec{Replicas: 2, NprocPerNode: 1}
598+
pCtx := dummyPluginCtx(buildTaskTemplate(spec), fakeClient)
599+
600+
handler := clusteredResourceHandler{}
601+
phase, err := handler.GetTaskPhase(context.Background(), pCtx, js)
602+
assert.NoError(t, err)
603+
assert.Equal(t, pluginsCore.PhaseRunning, phase.Phase())
604+
605+
lc := phase.Info().LogContext
606+
assert.NotNil(t, lc)
607+
assert.Equal(t, rank0, lc.PrimaryPodName)
608+
// Pending pod is excluded → only the two running pods remain.
609+
assert.Len(t, lc.Pods, 2)
610+
names := []string{lc.Pods[0].GetPodName(), lc.Pods[1].GetPodName()}
611+
assert.Contains(t, names, rank0)
612+
assert.Contains(t, names, rank1)
613+
614+
// Each pod's primary container comes from the JobSet annotation (not the
615+
// sidecar / first container), and container contexts are populated.
616+
for _, p := range lc.Pods {
617+
assert.Equal(t, primaryContainer, p.GetPrimaryContainerName())
618+
assert.GreaterOrEqual(t, len(p.GetContainers()), 1)
619+
}
620+
})
621+
622+
t.Run("primary falls back when rank-0 pod is pending", func(t *testing.T) {
623+
js := makeRunningJobSet()
624+
fakeClient := fake.NewClientBuilder().WithScheme(k8sscheme.Scheme).
625+
WithObjects(
626+
mkPod(rank0, corev1.PodPending),
627+
mkPod(rank1, corev1.PodRunning),
628+
).Build()
629+
630+
spec := &clusteredpb.ClusteredTaskSpec{Replicas: 2, NprocPerNode: 1}
631+
pCtx := dummyPluginCtx(buildTaskTemplate(spec), fakeClient)
632+
633+
handler := clusteredResourceHandler{}
634+
phase, err := handler.GetTaskPhase(context.Background(), pCtx, js)
635+
assert.NoError(t, err)
636+
637+
lc := phase.Info().LogContext
638+
assert.NotNil(t, lc)
639+
// rank-0 is pending and excluded → PrimaryPodName must still reference an
640+
// included pod so downstream log streaming can resolve it.
641+
assert.Len(t, lc.Pods, 1)
642+
assert.Equal(t, rank1, lc.PrimaryPodName)
643+
assert.Equal(t, lc.Pods[0].GetPodName(), lc.PrimaryPodName)
644+
})
645+
}
646+
530647
// --- IsTerminal / GetCompletionTime ---
531648

532649
func TestIsTerminal(t *testing.T) {

flyteplugins/go/tasks/plugins/k8s/clustered/logs.go

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,13 +3,17 @@ package clustered
33
import (
44
"context"
55
"fmt"
6+
"strings"
67
"time"
78

9+
v1 "k8s.io/api/core/v1"
810
jobsetv1alpha2 "sigs.k8s.io/jobset/api/jobset/v1alpha2"
911

1012
"github.com/flyteorg/flyte/v2/flyteplugins/go/tasks/logs"
13+
"github.com/flyteorg/flyte/v2/flyteplugins/go/tasks/pluginmachinery/flytek8s"
1114
"github.com/flyteorg/flyte/v2/flyteplugins/go/tasks/pluginmachinery/k8s"
1215
"github.com/flyteorg/flyte/v2/flyteplugins/go/tasks/pluginmachinery/tasklog"
16+
"github.com/flyteorg/flyte/v2/flytestdlib/logger"
1317
"github.com/flyteorg/flyte/v2/gen/go/flyteidl2/core"
1418
)
1519

@@ -76,3 +80,55 @@ func getTaskLogs(ctx context.Context, pluginContext k8s.PluginContext, jobSet *j
7680
}
7781
return taskLogs, nil
7882
}
83+
84+
// getLogContext builds the structured LogContext from the JobSet's live child pods.
85+
//
86+
// Unlike getTaskLogs (which synthesizes templated URIs from *predicted* pod names and
87+
// requires a pod-log template to be configured in cluster config), this uses the *real*
88+
// pods — actual names (including the Job-assigned random suffix), namespace, primary
89+
// container, and per-container names + process timestamps — so the console can fetch
90+
// logs natively regardless
91+
// of log-template config. Best-effort: returns nil on list error or when no pods are
92+
// ready yet, leaving the templated Logs path as the fallback.
93+
func getLogContext(ctx context.Context, pluginContext k8s.PluginContext, jobSet *jobsetv1alpha2.JobSet) *core.LogContext {
94+
// The plugin's K8sReader already scopes List calls to this node execution's
95+
// namespace and execution-id/node-id labels, so no extra filters are needed.
96+
podList := &v1.PodList{}
97+
if err := pluginContext.K8sReader().List(ctx, podList); err != nil {
98+
logger.Warnf(ctx, "failed to list pods for JobSet %s/%s log context: %v", jobSet.Namespace, jobSet.Name, err)
99+
return nil
100+
}
101+
102+
// rank0PodName returns "<jobset>-workers-0-0"; the real pod carries an additional
103+
// random suffix, so match on prefix to identify the primary (rank-0) pod.
104+
primaryPrefix := rank0PodName(jobSet.Name)
105+
// The authoritative primary container name is stored on the JobSet at build time
106+
// (see build.go). Child pods don't carry the annotations BuildPodLogContext infers
107+
// from, so set it explicitly to avoid resolving to the wrong container (e.g. a sidecar).
108+
primaryContainerName := jobSet.Annotations[primaryContainerAnnotation]
109+
logCtx := &core.LogContext{Pods: make([]*core.PodLogContext, 0, len(podList.Items))}
110+
for i := range podList.Items {
111+
pod := &podList.Items[i]
112+
// Pending pods have no logs yet and no container statuses to build contexts from.
113+
if pod.Status.Phase == v1.PodPending {
114+
continue
115+
}
116+
if strings.HasPrefix(pod.Name, primaryPrefix) {
117+
logCtx.PrimaryPodName = pod.Name
118+
}
119+
podLogCtx := flytek8s.BuildPodLogContext(pod)
120+
if primaryContainerName != "" {
121+
podLogCtx.PrimaryContainerName = primaryContainerName
122+
}
123+
logCtx.Pods = append(logCtx.Pods, podLogCtx)
124+
}
125+
if len(logCtx.Pods) == 0 {
126+
return nil
127+
}
128+
// Guarantee PrimaryPodName references a pod in Pods: if rank-0 was pending/absent,
129+
// fall back to the first included pod so downstream log streaming can resolve it.
130+
if logCtx.PrimaryPodName == "" {
131+
logCtx.PrimaryPodName = logCtx.Pods[0].GetPodName()
132+
}
133+
return logCtx
134+
}

flyteplugins/go/tasks/plugins/k8s/clustered/phase.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@ func (clusteredResourceHandler) GetTaskPhase(ctx context.Context, pluginContext
4545
}
4646
taskInfo := pluginsCore.TaskInfo{
4747
Logs: taskLogs,
48+
LogContext: getLogContext(ctx, pluginContext, jobSet),
4849
OccurredAt: &occurredAt,
4950
CustomInfo: statusDetails,
5051
}

0 commit comments

Comments
 (0)