Fix logs and describe for Pipelines-in-Pipelines by kamal2730 · Pull Request #2889 · tektoncd/cli

kamal2730 · 2026-05-27T06:41:04Z

Changes

Resolve child PipelineRun TaskRuns recursively in tkn pipelinerun logs and describe. TaskRuns from child PipelineRuns are now displayed with a ">" prefix (e.g., "call-child > greet") to indicate nesting depth. This works for any level of nesting and supports both available and follow/live modes.

Implementation Details:

Logs: recursive getOrderedTasks/getChildOrderedTasks and recursive GetTaskRunsWithStatus tracking.
Describe: replaced direct ChildReferences iteration with GetTaskRunsWithStatus for automatic PinP resolution.

Fixes #2886

Submitter Checklist

These are the criteria that every PR should meet, please check them off as you
review them:

Includes tests (if functionality changed/added)
Run the code checkers with make check
Regenerate the manpages, docs and go formatting with make generated
Commit messages follow commit message best practices

See the contribution guide
for more details.

Release Notes

tkn pipelinerun logs and describe now support Pipelines-in-Pipelines (PinP): TaskRuns inside child PipelineRuns are shown inline with a ">" prefix (e.g. "call-child > greet")

Resolve child PipelineRun TaskRuns recursively in tkn pipelinerun logs and describe. TaskRuns from child PipelineRuns are now displayed with a ">" prefix (e.g., "call-child > greet") to indicate nesting depth. This works for any level of nesting and supports both available and follow/live modes. Implementation Details: - Logs: recursive getOrderedTasks/getChildOrderedTasks and recursive GetTaskRunsWithStatus tracking. - Describe: replaced direct ChildReferences iteration with GetTaskRunsWithStatus for automatic PinP resolution.

tekton-robot · 2026-05-27T06:41:08Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
To complete the pull request process, please assign divyansh42 after the PR has been reviewed.
You can assign the PR to them by writing /assign @divyansh42 in a comment when ready.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

linux-foundation-easycla · 2026-05-27T06:41:11Z

The committers listed above are authorized under a signed CLA.

✅ login: kamal2730 / name: kamal2730 (9db9349)

kamal2730 · 2026-05-27T12:06:45Z

Hi @twoGiants , this PR is ready for review. Kindly review it when you have time.

danielfbm

Did some basic review here and left comments inline. These are mostly suggestions from my part, and does not replace a review from maintainers, just flagging what I found here.

Good work overal @kamal2730 👍
I will play with your branch to see how it works paired with tektoncd/pipeline#9432

danielfbm · 2026-06-02T05:57:11Z

+			}
+			childPRs[cr.PipelineTaskName] = childPR
+		}
+	}


This loop re-fetches every child PipelineRun from the API, but GetTaskRunsWithStatus on line 242 already does that recursively (see pkg/pipelinerun/tracker.go getTaskRunsWithStatusRecursive, case "PipelineRun"). For each child PR we now issue 2 GETs per call to getOrderedTasks, and the cost compounds with nesting depth.

Consider either: (a) dropping this loop and looking up child PRs via the prefixed entries already in trsMap, or (b) having GetTaskRunsWithStatus return the fetched child PR objects alongside the TR status map so we fetch once. Functionally fine; perf/clarity smell.

danielfbm · 2026-06-02T05:57:11Z

 		if pr.Spec.PipelineRef.Resolver != "" {
 			if pr.Status.PipelineSpec != nil {
 				tasks = append(tasks, pr.Status.PipelineSpec.Tasks...)
+				tasks = append(tasks, pr.Status.PipelineSpec.Finally...)


This is the actual functional fix in this file — adding Finally to the resolver branch — but no test exercises it. All current PinP log tests use embedded PipelineSpec or non-resolver PipelineRef. Recommend adding TestPipelineRunLogs_ResolverPipeline_WithFinally: a parent PR with Spec.PipelineRef.Resolver != "" and Status.PipelineSpec.Finally populated, asserting finally logs appear after task logs.

danielfbm · 2026-06-02T05:57:11Z

+				return nil, err
+			}
+			ordered = append(ordered, childTasks...)
+		}


