From 07248902302c3306c047a977a10b567e90e5d99b Mon Sep 17 00:00:00 2001 From: Deep Mistry Date: Thu, 16 Oct 2025 11:26:52 -0400 Subject: [PATCH 1/2] Fix to exclude creating slackreportconfig for excluded pattern --- pkg/config/load_test.go | 69 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 69 insertions(+) diff --git a/pkg/config/load_test.go b/pkg/config/load_test.go index 3b6414bd28..e6cb7f63c2 100644 --- a/pkg/config/load_test.go +++ b/pkg/config/load_test.go @@ -604,3 +604,72 @@ func TestValidateProwgenSkipOperatorPresubmits(t *testing.T) { }) } } + +func TestProwgen_MergeDefaults_SlackReporterConfigs(t *testing.T) { + testCases := []struct { + name string + base Prowgen + defaults Prowgen + expected []SlackReporterConfig + }{ + { + name: "slack reporter configs are never merged from defaults", + base: Prowgen{}, + defaults: Prowgen{ + SlackReporterConfigs: []SlackReporterConfig{ + { + Channel: "#test-channel", + JobStatesToReport: []prowv1.ProwJobState{"failure"}, + JobNamePatterns: []string{".*"}, + ExcludedJobPatterns: []string{".*-skip$"}, + }, + }, + }, + expected: nil, + }, + { + name: "existing slack reporter configs are preserved unchanged", + base: Prowgen{ + SlackReporterConfigs: []SlackReporterConfig{ + { + Channel: "#existing-channel", + JobStatesToReport: []prowv1.ProwJobState{"error"}, + JobNames: []string{"unit"}, + }, + }, + }, + defaults: Prowgen{ + SlackReporterConfigs: []SlackReporterConfig{ + { + Channel: "#default-channel", + JobStatesToReport: []prowv1.ProwJobState{"failure"}, + JobNamePatterns: []string{".*"}, + ExcludedJobPatterns: []string{".*-skip$"}, + }, + }, + }, + expected: []SlackReporterConfig{ + { + Channel: "#existing-channel", + JobStatesToReport: []prowv1.ProwJobState{"error"}, + JobNames: []string{"unit"}, + }, + }, + }, + { + name: "empty base with empty defaults stays empty", + base: Prowgen{}, + defaults: Prowgen{}, + expected: nil, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + tc.base.MergeDefaults(&tc.defaults) + if diff := cmp.Diff(tc.base.SlackReporterConfigs, tc.expected); diff != "" { + t.Fatalf("SlackReporterConfigs don't match expected, diff: %v", diff) + } + }) + } +} From cdbfe8067d7cc8c997da2941a1d5cd41b08e8e52 Mon Sep 17 00:00:00 2001 From: Deep Mistry Date: Fri, 17 Oct 2025 16:04:05 -0400 Subject: [PATCH 2/2] Add additional changes for slack reporter config exclusion --- pkg/config/load.go | 32 ++++++++++++ pkg/prowgen/jobbase.go | 12 +---- pkg/prowgen/jobbase_test.go | 22 ++++++++ pkg/prowgen/prowgen.go | 52 +++++++++++++++---- ...job_is_configured_for_slack_reporting.yaml | 6 +++ ...should_not_have_slack_reporter_config.yaml | 44 ++++++++++++++++ ...est_simple_with_slack_reporter_config.yaml | 6 --- 7 files changed, 148 insertions(+), 26 deletions(-) create mode 100644 pkg/prowgen/testdata/zz_fixture_TestNewProwJobBaseBuilderForTest_job_excluded_by_patterns_should_not_have_slack_reporter_config.yaml diff --git a/pkg/config/load.go b/pkg/config/load.go index 97ed27957b..437ec25d4a 100644 --- a/pkg/config/load.go +++ b/pkg/config/load.go @@ -96,6 +96,38 @@ func (p *Prowgen) GetSlackReporterConfigForTest(test, variant string) *SlackRepo return nil } +// GetSlackReporterConfigForJobName checks against full job names, allowing excluded_job_patterns +// to work with prefixes like "pull-", "periodic-", etc. +func (p *Prowgen) GetSlackReporterConfigForJobName(fullJobName, testName, variant string) *SlackReporterConfig { + for _, s := range p.SlackReporterConfigs { + if !slices.Contains(s.ExcludedVariants, variant) { + // Check if job is excluded by pattern (using full job name) + isExcluded := false + for _, excludePattern := range s.ExcludedJobPatterns { + if matched, err := regexp.MatchString(excludePattern, fullJobName); err == nil && matched { + isExcluded = true + break + } + } + + if !isExcluded { + // Check exact job name matches first (against test name for backward compatibility) + if slices.Contains(s.JobNames, testName) { + return &s + } + + // Check regex pattern matches (against test name for backward compatibility) + for _, pattern := range s.JobNamePatterns { + if matched, err := regexp.MatchString(pattern, testName); err == nil && matched { + return &s + } + } + } + } + } + return nil +} + func (p *Prowgen) MergeDefaults(defaults *Prowgen) { if defaults.Private { p.Private = true diff --git a/pkg/prowgen/jobbase.go b/pkg/prowgen/jobbase.go index 1078da4c45..ec60726140 100644 --- a/pkg/prowgen/jobbase.go +++ b/pkg/prowgen/jobbase.go @@ -139,16 +139,8 @@ func NewProwJobBaseBuilderForTest(configSpec *cioperatorapi.ReleaseBuildConfigur if testContainsLease(&test) { p.PodSpec.Add(LeaseClient()) } - if slackReporter := info.Config.GetSlackReporterConfigForTest(test.As, configSpec.Metadata.Variant); slackReporter != nil { - if p.base.ReporterConfig == nil { - p.base.ReporterConfig = &prowv1.ReporterConfig{} - } - p.base.ReporterConfig.Slack = &prowv1.SlackReporterConfig{ - Channel: slackReporter.Channel, - JobStatesToReport: slackReporter.JobStatesToReport, - ReportTemplate: slackReporter.ReportTemplate, - } - } + // Note: Slack reporter config is now set in individual job generation functions + // to support full job name matching in excluded_job_patterns switch { case test.MultiStageTestConfigurationLiteral != nil: diff --git a/pkg/prowgen/jobbase_test.go b/pkg/prowgen/jobbase_test.go index cb473d250c..fc4d9086dc 100644 --- a/pkg/prowgen/jobbase_test.go +++ b/pkg/prowgen/jobbase_test.go @@ -500,6 +500,28 @@ func TestNewProwJobBaseBuilderForTest(t *testing.T) { }, }, }, + { + name: "job excluded by patterns should not have slack reporter config", + test: ciop.TestStepConfiguration{ + As: "unit-skip", + Commands: "make unit", + ContainerTestConfiguration: &ciop.ContainerTestConfiguration{From: "src"}, + }, + info: &ProwgenInfo{ + Metadata: ciop.Metadata{Org: "o", Repo: "r", Branch: "b"}, + Config: config.Prowgen{ + SlackReporterConfigs: []config.SlackReporterConfig{ + { + Channel: "some-channel", + JobStatesToReport: []prowv1.ProwJobState{"error"}, + ReportTemplate: "some template", + JobNames: []string{"unit-skip", "e2e"}, + ExcludedJobPatterns: []string{".*-skip$"}, + }, + }, + }, + }, + }, } for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { diff --git a/pkg/prowgen/prowgen.go b/pkg/prowgen/prowgen.go index 073ae0f137..247b512946 100644 --- a/pkg/prowgen/prowgen.go +++ b/pkg/prowgen/prowgen.go @@ -146,16 +146,7 @@ func GenerateJobs(configSpec *cioperatorapi.ReleaseBuildConfiguration, info *Pro injectArchitectureLabels(jobBaseGen, configSpec.Images) jobBaseGen.PodSpec.Add(Promotion(), Targets(imageTargets.UnsortedList()...)) - if slackReporter := info.Config.GetSlackReporterConfigForTest(imagesTestName, configSpec.Metadata.Variant); slackReporter != nil { - if jobBaseGen.base.ReporterConfig == nil { - jobBaseGen.base.ReporterConfig = &prowv1.ReporterConfig{} - } - jobBaseGen.base.ReporterConfig.Slack = &prowv1.SlackReporterConfig{ - Channel: slackReporter.Channel, - JobStatesToReport: slackReporter.JobStatesToReport, - ReportTemplate: slackReporter.ReportTemplate, - } - } + // Note: Slack reporter config for images postsubmit is now handled in generatePostsubmitForTest postsubmit := generatePostsubmitForTest(jobBaseGen, info) postsubmit.MaxConcurrency = 1 if postsubmit.Labels == nil { @@ -266,6 +257,19 @@ func generatePresubmitForTest(jobBaseBuilder *prowJobBaseBuilder, name string, i shortName := info.TestName(name) base := jobBaseBuilder.Rehearsable(!opts.disableRehearsal).Build(jc.PresubmitPrefix) + + // Set slack reporter config using full job name for proper excluded_job_patterns matching + fullJobName := info.JobName(jc.PresubmitPrefix, name) + if slackReporter := info.Config.GetSlackReporterConfigForJobName(fullJobName, name, info.Metadata.Variant); slackReporter != nil { + if base.ReporterConfig == nil { + base.ReporterConfig = &prowv1.ReporterConfig{} + } + base.ReporterConfig.Slack = &prowv1.SlackReporterConfig{ + Channel: slackReporter.Channel, + JobStatesToReport: slackReporter.JobStatesToReport, + ReportTemplate: slackReporter.ReportTemplate, + } + } pipelineOpt := false if opts.pipelineRunIfChanged != "" { if base.Annotations == nil { @@ -312,6 +316,20 @@ func generatePostsubmitForTest(jobBaseBuilder *prowJobBaseBuilder, info *Prowgen } base := jobBaseBuilder.Build(jc.PostsubmitPrefix) + + // Set slack reporter config using full job name for proper excluded_job_patterns matching + testName := jobBaseBuilder.testName + fullJobName := info.JobName(jc.PostsubmitPrefix, testName) + if slackReporter := info.Config.GetSlackReporterConfigForJobName(fullJobName, testName, info.Metadata.Variant); slackReporter != nil { + if base.ReporterConfig == nil { + base.ReporterConfig = &prowv1.ReporterConfig{} + } + base.ReporterConfig.Slack = &prowv1.SlackReporterConfig{ + Channel: slackReporter.Channel, + JobStatesToReport: slackReporter.JobStatesToReport, + ReportTemplate: slackReporter.ReportTemplate, + } + } alwaysRun := opts.runIfChanged == "" && opts.skipIfOnlyChanged == "" pj := &prowconfig.Postsubmit{ JobBase: base, @@ -366,6 +384,20 @@ func GeneratePeriodicForTest(jobBaseBuilder *prowJobBaseBuilder, info *ProwgenIn // We are resetting PathAlias because it will be set on the `ExtraRefs` item base := jobBaseBuilder.Rehearsable(!opts.DisableRehearsal).PathAlias("").Build(jc.PeriodicPrefix) + // Set slack reporter config using full job name for proper excluded_job_patterns matching + testName := jobBaseBuilder.testName + fullJobName := info.JobName(jc.PeriodicPrefix, testName) + if slackReporter := info.Config.GetSlackReporterConfigForJobName(fullJobName, testName, info.Metadata.Variant); slackReporter != nil { + if base.ReporterConfig == nil { + base.ReporterConfig = &prowv1.ReporterConfig{} + } + base.ReporterConfig.Slack = &prowv1.SlackReporterConfig{ + Channel: slackReporter.Channel, + JobStatesToReport: slackReporter.JobStatesToReport, + ReportTemplate: slackReporter.ReportTemplate, + } + } + cron := opts.Cron if cron == "@daily" { cron = hashDailyCron(base.Name) diff --git a/pkg/prowgen/testdata/zz_fixture_TestGenerateJobs_images_job_is_configured_for_slack_reporting.yaml b/pkg/prowgen/testdata/zz_fixture_TestGenerateJobs_images_job_is_configured_for_slack_reporting.yaml index 5483fa70a2..8b5fa5136d 100644 --- a/pkg/prowgen/testdata/zz_fixture_TestGenerateJobs_images_job_is_configured_for_slack_reporting.yaml +++ b/pkg/prowgen/testdata/zz_fixture_TestGenerateJobs_images_job_is_configured_for_slack_reporting.yaml @@ -17,3 +17,9 @@ presubmits: labels: pj-rehearse.openshift.io/can-be-rehearsed: "true" name: pull-ci-organization-repository-branch-images + reporter_config: + slack: + channel: some-channel + job_states_to_report: + - error + report_template: some template diff --git a/pkg/prowgen/testdata/zz_fixture_TestNewProwJobBaseBuilderForTest_job_excluded_by_patterns_should_not_have_slack_reporter_config.yaml b/pkg/prowgen/testdata/zz_fixture_TestNewProwJobBaseBuilderForTest_job_excluded_by_patterns_should_not_have_slack_reporter_config.yaml new file mode 100644 index 0000000000..208a979da8 --- /dev/null +++ b/pkg/prowgen/testdata/zz_fixture_TestNewProwJobBaseBuilderForTest_job_excluded_by_patterns_should_not_have_slack_reporter_config.yaml @@ -0,0 +1,44 @@ +agent: kubernetes +decorate: true +decoration_config: + skip_cloning: true +name: prefix-ci-o-r-b-unit-skip +spec: + containers: + - args: + - --gcs-upload-secret=/secrets/gcs/service-account.json + - --image-import-pull-secret=/etc/pull-secret/.dockerconfigjson + - --report-credentials-file=/etc/report/credentials + - --target=unit-skip + command: + - ci-operator + image: quay-proxy.ci.openshift.org/openshift/ci:ci_ci-operator_latest + imagePullPolicy: Always + name: "" + resources: + requests: + cpu: 10m + volumeMounts: + - mountPath: /secrets/gcs + name: gcs-credentials + readOnly: true + - mountPath: /secrets/manifest-tool + name: manifest-tool-local-pusher + readOnly: true + - mountPath: /etc/pull-secret + name: pull-secret + readOnly: true + - mountPath: /etc/report + name: result-aggregator + readOnly: true + serviceAccountName: ci-operator + volumes: + - name: manifest-tool-local-pusher + secret: + secretName: manifest-tool-local-pusher + - name: pull-secret + secret: + secretName: registry-pull-credentials + - name: result-aggregator + secret: + secretName: result-aggregator diff --git a/pkg/prowgen/testdata/zz_fixture_TestNewProwJobBaseBuilderForTest_simple_with_slack_reporter_config.yaml b/pkg/prowgen/testdata/zz_fixture_TestNewProwJobBaseBuilderForTest_simple_with_slack_reporter_config.yaml index 806482c710..fa6cf161ff 100644 --- a/pkg/prowgen/testdata/zz_fixture_TestNewProwJobBaseBuilderForTest_simple_with_slack_reporter_config.yaml +++ b/pkg/prowgen/testdata/zz_fixture_TestNewProwJobBaseBuilderForTest_simple_with_slack_reporter_config.yaml @@ -3,12 +3,6 @@ decorate: true decoration_config: skip_cloning: true name: prefix-ci-o-r-b-unit -reporter_config: - slack: - channel: some-channel - job_states_to_report: - - error - report_template: some template spec: containers: - args: