search

kubernetes-sigs / cluster-api

Home for Cluster API, a subproject of sig-cluster-lifecycle
https://cluster-api.sigs.k8s.io
Apache License 2.0
3.48k stars 1.29k forks source link

Release notes tool generates duplicate release notes #10492

Open sbueringer opened 4 months ago

sbueringer commented 4 months ago

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 commented 4 months ago

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](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.
sbueringer commented 4 months ago

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

fabriziopandini commented 4 months ago

/priority important-soon

g-gaston commented 4 months ago

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

chrischdi commented 4 months ago

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

sbueringer commented 4 months ago

Sorry should have attached this. Too much :)

release-notes.md

g-gaston commented 3 months ago

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: https://github.com/kubernetes-sigs/cluster-api/pull/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 commented 3 months ago

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 commented 3 months ago

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 commented 3 months ago

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? https://github.com/kubernetes-sigs/cluster-api/blob/18f4d015ae44857941d60759b8b35ba7b85f7c5b/hack/tools/release/notes/print.go#L86

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 commented 3 months ago

I'm not sure how the check works, it may have been added because of this comment about the e2e test: https://github.com/kubernetes-sigs/cluster-api/pull/10091#issuecomment-1986175255

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 commented 3 months ago

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

chandankumar4 commented 2 months ago

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

Maybe I don't need to open up a separate issue for it, i'll take a look while working on this bug.