TestDAGPipelineRun is flaky

[mattmoor]

Expected Behavior

TestDAGPipelineRun passes consistently and actually verifies that the tasks that SHOULD run in parallel DO run in parallel.

Actual Behavior

On busy clusters TestDAGPipelineRun will frequently fail with a message like:

dag_test.go:248: Expected parallel tasks to execute more or less at the same time, but they were 6s apart

Steps to Reproduce the Problem

This is an intermittent flake on a fairly busy cluster.

Additional Info

The test as written today doesn't necessarily even verify that pipeline-task-2-parallel-1 and pipeline-task-2-parallel-2 run in parallel. If they run in sequence quickly (<5s) then the test will pass.

[mattmoor]

My proposal for how to fix this would be as follows:

1. Have the task definition executed by pipeline-task-2-parallel-* sleep for XX seconds (probably 30s)
2. Check that the tasks start within 30s of each other.

This:

• Gives the test a fairly significant amount of leeway to schedule the second TaskRun even on a congested cluster.
• Verifies that the tasks actually run in parallel because the second task MUST start before the first is done (b/c of the sleep).

I think this will be both more stable as well as provide more meaningful coverage.

[mattmoor]

Some pointers...

Here is the test (with a nice ASCII diagram):

pipeline/test/v1alpha1/dag_test.go

Lines 37 to 47 in 38b9f26

	// TestDAGPipelineRun creates a graph of arbitrary Tasks, then looks at the corresponding
	// TaskRun start times to ensure they were run in the order intended, which is:
	// |
	// pipeline-task-1
	// / \
	// pipeline-task-2-parallel-1 pipeline-task-2-parallel-2
	// \ /
	// pipeline-task-3
	// |
	// pipeline-task-4
	func TestDAGPipelineRun(t *testing.T) {

The body of echo-task which is what we run in parallel:

pipeline/test/v1alpha1/dag_test.go

Lines 75 to 79 in 38b9f26

	steps:
	- image: busybox
	script: 'echo $(params["text"])'
	- image: busybox
	script: 'ln -s $(resources.inputs.repo.path) $(resources.outputs.repo.path)'

Where we check that the tasks start within 5s:

pipeline/test/v1alpha1/dag_test.go

Lines 243 to 249 in 38b9f26

	// Check that the two tasks that can run in parallel did
	s1 := taskRuns[1].Status.StartTime.Time
	s2 := taskRuns[2].Status.StartTime.Time
	absDiff := time.Duration(math.Abs(float64(s2.Sub(s1))))
	if absDiff > (time.Second * 5) {
	t.Errorf("Expected parallel tasks to execute more or less at the same time, but they were %v apart", absDiff)
	}

[mattmoor]

There is also a version of this in ./test/dag_test.go (I linked v1alpha1 above)