-
Notifications
You must be signed in to change notification settings - Fork 2.7k
chore(release): chart for v0.17.0 #5479
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
chore(release): chart for v0.17.0 #5479
Conversation
029566b
to
4edecd7
Compare
/assign @szuecs |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried to run the step Run CHANGELOG check locally, with latest GH version, and it gave me this changes list:
- kind: changed
description: "Allow extraArgs to also be a map enabling overrides of individual values. ([#5293](https://github.com/kubernetes-sigs/external-dns/pull/5293)) _@frittentheke"
- kind: changed
description: " -----"
- kind: changed
description: "Update CRD. ([#5287](https://github.com/kubernetes-sigs/external-dns/pull/5287)) _@mloiseleur_"
- kind: changed
description: " -----"
- kind: changed
description: "Update CRD. ([#5446](https://github.com/kubernetes-sigs/external-dns/pull/5446)) _@mloiseleur_"
- kind: changed
description: " -----"
- kind: changed
description: "Update _ExternalDNS_ OCI image version to [v0.17.0](https://github.com/kubernetes-sigs/external-dns/releases/tag/v0.17.0). ([#5479](https://github.com/kubernetes-sigs/external-dns/pull/5479)) _@stevehipwell_"
- kind: fixed
description: "Fix wrong type definitions for webhook probes. ([#5297](https://github.com/kubernetes-sigs/external-dns/pull/5297)) _@semnell_"
Wdyt of
- Transform this step into a bash script commited into the repository (in
scripts/
) - Adapt this script so the computed changelog can be reviewed
4edecd7
to
ebad3b7
Compare
24d392e
to
5f157a6
Compare
@mloiseleur the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changelog LGTM.
I'm not sure to understand why this PR is modifying the CRD ?
Unless I missed something, it's not the purpose of this PR.
The change does not seem to be breaking, but it will be replaced each time we run make crd
.
@mloiseleur the PR is reverting the changes |
@szuecs could you please review this? |
Ok. Thanks, that's what I thought 👍. When #5372 will be ready, we will have 2 CRDs. Unless I'm missing something, it will then revert this revert. So, my question is: Would you please explain your motivation ? |
@mloiseleur when you copy the CRDs the unwanted annotations can be removed and the YAML files can be formatted with yamlfmt. This is how I was processing the CRDs before you added the automation and I'm pretty sure I added a comment to the PR where you automated this. TL;DR - The make script should be as follows but will need both #? crd: Generates CRD using controller-gen and copy it into chart
.PHONY: crd
crd: controller-gen-install
${CONTROLLER_GEN} object crd:crdVersions=v1 paths="./endpoint/..."
${CONTROLLER_GEN} object crd:crdVersions=v1 paths="./apis/..." output:crd:stdout > ./config/crd/standard/dnsendpoint.yaml
yq eval '.metadata.annotations |= with_entries(select(.key | test("kubernetes\.io")))' ./config/crd/standard/dnsendpoint.yaml | yamlfmt - > ./charts/external-dns/crds/dnsendpoint.yaml |
2654e56
to
e0b93ac
Compare
@mloiseleur RE the schema it looks like #5510 passed before v2.0.0 was released. I've updated the config for this so we should be all good. |
e0b93ac
to
f011bca
Compare
f011bca
to
3d8748f
Compare
@mloiseleur I've also updated the CRD copy logic to match the comments I made on other PRs. |
Thanks 👍 . That should help and avoid back-and-forth between PRs on CRDs. The change makes sense to me. I somehow understand that your motivation for yamlfmt: even if the yaml file is valid, you prefer to have it as clean and formatted the same way as all the other files in the Chart. I have not yet understand your motivation for removing
|
3d8748f
to
e1d1da1
Compare
Good spot @mloiseleur!
I'm fully aware of this, but neither of those goals are related to the Helm chart where the CRD is (or should be) a duplicate. The CRD in the Helm chart shouldn't be being used for debugging; firstly because it makes no logical sense, and secondly because Helm doesn't update CRDs (so the annotation wouldn't be being updated anyway). As the chart CRD should be a duplicate, from the goal perspective the annotation is at best neutral but potentially complicates the signals. Finally from a Helm and end-user perspective the annotation is meaningless but if the chart CRD is being used as a source of truth (as we use it) an annotation update can cause unnecessary churn. |
/retest |
Thanks for this detailed explanation.
Unless I'm missing something, it's not what this PR is currently doing with the Makefile. I tested the code update in the Makefile: it creates and regenerate CRDs only for the chart. There is no file created or updated in |
e1d1da1
to
992ab55
Compare
Thanks again @mloiseleur, I was rushing. This now updates the CRDs in the config/crd/standard directory and is compatible with multiple CRDs. It also means that any GitHub query for the annotations can use the file name for the full CRD name. |
Signed-off-by: Steve Hipwell <[email protected]>
992ab55
to
c79003b
Compare
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: mloiseleur 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:
Approvers can indicate their approval by writing |
* master: chore: Release chart for v0.17.0 (kubernetes-sigs#5479)
@stevehipwell it looks like this release didn't go through:
|
@jasonslay #5533 should release the chart. |
What does it do ?
This PR updates the Helm chart for the
v0.17.0
release. It also adds automation to auto-generate the chart changelog annotations to simplify the release process.Closes #5398
Motivation
More