Skip to content

fix(watcher): populate RecordSummary StartTime/EndTime from Status fields#1285

Open
SergK wants to merge 1 commit into
tektoncd:mainfrom
SergK:fix/509-recordsummary-timestamps
Open

fix(watcher): populate RecordSummary StartTime/EndTime from Status fields#1285
SergK wants to merge 1 commit into
tektoncd:mainfrom
SergK:fix/509-recordsummary-timestamps

Conversation

@SergK
Copy link
Copy Markdown

@SergK SergK commented Apr 1, 2026

RecordSummary.StartTime is always NULL and EndTime is NULL for
failed/timed-out/cancelled runs.

The watcher derived timestamps from Knative conditions via getTimestamp().
This is broken for two reasons:

  1. StartTime was read from ConditionReady. Tekton PipelineRun and TaskRun
    are batch resources (NewBatchConditionSet) whose happy condition is
    ConditionSucceeded. ConditionReady is for living resources like Knative
    Services and is never set on batch resources, so the lookup always returned
    nil.

  2. EndTime was read from ConditionSucceeded, but getTimestamp() returned
    nil when the condition IsFalse(). For failed, timed-out, and cancelled runs
    ConditionSucceeded is False, so EndTime was nil even though the run had
    completed.

Fix

Read Status.StartTime and Status.CompletionTime directly from the
PipelineRun/TaskRun object via type switch. These fields are set by the Tekton
controller for all terminal states. This follows the existing pattern used by
getCompletionTime() in the dynamic reconciler.

  • Add getStartTime() and getEndTime() helpers
  • Update ensureResult() to use the new helpers
  • Remove the now-unused getTimestamp() function
  • Add unit and integration tests covering both run types in success and failure
    states

/kind bug

Submitter Checklist

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

  • Has Docs included if any changes are user facing
  • Has Tests included if any functionality added or changed
  • Tested your changes locally (if this is a code change)
  • Follows the commit message standard
  • Meets the Tekton contributor standards (including functionality, content, code)
  • Has a kind label. You can add a comment on this PR that contains /kind <type>. Valid types are bug, cleanup, design, documentation, feature, flake, misc, question, tep
  • Release notes block below has been updated with any user-facing changes (API changes, bug fixes, changes requiring upgrade notices or deprecation warnings)
  • Release notes contain the string "action required" if the change requires additional action from users switching to the new release

Release Notes

Fix RecordSummary StartTime always being NULL and EndTime being NULL for
failed/timed-out/cancelled runs. Timestamps are now read directly from the
run's Status.StartTime and Status.CompletionTime fields instead of from
Knative conditions.

@SergK
fix(watcher): populate RecordSummary StartTime/EndTime from Status fi…
8b5f268 
…elds

Fixes tektoncd#509

RecordSummary.StartTime is always NULL and EndTime is NULL for
failed/timed-out/cancelled runs.

The watcher derived timestamps from Knative conditions via
getTimestamp(). This is broken for two reasons:

1. StartTime was read from ConditionReady. Tekton PipelineRun and
   TaskRun are batch resources (NewBatchConditionSet) whose happy
   condition is ConditionSucceeded. ConditionReady is for living
   resources like Knative Services and is never set on batch resources,
   so the lookup always returned nil.

2. EndTime was read from ConditionSucceeded, but getTimestamp() returned
   nil when the condition IsFalse(). For failed, timed-out, and
   cancelled runs ConditionSucceeded is False, so EndTime was nil even
   though the run had completed.

Read Status.StartTime and Status.CompletionTime directly from the
PipelineRun/TaskRun object via type switch. These fields are set by the
Tekton controller for all terminal states. This follows the existing
pattern used by getCompletionTime() in the dynamic reconciler.

Signed-off-by: Sergiy Kulanov <sergiy_kulanov@epam.com>
@tekton-robot tekton-robot added kind/bug Categorizes issue or PR as related to a bug. release-note Denotes a PR that will be considered when it comes time to generate release notes. labels Apr 1, 2026
@tekton-robot tekton-robot requested review from divyansh42 and khrm April 1, 2026 11:56
@tekton-robot tekton-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Apr 1, 2026
khrm
khrm reviewed Apr 1, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@khrm khrm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/approve

@tekton-robot
Copy link
Copy Markdown

tekton-robot commented Apr 1, 2026

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: khrm

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

The pull request process is described here

Details Needs approval from an approver in each of these files:

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

@tekton-robot tekton-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Apr 1, 2026
SergK added a commit to KubeRocketCI/krci-portal that referenced this pull request Apr 4, 2026
@SergK
EPMDEDP-16623: fix: Resolve pagination failure in Tekton Results Pipe…
028a942 
…lineRun query

The getPipelineRunResults endpoint uses `order_by: summary.end_time desc`
for sorting. However, RecordSummary.EndTime is NULL for many results due
to a watcher bug in tektoncd/results (tektoncd/results#1285). Ordering by
a NULL field corrupts the pagination cursor, causing page 2+ to return
empty results.

Changed to `order_by: create_time desc` which is:
- Always populated (never NULL)
- Semantically equivalent for recency-based sorting
- The client hook already re-sorts by creationTimestamp anyway

This unblocks the "Load more history" pagination control and allows users
to view all 100+ PipelineRuns in the history, not just the initial 50.

See: tektoncd/results#1285
Signed-off-by: Sergiy Kulanov <sergiy_kulanov@epam.com>
@SergK SergK mentioned this pull request Apr 4, 2026
Merged
SergK added a commit to KubeRocketCI/krci-portal that referenced this pull request Apr 4, 2026
@SergK
EPMDEDP-16623: fix: Resolve pagination failure in Tekton Results Pipe…
b7580ff 
…lineRun query

The getPipelineRunResults endpoint uses `order_by: summary.end_time desc`
for sorting. However, RecordSummary.EndTime is NULL for many results due
to a watcher bug in tektoncd/results (tektoncd/results#1285). Ordering by
a NULL field corrupts the pagination cursor, causing page 2+ to return
empty results.

Changed to `order_by: create_time desc` which is:
- Always populated (never NULL)
- Semantically equivalent for recency-based sorting
- The client hook already re-sorts by creationTimestamp anyway

This unblocks the "Load more history" pagination control and allows users
to view all 100+ PipelineRuns in the history, not just the initial 50.

See: tektoncd/results#1285
Signed-off-by: Sergiy Kulanov <sergiy_kulanov@epam.com>
SergK added a commit to KubeRocketCI/krci-portal that referenced this pull request Apr 4, 2026
@SergK
EPMDEDP-16623: fix: Resolve pagination failure in Tekton Results Pipe…
4409ea5 
…lineRun query

The getPipelineRunResults endpoint uses `order_by: summary.end_time desc`
for sorting. However, RecordSummary.EndTime is NULL for many results due
to a watcher bug in tektoncd/results (tektoncd/results#1285). Ordering by
a NULL field corrupts the pagination cursor, causing page 2+ to return
empty results.

Changed to `order_by: create_time desc` which is:
- Always populated (never NULL)
- Semantically equivalent for recency-based sorting
- The client hook already re-sorts by creationTimestamp anyway

This unblocks the "Load more history" pagination control and allows users
to view all 100+ PipelineRuns in the history, not just the initial 50.

See: tektoncd/results#1285
Signed-off-by: Sergiy Kulanov <sergiy_kulanov@epam.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@khrm khrm khrm left review comments

@divyansh42 divyansh42 Awaiting requested review from divyansh42

Assignees

No one assigned

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. kind/bug Categorizes issue or PR as related to a bug. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@SergK @tekton-robot @khrm