diff --git a/cmd/krel/cmd/release_notes.go b/cmd/krel/cmd/release_notes.go index 5cbc949bd22..d22b2245783 100644 --- a/cmd/krel/cmd/release_notes.go +++ b/cmd/krel/cmd/release_notes.go @@ -124,21 +124,63 @@ permissions to your fork of k/sig-release and k-sigs/release-notes.`, }, } +// rerunCmd represents the `krel release-notes rerun` subcommand. +var rerunCmd = &cobra.Command{ + Use: "rerun", + Short: "Rerun release notes generation against an existing draft PR branch", + Long: `krel release-notes rerun + +Re-generates the release notes draft against an existing PR branch. The process: + +1. Clone upstream k/sig-release and fetch the draft branch from --draft-pr-source-fork. +2. Gather release notes from k/k for the given --tag range. +3. Apply map files (from the branch and any extra --maps-from paths). +4. Write the updated markdown and JSON drafts, then commit. +5. If --draft-pr-push-fork is set, push the branch there (updating any open PR). + +The local clone is preserved after the run so you can inspect or push manually.`, + Example: ` # Rerun against an existing draft branch and push to a destination fork: + krel release-notes rerun \ + --tag v1.36.0 \ + --draft-pr-source-fork "myorg/sig-release-repo" \ + --draft-pr-push-fork "destorg" + + # Rerun with additional maps from a local directory: + krel release-notes rerun \ + --tag v1.36.0 \ + --draft-pr-source-fork "myorg/sig-release" \ + --maps-from "/path/to/extra/maps"`, + SilenceUsage: true, + SilenceErrors: true, + PreRunE: func(cmd *cobra.Command, args []string) error { + return validateRerunOpts(releaseNotesOpts) + }, + RunE: func(cmd *cobra.Command, args []string) error { + tag := releaseNotesOpts.tag + + return rerunDraftNotes(releaseNotesOpts.repoPath, tag) + }, +} + type releaseNotesOptions struct { - createDraftPR bool - createWebsitePR bool - fixNotes bool - interactiveMode bool - updateRepo bool - useSSH bool - repoPath string - tag string - userFork string - websiteRepo string - githubOrg string - draftRepo string - mapProviders []string - includeLabels []string + createDraftPR bool + createWebsitePR bool + fixNotes bool + interactiveMode bool + updateRepo bool + useSSH bool + repoPath string + tag string + userFork string + websiteRepo string + githubOrg string + draftRepo string + draftPRSourceFork string + draftPRSourceBranch string + draftPRPushFork string + draftPRPushBranch string + mapProviders []string + includeLabels []string } type releaseNotesResult struct { @@ -245,6 +287,36 @@ func init() { _ = releaseNotesCmd.PersistentFlags().MarkDeprecated("create-website-pr", "This flag is deprecated and will be removed in a future release. Use --create-draft-pr instead.") + // Register rerun subcommand flags + rerunCmd.Flags().StringVar( + &releaseNotesOpts.draftPRSourceFork, + "draft-pr-source-fork", + "", + "k/sig-release fork to fetch the existing draft branch from; 'org' or 'org/repo' (required)", + ) + + rerunCmd.Flags().StringVar( + &releaseNotesOpts.draftPRSourceBranch, + "draft-pr-source-branch", + "", + "branch to fetch from the source k/sig-release fork (default: release-notes-draft-)", + ) + + rerunCmd.Flags().StringVar( + &releaseNotesOpts.draftPRPushFork, + "draft-pr-push-fork", + "", + "k/sig-release fork to push the updated branch to; 'org' or 'org/repo' (omit to skip push)", + ) + + rerunCmd.Flags().StringVar( + &releaseNotesOpts.draftPRPushBranch, + "draft-pr-push-branch", + "", + "branch to push to on the destination k/sig-release fork (default: same as source branch)", + ) + + releaseNotesCmd.AddCommand(rerunCmd) rootCmd.AddCommand(releaseNotesCmd) } @@ -325,6 +397,231 @@ func runReleaseNotes() (err error) { return nil } +// rerunDraftNotes fetches an existing draft branch from a source fork of +// k/sig-release, regenerates the release notes (applying maps), +// and optionally pushes the result to a destination fork. +func rerunDraftNotes(repoPath, tag string) error { + tagVersion, err := helpers.TagStringToSemver(tag) + if err != nil { + return fmt.Errorf("reading tag: %s: %w", tag, err) + } + + // Parse source fork org/repo + sourceParts := strings.SplitN(releaseNotesOpts.draftPRSourceFork, "/", 2) + sourceOrg, sourceRepo := sourceParts[0], sourceParts[1] + + branchname := releaseNotesOpts.draftPRSourceBranch + + gh := github.New() + + // Verify source fork is a fork of k/sig-release + logrus.Infof("Verifying %s/%s is a fork of %s/%s", + sourceOrg, sourceRepo, git.DefaultGithubOrg, git.DefaultGithubReleaseRepo) + + isFork, err := gh.RepoIsForkOf( + sourceOrg, sourceRepo, + git.DefaultGithubOrg, git.DefaultGithubReleaseRepo, + ) + if err != nil { + return fmt.Errorf( + "while checking if %s/%s is a fork of %s/%s: %w", + sourceOrg, sourceRepo, + git.DefaultGithubOrg, git.DefaultGithubReleaseRepo, err, + ) + } + + if !isFork { + return fmt.Errorf( + "%s/%s is not a fork of %s/%s", + sourceOrg, sourceRepo, + git.DefaultGithubOrg, git.DefaultGithubReleaseRepo, + ) + } + + // Clone upstream k/sig-release + logrus.Info("Cloning upstream kubernetes/sig-release") + + opts := &gogit.CloneOptions{} + + sigReleaseRepo, err := git.CleanCloneGitHubRepo( + git.DefaultGithubOrg, git.DefaultGithubReleaseRepo, + false, true, opts, + ) + if err != nil { + return fmt.Errorf("cloning kubernetes/sig-release: %w", err) + } + + // Do NOT call Cleanup — preserve the local clone for user inspection + + // Add the source fork as a remote named "source" + logrus.Infof("Adding remote 'source' for %s/%s", sourceOrg, sourceRepo) + + // Use HTTPS for the source remote — it's a read-only fetch from a public fork + if err := sigReleaseRepo.AddRemote("source", sourceOrg, sourceRepo, false); err != nil { + return fmt.Errorf("adding source remote %s/%s: %w", sourceOrg, sourceRepo, err) + } + + // Fetch the source branch + logrus.Infof("Fetching branch %s from source remote", branchname) + + if err := command.NewWithWorkDir( + sigReleaseRepo.Dir(), "git", "fetch", "source", branchname, + ).RunSuccess(); err != nil { + // Handle error when the source branch does not exist + return fmt.Errorf( + "fetching branch %s from %s/%s: branch may not exist on the source fork: %w", + branchname, sourceOrg, sourceRepo, err, + ) + } + + // Checkout the fetched branch locally + logrus.Infof("Checking out branch %s", branchname) + + if err := command.NewWithWorkDir( + sigReleaseRepo.Dir(), "git", "checkout", "-B", branchname, "source/"+branchname, + ).RunSuccess(); err != nil { + return fmt.Errorf("checking out branch %s: %w", branchname, err) + } + + // Compute startTag (previous minor version, same as createDraftPR) + start := helpers.SemverToTagString(semver.Version{ + Major: tagVersion.Major, + Minor: tagVersion.Minor - 1, + Patch: 0, + }) + + // Gather release notes from k/k. + // The release path inside the sig-release repository + releasePath := filepath.Join("releases", fmt.Sprintf("release-%d.%d", tagVersion.Major, tagVersion.Minor)) + releaseDir := filepath.Join(sigReleaseRepo.Dir(), releasePath) + + // Add the fetched branch's maps directory to map providers + branchMapsDir := filepath.Join(releaseDir, releaseNotesWorkDir, mapsMainDirectory) + originalMapProviders := releaseNotesOpts.mapProviders + + if helpers.Exists(branchMapsDir) { + logrus.Infof("Loading maps from fetched branch: %s", branchMapsDir) + releaseNotesOpts.mapProviders = append([]string{branchMapsDir}, releaseNotesOpts.mapProviders...) + } + + releaseNotes, err := gatherNotesFrom(repoPath, start) + if err != nil { + return fmt.Errorf("gathering release notes for tag %s: %w", start, err) + } + + // Restore original map providers + releaseNotesOpts.mapProviders = originalMapProviders + + // Build the notes result (markdown + JSON) + result, err := buildNotesResult(start, releaseNotes) + if err != nil { + return fmt.Errorf("building release notes results: %w", err) + } + + // Write the regenerated files to the working tree + logrus.Debugf("Release notes draft files will be written to %s", releaseDir) + + if err := createNotesWorkDir(releaseDir); err != nil { + return fmt.Errorf("creating working directory: %w", err) + } + + //nolint:gosec // TODO(gosec): G306 + if err := os.WriteFile( + filepath.Join(releaseDir, releaseNotesWorkDir, draftMarkdownFile), + []byte(result.markdown), 0o644, + ); err != nil { + return fmt.Errorf("writing release notes draft: %w", err) + } + + logrus.Infof("Release Notes Markdown Draft written to %s", + filepath.Join(releaseDir, releaseNotesWorkDir, draftMarkdownFile)) + + //nolint:gosec // TODO(gosec): G306 + if err := os.WriteFile( + filepath.Join(releaseDir, releaseNotesWorkDir, draftJSONFile), + []byte(result.json), 0o644, + ); err != nil { + return fmt.Errorf("writing release notes json file: %w", err) + } + + logrus.Infof("Release Notes JSON version written to %s", + filepath.Join(releaseDir, releaseNotesWorkDir, draftJSONFile)) + + // Create a commit with the updated files + if err := createDraftCommit( + sigReleaseRepo, releasePath, "Release Notes draft for k/k "+tag, + ); err != nil { + return fmt.Errorf("creating release notes commit: %w", err) + } + + // Optional push to destination fork + if releaseNotesOpts.draftPRPushFork != "" { + pushParts := strings.SplitN(releaseNotesOpts.draftPRPushFork, "/", 2) + pushOrg, pushRepo := pushParts[0], pushParts[1] + pushBranch := releaseNotesOpts.draftPRPushBranch + + // Verify push fork is a fork of k/sig-release + logrus.Infof("Verifying %s/%s is a fork of %s/%s", + pushOrg, pushRepo, git.DefaultGithubOrg, git.DefaultGithubReleaseRepo) + + isPushFork, err := gh.RepoIsForkOf( + pushOrg, pushRepo, + git.DefaultGithubOrg, git.DefaultGithubReleaseRepo, + ) + if err != nil { + return fmt.Errorf( + "while checking if %s/%s is a fork of %s/%s: %w", + pushOrg, pushRepo, + git.DefaultGithubOrg, git.DefaultGithubReleaseRepo, err, + ) + } + + if !isPushFork { + return fmt.Errorf( + "%s/%s is not a fork of %s/%s", + pushOrg, pushRepo, + git.DefaultGithubOrg, git.DefaultGithubReleaseRepo, + ) + } + + // Add the push fork as a remote named "userfork" + if err := sigReleaseRepo.AddRemote(userForkName, pushOrg, pushRepo, false); err != nil { + return fmt.Errorf("adding push remote %s/%s: %w", pushOrg, pushRepo, err) + } + + // Push with --force-with-lease + logrus.Infof("Pushing to %s/%s branch %s with --force-with-lease", pushOrg, pushRepo, pushBranch) + + if err := command.NewWithWorkDir( + sigReleaseRepo.Dir(), + "git", "push", "--force-with-lease", userForkName, branchname+":"+pushBranch, + ).RunSuccess(); err != nil { + return fmt.Errorf("pushing to %s/%s branch %s: %w", pushOrg, pushRepo, pushBranch, err) + } + + // Log that existing PR will be updated + logrus.Infof("Pushed to %s/%s branch %s — any existing PR will be updated by this push", pushOrg, pushRepo, pushBranch) + } + + // Print the local clone path + fmt.Println() + fmt.Println("Local sig-release clone preserved at:") + fmt.Println(sigReleaseRepo.Dir()) + + if releaseNotesOpts.draftPRPushFork == "" { + fmt.Println() + fmt.Println("No --draft-pr-push-fork was specified, so no push was performed.") + fmt.Println("To push manually, run:") + fmt.Printf(" cd %s\n", sigReleaseRepo.Dir()) + fmt.Printf(" git remote add userfork \n") + fmt.Printf(" git push --force-with-lease userfork %s\n", branchname) + } + + logrus.Info("Rerun complete!") + + return nil +} + // createDraftPR pushes the release notes draft to the users fork. // //nolint:maintidx // complex but acceptable @@ -1042,6 +1339,54 @@ func (o *releaseNotesOptions) Validate() error { return nil } +// validateRerunOpts validates options specific to the rerun subcommand. +func validateRerunOpts(o *releaseNotesOptions) error { + token, isset := os.LookupEnv(github.TokenEnvKey) + if !isset || token == "" { + return fmt.Errorf("cannot generate release notes if %s env variable is not set", github.TokenEnvKey) + } + + if o.tag == "" { + return errors.New("--tag is required") + } + + if o.draftPRSourceFork == "" { + return errors.New("--draft-pr-source-fork is required") + } + + // Parse --draft-pr-source-fork: if no "/" present, append "/sig-release" + if !strings.Contains(o.draftPRSourceFork, "/") { + o.draftPRSourceFork = o.draftPRSourceFork + "/" + git.DefaultGithubReleaseRepo + } + + // Parse --draft-pr-push-fork: if no "/" present, append "/sig-release" + if o.draftPRPushFork != "" && !strings.Contains(o.draftPRPushFork, "/") { + o.draftPRPushFork = o.draftPRPushFork + "/" + git.DefaultGithubReleaseRepo + } + + // Resolve --draft-pr-source-branch default + if o.draftPRSourceBranch == "" { + o.draftPRSourceBranch = draftBranchPrefix + o.tag + } + + // Resolve --draft-pr-push-branch default to source branch + if o.draftPRPushBranch == "" { + o.draftPRPushBranch = o.draftPRSourceBranch + } + + // Warn if rerun without --maps-from + if len(o.mapProviders) == 0 { + mapsPath := "releases/release-x.yz/release-notes/maps" + if tagVersion, err := helpers.TagStringToSemver(o.tag); err == nil { + mapsPath = fmt.Sprintf("releases/release-%d.%d/release-notes/maps", tagVersion.Major, tagVersion.Minor) + } + + logrus.Warnf("Running rerun without --maps-from will automatically pick up map files from %s folder", mapsPath) + } + + return nil +} + // Save the session to a file. func (sd *sessionData) Save() error { if sd.Date == 0 { diff --git a/cmd/krel/cmd/release_notes_test.go b/cmd/krel/cmd/release_notes_test.go new file mode 100644 index 00000000000..21f801afefd --- /dev/null +++ b/cmd/krel/cmd/release_notes_test.go @@ -0,0 +1,158 @@ +/* +Copyright 2026 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package cmd + +import ( + "bytes" + "testing" + + "github.com/sirupsen/logrus" + "github.com/stretchr/testify/require" +) + +func setGitHubToken(t *testing.T) { + t.Helper() + t.Setenv("GITHUB_TOKEN", "fake-token-for-testing") +} + +func TestValidateRerunOpts_RequiredFlags(t *testing.T) { + setGitHubToken(t) + t.Run("missing draftPRSourceFork", func(t *testing.T) { + opts := &releaseNotesOptions{ + tag: "v1.32.0", + draftPRSourceFork: "", + } + err := validateRerunOpts(opts) + require.Error(t, err) + require.Contains(t, err.Error(), "--draft-pr-source-fork is required") + }) + + t.Run("missing tag", func(t *testing.T) { + opts := &releaseNotesOptions{ + tag: "", + draftPRSourceFork: "myorg/sig-release", + } + err := validateRerunOpts(opts) + require.Error(t, err) + require.Contains(t, err.Error(), "--tag is required") + }) +} + +func TestValidateRerunOpts_ForkNormalization(t *testing.T) { + setGitHubToken(t) + t.Run("org-only source fork gets /sig-release appended", func(t *testing.T) { + opts := &releaseNotesOptions{ + tag: "v1.32.0", + draftPRSourceFork: "myorg", + } + err := validateRerunOpts(opts) + require.NoError(t, err) + require.Equal(t, "myorg/sig-release", opts.draftPRSourceFork) + }) + + t.Run("full org/repo source fork unchanged", func(t *testing.T) { + opts := &releaseNotesOptions{ + tag: "v1.32.0", + draftPRSourceFork: "myorg/myrepo", + } + err := validateRerunOpts(opts) + require.NoError(t, err) + require.Equal(t, "myorg/myrepo", opts.draftPRSourceFork) + }) + + t.Run("org-only push fork gets /sig-release appended", func(t *testing.T) { + opts := &releaseNotesOptions{ + tag: "v1.32.0", + draftPRSourceFork: "myorg/sig-release", + draftPRPushFork: "pushorg", + } + err := validateRerunOpts(opts) + require.NoError(t, err) + require.Equal(t, "pushorg/sig-release", opts.draftPRPushFork) + }) +} + +func TestValidateRerunOpts_BranchDefaults(t *testing.T) { + setGitHubToken(t) + t.Run("empty source branch defaults to release-notes-draft-", func(t *testing.T) { + opts := &releaseNotesOptions{ + tag: "v1.32.0", + draftPRSourceFork: "myorg/sig-release", + draftPRSourceBranch: "", + } + err := validateRerunOpts(opts) + require.NoError(t, err) + require.Equal(t, "release-notes-draft-v1.32.0", opts.draftPRSourceBranch) + }) + + t.Run("empty push branch defaults to source branch", func(t *testing.T) { + opts := &releaseNotesOptions{ + tag: "v1.32.0", + draftPRSourceFork: "myorg/sig-release", + draftPRSourceBranch: "my-branch", + draftPRPushBranch: "", + } + err := validateRerunOpts(opts) + require.NoError(t, err) + require.Equal(t, "my-branch", opts.draftPRPushBranch) + }) +} + +func TestValidateRerunOpts_ValidOptions(t *testing.T) { + setGitHubToken(t) + + opts := &releaseNotesOptions{ + tag: "v1.32.0", + draftPRSourceFork: "myorg/sig-release", + } + err := validateRerunOpts(opts) + require.NoError(t, err) +} + +func TestValidateRerunOpts_DynamicMapsWarning(t *testing.T) { + setGitHubToken(t) + + tests := []struct { + name string + tag string + expected string + }{ + {"alpha tag", "v1.32.0-alpha.1", "releases/release-1.32/release-notes/maps"}, + {"beta tag", "v1.33.0-beta.0", "releases/release-1.33/release-notes/maps"}, + {"rc tag", "v1.36.0-rc.2", "releases/release-1.36/release-notes/maps"}, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + orig := logrus.StandardLogger().Out + + var buf bytes.Buffer + + logrus.SetOutput(&buf) + defer logrus.SetOutput(orig) + + opts := &releaseNotesOptions{ + tag: tc.tag, + draftPRSourceFork: "myorg/sig-release", + mapProviders: []string{}, + } + err := validateRerunOpts(opts) + require.NoError(t, err) + require.Contains(t, buf.String(), tc.expected) + }) + } +} diff --git a/docs/krel/release-notes.md b/docs/krel/release-notes.md index 643073b4fef..77ddb6fd53f 100644 --- a/docs/krel/release-notes.md +++ b/docs/krel/release-notes.md @@ -64,7 +64,7 @@ Global Flags: ### Examples -`krel release-notes` has two main modes of operation: +`krel release-notes` has three main modes of operation: #### Update the Release Notes markdown draft @@ -106,6 +106,54 @@ As with `--create-draft-pr`, `--tag` is optional and will default to the latest You can override the name of your fork of kubernetes-sigs/release-notes by specifying the full repository slug: `--fork=myorg/myreponame`. +#### Rerun against an existing draft branch + +After the initial `--create-draft-pr` run creates a PR against k/sig-release, reviewers may suggest changes. To incorporate those changes (via [map files](../release-notes-maps.md)), you need to regenerate the draft against the existing branch. + +The `rerun` subcommand handles this workflow — it fetches an existing draft branch from any fork, regenerates the notes with maps applied, and optionally pushes the result to a destination fork. + +This solves the problem where re-running `--create-draft-pr` fails because the branch already exists on the fork, and also supports handoffs where a different team member needs to pick up the rerun work from a colleague's fork. + +The process: + +1. Clone upstream k/sig-release and fetch the draft branch from `--draft-pr-source-fork` +2. Gather release notes from k/k for the given `--tag` range +3. Apply map files (from the branch and any extra `--maps-from` paths) +4. Write the updated markdown and JSON drafts, then commit +5. If `--draft-pr-push-fork` is set, push the branch there (updating any open PR) + +The local clone is preserved after the run so you can inspect or push manually. + +```bash +# Rerun and push to your own fork (updates the open PR): +krel release-notes rerun \ + --tag v1.36.0-beta.0 \ + --draft-pr-source-fork colleague \ + --draft-pr-push-fork myfork + +# Rerun with additional local maps, no push (inspect locally): +krel release-notes rerun \ + --tag v1.36.0-beta.0 \ + --draft-pr-source-fork myfork \ + --maps-from /path/to/extra/maps +``` + +The `--draft-pr-source-fork` flag accepts either an org name (`myorg`, which expands to `myorg/sig-release`) or a full slug (`myorg/myrepo`). +The same applies to `--draft-pr-push-fork`. + +If `--draft-pr-source-branch` is not specified, it defaults to `release-notes-draft-`. + +Flags specific to `rerun`: + +``` + --draft-pr-source-fork string k/sig-release fork to fetch the existing draft branch from (required) + --draft-pr-source-branch string branch to fetch (default: release-notes-draft-) + --draft-pr-push-fork string k/sig-release fork to push the updated branch to (omit to skip push) + --draft-pr-push-branch string branch to push to on the destination fork (default: same as source branch) +``` + +Inherited flags from the parent command (`--tag`, `--maps-from`, `--repo`, `--use-ssh`, `--update-repo`, `--include-labels`) are also available. + ### Usage notes You can run `--create-draft-pr` and `--create-website-pr` in the same invocation of krel. diff --git a/pkg/notes/notes.go b/pkg/notes/notes.go index 811a6d1a50e..922bde08a2b 100644 --- a/pkg/notes/notes.go +++ b/pkg/notes/notes.go @@ -1095,12 +1095,16 @@ func (rn *ReleaseNote) ApplyMap(noteMap *ReleaseNotesMap, markdownLinks bool) er rn.IsMapped = true - if noteMap.PRBody != nil && rn.PRBody != "" && rn.PRBody != *noteMap.PRBody { - logrus.Warnf("Original PR body of release note mapping changed for PR: #%d", rn.PrNumber) + if noteMap.PRBody != nil { + if rn.PRBody != "" && rn.PRBody != *noteMap.PRBody { + logrus.Warnf("Original PR body of release note mapping changed for PR: #%d", rn.PrNumber) - dmp := diffmatchpatch.New() - diffs := dmp.DiffMain(rn.PRBody, *noteMap.PRBody, false) - logrus.Warnf("The diff between actual release note body and mapped one is:\n%s", dmp.DiffPrettyText(diffs)) + dmp := diffmatchpatch.New() + diffs := dmp.DiffMain(rn.PRBody, *noteMap.PRBody, false) + logrus.Warnf("The diff between actual release note body and mapped one is:\n%s", dmp.DiffPrettyText(diffs)) + } + + rn.PRBody = *noteMap.PRBody } reRenderMarkdown := false @@ -1130,6 +1134,7 @@ func (rn *ReleaseNote) ApplyMap(noteMap *ReleaseNotesMap, markdownLinks bool) er if noteMap.ReleaseNote.SIGs != nil { rn.SIGs = *noteMap.ReleaseNote.SIGs + reRenderMarkdown = true } if noteMap.ReleaseNote.Feature != nil { @@ -1163,7 +1168,7 @@ func (rn *ReleaseNote) ApplyMap(noteMap *ReleaseNotesMap, markdownLinks bool) er indented, rn.PrNumber, rn.PrURL, rn.Author, rn.AuthorURL) } // Add sig labels to markdown - if len(rn.SIGs) > 1 { + if len(rn.SIGs) >= 1 { markdown = fmt.Sprintf("%s [%s]", markdown, prettifySIGList(rn.SIGs)) } // Uppercase the first character of the markdown to make it look uniform diff --git a/pkg/notes/notes_map.go b/pkg/notes/notes_map.go index 6a794390597..eb149ee5889 100644 --- a/pkg/notes/notes_map.go +++ b/pkg/notes/notes_map.go @@ -22,6 +22,7 @@ import ( "io" "os" "path/filepath" + "sync" "github.com/sirupsen/logrus" "go.yaml.in/yaml/v4" @@ -131,17 +132,20 @@ type ReleaseNotesDataField any // DirectoryMapProvider is a provider that gets maps from a directory. type DirectoryMapProvider struct { - Path string - Maps map[int][]*ReleaseNotesMap + Path string + Maps map[int][]*ReleaseNotesMap + once sync.Once + loadErr error } // GetMapsForPR get the release notes maps for a specific PR number. func (mp *DirectoryMapProvider) GetMapsForPR(pr int) (notesMap []*ReleaseNotesMap, err error) { - if mp.Maps == nil { - err := mp.readMaps() - if err != nil { - return nil, fmt.Errorf("while reading release notes maps: %w", err) - } + mp.once.Do(func() { + mp.loadErr = mp.readMaps() + }) + + if mp.loadErr != nil { + return nil, fmt.Errorf("while reading release notes maps: %w", mp.loadErr) } if notesMap, ok := mp.Maps[pr]; ok { diff --git a/pkg/notes/notes_map_test.go b/pkg/notes/notes_map_test.go index 5ad33fd0c2b..d680db8796f 100644 --- a/pkg/notes/notes_map_test.go +++ b/pkg/notes/notes_map_test.go @@ -17,6 +17,7 @@ limitations under the License. package notes import ( + "sync" "testing" "github.com/stretchr/testify/require" @@ -66,6 +67,43 @@ func TestGetMapsForPR(t *testing.T) { require.GreaterOrEqual(t, 4, len(maps)) } +func TestGetMapsForPR_Concurrent(t *testing.T) { + provider := &DirectoryMapProvider{Path: "maps/testdata"} + + const numGoroutines = 100 + + var wg sync.WaitGroup + + wg.Add(numGoroutines) + + results := make([][]*ReleaseNotesMap, numGoroutines) + errs := make([]error, numGoroutines) + + for i := range numGoroutines { + go func(idx int) { + defer wg.Done() + + results[idx], errs[idx] = provider.GetMapsForPR(95000) + }(i) + } + + wg.Wait() + + // All calls should succeed + for i, err := range errs { + require.NoError(t, err, "goroutine %d returned error", i) + } + + // All calls should return consistent results + for i := 1; i < numGoroutines; i++ { + require.Len(t, results[i], len(results[0]), + "goroutine %d returned different number of maps", i) + } + + // Maps should be populated (non-empty result for known PR) + require.NotEmpty(t, results[0], "expected non-empty maps for PR 95000") +} + func TestReleaseNotesMapIntegrity(t *testing.T) { maps, err := ParseReleaseNotesMap("maps/testdata/fullmap.yaml") require.NoError(t, err)