Release notes tool generates duplicate release notes

[sbueringer]

Hey folks,
I was trying to use the release-notes tool to generate release notes for CR. I hit two issues:

1. It was necessary to specifcy branch because we don't create release branches before the release in CR. Is this actually necessary? Could we e.g. maybe fallback if the branch doesn't exist? What was the difference again between calculating the diff based on a release branch or main?

2. It generates duplicate notes

 $CAPI_HOME/bin/notes --release v0.18.0 --branch=main --previous-release-version=tags/v0.17.0 --repository kubernetes-sigs/controller-runtime --prefix-area-label=false --deprecation=false --add-kubernetes-version-support=false > release-notes.md
2024/04/23 13:58:11 Computing diff between v0.17.0 and heads/main
2024/04/23 13:58:11 Calling endpoint repos/kubernetes-sigs/controller-runtime/compare/v0.17.0...main?per_page=250&page=1&
2024/04/23 13:58:12 Total of 1 pages and 133 elements read
2024/04/23 13:58:12 Reading ref heads/main for upper limit
2024/04/23 13:58:12 Reading commit b74908fd50f01704785762e94e79484d35ef6814 for upper limit
2024/04/23 13:58:12 Listing PRs from 2024-01-16 07:21:39 +0000 UTC to 2024-04-23 09:29:40 +0000 UTC
2024/04/23 13:58:12 Calling endpoint search/issues?per_page=100&page=1&q=repo:kubernetes-sigs/controller-runtime+is:pr+is:merged+merged:2024-01-16T07:21:39Z..2024-04-23T09:29:40Z+base:main+base:main
2024/04/23 13:58:14 Total of 1 pages and 64 elements read
2024/04/23 13:58:14 Found 64 PRs in github
2024/04/23 13:58:14 63 PRs match the commits from the git diff
2024/04/23 13:58:14 Ignoring PR [:seedling: Bump actions/upload-artifact from 4.3.1 to 4.3.3 (#2794)]
2024/04/23 13:58:14 Ignoring PR [:seedling: Bump actions/checkout from 4.1.2 to 4.1.3 (#2793)]
2024/04/23 13:58:14 Ignoring PR [:seedling: Bump golang.org/x/sys from 0.18.0 to 0.19.0 (#2763)]
2024/04/23 13:58:14 Ignoring PR [:seedling: Bump github.com/onsi/ginkgo/v2 from 2.17.0 to 2.17.1 (#2731)]
2024/04/23 13:58:14 Ignoring PR [:seedling: Bump github.com/onsi/gomega from 1.31.1 to 1.32.0 (#2718)]
2024/04/23 13:58:14 Ignoring PR [:seedling: Bump github.com/onsi/ginkgo/v2 from 2.16.0 to 2.17.0 (#2716)]
2024/04/23 13:58:14 Ignoring PR [:seedling: Bump github.com/onsi/ginkgo/v2 from 2.15.0 to 2.16.0 (#2704)]
2024/04/23 13:58:14 Ignoring PR [:seedling: Bump golang.org/x/sys from 0.17.0 to 0.18.0 (#2700)]
2024/04/23 13:58:14 Ignoring PR [:seedling: Bump github.com/prometheus/client_golang from 1.18.0 to 1.19.0 (#2699)]
2024/04/23 13:58:14 Ignoring PR [:seedling: Bump go.uber.org/zap from 1.26.0 to 1.27.0 (#2696)]
2024/04/23 13:58:14 Ignoring PR [:seedling: Bump github.com/prometheus/client_model from 0.5.0 to 0.6.0 (#2691)]
2024/04/23 13:58:14 Ignoring PR [:seedling: Bump golangci/golangci-lint-action from 3 to 4 (#2683)]
2024/04/23 13:58:14 Ignoring PR [:seedling: Bump golang.org/x/sys from 0.16.0 to 0.17.0 (#2682)]
2024/04/23 13:58:14 Ignoring PR [:seedling: Bump EndBug/add-and-commit from 9.1.3 to 9.1.4 (#2667)]
2024/04/23 13:58:14 Ignoring PR [:seedling: Bump github.com/evanphx/json-patch/v5 from 5.8.1 to 5.9.0 (#2666)]
2024/04/23 13:58:14 Ignoring PR [:seedling: Bump k8s.io/klog/v2 from 2.120.0 to 2.120.1 (#2661)]
2024/04/23 13:58:14 Ignoring PR [:seedling: Bump github.com/evanphx/json-patch/v5 from 5.8.0 to 5.8.1 (#2658)]
2024/04/23 13:58:14 Ignoring PR [:seedling: Bump github.com/onsi/gomega from 1.30.0 to 1.31.1 (#2657)]
2024/04/23 13:58:14 Ignoring PR [:seedling: Bump k8s.io/klog/v2 from 2.110.1 to 2.120.0 (#2653)]
INFO[0003] Setting up repository github.com/kubernetes-sigs/controller-runtime
INFO[0003] Retrieving modules of v0.17.0
INFO[0004] Retrieving modules of main
INFO[0015] 173 modules found
INFO[0015] 4 modules added
INFO[0015] 28 modules changed
INFO[0015] 3 modules removed
2024/04/23 13:58:26 Computing diff between tags/v0.17.0 and heads/main
2024/04/23 13:58:26 Calling endpoint repos/kubernetes-sigs/controller-runtime/compare/v0.17.0...main?per_page=250&page=1&
2024/04/23 13:58:27 Total of 1 pages and 133 elements read
2024/04/23 13:58:27 Reading ref heads/main for upper limit
2024/04/23 13:58:28 Reading commit b74908fd50f01704785762e94e79484d35ef6814 for upper limit
2024/04/23 13:58:28 Listing PRs from 2024-01-16 07:21:39 +0000 UTC to 2024-04-23 09:29:40 +0000 UTC
2024/04/23 13:58:28 Calling endpoint search/issues?per_page=100&page=1&q=repo:kubernetes-sigs/controller-runtime+is:pr+is:merged+merged:2024-01-16T07:21:39Z..2024-04-23T09:29:40Z+base:main+base:main
2024/04/23 13:58:29 Total of 1 pages and 64 elements read
2024/04/23 13:58:29 Found 64 PRs in github
2024/04/23 13:58:29 63 PRs match the commits from the git diff
2024/04/23 13:58:29 Ignoring PR [:seedling: Bump actions/upload-artifact from 4.3.1 to 4.3.3 (#2794)]
2024/04/23 13:58:29 Ignoring PR [:seedling: Bump actions/checkout from 4.1.2 to 4.1.3 (#2793)]
2024/04/23 13:58:29 Ignoring PR [:seedling: Bump golang.org/x/sys from 0.18.0 to 0.19.0 (#2763)]
2024/04/23 13:58:29 Ignoring PR [:seedling: Bump github.com/onsi/ginkgo/v2 from 2.17.0 to 2.17.1 (#2731)]
2024/04/23 13:58:29 Ignoring PR [:seedling: Bump github.com/onsi/gomega from 1.31.1 to 1.32.0 (#2718)]
2024/04/23 13:58:29 Ignoring PR [:seedling: Bump github.com/onsi/ginkgo/v2 from 2.16.0 to 2.17.0 (#2716)]
2024/04/23 13:58:29 Ignoring PR [:seedling: Bump github.com/onsi/ginkgo/v2 from 2.15.0 to 2.16.0 (#2704)]
2024/04/23 13:58:29 Ignoring PR [:seedling: Bump golang.org/x/sys from 0.17.0 to 0.18.0 (#2700)]
2024/04/23 13:58:29 Ignoring PR [:seedling: Bump github.com/prometheus/client_golang from 1.18.0 to 1.19.0 (#2699)]
2024/04/23 13:58:29 Ignoring PR [:seedling: Bump go.uber.org/zap from 1.26.0 to 1.27.0 (#2696)]
2024/04/23 13:58:29 Ignoring PR [:seedling: Bump github.com/prometheus/client_model from 0.5.0 to 0.6.0 (#2691)]
2024/04/23 13:58:29 Ignoring PR [:seedling: Bump golangci/golangci-lint-action from 3 to 4 (#2683)]
2024/04/23 13:58:29 Ignoring PR [:seedling: Bump golang.org/x/sys from 0.16.0 to 0.17.0 (#2682)]
2024/04/23 13:58:29 Ignoring PR [:seedling: Bump EndBug/add-and-commit from 9.1.3 to 9.1.4 (#2667)]
2024/04/23 13:58:29 Ignoring PR [:seedling: Bump github.com/evanphx/json-patch/v5 from 5.8.1 to 5.9.0 (#2666)]
2024/04/23 13:58:29 Ignoring PR [:seedling: Bump k8s.io/klog/v2 from 2.120.0 to 2.120.1 (#2661)]
2024/04/23 13:58:29 Ignoring PR [:seedling: Bump github.com/evanphx/json-patch/v5 from 5.8.0 to 5.8.1 (#2658)]
2024/04/23 13:58:29 Ignoring PR [:seedling: Bump github.com/onsi/gomega from 1.30.0 to 1.31.1 (#2657)]
2024/04/23 13:58:29 Ignoring PR [:seedling: Bump k8s.io/klog/v2 from 2.110.1 to 2.120.0 (#2653)]
INFO[0018] Setting up repository github.com/kubernetes-sigs/controller-runtime
INFO[0018] Retrieving modules of v0.17.0
INFO[0019] Retrieving modules of main
INFO[0025] 173 modules found
INFO[0025] 4 modules added
INFO[0025] 28 modules changed

[k8s-ci-robot]

This issue is currently awaiting triage.

CAPI contributors will take a look as soon as possible, apply one of the triage/* labels and provide further guidance.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[sbueringer]

cc @g-gaston In case you have some context :)

[fabriziopandini]

/priority important-soon

[g-gaston]

@sbueringer how many PR entries you get in the markdown output?

[chrischdi]

Dropping the --previous-release-version flag prevents the duplicated output. However, maybe the tool could auto-detect if both would be the same.

[sbueringer]

Sorry should have attached this. Too much :)

release-notes.md

[g-gaston]

hey yall sorry it took me a bit
Looking at the code, I believe this duplication is related to the new functionality added to print notes for pre-releases: #10091

Although I understand how the notes should look for pre-releases (and it's true that two sets of notes need to be generated), I do lack context about this feature and new code. I'm not sure what --previous-release-version is for.

Maybe @chandankumar4 or @kubernetes-sigs/cluster-api-release-team may know something?

[willie-yao]

Hmm that's weird, it definitely is related to #10091 where it prints the changes from the previous release version if it's a beta or rc release, then prints the changes since the last minor release in a dropdown. --previous-release-version is for the first part of the notes to print the changes since the last beta or rc release.

In your case, --previous-release-version is not needed since it's not a beta or rc release. This should probably be specified in the description for the flag.

[willie-yao]

#10324 added logic to detect the release type: https://github.com/kubernetes-sigs/cluster-api/pull/10324/files#diff-42e246c09fe01942779ff9af547e932b606a68a6f692fe137b254e4a9a10da78R104

Maybe we can prevent --previous-release-version to be set if the release type is not beta or rc to prevent confusion.

[sbueringer]

Ah okay. Seems to work when I just don't specificy --previous-release-version.

In the case I hit above it didn't add any details tag, it just ~ printed the entire release notes twice

Ah, seems like that's because the release type is an empty string in my case.

Maybe we can prevent --previous-release-version to be set if the release type is not beta or rc to prevent confusion.

I guess that makes sense

I wonder why we only add details tags if previousRelease is also empty?

cluster-api/hack/tools/release/notes/print.go

Line 86 in 18f4d01

if p.releaseType != "" && previousReleaseRef.value == "" {

Maybe that flags is defined in a weird way to be honest. The flag seem to indirectly influence behavior based on the cases when we use the flag? Or maybe I"m just missing why details are only added if previous release is not set

[willie-yao]

I'm not sure how the check works, it may have been added because of this comment about the e2e test: #10091 (comment)

cc @chandankumar4 the current comms lead. I think there can be an improvement task for the comms team for simplifying the logic of --previous-release-version or finding a way to automatically detect it based on the beta/rc version. wdyt?

[chandankumar4]

Yeah, I agree!! I’ll open up an issue for simplifying the logic of “previous-release-version”.