Skip to content

Commit 365b061

Browse files
committed
Add test for when concurrency and startPR error
when we cannot create the PipelineRun and we got an error from the controller, if we had concurrency set the controller would crash. This was fixed in e553256 but added test for this ``` 💡 11:25:51 pac-controller pipelinerun test-gh-kjtd8 has been created in namespace pac-e2e-ns-7bw6b for SHA: 4a82af8eb94e78576bc0f9ab3e1a7c5582825412 Target Branch: main 🚨 11:25:51 pac-controller There was an error starting the PipelineRun 00-bad-apple-tdza-, creating pipelinerun 00-bad-apple-tdza- in namespace pac-e2e-ns-7bw6b has failed. Tekton Controller has reported this error: ```admission webhook "validation.webhook.pipeline.tekton.dev" denied the request: validation failed: invalid value: couldn't add link between noexist and donotexist: task noexist depends on donotexist but donotexist wasn't present in Pipeline: spec.pipelineSpec.tasks``` 💡 11:25:52 pac-controller patched pipelinerun with checkRunID and logURL: pac-e2e-ns-7bw6b/test-gh-1-6x879 💡 11:25:52 pac-controller patched pipelinerun with checkRunID and logURL: pac-e2e-ns-7bw6b/test-gh-kjtd8 💡 11:25:52 pac-controller patched pipelinerun with checkRunID and logURL: pac-e2e-ns-7bw6b/prlongrunnning-1-fjrs-qckbv 💡 11:25:52 pac-controller patched pipelinerun with checkRunID and logURL: pac-e2e-ns-7bw6b/test-gh-2-tjdwf 💡 11:25:52 pac-controller skipping event: check_run: unsupported action "created" 💡 11:25:52 pac-controller skipping event: check_run: unsupported action "created" 💡 11:25:52 pac-controller skipping event: check_run: unsupported action "created" 💡 11:25:52 pac-controller skipping event: check_run: unsupported action "created" 💡 11:25:52 pac-controller skipping event: check_run: unsupported action "created" pipelines-as-code/ghe-controller-6b68d9cfb-dc6lg[pac-controller]: panic: reflect: call of reflect.Value.Interface on zero Value pipelines-as-code/ghe-controller-6b68d9cfb-dc6lg[pac-controller]: pipelines-as-code/ghe-controller-6b68d9cfb-dc6lg[pac-controller]: goroutine 563 [running]: pipelines-as-code/ghe-controller-6b68d9cfb-dc6lg[pac-controller]: reflect.valueInterface({0x0?, 0x0?, 0x4000da0d40?}, 0xe0?) pipelines-as-code/ghe-controller-6b68d9cfb-dc6lg[pac-controller]: reflect/value.go:1501 +0xfc pipelines-as-code/ghe-controller-6b68d9cfb-dc6lg[pac-controller]: ``` fixing and add e2e tests for it.
1 parent fa5c20d commit 365b061

File tree

1 file changed

+45
-9
lines changed

1 file changed

+45
-9
lines changed

test/github_pullrequest_concurrency_test.go

