Fix empty release note parsing

[npolshakova]

What type of PR is this?

/kind bug

What this PR does / why we need it:

Fixes the empty release note parsing when an empty or whitespace-only release note is left on the PR description.

Which issue(s) this PR fixes:

Fixes https://github.com/kubernetes/release/issues/3516

Special notes for your reviewer:

Release notes collection automation has resulted in incorrect text and markdown being collected: https://github.com/kubernetes/sig-release/pull/2449#discussion_r1521495247

You can see examples of this merged in sig-release for 1.30 due to past release notes collection:

• PR 123082
• PR 121413
• PR 122893

Updated examples from 1.29 that are still in the repo after 1.30 was cleaned up:

• https://github.com/kubernetes/kubernetes/pull/119394, release notes here.

Does this PR introduce a user-facing change?

NONE

[cpanato]

Thanks!!

[npolshakova]

Thanks! Do you mind looking into the failing integration tests?

Okay, so it's failing the integration test because now we're exiting early here if any PRs match the exclusion regex:

// If we match exclusion filter (release-note-none), we don't look further,
		// instead we return that PR. We return that PR because it might have a map,
		// which is checked after this function returns
		if MatchesExcludeFilter(prBody) {
			res := &Result{commit: commit, pullRequest: pr}
			logrus.Infof("PR #%d contains exclusion (release-note-none)", pr.GetNumber())

			return res, nil
		}

There are a couple options:

1. Continue checking all prs, skipping the ones that contain the exclusion regex. The empty release note would still be in the exclusion regex.
2. Treat the explicit exclusion regex separately from the empty release note. Setting the /release-notes-none or NONE/N/A in the release note section would mean all PRs don't have a release note. An empty release note is ignored only if all PRs don't have a release note. We're already kind of doing this by checking noteTextFromString is not empty. This is the approach https://github.com/kubernetes/release/pull/3764/commits/fe7fa2eea19906aff55247c9a2efe6fd07bdd94f has.

Multiple PRs happen in cherry-pick cases like here:

• https://github.com/kubernetes/kubernetes/pull/113425
• https://github.com/kubernetes/kubernetes/pull/112693

[xmudrii]

I think this looks solid, but I'd like to bring a topic up for this discussion. We originally allowed cherry-picks to have an empty release note, in which case the release note would be inherited from the original PR. However, we had some troubles in the past with that, e.g. https://github.com/kubernetes/release/pull/2689 (this was quite a journey as can be told from the PR description).

With the risk that I lack context, I would propose that we stop doing this and explicitly forbid empty release notes, even for cherry-picks.

It has two huge benefits from my point of view:

• We'll be able to drop this edge case, which would make our code easier to maintain and understand
• It'll make it easier for end users to figure out what's changed in the cherry-pick PR without having to search for the original PR

I'm going to tag leads on this PR (@kubernetes/sig-release-leads) so that they can provide 2 cents on this. Please correct me if I'm wrong or if I'm lacking context about this.

About this PR, I'll go ahead and lgtm it, but I'll leave the hold for other folks to take a look. We can move this conversation to a new GitHub issue if needed.

[npolshakova]

@kubernetes/sig-release-leads @xmudrii Can you give this another pass when you get a chance? Thanks!

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cpanato, npolshakova, xmudrii

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• ~~pkg/notes/OWNERS~~ [cpanato,xmudrii]

Approvers can indicate their approval by writing /approve in a comment Approvers can cancel approval by writing /approve cancel in a comment