prowgen: remove .config.prowgen support#5167
Draft
Prucek wants to merge 1 commit intoopenshift:mainfrom
Draft
Conversation
All features previously configured via .config.prowgen files are now available inline in ci-operator configuration YAML via the `prowgen:` field (private, expose, disable_rehearsals, skip_operator_presubmits, enable_secrets_store_csi_driver) and per-test `reporter_config` / `disable_rehearsal` fields. This removes: - config.Prowgen struct and all associated types/functions from pkg/config/load.go (Rehearsals, SlackReporterConfig, SkipOperatorPresubmits, LoadProwgenConfig, validateProwgenConfig, MergeDefaults, GetSlackReporterConfigForJobName, SkipPresubmits) - ProwgenInfo.Config field — ProwgenInfo now only contains Metadata - .config.prowgen loading from ci-operator-prowgen and image-graph-generator - .config.prowgen skip from check-gh-automation Integration test fixtures migrated from .config.prowgen to inline ci-operator config equivalents. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
|
Pipeline controller notification For optional jobs, comment This repository is configured in: automatic mode |
openshift-ci
Bot
added
the
do-not-merge/work-in-progress
Contributor
|
Skipping CI for Draft Pull Request. |
📝 WalkthroughWalkthroughThis PR migrates Prowgen configuration from centralized ChangesMigrate Prowgen Configuration to Inline CI Operator Config
🎯 4 (Complex) | ⏱️ ~55 minutes 🚥 Pre-merge checks | ✅ 12 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (12 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Contributor
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: Prucek The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
Bot
added
the
approved
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
pkg/prowgen/prowgen.go (1)
43-65:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPropagate repo-level
disable_rehearsalsto non-test presubmits.
disableAllRehearsalsis only threaded through theconfigSpec.Testsloop. The generatedimagesand operator bundle presubmits below still callgeneratePresubmitForTest(...)withoutdisableRehearsal, so a repo-wideprowgen.disable_rehearsals: truewill stop suppressing rehearsals for those jobs after this migration.Proposed fix
presubmits[orgrepo] = append(presubmits[orgrepo], *generatePresubmitForTest(jobBaseGen, imagesTestName, info, func(options *generatePresubmitOptions) { + options.disableRehearsal = disableAllRehearsals options.optional = optional options.runIfChanged = configSpec.Images.RunIfChanged options.skipIfOnlyChanged = configSpec.Images.SkipIfOnlyChanged options.pipelineRunIfChanged = configSpec.Images.PipelineRunIfChanged options.pipelineSkipIfOnlyChanged = configSpec.Images.PipelineSkipIfOnlyChanged @@ presubmits[orgrepo] = append(presubmits[orgrepo], *generatePresubmitForTest(jobBaseGen, testName, info, func(options *generatePresubmitOptions) { + options.disableRehearsal = disableAllRehearsals options.optional = bundle.Optional options.Capabilities = bundle.Capabilities })) @@ - presubmits[orgrepo] = append(presubmits[orgrepo], *generatePresubmitForTest(jobBaseGen, name, info)) + presubmits[orgrepo] = append(presubmits[orgrepo], *generatePresubmitForTest(jobBaseGen, name, info, func(options *generatePresubmitOptions) { + options.disableRehearsal = disableAllRehearsals + }))Also applies to: 143-149, 181-208
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/prowgen/prowgen.go` around lines 43 - 65, The repo-level disableAllRehearsals flag is computed when iterating configSpec.Tests but never propagated into the other presubmit generators, so generatePresubmitForTest(...) calls for images and operator bundles ignore it; update the call sites that create non-test presubmits (calls to generatePresubmitForTest and any wrappers that build presubmits) to accept and pass a disableRehearsal boolean (derived from disableAllRehearsals || element.DisableRehearsal or appropriate repo-level value), and adjust the signature of generatePresubmitForTest (and any helper like NewProwJobBaseBuilderForTest usage) to honor that parameter so rehearsals are suppressed consistently. Ensure all places noted in the review (the additional presubmit generation blocks) pass the propagated disableRehearsal flag.
🧹 Nitpick comments (1)
cmd/ci-operator-prowgen/main.go (1)
123-136: ⚡ Quick winWrap
GenerateJobsfailures with repo context.The raw
return errhere drops whichorg/repowas being processed, so one bad config becomes hard to isolate during a full tree run.Proposed fix
generated, err := prowgen.GenerateJobs(configSpec, pInfo) if err != nil { - return err + return fmt.Errorf("generate jobs for %s: %w", orgRepo, err) }As per coding guidelines, "Wrap errors with fmt.Errorf("context: %w", err), lowercase messages, no trailing punctuation"
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cmd/ci-operator-prowgen/main.go` around lines 123 - 136, The call to prowgen.GenerateJobs in generateJobs returns an error that is currently returned raw, losing repo context; update the error return to wrap the failure with the org/repo context (use orgRepo) using fmt.Errorf with the %w verb and a lowercase, no-trailing-punctuation message (e.g., "generate jobs for %s: %w", orgRepo, err) so failures from prowgen.GenerateJobs include which repository failed while keeping existing variables (configSpec, pInfo, generated) unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@pkg/prowgen/prowgen.go`:
- Around line 262-275: slackReporterConfig currently only reads
tests[*].reporter_config and thus drops reporter settings for
synthesized/non-test jobs; update slackReporterConfig (and its callers) to
accept and merge a repo-level reporter config (e.g., a second parameter
repoSlackConfig *cioperatorapi.SlackReporterConfig) so that if testSlackConfig
is nil or missing fields you fall back to repoSlackConfig, then apply the
existing default-job-states logic and return the merged prowv1.ReporterConfig;
reference slackReporterConfig, cioperatorapi.SlackReporterConfig and
prowv1.ReporterConfig when locating where to implement the merge.
---
Outside diff comments:
In `@pkg/prowgen/prowgen.go`:
- Around line 43-65: The repo-level disableAllRehearsals flag is computed when
iterating configSpec.Tests but never propagated into the other presubmit
generators, so generatePresubmitForTest(...) calls for images and operator
bundles ignore it; update the call sites that create non-test presubmits (calls
to generatePresubmitForTest and any wrappers that build presubmits) to accept
and pass a disableRehearsal boolean (derived from disableAllRehearsals ||
element.DisableRehearsal or appropriate repo-level value), and adjust the
signature of generatePresubmitForTest (and any helper like
NewProwJobBaseBuilderForTest usage) to honor that parameter so rehearsals are
suppressed consistently. Ensure all places noted in the review (the additional
presubmit generation blocks) pass the propagated disableRehearsal flag.
---
Nitpick comments:
In `@cmd/ci-operator-prowgen/main.go`:
- Around line 123-136: The call to prowgen.GenerateJobs in generateJobs returns
an error that is currently returned raw, losing repo context; update the error
return to wrap the failure with the org/repo context (use orgRepo) using
fmt.Errorf with the %w verb and a lowercase, no-trailing-punctuation message
(e.g., "generate jobs for %s: %w", orgRepo, err) so failures from
prowgen.GenerateJobs include which repository failed while keeping existing
variables (configSpec, pInfo, generated) unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: dc6a2fde-2869-489f-84b3-9bb7477a288f
📒 Files selected for processing (29)
cmd/check-gh-automation/main.gocmd/ci-operator-prowgen/main.gocmd/ci-operator-prowgen/main_test.gopkg/config/load.gopkg/config/load_test.gopkg/image-graph-generator/operator.gopkg/prowgen/jobbase.gopkg/prowgen/jobbase_test.gopkg/prowgen/prowgen.gopkg/prowgen/prowgen_test.gopkg/prowgen/testdata/zz_fixture_TestGenerateJobs_ci_operator_config_takes_precedence_over_prowgen_config.yamlpkg/prowgen/testdata/zz_fixture_TestGenerateJobs_images_job_is_configured_for_slack_reporting.yamlpkg/prowgen/testdata/zz_fixture_TestNewProwJobBaseBuilderForTest_job_excluded_by_patterns_should_not_have_slack_reporter_config.yamltest/integration/ci-operator-prowgen/input/config/norehearsals/duper/.config.prowgentest/integration/ci-operator-prowgen/input/config/norehearsals/duper/norehearsals-duper-master.yamltest/integration/ci-operator-prowgen/input/config/norehearsals/stuper/.config.prowgentest/integration/ci-operator-prowgen/input/config/norehearsals/stuper/norehearsals-stuper-master.yamltest/integration/ci-operator-prowgen/input/config/private-org/.config.prowgentest/integration/ci-operator-prowgen/input/config/private-org/duper/private-org-duper-master.yamltest/integration/ci-operator-prowgen/input/config/private-org/super/private-org-super-master.yamltest/integration/ci-operator-prowgen/input/config/private/duper/.config.prowgentest/integration/ci-operator-prowgen/input/config/private/duper/private-duper-master.yamltest/integration/ci-operator-prowgen/input/config/slack-report/duper/.config.prowgentest/integration/ci-operator-prowgen/input/config/slack-report/duper/slack-report-duper-master.yamltest/integration/ci-operator-prowgen/input/config/super/duper/.config.prowgentest/integration/ci-operator-prowgen/input/config/super/duper/super-duper-release-4.19__periodics.yamltest/integration/ci-operator-prowgen/output/jobs/slack-report-inline/duper/slack-report-inline-duper-master-periodics.yamltest/integration/ci-operator-prowgen/output/jobs/slack-report-inline/duper/slack-report-inline-duper-master-postsubmits.yamltest/integration/ci-operator-prowgen/output/jobs/slack-report-inline/duper/slack-report-inline-duper-master-presubmits.yaml
💤 Files with no reviewable changes (16)
- test/integration/ci-operator-prowgen/input/config/norehearsals/stuper/.config.prowgen
- test/integration/ci-operator-prowgen/input/config/private-org/.config.prowgen
- pkg/prowgen/testdata/zz_fixture_TestGenerateJobs_ci_operator_config_takes_precedence_over_prowgen_config.yaml
- test/integration/ci-operator-prowgen/input/config/norehearsals/duper/.config.prowgen
- pkg/prowgen/testdata/zz_fixture_TestNewProwJobBaseBuilderForTest_job_excluded_by_patterns_should_not_have_slack_reporter_config.yaml
- test/integration/ci-operator-prowgen/input/config/super/duper/.config.prowgen
- test/integration/ci-operator-prowgen/input/config/private/duper/.config.prowgen
- test/integration/ci-operator-prowgen/output/jobs/slack-report-inline/duper/slack-report-inline-duper-master-presubmits.yaml
- test/integration/ci-operator-prowgen/output/jobs/slack-report-inline/duper/slack-report-inline-duper-master-periodics.yaml
- test/integration/ci-operator-prowgen/input/config/slack-report/duper/.config.prowgen
- test/integration/ci-operator-prowgen/output/jobs/slack-report-inline/duper/slack-report-inline-duper-master-postsubmits.yaml
- pkg/image-graph-generator/operator.go
- pkg/prowgen/testdata/zz_fixture_TestGenerateJobs_images_job_is_configured_for_slack_reporting.yaml
- pkg/config/load_test.go
- cmd/check-gh-automation/main.go
- pkg/config/load.go
Comment on lines
+262
to
+275
| func slackReporterConfig(testSlackConfig *cioperatorapi.SlackReporterConfig) *prowv1.ReporterConfig { | ||
| if testSlackConfig == nil { | ||
| return nil | ||
| } | ||
| if slackReporter := info.Config.GetSlackReporterConfigForJobName(jobName, testName, info.Metadata.Variant); slackReporter != nil { | ||
| return &prowv1.ReporterConfig{ | ||
| Slack: &prowv1.SlackReporterConfig{ | ||
| Channel: slackReporter.Channel, | ||
| JobStatesToReport: slackReporter.JobStatesToReport, | ||
| ReportTemplate: slackReporter.ReportTemplate, | ||
| }, | ||
| } | ||
| jobStatesToReport := testSlackConfig.JobStatesToReport | ||
| if len(jobStatesToReport) == 0 { | ||
| jobStatesToReport = cioperatorapi.DefaultSlackReporterJobStatesToReport | ||
| } | ||
| return &prowv1.ReporterConfig{ | ||
| Slack: &prowv1.SlackReporterConfig{ | ||
| Channel: testSlackConfig.Channel, | ||
| JobStatesToReport: jobStatesToReport, | ||
| ReportTemplate: testSlackConfig.ReportTemplate, | ||
| }, |
There was a problem hiding this comment.
This migration drops reporter config for generated images jobs.
After narrowing slackReporterConfig to tests[*].reporter_config only, synthesized jobs like the images promotion postsubmit have no inline field that can ever reach this helper. Repos that previously configured Slack reporting for those jobs via .config.prowgen lose that behavior with no replacement.
If that deprecation is intentional, it should be called out explicitly; otherwise this needs a non-test reporter-config source before the old path is removed.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@pkg/prowgen/prowgen.go` around lines 262 - 275, slackReporterConfig currently
only reads tests[*].reporter_config and thus drops reporter settings for
synthesized/non-test jobs; update slackReporterConfig (and its callers) to
accept and merge a repo-level reporter config (e.g., a second parameter
repoSlackConfig *cioperatorapi.SlackReporterConfig) so that if testSlackConfig
is nil or missing fields you fall back to repoSlackConfig, then apply the
existing default-job-states logic and return the merged prowv1.ReporterConfig;
reference slackReporterConfig, cioperatorapi.SlackReporterConfig and
prowv1.ReporterConfig when locating where to implement the merge.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
.config.prowgenfile support entirely from prowgen.config.prowgenare now available inline in ci-operator configuration YAML:prowgen: {private, expose, disable_rehearsals, skip_operator_presubmits, enable_secrets_store_csi_driver}reporter_configanddisable_rehearsalfieldsconfig.Prowgenstruct and all associated types/functions frompkg/config/load.goProwgenInfo.Configfield —ProwgenInfonow only containsMetadata.config.prowgenloading fromci-operator-prowgenandimage-graph-generator.config.prowgenskip fromcheck-gh-automation.config.prowgento inline ci-operator config equivalents🤖 Generated with Claude Code