From 4bbaa44d61047f7c069455dbb7bcc795951bd5a3 Mon Sep 17 00:00:00 2001 From: Piyush Garg Date: Wed, 21 Feb 2024 15:49:03 +0530 Subject: [PATCH] Fix logs command not showing logs This will fix the logs command not showing logs if there are retries and task has completed in first go Failure and Completion of taskrun should not depend on no. of retries. Also retries will not scheduled be if task is done so add a check for that. Add e2e test for this scenarioi Fix #2208 --- pkg/cmd/taskrun/logs_test.go | 4 ++-- pkg/log/task_reader.go | 29 ++++++++++++++--------------- test/e2e/pipeline/start_test.go | 6 ++++++ test/resources/pipeline.yaml | 2 +- 4 files changed, 23 insertions(+), 18 deletions(-) diff --git a/pkg/cmd/taskrun/logs_test.go b/pkg/cmd/taskrun/logs_test.go index 35beefc27..52334fe1c 100644 --- a/pkg/cmd/taskrun/logs_test.go +++ b/pkg/cmd/taskrun/logs_test.go @@ -2844,7 +2844,7 @@ func TestLog_taskrun_follow_mode_update_timeout_v1beta1(t *testing.T) { t.Errorf("Unexpected error: %v", err) } - expectedOut := "task output-task create has not started yet or pod for task not yet available\n" + expectedOut := "task output-task has not started yet or pod for task not yet available\n" test.AssertOutput(t, expectedOut, output) } @@ -2992,7 +2992,7 @@ func TestLog_taskrun_follow_mode_update_timeout(t *testing.T) { t.Errorf("Unexpected error: %v", err) } - expectedOut := "task output-task create has not started yet or pod for task not yet available\n" + expectedOut := "task output-task has not started yet or pod for task not yet available\n" test.AssertOutput(t, expectedOut, output) } diff --git a/pkg/log/task_reader.go b/pkg/log/task_reader.go index 530dece71..7ba470bb2 100644 --- a/pkg/log/task_reader.go +++ b/pkg/log/task_reader.go @@ -52,7 +52,7 @@ func (r *Reader) readTaskLog() (<-chan Log, <-chan error, error) { r.formTaskName(tr) - if !isDone(tr, r.retries) && r.follow { + if !tr.IsDone() && r.follow { return r.readLiveTaskLogs(tr) } return r.readAvailableTaskLogs(tr) @@ -91,7 +91,7 @@ func (r *Reader) readAvailableTaskLogs(tr *v1.TaskRun) (<-chan Log, <-chan error } // Check if taskrun failed on start up - if err := hasTaskRunFailed(tr, r.task, r.retries); err != nil { + if err := hasTaskRunFailed(tr, r.task); err != nil { if r.stream != nil { fmt.Fprintf(r.stream.Err, "%s\n", err.Error()) } else { @@ -262,24 +262,24 @@ func (r *Reader) getTaskRunPodNames(run *v1.TaskRun) (<-chan string, <-chan erro } if run.Status.PodName != "" { addPod(run.Status.PodName) - if areRetriesScheduled(run, r.retries) { + if !areRetriesScheduled(run, r.retries) { return } } case <-timeout: // Check if taskrun failed on start up - if err := hasTaskRunFailed(run, r.task, r.retries); err != nil { + if err := hasTaskRunFailed(run, r.task); err != nil { errC <- err return } // check if pod has been started and has a name if run.HasStarted() && run.Status.PodName != "" { - if !areRetriesScheduled(run, r.retries) { + if areRetriesScheduled(run, r.retries) { continue } return } - errC <- fmt.Errorf("task %s create has not started yet or pod for task not yet available", r.task) + errC <- fmt.Errorf("task %s has not started yet or pod for task not yet available", r.task) return } } @@ -351,8 +351,8 @@ func getSteps(pod *corev1.Pod) []*step { return steps } -func hasTaskRunFailed(tr *v1.TaskRun, taskName string, retries int) error { - if isFailure(tr, retries) { +func hasTaskRunFailed(tr *v1.TaskRun, taskName string) error { + if isFailure(tr) { return fmt.Errorf("task %s has failed: %s", taskName, tr.Status.Conditions[0].Message) } return nil @@ -370,16 +370,15 @@ func cast2taskrun(obj runtime.Object) (*v1.TaskRun, error) { return run, nil } -func isDone(tr *v1.TaskRun, retries int) bool { - return tr.IsDone() || !areRetriesScheduled(tr, retries) -} - -func isFailure(tr *v1.TaskRun, retries int) bool { +func isFailure(tr *v1.TaskRun) bool { conditions := tr.Status.Conditions - return len(conditions) != 0 && conditions[0].Status == corev1.ConditionFalse && areRetriesScheduled(tr, retries) + return len(conditions) != 0 && conditions[0].Status == corev1.ConditionFalse } func areRetriesScheduled(tr *v1.TaskRun, retries int) bool { + if tr.IsDone() { + return false + } retriesDone := len(tr.Status.RetriesStatus) - return retriesDone >= retries + return retriesDone < retries } diff --git a/test/e2e/pipeline/start_test.go b/test/e2e/pipeline/start_test.go index 63ec41cde..f8ba360a5 100644 --- a/test/e2e/pipeline/start_test.go +++ b/test/e2e/pipeline/start_test.go @@ -77,6 +77,12 @@ func TestPipelineInteractiveStartE2E(t *testing.T) { }) }) + t.Run("Validate pipeline logs, with follow mode (-f) and --last ", func(t *testing.T) { + res := tkn.Run(t, "pipeline", "logs", "--last", "-f") + expected := "\n\ntest-e2e\n\n" + helper.AssertOutput(t, expected, res.Stdout()) + }) + t.Run("Start PipelineRun using pipeline start interactively using --use-param-defaults and some of the params not having default ", func(t *testing.T) { tkn.RunInteractiveTests(t, &cli.Prompt{ CmdArgs: []string{ diff --git a/test/resources/pipeline.yaml b/test/resources/pipeline.yaml index f23d21914..278d9ba4f 100644 --- a/test/resources/pipeline.yaml +++ b/test/resources/pipeline.yaml @@ -25,7 +25,7 @@ spec: steps: - name: create-new-file image: ubuntu - script: touch $(workspaces.shared-data.path)/$(params.filename) + script: sleep 30s;touch $(workspaces.shared-data.path)/$(params.filename) - name: write-new-stuff image: ubuntu script: echo $(params.message) > $(workspaces.shared-data.path)/$(params.filename)