Ensure release-notes tool doesn't skip automated cherry picks

[xmudrii]

What type of PR is this?

/kind bug

What this PR does / why we need it:

Motivation

We've recently discovered a bug in the release-notes tool causing automated cherry picks to be skipped. This is because those PRs have an empty release note block, which is handled like the release note is present. This is not correct behavior, instead, those PRs should be skipped. Instead of those PRs, the release-notes tool should take the parent PR and its release note.

Problem explained

Before I start with the problem explanation, the following diagram shows our workflow:

flowchart TD
    A[release-notes] --> B[GatherReleaseNotes]
    B --> C[ListReleaseNotes]
    C --> D[gatherNotes]
    D --> E[notesForCommit]
    E --> F[prsFromCommit]
    F --> G[prsForCommitFromMessage]
    G --> H[prsNumForCommitFromMessage]

    H --> |slice of PR numbers| G
    G --> |slice of PRs| F
    F --> |slice of PRs| E
    E --> |SINGLE PR selected based  on heuristics| D
    D --> |slice of release notes for all PRs| C
    C --> |complete release notes for all PRs| B

Loading

The release-notes tool calls a bunch of functions to determine a release note for PRs. We first take a list of commits for which we need to determine PRs. Those list of commits is passed to gatherNotes which takes each commit and passes it to notesForCommit. After calling a few more functions (as shown in the diagram), we end up with a list of candidate PRs for that commit (prsNumForCommitFromMessage function). Those candidate PRs are the original PR, parent PR if it's a cherry-pick, and we have special handling for squashed commits.

Those candidate PRs are passed back to notesForCommit where we select a single PR out of those candidate PRs.

This is where the problem appears. This function takes the PR body and checks if it contains the release-note block OR the Does this PR introduce a user-facing change? text. However, we do NOT check if it's a valid release note. We pass that single PR to gatherNotes where we check if it's actually a valid release note. However, we pass only that one single PR, we don't pass other candidate PRs. If it turns out to be an invalid release note, we can't check other PRs because we discarded them.

That's not a problem for PRs targeting the master/main branch. However, that is a problem for cherry picks where we have an empty release note block and where we excpect to inherit release note from the parent PR. In this case, notesForCommit will think that the cherry pick has a valid release note (even if it's empty) and will discard the parent PR. This causes release notes to lose release note for that change/PR.

Fix explained

This PR introduces two major changes:

• notesForCommit checks if the release note is valid instead of just checking for the presence of the release-note block or Does this PR introduce a user-facing change? text
  • If the PR is supposed to be excluded (NONE/release-note-none), we return that PR because it might have a map with a release note. If it doesn't, it will be skipped and that's okay because it has NONE/release-note-none.
• noteExclusionFilters is updated so that the release note MUST contain NONE/NO/NA. Currently, it excludes PRs with empty release notes, but this seems wrong because a PR without a release note should be considered as invalid

Which issue(s) this PR fixes:

xref #2679
ListReleaseNotesV2 is affected as well, but not fixed by this PR. It'll be done in a follow up PR.

Special notes for your reviewer:

Release note for this PR might include more details, I can update it if needed.

Does this PR introduce a user-facing change?

Ensure release-notes tool doesn't skip automated cherry picks

/assign @puerco @jeremyrickard @cpanato @Verolop @palnabarun
cc @kubernetes/release-engineering

[xmudrii]

/hold
for discussion

[saschagrunert]

LGTM, this seems to fix a valid bug and simplifies the code. Great work!

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: saschagrunert, 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 [saschagrunert,xmudrii]

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

[xmudrii]

/hold cancel