Skip to content
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

feat(helm): remove "v" prefix from chart version #565

Merged
merged 3 commits into from
Dec 1, 2023
Merged

feat(helm): remove "v" prefix from chart version #565

merged 3 commits into from
Dec 1, 2023

Conversation

kranurag7
Copy link
Contributor

@kranurag7 kranurag7 commented Nov 18, 2023

This commit updates the chart version of hcloud-cloud-controller-manager We only need to update the version in Chart.yaml because goreleaser already strips the v from {{ .Version }}

To patch the extra v with image tag, I updated the template to use tag: 'v{{ $.Chart.Version }}'

Ref: https://goreleaser.com/customization/release/#github
Above, the name template looks something like this.

name_template: "{{.ProjectName}}-v{{.Version}} {{.Env.USER}}"

On packer-plugin-hcloud we manually include v in the binary name.
https://github.com/syself/packer-plugin-hcloud/blob/7a934739f43c36e2818d1816f22d388231e2eac1/.goreleaser.yml#L28

Release Assets for packer-plugin-hcloud

So, IMHO, we don't need to make any change in .goreleaser.yaml
For testing and Ref:
I created a snapshot release locally using goreleaser release --snapshot and the diff updates the chart version and image tag as per the requirement described in the issue.

Attached Screenshot

image

Fixes #529

this commit updates the chart version of hcloud-cloud-controller-manager
We only need to update the version in Chart.yaml because goreleaser
already strips the `v` from {{ .Version }}

Signed-off-by: kranurag7 <81210977+kranurag7@users.noreply.github.com>
@kranurag7 kranurag7 requested a review from a team as a code owner November 18, 2023 20:20
@apricote apricote self-assigned this Nov 20, 2023
@apricote apricote added the enhancement New feature or request label Nov 20, 2023
Copy link
Member

@apricote apricote left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @kranurag7,

thanks for the PR! I think we do need to make some changes in the goreleaser configuration, because we explicitly added the v back into the version when calling scripts/release-generate-deployment-yamls.sh and ./scripts/publish-helm-chart.sh. See my comment #529 (comment)

The first script actually overwrites the version in Chart.yaml to be whatever the script was called with, currently including the v:

sed -e "s/version: .*/version: $VERSION/" --in-place chart/Chart.yaml

I also think this is worth pointing out in the release notes, so I will update the title of this PR to feat(helm): remove "v" prefix from chart version.

@apricote apricote changed the title chore: use semver with helm chart feat(helm): remove "v" prefix from chart version Nov 20, 2023
@apricote
Copy link
Member

Hey @kranurag7, do you have some time to work on this within the next week? Otherwise I would implement the fixes to make sure this will be included in the next version.

@kranurag7
Copy link
Contributor Author

This PR was always in back of my head but I was little busy. I'll for sure push the changes tomorrow.

Signed-off-by: kranurag7 <81210977+kranurag7@users.noreply.github.com>
@kranurag7
Copy link
Contributor Author

@apricote I tried, this today, but I was not able to test this entirely on my machine (probably my issues with Docker buildx build). I just removed things as per your issue feedback and some minor tinkering with the scripts.
If you intend to create a release soon, then please feel free to add additional commits and proceed with the release.
I'm just one ping away if there's anything where I can be helpful.

@apricote
Copy link
Member

apricote commented Dec 1, 2023

Yea, running the goreleaser config outside of CI (and without actually publishing something) is a PITA :(

Changes look good, i will push one more cleanup commit and then do a release-candidate to verify its working 🚀

Thanks for the patches!

@apricote apricote merged commit f11aa0d into hetznercloud:main Dec 1, 2023
3 of 8 checks passed
@kranurag7 kranurag7 deleted the kr/helm-semver branch December 1, 2023 11:08
apricote pushed a commit that referenced this pull request Dec 1, 2023
🤖 I have created a release *beep* *boop*
---


##
[1.19.0-rc.0](v1.18.0...v1.19.0-rc.0)
(2023-12-01)


### Features

* **chart:** add daemonset and node selector
([#537](#537))
([a94384f](a94384f))
* **config:** stricter validation for settings
`HCLOUD_LOAD_BALANCERS_ENABLED`, `HCLOUD_METRICS_ENABLED` &
`HCLOUD_NETWORK_ROUTES_ENABLED`
([#546](#546))
([335a2c9](335a2c9))
* **helm:** remove "v" prefix from chart version
([#565](#565))
([f11aa0d](f11aa0d)),
closes
[#529](#529)
* **load-balancer:** handle planned targets exceedings max targets
([#570](#570))
([8bb131f](8bb131f))
* remove unused variable NODE_NAME
([#545](#545))
([a659408](a659408))
* **robot:** handle ratelimiting with constant backoff
([#572](#572))
([2ddc201](2ddc201))
* support for Robot servers
([#561](#561))
([65dea11](65dea11))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).
@apricote
Copy link
Member

apricote commented Dec 1, 2023

Building the Helm chart with the new version was successful :)

➜ helm search repo hcloud --versions --devel | grep 1.19
hcloud/hcloud-cloud-controller-manager  1.19.0-rc.0

apricote added a commit that referenced this pull request Dec 7, 2023
## [1.19.0](v1.18.0...v1.19.0)
(2023-12-07)


### Features

* **chart:** add daemonset and node selector
([#537](#537))
([a94384f](a94384f))
* **config:** stricter validation for settings
`HCLOUD_LOAD_BALANCERS_ENABLED`, `HCLOUD_METRICS_ENABLED` &
`HCLOUD_NETWORK_ROUTES_ENABLED`
([#546](#546))
([335a2c9](335a2c9))
* **helm:** remove "v" prefix from chart version
([#565](#565))
([f11aa0d](f11aa0d)),
closes
[#529](#529)
* **load-balancer:** handle planned targets exceedings max targets
([#570](#570))
([8bb131f](8bb131f))
* remove unused variable NODE_NAME
([#545](#545))
([a659408](a659408))
* **robot:** handle ratelimiting with constant backoff
([#572](#572))
([2ddc201](2ddc201))
* support for Robot servers
([#561](#561))
([65dea11](65dea11))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[bug] helm chart version is not semver compliant
2 participants