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

Adding option to skip existing releases #111

Merged
merged 3 commits into from
Mar 15, 2021
Merged

Adding option to skip existing releases #111

merged 3 commits into from
Mar 15, 2021

Conversation

eleduardo
Copy link
Contributor

I just ran into bug #65 with the exact same use case that the author of the bug reported. I have a repository for all the charts in my organization what I want is for a CI pipeline to automatically push the charts that have changed. I am trying to keep the CI logic simple and cr is helping me a lot but I do have the issue that not all charts are changing at the same time. What I would like is to be able to tell cr to skip charts that have a name/tag that is already released (a warning can be added).

This seems to be simple, I just added a "skip" flag and if a release already exists it just skips it.

I did not add a test yet since I would like feedback on it before moving forward. Any thoughts on this fix?

thank you for CR!!

Eduardo Solis added 2 commits March 12, 2021 15:51
Signed-off-by: Eduardo Solis <eduardo.solis@udemy.com>
Signed-off-by: Eduardo Solis <eduardo.solis@udemy.com>
Copy link
Member

@cpanato cpanato left a comment

Choose a reason for hiding this comment

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

IMHO I think this is ok to have the user can control that part.
LGTM but I would like to hear from the senior maintainer @unguiculus

@cpanato cpanato added the enhancement New feature or request label Mar 14, 2021
Copy link
Member

@davidkarlsen davidkarlsen left a comment

Choose a reason for hiding this comment

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

lgtm


if r.config.SkipExisting {
existingRelease, _ := r.github.GetRelease(context.TODO(), releaseName)
if existingRelease != nil && len(existingRelease.Assets) > 0 {
Copy link
Member

Choose a reason for hiding this comment

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

I wonder if len(existingRelease.Assets) > 0 makes sense. I'd tend to remove that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@unguiculus thank you! removed the check

Signed-off-by: Eduardo Solis <eduardo.solis@udemy.com>
@unguiculus unguiculus merged commit 30f8b9d into helm:master Mar 15, 2021
@eleduardo eleduardo deleted the lenientupload branch March 17, 2021 14:55
@thesuperzapper
Copy link

We should probably reference that the CR_SKIP_EXISTING config exists in the README, otherwise people wont get to use this very important feature.

alokjani added a commit to alokjani/chart-releaser that referenced this pull request Sep 26, 2021
Adds reference for using --skip-existing and CR_SKIP_EXISTING
alokjani added a commit to alokjani/chart-releaser that referenced this pull request Sep 26, 2021
Adds reference for using --skip-existing and CR_SKIP_EXISTING
@alokjani
Copy link
Contributor

Updated reference to this in the README in #136 @thesuperzapper

@eleduardo thanks for the feature !

alokjani added a commit to alokjani/chart-releaser that referenced this pull request Sep 26, 2021
Adds reference for using --skip-existing and CR_SKIP_EXISTING

Signed-off-by: Alok Jani <alokjani.web@gmail.com>
davidkarlsen pushed a commit that referenced this pull request Sep 28, 2021
Adds reference for using --skip-existing and CR_SKIP_EXISTING

Signed-off-by: Alok Jani <alokjani.web@gmail.com>
StevenJDH added a commit to StevenJDH/helm-charts that referenced this pull request May 6, 2024
* Fixed 422 scenario in release workflow when commits not tied to a release are detected in other charts, helm/chart-releaser#111.
* Fixed helm docs rendering of ingress properties showing as json instead of individual values.
* Fixed extra whitespace in copy paste example in each template docs.
* Fixed missing template docs in test connection pod and notes templates.
* Fixed a typo in each template docs.
* Fixed job name not using correct template for naming convention.
* Fixed incorrect indentation for env field in job template, which was 14 instead of 12.
* Removed template.metadata.name field from job because it's not used by k8s, kubernetes/kubernetes#108977.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request size/XS
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants