Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add rendering for retried taskRuns #1439

Merged
merged 1 commit into from
May 27, 2020

Conversation

steveodonovan
Copy link
Member

Changes

#936

Adds a rendering for taskRun retries.

Submitter Checklist

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

See the contribution guide
for more details.

One thing to follow up on. Why do we sort by step started time on the taskRun tree. https://github.com/tektoncd/dashboard/blob/master/packages/components/src/components/PipelineRun/PipelineRun.js#L67

The values can be blank - below is a sample

conditions:
  - type: Succeeded
    status: Unknown
    lastTransitionTime: '2020-05-26T16:14:42Z'
    reason: Pending
    message: >-
      pod status "Ready":"False"; message: "containers with unready status:
      [step-task-one-step-one step-task-one-step-two]"
podName: sample-piperun-on-tenbase7-task2-hjrw2-pod-xh4n6
startTime: '2020-05-26T16:14:39Z'
steps:
  - waiting:
      reason: PodInitializing
    name: task-one-step-one
    container: step-task-one-step-one
  - waiting:
      reason: PodInitializing
    name: task-one-step-two
    container: step-task-one-step-two

@tekton-robot tekton-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label May 26, 2020
@AlanGreene
Copy link
Member

Why do we sort by step started time on the taskRun tree

Some of the discussion about the sorting is on the original PR here #1077 but I don't think it was all captured 😿 There was an issue with the TaskRun times in some cases, but we've had Tekton Pipelines 0.11 and 0.12 released since then (end of Feb) with many potentially relevant changes in that area so probably worth revisiting that decision.

Copy link
Member

@AlanGreene AlanGreene left a comment

Choose a reason for hiding this comment

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

suggestion on the string id, otherwise code looks good
testing this out now and so far it's working great!

retryPodIndex[podName] || taskRun.status.retriesStatus.length;
pipelineTaskName = intl.formatMessage(
{
id: 'dashboard.pipelineRun.pipelineTaskName.retryAppendage',
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
id: 'dashboard.pipelineRun.pipelineTaskName.retryAppendage',
id: 'dashboard.pipelineRun.pipelineTaskName.retry',

Copy link
Member

@AlanGreene AlanGreene left a comment

Choose a reason for hiding this comment

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

:shipit:
/lgtm

@tekton-robot tekton-robot added the lgtm Indicates that a PR is ready to be merged. label May 27, 2020
@tekton-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: AlanGreene

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

The pull request process is described here

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 May 27, 2020
@tekton-robot tekton-robot merged commit 2042c53 into tektoncd:master May 27, 2020
@steveodonovan steveodonovan deleted the showRerunSteps branch May 27, 2020 13:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants