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 #3516

Special notes for your reviewer:

Release notes collection automation has resulted in incorrect text and markdown being collected: kubernetes/sig-release#2449 (comment)

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:

• Fix stale conntrack flow detection logic kubernetes#119394, release notes here.

Does this PR introduce a user-facing change?

NONE

[cpanato]

/lgtm

/hold for extra eyes

[cpanato]

Thanks!!

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

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

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]

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

[saschagrunert]

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

[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 fe7fa2e has.

Multiple PRs happen in cherry-pick cases like here:

• Automated cherry pick of #112693: Fixed (CVE-2022-27664) Bump golang.org/x/net to kubernetes#113425
• Fixed CVE-2022-27664 Bump golang.org/x/net to v0.1.1-0.20221027164007-c63010009c80 kubernetes#112693

[k8s-ci-robot]

New changes are detected. LGTM label has been removed.

[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. #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.

[xmudrii]

I found a nit, but otherwise it looks good