Lines changed: 45 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,11 @@ import (
77
"context"
88
"fmt"
99
"net/http"
10+
"strings"
1011
"testing"
1112
"time"
1213

14+
"github.com/openshift-pipelines/pipelines-as-code/pkg/sort"
1315
tgithub "github.com/openshift-pipelines/pipelines-as-code/test/pkg/github"
1416
"github.com/openshift-pipelines/pipelines-as-code/test/pkg/options"
1517
"github.com/openshift-pipelines/pipelines-as-code/test/pkg/payload"
@@ -20,14 +22,37 @@ import (
2022
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2123
)
2224

23-
func TestGithubPullRequestConcurrency(t *testing.T) {
25+
func TestGithubSecondPullRequestConcurrency1by1(t *testing.T) {
2426
ctx := context.Background()
25-
label := "Github PullRequest Concurrent"
27+
label := "Github PullRequest Concurrent, sequentially one by one"
28+
numberOfPipelineRuns := 5
29+
maxNumberOfConcurrentPipelineRuns := 1
30+
testGithubConcurrency(ctx, t, maxNumberOfConcurrentPipelineRuns, numberOfPipelineRuns, label, true, map[string]string{})
31+
}
32+
33+
func TestGithubSecondPullRequestConcurrency3by3(t *testing.T) {
34+
ctx := context.Background()
35+
label := "Github PullRequest Concurrent three at time"
2636
numberOfPipelineRuns := 10
2737
maxNumberOfConcurrentPipelineRuns := 3
38+
testGithubConcurrency(ctx, t, maxNumberOfConcurrentPipelineRuns, numberOfPipelineRuns, label, false, map[string]string{})
39+
}
40+
41+
// TestGithubSecondPullRequestConcurrency1by1WithError will test concurrency and an error, this used to crash for us previously.
42+
func TestGithubSecondPullRequestConcurrency1by1WithError(t *testing.T) {
43+
ctx := context.Background()
44+
label := "Github PullRequest Concurrent, sequentially one by one with one bad apple"
45+
numberOfPipelineRuns := 1
46+
maxNumberOfConcurrentPipelineRuns := 1
47+
testGithubConcurrency(ctx, t, maxNumberOfConcurrentPipelineRuns, numberOfPipelineRuns, label, false, map[string]string{
48+
".tekton/00-bad-apple.yaml": "testdata/failures/bad-runafter-task.yaml",
49+
})
50+
}
2851

52+
func testGithubConcurrency(ctx context.Context, t *testing.T, maxNumberOfConcurrentPipelineRuns, numberOfPipelineRuns int, label string, checkOrdering bool, yamlFiles map[string]string) {
53+
pipelineRunFileNamePrefix := "prlongrunnning-"
2954
targetNS := names.SimpleNameGenerator.RestrictLengthWithRandomSuffix("pac-e2e-ns")
30-
_, runcnx, opts, ghcnx, err := tgithub.Setup(ctx, false, false)
55+
_, runcnx, opts, ghcnx, err := tgithub.Setup(ctx, true, false)
3156
assert.NilError(t, err)
3257

3358
logmsg := fmt.Sprintf("Testing %s with Github APPS integration on %s", label, targetNS)
@@ -45,12 +70,11 @@ func TestGithubPullRequestConcurrency(t *testing.T) {
4570
err = tgithub.CreateCRD(ctx, t, repoinfo, runcnx, opts, targetNS)
4671
assert.NilError(t, err)
4772

48-
yamlFiles := map[string]string{}
4973
for i := 1; i <= numberOfPipelineRuns; i++ {
50-
yamlFiles[fmt.Sprintf(".tekton/prlongrunnning-%d.yaml", i)] = "testdata/pipelinerun_long_running.yaml"
74+
yamlFiles[fmt.Sprintf(".tekton/%s%d.yaml", pipelineRunFileNamePrefix, i)] = "testdata/pipelinerun_long_running.yaml"
5175
}
5276

53-
entries, err := payload.GetEntries(yamlFiles, targetNS, options.MainBranch, options.PullRequestEvent, map[string]string{})
77+
entries, err := payload.GetEntries(yamlFiles, targetNS, options.MainBranch, "pull_request", map[string]string{})
5478
assert.NilError(t, err)
5579

5680
targetRefName := fmt.Sprintf("refs/heads/%s",
@@ -79,7 +103,7 @@ func TestGithubPullRequestConcurrency(t *testing.T) {
79103
assert.NilError(t, wait.UntilMinPRAppeared(ctx, runcnx.Clients, waitOpts, numberOfPipelineRuns))
80104

81105
finished := false
82-
maxLoop := 15
106+
maxLoop := 30
83107
for i := 0; i < maxLoop; i++ {
84108
unsuccessful := 0
85109
prs, err := runcnx.Clients.Tekton.TektonV1().PipelineRuns(targetNS).List(ctx, metav1.ListOptions{})
@@ -101,11 +125,23 @@ func TestGithubPullRequestConcurrency(t *testing.T) {
101125
finished = true
102126
break
103127
}
104-
runcnx.Clients.Log.Infof("number of unsuccessful PR %d out of %d, waiting 10s more, %d/%d", unsuccessful, numberOfPipelineRuns*2, i, maxLoop)
128+
runcnx.Clients.Log.Infof("number of unsuccessful PR %d out of %d, waiting 10s more in the waiting loop: %d/%d", unsuccessful, numberOfPipelineRuns, i, maxLoop)
105129
// it's high because it takes time to process on kind
106-
time.Sleep(15 * time.Second)
130+
time.Sleep(10 * time.Second)
107131
}
108132
if !finished {
109133
t.Errorf("the %d pipelineruns has not successfully finished, some of them are still pending or it's abnormally slow to process the Q", numberOfPipelineRuns)
110134
}
135+
136+
// sort all the PR by when they have started
137+
if checkOrdering {
138+
prs, err := runcnx.Clients.Tekton.TektonV1().PipelineRuns(targetNS).List(ctx, metav1.ListOptions{})
139+
assert.NilError(t, err)
140+
sort.PipelineRunSortByStartTime(prs.Items)
141+
for i := 0; i < numberOfPipelineRuns; i++ {
142+
prExpectedName := fmt.Sprintf("%s%d", pipelineRunFileNamePrefix, len(prs.Items)-i)
143+
prActualName := prs.Items[i].GetName()
144+
assert.Assert(t, strings.HasPrefix(prActualName, prExpectedName), "prActualName: %s does not start with expected prefix %s, was is ordered properly at start time", prActualName, prExpectedName)
145+
}
146+
}
111147
}

0 commit comments

Comments
 (0)