diff --git a/cmd/pipeline-controller/config_data_provider.go b/cmd/pipeline-controller/config_data_provider.go index d1b5807e6f..f548451a71 100644 --- a/cmd/pipeline-controller/config_data_provider.go +++ b/cmd/pipeline-controller/config_data_provider.go @@ -10,7 +10,7 @@ import ( "sigs.k8s.io/prow/pkg/config" ) -// RepoLister is a function that returns a list of "org/repo" strings that should be processed +// RepoLister i s a function that returns a list of "org/repo" strings that should be processed type RepoLister func() []string type presubmitTests struct { diff --git a/cmd/pipeline-controller/helpers.go b/cmd/pipeline-controller/helpers.go index f6dc132afb..71f256d244 100644 --- a/cmd/pipeline-controller/helpers.go +++ b/cmd/pipeline-controller/helpers.go @@ -21,7 +21,7 @@ func sendComment(presubmits presubmitTests, pj *v1.ProwJob, ghc minimalGhClient, } func sendCommentWithMode(presubmits presubmitTests, pj *v1.ProwJob, ghc minimalGhClient, deleteIds func()) error { - testContexts, err := acquireConditionalContexts(pj, presubmits.pipelineConditionallyRequired, ghc, deleteIds) + testContexts, manualControlMessage, err := acquireConditionalContexts(pj, presubmits.pipelineConditionallyRequired, ghc, deleteIds) if err != nil { deleteIds() return err @@ -29,9 +29,8 @@ func sendCommentWithMode(presubmits presubmitTests, pj *v1.ProwJob, ghc minimalG var comment string - // Check if testContexts contains a manual control message (starts with "Pipeline can be controlled") - if strings.HasPrefix(testContexts, "Pipeline can be controlled") { - comment = testContexts + if manualControlMessage != "" { + comment = manualControlMessage } else { var protectedCommands string for _, presubmit := range presubmits.protected { @@ -56,7 +55,7 @@ func sendCommentWithMode(presubmits presubmitTests, pj *v1.ProwJob, ghc minimalG return nil } -func acquireConditionalContexts(pj *v1.ProwJob, pipelineConditionallyRequired []config.Presubmit, ghc minimalGhClient, deleteIds func()) (string, error) { +func acquireConditionalContexts(pj *v1.ProwJob, pipelineConditionallyRequired []config.Presubmit, ghc minimalGhClient, deleteIds func()) (string, string, error) { repoBaseRef := pj.Spec.Refs.Repo + "-" + pj.Spec.Refs.BaseRef var testCommands string if len(pipelineConditionallyRequired) != 0 { @@ -76,12 +75,12 @@ func acquireConditionalContexts(pj *v1.ProwJob, pipelineConditionallyRequired [] psList[0].RegexpChangeMatcher = config.RegexpChangeMatcher{RunIfChanged: run} if err := config.SetPresubmitRegexes(psList); err != nil { deleteIds() - return "", err + return "", "", err } _, shouldRunResult, err := psList[0].RegexpChangeMatcher.ShouldRun(cfp) if err != nil { deleteIds() - return "", err + return "", "", err } shouldRun = shouldRunResult } else if skip, ok := presubmit.Annotations["pipeline_skip_if_only_changed"]; ok && skip != "" { @@ -90,12 +89,12 @@ func acquireConditionalContexts(pj *v1.ProwJob, pipelineConditionallyRequired [] psList[0].RegexpChangeMatcher = config.RegexpChangeMatcher{SkipIfOnlyChanged: skip} if err := config.SetPresubmitRegexes(psList); err != nil { deleteIds() - return "", err + return "", "", err } _, shouldRunResult, err := psList[0].RegexpChangeMatcher.ShouldRun(cfp) if err != nil { deleteIds() - return "", err + return "", "", err } shouldRun = shouldRunResult } else { @@ -112,7 +111,7 @@ func acquireConditionalContexts(pj *v1.ProwJob, pipelineConditionallyRequired [] statuses, err := ghc.ListStatuses(pj.Spec.Refs.Org, pj.Spec.Refs.Repo, pj.Spec.Refs.Pulls[0].SHA) if err != nil { deleteIds() - return "", err + return "", "", err } // Check if any of the tests we want to run have already been triggered manually @@ -123,7 +122,7 @@ func acquireConditionalContexts(pj *v1.ProwJob, pipelineConditionallyRequired [] for _, status := range statuses { if strings.Contains(status.Context, testName) { deleteIds() - return "Tests from second stage were triggered manually. Pipeline can be controlled only manually, until HEAD changes. Use `/pipeline required` to trigger second stage.", nil + return "", "Tests from second stage were triggered manually. Pipeline can be controlled only manually, until HEAD changes. Use `/pipeline required` to trigger second stage.", nil } } } @@ -133,5 +132,5 @@ func acquireConditionalContexts(pj *v1.ProwJob, pipelineConditionallyRequired [] testCommands += "\n" + presubmit.RerunCommand } } - return testCommands, nil + return testCommands, "", nil } diff --git a/cmd/pipeline-controller/helpers_test.go b/cmd/pipeline-controller/helpers_test.go index 3adaf7b267..542cf30823 100644 --- a/cmd/pipeline-controller/helpers_test.go +++ b/cmd/pipeline-controller/helpers_test.go @@ -82,6 +82,7 @@ func TestAcquireConditionalContexts(t *testing.T) { changes []github.PullRequestChange statuses []github.Status expectedTestCommands []string + expectedManualControlMessage string expectedError string }{ { @@ -104,9 +105,10 @@ func TestAcquireConditionalContexts(t *testing.T) { {Filename: "main.go"}, {Filename: "README.md"}, }, - statuses: []github.Status{}, - expectedTestCommands: []string{"/test test"}, - expectedError: "", + statuses: []github.Status{}, + expectedTestCommands: []string{"/test test"}, + expectedManualControlMessage: "", + expectedError: "", }, { name: "pipeline_run_if_changed does not match files", @@ -129,9 +131,10 @@ func TestAcquireConditionalContexts(t *testing.T) { {Filename: "README.md"}, {Filename: "docs/guide.md"}, }, - statuses: []github.Status{}, - expectedTestCommands: []string{}, - expectedError: "", + statuses: []github.Status{}, + expectedTestCommands: []string{}, + expectedManualControlMessage: "", + expectedError: "", }, { name: "pipeline_skip_if_only_changed skips when only matching files changed", @@ -154,9 +157,10 @@ func TestAcquireConditionalContexts(t *testing.T) { {Filename: "docs/guide.md"}, {Filename: "README.md"}, }, - statuses: []github.Status{}, - expectedTestCommands: []string{}, - expectedError: "", + statuses: []github.Status{}, + expectedTestCommands: []string{}, + expectedManualControlMessage: "", + expectedError: "", }, { name: "pipeline_skip_if_only_changed runs when other files are changed", @@ -178,9 +182,10 @@ func TestAcquireConditionalContexts(t *testing.T) { {Filename: "docs/guide.md"}, {Filename: "main.go"}, }, - statuses: []github.Status{}, - expectedTestCommands: []string{"/test test"}, - expectedError: "", + statuses: []github.Status{}, + expectedTestCommands: []string{"/test test"}, + expectedManualControlMessage: "", + expectedError: "", }, { name: "pipeline_run_if_changed takes precedence over pipeline_skip_if_only_changed", @@ -202,9 +207,10 @@ func TestAcquireConditionalContexts(t *testing.T) { changes: []github.PullRequestChange{ {Filename: "test/test.go"}, }, - statuses: []github.Status{}, - expectedTestCommands: []string{"/test test"}, - expectedError: "", + statuses: []github.Status{}, + expectedTestCommands: []string{"/test test"}, + expectedManualControlMessage: "", + expectedError: "", }, { name: "multiple jobs with different annotations", @@ -238,9 +244,10 @@ func TestAcquireConditionalContexts(t *testing.T) { {Filename: "main.go"}, {Filename: "docs/guide.md"}, }, - statuses: []github.Status{}, - expectedTestCommands: []string{"/test test1", "/test test2"}, - expectedError: "", + statuses: []github.Status{}, + expectedTestCommands: []string{"/test test1", "/test test2"}, + expectedManualControlMessage: "", + expectedError: "", }, { name: "job name does not contain repo-baseRef", @@ -261,9 +268,10 @@ func TestAcquireConditionalContexts(t *testing.T) { changes: []github.PullRequestChange{ {Filename: "main.go"}, }, - statuses: []github.Status{}, - expectedTestCommands: []string{}, - expectedError: "", + statuses: []github.Status{}, + expectedTestCommands: []string{}, + expectedManualControlMessage: "", + expectedError: "", }, { name: "test already running - should return manual control message", @@ -290,15 +298,16 @@ func TestAcquireConditionalContexts(t *testing.T) { State: "pending", }, }, - expectedTestCommands: []string{"Pipeline can be controlled only manually, until HEAD changes."}, - expectedError: "", + expectedTestCommands: []string{}, + expectedManualControlMessage: "Tests from second stage were triggered manually. Pipeline can be controlled only manually, until HEAD changes. Use `/pipeline required` to trigger second stage.", + expectedError: "", }, } for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { ghc := &fakeGhClientWithStatuses{changes: tc.changes, statuses: tc.statuses} - testCmds, err := acquireConditionalContexts(basePJ, tc.pipelineConditionallyRequired, ghc, func() {}) + testCmds, manualControlMessage, err := acquireConditionalContexts(basePJ, tc.pipelineConditionallyRequired, ghc, func() {}) // Check expected error if tc.expectedError != "" { @@ -314,6 +323,14 @@ func TestAcquireConditionalContexts(t *testing.T) { t.Errorf("unexpected error: %v", err) } + // Check manual control message + if tc.expectedManualControlMessage != "" { + if manualControlMessage != tc.expectedManualControlMessage { + t.Errorf("expected manual control message %q, got %q", tc.expectedManualControlMessage, manualControlMessage) + } + return + } + // Check test commands for _, expected := range tc.expectedTestCommands { if !strings.Contains(testCmds, expected) {