Implicit else branch: if a tasks entry matches neither trNames nor childPRs, it is silently dropped. This was effectively the prior behavior of SortTasksBySpecOrder, but the new control flow makes it easier to introduce a regression where a misnamed child key drops the task without any signal. Worth either (a) logging at debug level, or (b) adding a test that confirms a pending/not-yet-started child task is dropped cleanly without panic, so the behavior is locked in.

danielfbm · 2026-06-02T05:57:11Z

+			}
 			trStatuses[cr.Name] = &v1.PipelineRunTaskRunStatus{
-				PipelineTaskName: cr.PipelineTaskName,
+				PipelineTaskName: taskName,


Behavior regression for nested retries caused by this prefixing. findNewTaskruns (tracker.go:183, outside this diff) does if trs.PipelineTaskName == pipelineTask.Name { retries = pipelineTask.Retries }. With the new prefix, trs.PipelineTaskName for any nested TR becomes "call-child > greet", while pipelineTask.Name there is "call-child" (the parent's pipeline task). They will never match, so retries is always 0 for any TaskRun inside a child PipelineRun.

Result: in live/follow mode, retries on nested TaskRuns are not tracked and retry log streams may be missed. The PR description claims follow mode is supported for PinP, so this should be exercised. Suggested fix: when trs.PipelineTaskName contains " > ", look up the retries from the corresponding child PR's Status.PipelineSpec.Tasks (you already fetch those PRs in this function).

Also: no test asserts retry counts for PinP children — add TestFindNewTaskruns_PinP_RetryCount.

danielfbm · 2026-06-02T05:57:11Z

+		case "PipelineRun":
+			childPR, err := GetPipelineRun(pipelineRunGroupResource, c, cr.Name, ns)
+			if err != nil {
+				return nil, err


If GetPipelineRun fails here (child PR not yet visible to informer cache, RBAC issue, network glitch), the entire GetTaskRunsWithStatus call returns an error and both tkn pipelinerun logs and tkn pipelinerun describe fail with no partial output. No test simulates this. In live mode especially, this is plausible — the parent PR may reference a child that hasn't propagated yet. Consider either (a) tolerating NotFound (skip the child for this pass), or (b) at minimum adding a test that pins the fail-fast behavior so it's an explicit choice.

danielfbm · 2026-06-02T05:57:11Z

+			TaskRunName:      trName,
+			PipelineTaskName: trs.PipelineTaskName,
+			Status:           trs.Status,
+		})


Downstream sort.Sort(taskRunList) (description.go:192) relies on TaskRunWithStatusList.Less, which dereferences trs[i].Status.StartTime. Child TaskRuns fetched via the new recursive path may not yet have a StartTime if the child PR is still starting. The comparator handles nil StartTime via early returns, but no test exercises a PinP describe where a nested TR has nil Status.StartTime. Recommend a TestPipelineRunDescribe_PinP_NilStartTime to lock ordering behavior.

danielfbm · 2026-06-02T07:24:24Z

+	for _, pt := range tasks {
+		if _, ok := trNames[pt.Name]; ok {
+			// Direct TaskRun child — use existing sort logic
+			ordered = append(ordered, taskrunpkg.SortTasksBySpecOrder([]v1.PipelineTask{pt}, trsMap)...)


--task filter breaks for PinP tasks. readAvailablePipelineLogs calls taskrunpkg.Filter(ordered, r.tasks), which does exact-string matching against tr.Task (taskrun.go:74 — if filter[tr.Task]). With this PR, Run.Task is now a prefixed string like "call-child > greet", so:

# User runs Result

1 tkn pr logs --task "call-child > greet" works

2 tkn pr logs --task greet silently drops all child logs

3 tkn pr logs --task call-child silently drops all child logs

4 no --task flag works (all PinP tests cover this)

The error message for case 2/3 will be passed filtered tasks: [greet] is not available, available tasks are: [call-child > greet], which is confusing UX. The same issue affects live-follow via taskrunpkg.IsFiltered (taskrun.go:50).

At minimum, add a test covering case 1 and document the required parent > child format. Better: change Filter to a substring/suffix match for PinP entries, so --task greet matches "call-child > greet". Wdys?

danielfbm · 2026-06-02T07:24:24Z

+		return nil, err
+	}
+	for i := range childOrdered {
+		childOrdered[i].Task = parentTaskName + " > " + childOrdered[i].Task


DisplayName is not prefixed alongside Task. setUpTask (pipeline_reader.go:211) passes tr.DisplayName straight to r.setDisplayName, and that ends up in Log.TaskDisplayName (line 195). If a child task defines displayName, users will see an unprefixed display name next to a prefixed task name — the opposite of the consistency this PR is trying to establish.

Suggested change

childOrdered[i].Task = parentTaskName + " > " + childOrdered[i].Task

for i := range childOrdered {

childOrdered[i].Task = parentTaskName + " > " + childOrdered[i].Task

if childOrdered[i].DisplayName != "" {

childOrdered[i].DisplayName = parentTaskName + " > " + childOrdered[i].DisplayName

}

}

Also worth adding a test where a child PipelineTask carries DisplayName to lock the format.

danielfbm · 2026-06-02T07:24:24Z

-
+			taskName := cr.PipelineTaskName
+			if prefix != "" {
+				taskName = prefix + " > " + taskName


SUGGESTION: The separator " > " is now a magic string in three production sites: tracker.go:236, tracker.go:249, and pipeline_reader.go:284. If the display format ever changes (/, |, indentation), the three sites plus the golden file TestPipelineRunDescribe_PinP.golden all need to be touched in lockstep. Extracting a constant keeps them aligned:

// In pkg/pipelinerun/tracker.go top-level: // ChildTaskSeparator separates parent and child pipeline task names in // Pipelines-in-Pipelines describe/logs output (e.g. "call-child > greet"). const ChildTaskSeparator = " > "

Then replace the three literals with pipelinerunpkg.ChildTaskSeparator / ChildTaskSeparator. Optional but cheap.

danielfbm · 2026-06-02T07:24:24Z

+	}
+	return ordered, nil
+}
+func (r *Reader) getChildOrderedTasks(childPR *v1.PipelineRun, parentTaskName string) ([]taskrunpkg.Run, error) {


nit: missing blank line between getOrderedTasks and getChildOrderedTasks. Every other top-level function pair in this file is separated by a blank line.

Suggested change

func (r *Reader) getChildOrderedTasks(childPR *v1.PipelineRun, parentTaskName string) ([]taskrunpkg.Run, error) {

return ordered, nil

}

func (r *Reader) getChildOrderedTasks(childPR *v1.PipelineRun, parentTaskName string) ([]taskrunpkg.Run, error) {

tekton-robot added the release-note Denotes a PR that will be considered when it comes time to generate release notes. label May 27, 2026

tekton-robot requested review from divyansh42 and pratap0007 May 27, 2026 06:41

tekton-robot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label May 27, 2026

danielfbm reviewed Jun 2, 2026

View reviewed changes

danielfbm reviewed Jun 3, 2026

View reviewed changes

#	User runs	Result
1	`tkn pr logs --task "call-child > greet"`	works
2	`tkn pr logs --task greet`	silently drops all child logs
3	`tkn pr logs --task call-child`	silently drops all child logs
4	no `--task` flag	works (all PinP tests cover this)

Conversation

kamal2730 commented May 27, 2026

Changes

Submitter Checklist

Release Notes

Uh oh!

tekton-robot commented May 27, 2026

Uh oh!

linux-foundation-easycla Bot commented May 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kamal2730 commented May 27, 2026

Uh oh!

danielfbm left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

linux-foundation-easycla Bot commented May 27, 2026 •

edited

Loading