diff --git a/prow/crier/controller.go b/prow/crier/controller.go index 88cb3b4c9a78..a1e9ff96521f 100644 --- a/prow/crier/controller.go +++ b/prow/crier/controller.go @@ -178,7 +178,7 @@ func (r *reconciler) reconcile(ctx context.Context, log *logrus.Entry, req recon log = log.WithField("jobName", pj.Spec.Job) - if !pj.Spec.Report || !r.reporter.ShouldReport(ctx, log, &pj) { + if !r.reporter.ShouldReport(ctx, log, &pj) { return nil, nil } diff --git a/prow/crier/reporters/gerrit/reporter.go b/prow/crier/reporters/gerrit/reporter.go index 3a10d52e2c8b..cd8b31cf99ad 100644 --- a/prow/crier/reporters/gerrit/reporter.go +++ b/prow/crier/reporters/gerrit/reporter.go @@ -111,6 +111,10 @@ func (c *Client) GetName() string { // ShouldReport returns if this prowjob should be reported by the gerrit reporter func (c *Client) ShouldReport(ctx context.Context, log *logrus.Entry, pj *v1.ProwJob) bool { + if !pj.Spec.Report { + return false + } + ctx, cancel := context.WithTimeout(ctx, 10*time.Second) defer cancel() diff --git a/prow/crier/reporters/gerrit/reporter_test.go b/prow/crier/reporters/gerrit/reporter_test.go index 9cc0e969074a..c384afb594d8 100644 --- a/prow/crier/reporters/gerrit/reporter_test.go +++ b/prow/crier/reporters/gerrit/reporter_test.go @@ -103,6 +103,9 @@ func TestReport(t *testing.T) { { name: "1 job, unfinished, should not report", pj: &v1.ProwJob{ + Spec: v1.ProwJobSpec{ + Report: true, + }, Status: v1.ProwJobStatus{ State: v1.PendingState, }, @@ -111,6 +114,9 @@ func TestReport(t *testing.T) { { name: "1 job, finished, no labels, should not report", pj: &v1.ProwJob{ + Spec: v1.ProwJobSpec{ + Report: true, + }, Status: v1.ProwJobStatus{ State: v1.SuccessState, }, @@ -119,6 +125,9 @@ func TestReport(t *testing.T) { { name: "1 job, finished, missing gerrit-id label, should not report", pj: &v1.ProwJob{ + Spec: v1.ProwJobSpec{ + Report: true, + }, ObjectMeta: metav1.ObjectMeta{ Labels: map[string]string{ client.GerritRevision: "abc", @@ -137,6 +146,9 @@ func TestReport(t *testing.T) { { name: "1 job, finished, missing gerrit-revision label, should not report", pj: &v1.ProwJob{ + Spec: v1.ProwJobSpec{ + Report: true, + }, ObjectMeta: metav1.ObjectMeta{ Annotations: map[string]string{ client.GerritID: "123-abc", @@ -152,6 +164,9 @@ func TestReport(t *testing.T) { { name: "1 job, finished, missing gerrit-instance label, should not report", pj: &v1.ProwJob{ + Spec: v1.ProwJobSpec{ + Report: true, + }, ObjectMeta: metav1.ObjectMeta{ Labels: map[string]string{ client.GerritRevision: "abc", @@ -189,7 +204,8 @@ func TestReport(t *testing.T) { Refs: &v1.Refs{ Repo: "foo", }, - Job: "ci-foo", + Job: "ci-foo", + Report: true, }, }, expectReport: true, @@ -197,6 +213,33 @@ func TestReport(t *testing.T) { expectLabel: map[string]string{codeReview: lgtm}, numExpectedReport: 1, }, + { + name: "1 job, passed, skip report set true, should not report", + pj: &v1.ProwJob{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{ + client.GerritRevision: "abc", + kube.ProwJobTypeLabel: presubmit, + client.GerritReportLabel: "Code-Review", + }, + Annotations: map[string]string{ + client.GerritID: "123-abc", + client.GerritInstance: "gerrit", + }, + }, + Status: v1.ProwJobStatus{ + State: v1.SuccessState, + URL: "guber/foo", + }, + Spec: v1.ProwJobSpec{ + Refs: &v1.Refs{ + Repo: "foo", + }, + Job: "ci-foo", + Report: false, + }, + }, + }, { name: "1 job, passed, bad label, should report without label", pj: &v1.ProwJob{ @@ -219,7 +262,8 @@ func TestReport(t *testing.T) { Refs: &v1.Refs{ Repo: "foo", }, - Job: "ci-foo", + Job: "ci-foo", + Report: true, }, }, expectReport: true, @@ -248,7 +292,8 @@ func TestReport(t *testing.T) { Refs: &v1.Refs{ Repo: "foo", }, - Job: "ci-foo", + Job: "ci-foo", + Report: true, }, }, expectReport: true, @@ -277,7 +322,8 @@ func TestReport(t *testing.T) { Refs: &v1.Refs{ Repo: "foo", }, - Job: "ci-foo", + Job: "ci-foo", + Report: true, }, }, expectReport: false, @@ -304,7 +350,8 @@ func TestReport(t *testing.T) { Refs: &v1.Refs{ Repo: "foo", }, - Job: "ci-foo", + Job: "ci-foo", + Report: true, }, }, expectReport: true, @@ -335,7 +382,8 @@ func TestReport(t *testing.T) { Refs: &v1.Refs{ Repo: "foo", }, - Job: "ci-foo", + Job: "ci-foo", + Report: true, }, }, expectReport: true, @@ -365,7 +413,8 @@ func TestReport(t *testing.T) { Refs: &v1.Refs{ Repo: "foo/bar", }, - Job: "ci-foo-bar", + Job: "ci-foo-bar", + Report: true, }, }, expectReport: true, @@ -396,7 +445,8 @@ func TestReport(t *testing.T) { Refs: &v1.Refs{ Repo: "foo", }, - Job: "ci-foo", + Job: "ci-foo", + Report: true, }, }, existingPJs: []*v1.ProwJob{ @@ -420,7 +470,8 @@ func TestReport(t *testing.T) { Refs: &v1.Refs{ Repo: "bar", }, - Job: "ci-bar", + Job: "ci-bar", + Report: true, }, }, }, @@ -452,7 +503,8 @@ func TestReport(t *testing.T) { Refs: &v1.Refs{ Repo: "foo", }, - Job: "ci-foo", + Job: "ci-foo", + Report: true, }, }, existingPJs: []*v1.ProwJob{ @@ -476,7 +528,8 @@ func TestReport(t *testing.T) { Refs: &v1.Refs{ Repo: "bar", }, - Job: "ci-bar", + Job: "ci-bar", + Report: true, }, }, }, @@ -503,7 +556,8 @@ func TestReport(t *testing.T) { Refs: &v1.Refs{ Repo: "foo", }, - Job: "ci-foo", + Job: "ci-foo", + Report: true, }, }, existingPJs: []*v1.ProwJob{ @@ -527,7 +581,8 @@ func TestReport(t *testing.T) { Refs: &v1.Refs{ Repo: "bar", }, - Job: "ci-bar", + Job: "ci-bar", + Report: true, }, }, }, @@ -559,7 +614,8 @@ func TestReport(t *testing.T) { Refs: &v1.Refs{ Repo: "foo", }, - Job: "ci-foo", + Job: "ci-foo", + Report: true, }, }, expectReport: true, @@ -589,7 +645,8 @@ func TestReport(t *testing.T) { Refs: &v1.Refs{ Repo: "foo", }, - Job: "ci-foo", + Job: "ci-foo", + Report: true, }, }, existingPJs: []*v1.ProwJob{ @@ -614,7 +671,8 @@ func TestReport(t *testing.T) { Refs: &v1.Refs{ Repo: "bar", }, - Job: "ci-bar", + Job: "ci-bar", + Report: true, }, }, }, @@ -646,7 +704,8 @@ func TestReport(t *testing.T) { Refs: &v1.Refs{ Repo: "foo", }, - Job: "ci-foo", + Job: "ci-foo", + Report: true, }, }, existingPJs: []*v1.ProwJob{ @@ -670,7 +729,8 @@ func TestReport(t *testing.T) { Refs: &v1.Refs{ Repo: "bar", }, - Job: "ci-bar", + Job: "ci-bar", + Report: true, }, }, }, @@ -702,8 +762,9 @@ func TestReport(t *testing.T) { Refs: &v1.Refs{ Repo: "foo", }, - Job: "ci-foo", - Type: v1.PresubmitJob, + Job: "ci-foo", + Type: v1.PresubmitJob, + Report: true, }, }, existingPJs: []*v1.ProwJob{ @@ -727,8 +788,9 @@ func TestReport(t *testing.T) { Refs: &v1.Refs{ Repo: "bar", }, - Job: "ci-bar", - Type: v1.PresubmitJob, + Job: "ci-bar", + Type: v1.PresubmitJob, + Report: true, }, }, }, @@ -759,7 +821,8 @@ func TestReport(t *testing.T) { Refs: &v1.Refs{ Repo: "foo", }, - Job: "ci-foo", + Job: "ci-foo", + Report: true, }, }, existingPJs: []*v1.ProwJob{ @@ -783,7 +846,8 @@ func TestReport(t *testing.T) { Refs: &v1.Refs{ Repo: "bar", }, - Job: "ci-bar", + Job: "ci-bar", + Report: true, }, }, }, @@ -814,7 +878,8 @@ func TestReport(t *testing.T) { Refs: &v1.Refs{ Repo: "foo", }, - Job: "ci-foo", + Job: "ci-foo", + Report: true, }, }, existingPJs: []*v1.ProwJob{ @@ -838,7 +903,8 @@ func TestReport(t *testing.T) { Refs: &v1.Refs{ Repo: "bar", }, - Job: "ci-bar", + Job: "ci-bar", + Report: true, }, }, }, @@ -872,7 +938,8 @@ func TestReport(t *testing.T) { Refs: &v1.Refs{ Repo: "foo", }, - Job: "ci-foo", + Job: "ci-foo", + Report: true, }, }, existingPJs: []*v1.ProwJob{ @@ -902,7 +969,8 @@ func TestReport(t *testing.T) { Refs: &v1.Refs{ Repo: "foo", }, - Job: "ci-foo", + Job: "ci-foo", + Report: true, }, }, }, @@ -933,7 +1001,8 @@ func TestReport(t *testing.T) { Refs: &v1.Refs{ Repo: "foo", }, - Job: "ci-foo", + Job: "ci-foo", + Report: true, }, }, existingPJs: []*v1.ProwJob{ @@ -957,7 +1026,8 @@ func TestReport(t *testing.T) { Refs: &v1.Refs{ Repo: "bar", }, - Job: "ci-bar", + Job: "ci-bar", + Report: true, }, }, }, @@ -992,7 +1062,8 @@ func TestReport(t *testing.T) { Refs: &v1.Refs{ Repo: "foo", }, - Job: "ci-foo", + Job: "ci-foo", + Report: true, }, }, existingPJs: []*v1.ProwJob{ @@ -1022,8 +1093,9 @@ func TestReport(t *testing.T) { Refs: &v1.Refs{ Repo: "bar", }, - Job: "ci-bar", - Type: v1.PresubmitJob, + Job: "ci-bar", + Type: v1.PresubmitJob, + Report: true, }, }, { @@ -1052,8 +1124,9 @@ func TestReport(t *testing.T) { Refs: &v1.Refs{ Repo: "foo", }, - Job: "ci-foo", - Type: v1.PresubmitJob, + Job: "ci-foo", + Type: v1.PresubmitJob, + Report: true, }, }, }, @@ -1092,7 +1165,8 @@ func TestReport(t *testing.T) { Refs: &v1.Refs{ Repo: "foo", }, - Job: "ci-foo", + Job: "ci-foo", + Report: true, }, }, existingPJs: []*v1.ProwJob{ @@ -1126,8 +1200,9 @@ func TestReport(t *testing.T) { Refs: &v1.Refs{ Repo: "bar", }, - Job: "ci-bar", - Type: v1.PresubmitJob, + Job: "ci-bar", + Type: v1.PresubmitJob, + Report: true, }, }, { @@ -1160,8 +1235,9 @@ func TestReport(t *testing.T) { Refs: &v1.Refs{ Repo: "foo", }, - Job: "ci-foo", - Type: v1.PresubmitJob, + Job: "ci-foo", + Type: v1.PresubmitJob, + Report: true, }, }, { @@ -1191,8 +1267,9 @@ func TestReport(t *testing.T) { Refs: &v1.Refs{ Repo: "foo", }, - Job: "ci-foo", - Type: v1.PresubmitJob, + Job: "ci-foo", + Type: v1.PresubmitJob, + Report: true, }, }, }, @@ -1228,7 +1305,8 @@ func TestReport(t *testing.T) { Refs: &v1.Refs{ Repo: "foo", }, - Job: "ci-foo", + Job: "ci-foo", + Report: true, }, }, existingPJs: []*v1.ProwJob{ @@ -1262,8 +1340,9 @@ func TestReport(t *testing.T) { Refs: &v1.Refs{ Repo: "bar", }, - Job: "ci-bar", - Type: v1.PresubmitJob, + Job: "ci-bar", + Type: v1.PresubmitJob, + Report: true, }, }, { @@ -1296,8 +1375,9 @@ func TestReport(t *testing.T) { Refs: &v1.Refs{ Repo: "foo", }, - Job: "ci-foo", - Type: v1.PresubmitJob, + Job: "ci-foo", + Type: v1.PresubmitJob, + Report: true, }, }, { @@ -1327,8 +1407,9 @@ func TestReport(t *testing.T) { Refs: &v1.Refs{ Repo: "foo", }, - Job: "ci-foo", - Type: v1.PresubmitJob, + Job: "ci-foo", + Type: v1.PresubmitJob, + Report: true, }, }, }, @@ -1357,7 +1438,8 @@ func TestReport(t *testing.T) { Refs: &v1.Refs{ Repo: "foo", }, - Job: "ci-foo", + Job: "ci-foo", + Report: true, }, }, expectReport: true, @@ -1387,7 +1469,8 @@ func TestReport(t *testing.T) { Refs: &v1.Refs{ Repo: "foo", }, - Job: "ci-foo", + Job: "ci-foo", + Report: true, }, }, expectReport: true, diff --git a/prow/crier/reporters/github/reporter.go b/prow/crier/reporters/github/reporter.go index 80197335ed83..587e8d7802f2 100644 --- a/prow/crier/reporters/github/reporter.go +++ b/prow/crier/reporters/github/reporter.go @@ -120,6 +120,9 @@ func (c *Client) GetName() string { // ShouldReport returns if this prowjob should be reported by the github reporter func (c *Client) ShouldReport(_ context.Context, _ *logrus.Entry, pj *v1.ProwJob) bool { + if !pj.Spec.Report { + return false + } switch { case pj.Labels[client.GerritReportLabel] != "":