fix(helm): allow a previously failed release to be upgraded #7653
Conversation
helm-bot
added
the
size/M
yinzara
force-pushed
the
feature/upgrade-replace
branch
2 times, most recently
from
bfd7b96 to
dcd08cd
Compare
|
Taking a quick glance at the feature flag description...
If it's unsafe for production use, why are we advocating for others to use this? I ask this because when we discuss introducing new features to Helm, we aim to implement features that can be used safely in production. If there are features unsafe for production use, they are temporarily hidden behind a feature gate until the feature is considered stable. Why should we ship this? |
|
I only added the warning because it also exists on the original replace flag. I honestly don't believe it's "unsafe for production". I would more say it's smarter to only use this flag when necessary after you have determined ignoring the "no deployed releases" error is safe (i.e. you intended to upgrade a failed release). I'm happy to reword. Right now there is no way to recover from a failed release with no prior successful deployments other than removing that release entirely (which may not be what you want or may be difficult). Maybe "This flag is not recommended for use in an automated production deployment workflow. It is best used to correct issues in failed production deployments after being verified by an engineer or for development workflows that tolerate outages" is a better description but I feel probably too wordy :) I will use "--replace" in my development automated build process but will use "--atomic" in production and let me explain why. It's very common in our automated build system for a new microservice to fail its first deployment (because of a mistake by the dev or otherwise). We would like this solved by pushing another commit that fixes the deployment issue. Right now, without the "--replace" flag, we have to manually clean up every release if they failed on the first deploy (the main cause of pain outlined in #5595 ). I would like it for our dev builds, that if a release fails, it remains in that state so I can debug what occurred with it but if I push a new commit that fixes the failure, that the deploy succeeds. However in production, if a release fails, I would like that release to rollback so that my production environment remains pristine. I'd like to know the release failed but I want my systems to remain in their prior-to-rollout state if that failure occurs. Without this flag, there is no way to allow the former scenario and why so many have reported the issue in #5595 . |
yinzara
force-pushed
the
feature/upgrade-replace
branch
2 times, most recently
from
e04fb9e to
dcd08cd
Compare
|
@bacongobbler Any chance we could get some love here (and on #7668 if you're cool with this PR) |
|
I’m busy with internal work this week. Won’t be able to review anything for a little while. Sorry! |
|
I friend called out that this PR seems similar to #7431 I do not believe that #7431 solves the issue that "--replace" does and I believe "--replace" follows the spirit of the same flag in the "install" command without introducing any new concepts and without violating the general process flow of the current helm upgrade nor introducing a lot of code changes. |
|
@yinzara does this flag only replaces the failed release or also the resources within the cluster? |
|
This flag acts as if the most recent failed release (if there are no successful ones) was successful. i.e. any resources that it should be deployed will be synchronized with the rendered chart. So let's say Chart V1 had resource A, B, C and Chart V2 had resource C, D and E. You install Chart V1 by using "helm upgrade --install --replace release-name chart-name ..." and it fails however A and C were installed correctly and B failed to install. You then update the source to V2 using the same command which is missing resources A and B but has new resources D and E. Helm will act as if the first release was successful and assume A, B and C were all installed. It will attempt to delete resources A and C, upgrade B to match V2's config, and install D and E with V2s config. Resource A will be deleted as it was successfully created in V1. C will attempt delete but will fail and print a warning to the log that helm attempted to delete a non-existent resource. Helm will patch B and install D and E with the config from V2. Maintaining existing functionality, if resource E was already found in the cluster when running the upgrade command on V2, helm will error indicating there was a resource not managed by the chart that already existed in the cluster. |
|
@yinzara it's not very straight forward to understand this behaviour. How would you describe in in the docs? |
|
I'm confused why that behavior is not straightforward. It's literally how Helm always works in a "helm upgrade" scenario even without the "--replace" flag. The only thing the replace flag does is in the case where you have no successful releases, it allows the most recent failed release to act as if it was successful. See the actual changed code in my PR, all it does is ignore an error. It does not actually change any functionality of how helm manages the upgrade. I also described above how I would use this in an automated build/release workflow and why I would use it over the "--atomic" flag. What else would make this easier to swallow? The documentation provided with the flag (as listed above is): |
|
Any thing else needed to get this merged? |
|
@bacongobbler |
|
Thank you for the ping. At this time, I'm focused on other work. I do not have enough time in my work schedule to review your code in the near future. That being said, there are plenty of other Helm maintainers who may have the time to review your work. I'd reach out and see if someone else is available to review this. Sorry for the inconvenience. |
|
Thanks man! I appreciate it anyway. How about @mattfarina ? |
pkg/action/upgrade.go
Outdated
| if err != nil { | ||
| // to keep existing behavior of returning the "%q has no deployed releases" error when an existing release does not exist | ||
| if strings.Contains(err.Error(), "no revision for release") { |
There was a problem hiding this comment.
Would be nice if this were more statically typed. Can errors.Is be used?
There was a problem hiding this comment.
The error is not a constant. It's a string built with the name of the release in it (hence why I used strings.Contains. Is there another method I don't know?
There was a problem hiding this comment.
My apologies, was not familiar with the codebase, it just looked incredibly.. Wrong..
But alas this seems to be the intended design throughout the codebase, so nothing wrong with it here.
|
@technosophos Can we get this merged? |
|
Can you describe the cases in which this is preferable to |
|
We had an edge-case where the new Helm upgrade, run by CD, using --atomic failed and it wanted to rollback, however the version it rolled back to failed as well. Helm retains a list of deployments with max size 10 it seems and uses it to rollback. The atomic action of install, rollback added 2 failed deployments to that list. At some point we messed up so badly that we failed 5 upgrades in a row, meaning 5*2 (due to rollbacks) failed deployments. So there were only failed deployments in the list. The result was that upgrade, even with atomic, no longer worked because there were no successful deployment in the history so Helm would early out. As I understand it, this change would alleviate such errors by doing the upgrade in spite of the lack of a successful deployment. |
|
@technosophos I will use "--replace" in my development automated build process but will use "--atomic" in production and let me explain why. It's very common in our automated build system for a new microservice to fail its first deployment (because of a mistake by the dev or otherwise). We would like this solved by pushing another commit that fixes the deployment issue. Right now, without the "--replace" flag, we have to manually clean up every release if they failed on the first deploy (the main cause of pain outlined in #5595 ). I would like it for our dev builds, that if a release fails, it remains in that state so I can debug what occurred with it but if I push a new commit that fixes the failure, that the deploy succeeds. However in production, if a release fails, I would like that release to rollback so that my production environment remains pristine. I'd like to know the release failed but I want my systems to remain in their prior-to-rollout state if that failure occurs. The '--replace' flag is necessary in the above use case as there is no other combination of options that solves this issue. Additionally, there is the case where a user has installed a large public helm chart (without --atomic) and is now in a state where the release only failed because of a timeout and not because of an actual error and without "helm upgrade --replace" there is no way to complete the installation of the chart without fully uninstalling the entire installation which most of the time is unnecessary. |
helm-bot
removed
the
size/M
|
for me, it is not even keeping good deployment. I had to roll back to the previous version I always run: helm upgrade --install REVISION UPDATED STATUS REVISION UPDATED STATUS |
|
@jawlitkp the feature was merged into rc2. Please update and try again. :) EDIT: oh, it looks like rc.2 doesn't exist yet. Regardless, the feature did not make it into rc.1, which would explain why it wouldn't work for you. |
|
@bacongobbler @jawlitkp The "git commit" from his version statement shows "1911870958098b774973c6fe56bfdf4441f61596" which is the right commit ID to have the "no deployed release" functionality. I'm not sure what's going on. |
|
Ok, I will just hold for now cuz I broke our ci/cd and had to revert back. |
|
It doesn't seem to work if the release is stuck in a |
|
It will not work for any
What we are open to is finding cases where Currently, for releases stuck in "pending" you must either manually restore (which is not something we provide guidance on) or you must delete and recreate. |
|
Thanks @technosophos ! Appreciate the insight. Our release ended up in a weird state I suppose: Somehow our build made it so we ended up with a bunch of failed releases and failed rollbacks likely because of a missing secret that because required for a container. Now during this whole time the pods from the last successful deploy continued running because of how we do our rolling-updates (surge by 100% to bring up new containers before old ones are taken down). Deleting and recreating a production app is risky and was something we only wanted to do as a last resort and as such we were looking for workarounds. I came across this issue: #7476 which inspired me to try something and it worked! What I did:
Documenting this here so it can help others / might help identify a scenario to improve helm further. |
|
Hi, is this released as part of |
|
@nareshnayini This will be in 3.2.1. |
|
I had to re-arrange the 3.2.0 release because of the security issue, and because RC.1 met the release criteria (so there was no need for an RC.2). So I bumped this to 3.2.1. |
hickeyma
added
the
picked
|
Is it possible to know, programmatically, that the deployment succeeded but that in actuality it deployed/kept the current release because the new release failed and thus this flag kicked the not action? |
* fix(install): use ca file for install (helm#7140) Fixes a few bugs related to tls config when installing charts: 1. When installing via relative path, tls config for the selected repository was not being set. 2. The `--ca-file` flag was not being passed when constructing the downloader. 3. Setting tls config was not checking for zero value in repo config, causing flag to get overwritten with empty string. There's still a few oddities here. I would expect that the flag passed in on the command line would override the repo config, but that's not currently the case. Also, we always set the cert, key and ca files as a trio, when they should be set individually depending on combination of flags / repo config. Signed-off-by: James McElwain <jmcelwain@gmail.com> * Repair failing unit tests - failure caused by os.Stat return values for directory size on Linux. Signed-off-by: Pavel Macík <pavel.macik@gmail.com> * Add corresponding unit test to the function in parser.go Signed-off-by: Guangwen Feng <fenggw-fnst@cn.fujitsu.com> * Add hpa boilerplate Signed-off-by: Naseem <naseem@transit.app> * Add unit test for Reverse() in pkg/releaseutil.go Signed-off-by: Dao Cong Tien <tiendc@vn.fujitsu.com> * Use /usr/bin/env for bash After this change, make works on nixos. Signed-off-by: Daniel Poelzleithner <git@poelzi.org> * fix(helm): sort hooks by kind for equal weight Use the same install order for hooks as for normal resources (non-hooks) for hooks with equal weight. This makes resource handling more consistent and helps, when there are hook consisting of several resources like e.g. a service account and a job using this service account. The sort functions are changed from an in place search to an out of place sort to avoid inout parameters. Closes helm#7416. Signed-off-by: Daniel Strobusch <1847260+dastrobu@users.noreply.github.com> * fixed dependencies processing in case of helm install or upgrade for disabled/enabled sub charts Signed-off-by: Florian Hopfensperger <f.hopfensperger@gmail.com> * fix(helm): Don't wait for service to be ready when external IP are set Resolves helm#7513 As the externalIPs are not managed by k8s (according to the doc: https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.17/#servicespec-v1-core) helm should not wait for services which set al least one externalIPs. Signed-off-by: Federico Bevione <f.bevione@cognitio.it> * ref(kind_sorter): use an in-place sort for sortManifestsByKind, reduce code duplication Signed-off-by: Matthew Fisher <matt.fisher@microsoft.com> * fix(helm): Reworded logs for clarity Signed-off-by: Federico Bevione <f.bevione@cognitio.it> * fixed missing bullet Signed-off-by: Matt Butcher <matt.butcher@microsoft.com> * Fix 'helm template' to also print invalid yaml Signed-off-by: Reinhard Naegele <unguiculus@gmail.com> * fix(helm): improved logs Signed-off-by: Federico Bevione <f.bevione@cognitio.it> * fix(kube): use non global Scheme to convert But instead use a newly initialized Scheme with only Kubernetes native resources added. This ensures the 3-way-merge patch strategy is not accidentally chosen for custom resources due to them being added to the global Scheme by e.g. versioned clients while using Helm as a package, and not a self-contained binary. Signed-off-by: Hidde Beydals <hello@hidde.co> * fix(kube): generate k8s native scheme only once Signed-off-by: Hidde Beydals <hello@hidde.co> * Place rendering invalid YAML under --debug flag Signed-off-by: Reinhard Naegele <unguiculus@gmail.com> * Pass kube user token via cli, introduce HELM_KUBETOKEN envvar Signed-off-by: Vibhav Bobade <vibhav.bobde@gmail.com> * Making fetch-dist get the sha256sum files Fetching these files is part of the release process. When the new file type was added this step was missed. It will cause the sign make target to fail. Signed-off-by: Matt Farina <matt@mattfarina.com> * ref(go.mod): k8s api 0.17.3 Signed-off-by: Rui Chen <chenrui333@gmail.com> * IsReachable() needs to give detailed error message. Signed-off-by: ylvmw <yngliu@vmware.com> * pkg/gates: add unit test for String Signed-off-by: Zhou Hao <zhouhao@cn.fujitsu.com> * cmd/helm/search_repo: print info to stderr Signed-off-by: Zhou Hao <zhouhao@cn.fujitsu.com> * fix golint failure in pkg/action Signed-off-by: Song Shukun <song.shukun@fujitsu.com> * pkg/helmpath: fix unit test for Windows Signed-off-by: Song Shukun <song.shukun@fujitsu.com> * Adds script to help craft release notes Signed-off-by: Paul Czarkowski <username.taken@gmail.com> * add license headers to release-notes.sh script Signed-off-by: Paul Czarkowski <username.taken@gmail.com> * Pass the apiserver address/port via cli, introduce HELM_KUBEAPISERVER envvar Signed-off-by: Vibhav Bobade <vibhav.bobde@gmail.com> * Fix output of list action when it is failed Signed-off-by: Song Shukun <song.shukun@fujitsu.com> * feat(install): introduce --create-namespace Signed-off-by: Matthew Fisher <matt.fisher@microsoft.com> * feat(comp): Move kube-context completion to Go Signed-off-by: Marc Khouzam <marc.khouzam@montreal.ca> * Adopt resources into release with correct instance and managed-by labels Signed-off-by: Jacob LeGrone <git@jacob.work> * Alternative: annotation-only solution Signed-off-by: Jacob LeGrone <git@jacob.work> * feat(comp): Static completion for plugins Allow plugins to optionally specify their sub-cmds and flags through a simple yaml file. When generating the completion script with the command 'helm completion <bash|zsh>' (and only then), helm will look for that yaml file in the plugin's directory. If the file exists, helm will create cobra commands and flags so that the completion script will handle them. Signed-off-by: Marc Khouzam <marc.khouzam@montreal.ca> * feat(comp): Dynamic completion for plugins For each plugin, helm registers a ValidArgsFunction which the completion script will call when it needs dynamic completion. This ValidArgsFunction will setup the plugin environment and then call the executable `plugin.complete`, if it is provided by the plugin. The output of the call to `plugin.complete` will be used as completion choices. The last line of the output can optionally be used by the plugin to specify a completion directive. Signed-off-by: Marc Khouzam <marc.khouzam@montreal.ca> * feat(tests): Allow to provision memory driver The memory driver is used for go tests. It can also be used from the command-line by setting the environment variable HELM_DRIVER=memory. In the latter case however, there was no way to pre-provision some releases. This commit introduces the HELM_MEMORY_DRIVER_DATA variable which can be used to provide a colon-separated list of yaml files specifying releases to provision automatically. For example: HELM_DRIVER=memory \ HELM_MEMORY_DRIVER_DATA=./testdata/releases.yaml \ helm list --all-namespaces Signed-off-by: Marc Khouzam <marc.khouzam@montreal.ca> * fix(helm): add --skipCRDs flag to 'helm upgrade' When 'helm upgrade --install' is run, this will allow to skip installing CRDs Closes helm#7452 Signed-off-by: akash-gautam <gautam.akash04@gmail.com> * pkg/storage/records: add unit test for Get Signed-off-by: Zhou Hao <zhouhao@cn.fujitsu.com> * pkg/storage/records: add unit test for Index Signed-off-by: Zhou Hao <zhouhao@cn.fujitsu.com> * pkg/storage/records: add unit test for Exists Signed-off-by: Zhou Hao <zhouhao@cn.fujitsu.com> * Printing name of chart that do not have requested import value. Signed-off-by: Tomas Kohout <tomas.kohout@leveris.com> * fix(cmd/helm): upgrade go-shellwords Removes workaround introduced in helm#7323 Signed-off-by: Adam Reese <adam@reese.io> * Fix dep build to be compatiable with Helm 2 when requirements use repo alias Signed-off-by: Song Shukun <song.shukun@fujitsu.com> * Fix golangci-lint errors. Signed-off-by: Pavel Macík <pavel.macik@gmail.com> * Return "unknown command" error for unknown subcommands Signed-off-by: Xiangxuan Liu <xiangxuan.liu@rightcapital.com> * Add test for unknown subcommand Signed-off-by: Xiangxuan Liu <xiangxuan.liu@rightcapital.com> * Update README.md Typo fix: Space missed in Markdown header. Signed-off-by: Evgeniy Yablokov <ey@odoscope.com> * fix(helm): stdin values for helm upgrade --install Signed-off-by: Matt Morrissette <yinzara@gmail.com> * Fixes verification output on pull command When using the --verify flag on the pull command the output was an internal Go object rather than useful detail. This is a bug. The output new displays who signed the chart along with the hash. Fixes helm#7624 Signed-off-by: Matt Farina <matt@mattfarina.com> * Add verification output to the verify command This complements the verification output fixed in helm#7706. On verify there should be some detail about the verification rather than no information. Signed-off-by: Matt Farina <matt@mattfarina.com> * fix(ADOPTERS): alphabetize org list (helm#7645) Signed-off-by: Matthew Fisher <matt.fisher@microsoft.com> * add unit test for SecretDelete Signed-off-by: Zhou Hao <zhouhao@cn.fujitsu.com> * add unit test for ConfigMapDelete Signed-off-by: Zhou Hao <zhouhao@cn.fujitsu.com> * fix(helm): respect resource policy on ungrade Don't delete a resource on upgrade if it is annotated with helm.io/resource-policy=keep. This can cause data loss for users if the annotation is ignored(e.g. for a PVC) Close helm#7677 Signed-off-by: Dong Gang <dong.gang@daocloud.io> * add unit test for RecordsReplace Signed-off-by: Zhou Hao <zhouhao@cn.fujitsu.com> * fix(helm): polish goimport Signed-off-by: Dong Gang <dong.gang@daocloud.io> * test(helm): fix client update error Signed-off-by: Dong Gang <dong.gang@daocloud.io> * Save Chart.lock to helm package tar Signed-off-by: Song Shukun <song.shukun@fujitsu.com> * ref(environment): use string checking instead It is more idiomatic to compare the string against the empty string than to check the string's length. Signed-off-by: Matthew Fisher <matt.fisher@microsoft.com> * Add --insecure-skip-tls-verify for repositories (helm#7254) * added --insecure-skip-tls-verify for chart repos Signed-off-by: Matthias Riegler <me@xvzf.tech> * fixed not passing the insecureSkipTLSverify option Signed-off-by: Matthias Riegler <me@xvzf.tech> * fixed testcase Signed-off-by: Matthias Riegler <me@xvzf.tech> * pass proxy when using insecureSkipVerify Signed-off-by: Matthias Riegler <me@xvzf.tech> * Add testcases for insecureSkipVerifyTLS Signed-off-by: Matthias Riegler <me@xvzf.tech> * added missing err check Signed-off-by: Matthias Riegler <me@xvzf.tech> * panic after json marshal fails Signed-off-by: Matthias Riegler <me@xvzf.tech> * fix: add new static linter and fix issues it found (helm#7655) * fix: add new static linter and fix issues it found Signed-off-by: Matt Butcher <matt.butcher@microsoft.com> * fixed two additional linter errors. Signed-off-by: Matt Butcher <matt.butcher@microsoft.com> * Make the charts cache safe in presence of several Helm instances If several instances of Helm are run at the same moment and try to download the same chart, some of them might see an empty or incomplete file in cache. Prevent that by saving the dowloaded file atomically. Closes helm#7600 Signed-off-by: Mikhail Gusarov <misha@ridge.co> * chore(go.mod): run `go mod tidy` Signed-off-by: Matthew Fisher <matt.fisher@microsoft.com> * Use Create method if no resources need to be adopted Signed-off-by: Jacob LeGrone <git@jacob.work> * Port --devel flag from v2 to v3 Helm v2 PR helm#5141 Signed-off-by: Martin Hickey <martin.hickey@ie.ibm.com> * Fix stray modules Signed-off-by: Martin Hickey <martin.hickey@ie.ibm.com> * Add unit test Signed-off-by: Martin Hickey <martin.hickey@ie.ibm.com> * Solve the issue helm#7749 where proper formating was not being done if --short(-q) option was used with other formating options like json, yaml Signed-off-by: Anshul Verma <ansverma@localhost.localdomain> * Solve the issue helm#7749 where proper formating was not being done if --short(-q) option was used with other formating options like json, yaml Signed-off-by: Anshul Verma <ansverma@localhost.localdomain> * Solve the issue helm#7749 where proper formating was not being done if --short(-q) option was used with other formating options like json, yaml Signed-off-by: Anshul Verma <ansverma@localhost.localdomain> * fix(install): correct append tls config. Signed-off-by: James McElwain <jmcelwain@gmail.com> * Fixing issue where archives created on windows have broken paths When archives are created on windows the path spearator in the archive file is \\. This causes issues when the file is unpacked. For example, on Linux the files are unpacked in a flat structure and \ is part of the file name. This causes comp issues. In Helm v2 the path was set as / when the archive was written. This works on both Windows and POSIX systems. The fix being implemented is to use the ToSlash function to ensure / is used as the separator. Fixes helm#7748 Signed-off-by: Matt Farina <matt@mattfarina.com> * Solve the issue helm#7749 where proper formating was not being done if --short(-q) option was used with other formating options like json, yaml Signed-off-by: Anshul Verma <ansverma@localhost.localdomain> * Add more detail to error messages and support a non-force mode in metadata visitor Signed-off-by: Jacob LeGrone <git@jacob.work> * Add tests Signed-off-by: Jacob LeGrone <git@jacob.work> * Correcting links for release notes Signed-off-by: Bridget Kromhout <bridget@kromhout.org> * fix(cli): Make upgrade check if cluster reachable The 'helm upgrade' command was not checking if the cluster was reachable. Also, 'helm upgrade --install' first checks if the release exists already. If that check fails there is no point in continuing the upgrade. This optimization avoids a second timeout of 30 seconds when trying to do the upgrade. Signed-off-by: Marc Khouzam <marc.khouzam@montreal.ca> * Fix a bug in storage/driver/secrets.go Delete() (helm#7348) * Fix a bug in storage/driver/secrets.go * Fix a bug in Delete() in storage/driver/cfgmaps.go (helm#7367) * Add unit test for lint/values.go Signed-off-by: Kim Bao Long <longkb@vn.fujitsu.com> * Improve error message to check in unit test Signed-off-by: Martin Hickey <martin.hickey@ie.ibm.com> * helm create command's templates more consistent - Removed most right whitespace chomps except those directly following a template definition where it make sense to not lead with a blank line. The system applied is now to almost always left whitespace chomp but also whitespace chomp right if its the first thing in a file or template definition. - Updated indentation to be systematic throughout all the boilerplace files. Signed-off-by: Erik Sundell <erik.i.sundell@gmail.com> * Snapcraft installation instructions added Helm is available as a snap - https://snapcraft.io/helm; added this to the installation options Signed-off-by: Ihor Dvoretskyi <ihor@linux.com> * pass subchart notes option to install client Signed-off-by: Jon Leonard <jgleonard@gmail.com> * add testing for upgrade --install with subchart notes Signed-off-by: Jon Leonard <jgleonard@gmail.com> * Delete unneeded chart output Signed-off-by: Jon Leonard <jgleonard@gmail.com> * Add fromYamlArray and fromJsonArray template helpers (helm#7712) Signed-off-by: Tuan Nguyen <nmtuan.dev@gmail.com> * fix: update unit test for go 1.14 error string change (helm#7835) * fix: update unit test for go 1.14 error string change Signed-off-by: Matt Butcher <matt.butcher@microsoft.com> * changed strategy based on conversation with Adam Signed-off-by: Matt Butcher <matt.butcher@microsoft.com> * update test chart to helm3 format Signed-off-by: Jon Leonard <jgleonard@gmail.com> * remove unneeded values files from testchart Signed-off-by: Jon Leonard <jgleonard@gmail.com> * Add unit test for pkg/chart/chart.go Signed-off-by: Hu Shuai <hus.fnst@cn.fujitsu.com> * Improve --show-only flag (helm#7816) * Improve --show-only flag * Ensure consistent manifest ordering * fix(helm): Data race in kube/client Delete func. (helm#7820) helm uninstall has a data race in its Delete function. This resolves it using a mutex. Signed-off-by: Liam Nattrass <liam.d.nattrass+git@gmail.com> * Avoid downloading same chart multiple times Signed-off-by: Andrey Voronkov <voronkovaa@gmail.com> * feat: add pod annotations With the rise of sidecar injectors, pod annotations configuration is becoming more and more important. Signed-off-by: Naseem <naseem@transit.app> * feat: allow image tag override While using the chart version as image tag is the sanest default, it is not uncommon to want to override this if using a custom image, or using helm to manage an in-house app running different tags across different environments. Signed-off-by: Naseem <naseem@transit.app> * Adding notes on semver to create Chart.yaml The version field in the Chart.yaml has a comment describing it but it did not note the version needs to follow SemVer. There have been numerous questions, over time, about this format. Add note here so it's exposed in more places. Signed-off-by: Matt Farina <matt@mattfarina.com> * fix: fixed bug in Dependency.List() (helm#7852) * fix: fixed bug in Dependency.List() A bug in Dependency.List() caused all compressed charts to flag their dependencies as "missing". Closes helm#4431 Signed-off-by: Matt Butcher <matt.butcher@microsoft.com> * removed some files from test fixtures Signed-off-by: Matt Butcher <matt.butcher@microsoft.com> * Implement support for multiple args for repo remove (helm#7791) * Implement support for multiple args for repo remove Signed-off-by: Andreas Lindhé <andreas@lindhe.io> * Adding template docs to the version command Signed-off-by: Matt Farina <matt@mattfarina.com> * ref(*): kubernetes v1.18 (helm#7831) Upgrade Kubernetes libraries to v0.18.0 Add new lazy load KubernetesClientSet to avoid missing kubeconfig error In kubernetes v1.18 kubeconfig validation was added. Minikube and Kind both remove kubeconfig when stopping clusters. This causes and error when running any helm commands because we initialize the client before executing the command. Signed-off-by: Adam Reese <adam@reese.io> * chore(comp): Remove unnecessary code After the introduction of lazy loading of the Kubernetes client as part of PR helm#7831, there is no longer a need to protect against an incomplete --kube-context value when doing completion. Signed-off-by: Marc Khouzam <marc.khouzam@montreal.ca> * fixed capitalization in a few help messages. (helm#7898) Signed-off-by: Matt Butcher <matt.butcher@microsoft.com> * fix(storage): preserve last deployed revision (helm#7806) Signed-off-by: Eric Bailey <eric@ericb.me> * feat(cmd/helm): Update Cobra to 1.0.0 release Signed-off-by: Marc Khouzam <marc.khouzam@montreal.ca> * updated help text for install --atomic, which was completely inaccurate (helm#7912) Signed-off-by: Matt Butcher <matt.butcher@microsoft.com> * add unit test for IsRoot Signed-off-by: Zhou Hao <zhouhao@cn.fujitsu.com> * add unit test for ChartPath Signed-off-by: Zhou Hao <zhouhao@cn.fujitsu.com> * add unit test for ChartFullPath Signed-off-by: Zhou Hao <zhouhao@cn.fujitsu.com> * add unit test for metadata Validate Signed-off-by: Zhou Hao <zhouhao@cn.fujitsu.com> * docs: Update inline docs on action/upgrade.go (helm#7834) * docs: Update inline docs on action/upgrade.go Signed-off-by: Matt Butcher <matt.butcher@microsoft.com> * clarify atomic and cleanup-on-fail Signed-off-by: Matt Butcher <matt.butcher@microsoft.com> * updated the post-render documentation on action.Upgrade Signed-off-by: Matt Butcher <matt.butcher@microsoft.com> * Add unit test for Secrets/ConfigMaps (helm#7765) * test(pkg/storage/secrets): make MockSecretsInterface.List follow ListOptions Signed-off-by: Lu Fengqi <lufq.fnst@cn.fujitsu.com> * test(pkg/storage/secrets): add unit test for Secrets.Query Signed-off-by: Lu Fengqi <lufq.fnst@cn.fujitsu.com> * test(pkg/storage/cfgmaps): make MockConfigMapsInterface.List follow ListOptions Signed-off-by: Lu Fengqi <lufq.fnst@cn.fujitsu.com> * test(pkg/storage/cfgmaps): add unit test for ConfigMaps.Query Signed-off-by: Lu Fengqi <lufq.fnst@cn.fujitsu.com> * fix(tests): fix broken unit tests in storage (helm#7928) * fix(pkg/kube): continue deleting objects when one fails * Continue deleting objects when one fails to minimize the risk of an upgrade ending in an unrecoverable state * Exclude failed deleted object from the returned result set Signed-off-by: Adam Reese <adam@reese.io> * Add an improved user error message for removed k8s apis The error message returned from Kubernetes when APIs are removed is not very informative. This PR adds additional information to the user. It covers the current release manifest APIs. Partial helm#7219 Signed-off-by: Martin Hickey <martin.hickey@ie.ibm.com> * test: forward-port regression test from Helm 2 (helm#7927) Signed-off-by: Matt Butcher <matt.butcher@microsoft.com> * add softonic to adopters (helm#7918) Signed-off-by: Riccardo Piccoli <riccardo.piccoli@softonic.com> Co-authored-by: Riccardo Piccoli <riccardo.piccoli@softonic.com> * Make get script eaiser for helm versions to live side by side (helm3 etc) (helm#7752) * Make get script eaiser for helm versions to live side by side (helm3 etc) Signed-off-by: Scott Rigby <scott@r6by.com> * Change PROJECT_NAME to BINARY_NAME for purpose clarity Signed-off-by: Scott Rigby <scott@r6by.com> * fix: rebuild chart after dependency update on install (helm#7897) * fix: rebuild chart after dependency update on install Signed-off-by: Matt Butcher <matt.butcher@microsoft.com> * add correct debug settings Signed-off-by: Matt Butcher <matt.butcher@microsoft.com> * add unit test for function FindPlugins Signed-off-by: ZouYu <zouy.fnst@cn.fujitsu.com> * Updating sprig and semver to newer versions Note, there is an issue with a dependency of sprig changing behavior. A test has been added with a description to catch if a behavior breaking change of mergo is used. See darccio/mergo#139 for the mergo issue and sprig for further details on handling this in the future. Closes helm#7533 Signed-off-by: Matt Farina <matt@mattfarina.com> * Fix nested null value overrides (helm#7743) * Fix nested null value overrides Signed-off-by: Mingxiang Xue <mingxiangxue@gmail.com> * Fix subchart value deletion Signed-off-by: Mingxiang Xue <mingxiangxue@gmail.com> * fix: Fixed a regression that was introduced with changed nil handling (helm#7938) Signed-off-by: Matt Butcher <matt.butcher@microsoft.com> * Migrate SQL storage driver to Helm 3 (helm#7635) * Migrate SQL storage driver to Helm 3 Signed-off-by: Elliot Maincourt <e.maincourt@gmail.com> * Update pkg/storage/driver/sql.go Co-Authored-By: Sebastian Pöhn <sebastian.poehn@gmail.com> Signed-off-by: Elliot Maincourt <e.maincourt@gmail.com> * Add authentication to releases_v3 Signed-off-by: Elliot Maincourt <e.maincourt@gmail.com> * Fix migration Signed-off-by: Elliot Maincourt <e.maincourt@gmail.com> * Template the init migration Signed-off-by: Elliot Maincourt <e.maincourt@gmail.com> * Prevent potential SQL injection Signed-off-by: Elliot Maincourt <e.maincourt@gmail.com> * Use an SQL querybuilder Signed-off-by: Elliot Maincourt <e.maincourt@gmail.com> * Remove references to HELM_DRIVER_SQL_DIALECT Signed-off-by: Elliot Maincourt <e.maincourt@gmail.com> Co-authored-by: Sebastian Pöhn <sebastian.poehn@gmail.com> Co-authored-by: Matt Butcher <matt.butcher@microsoft.com> * fix: removed inaccurate comment (helm#7937) Signed-off-by: Matt Butcher <matt.butcher@microsoft.com> * Updating get stripts to skip pre-releases A recent change to the get scripts causes them to pickup pre-releases in addition to stable releases. This update causes only stable releases to be fetched by the get scripts. Fixed helm#7941 Signed-off-by: Matt Farina <matt@mattfarina.com> * fix(helm): allow a previously failed release to be upgraded (helm#7653) Signed-off-by: Matt Morrissette <yinzara@gmail.com> * fs_test: use os.Getuid() instead user.Current() to determine if a test is executed with root privileges. This change lower the expectations on test env setup, i.e. tests could be executed in a container under a random UID, without require an user in /etc/passwd Signed-off-by: Predrag Knezevic <pknezevi@redhat.com> * Parse reference templates in predictable order (helm#7702) * Parse reference templates in predictable order Fix issue helm#7701 Signed-off-by: Andre Sencioles <asenci@gmail.com> * Add test case for issue helm#7701 regression Signed-off-by: Andre Sencioles <asenci@gmail.com> * gofmt Signed-off-by: Andre Sencioles <asenci@gmail.com> * fix linting error with lookup function (helm#7969) Signed-off-by: Matt Butcher <matt.butcher@microsoft.com> * fix(pkg/plugin): copy plugins directly to the data directory (helm#7962) Copy plugins from the cache rather than create a symlink. fixes: helm#7206 Signed-off-by: Adam Reese <adam@reese.io> * Helm upgrades with --reuse-values and nil user values -- with tests (helm#7959) * return the new values if modifications dont yet exist Signed-off-by: David Pait <DP19@users.noreply.github.com> * fix tests Signed-off-by: David Pait <DP19@users.noreply.github.com> * removed outter if statement as its not needed now Signed-off-by: David Pait <DP19@users.noreply.github.com> * fix(*): remove bom in utf files when loading chart files (helm#6081) Removes the BOM prefix if present, in read files before processing the data. Affects the following pkg: - pkg/chart/loader: directory and archive loader - internal/ignore: when loading .helmignore file Signed-off-by: Thomas FREYSS <thomas.freyss@gmail.com> * fix(cmd/env): make helm env command respect cli flags (helm#7978) Running `helm env` should respect cli flag overrides. Signed-off-by: Adam Reese <adam@reese.io> * fix(pkg/cli): ensure correct configuration from kubeconfig file Bind Helm flags to Kubernetes configuration loader to get a merged config with kubeconfig. Fixes: helm#7539 Signed-off-by: Adam Reese <adam@reese.io> * Modify Circle config to use Go 1.14 (helm#7980) Signed-off-by: Josh Dolitsky <393494+jdolitsky@users.noreply.github.com> * Fixing docs from version to appVersion (helm#7975) In the created chart from `helm create` is notes a tag overrides version. It actually overrides appVersion. Updating the docs to reflect reality. Signed-off-by: Matt Farina <matt@mattfarina.com> * test: add test for bom test data integrity Signed-off-by: Thomas FREYSS <thomas.freyss@gmail.com> * fix: write index.yaml file atomically (helm#7954) * fix: write index.yaml file atomically This refactors the already-existing `AtomicWriteFile` utility to a central location and uses it to write index files atomically. This is done to avoid having half-written index files break client requests. Drive-bys: - Add test for AtomicWriteFile. - Add test IndexFile.WriteFile. Signed-off-by: rabadin <rvbadin@gmail.com> * Review fix: use RenameWithFallback instead of os.Rename Signed-off-by: rabadin <rvbadin@gmail.com> Co-authored-by: rabadin <rvbadin@gmail.com> * Add unit test for pkg/chart/chart.go Signed-off-by: Hu Shuai <hus.fnst@cn.fujitsu.com> * Adding PR template from dev-v2 branch Signed-off-by: Bridget Kromhout <bridget@kromhout.org> * Updating CONTRIBUTING to match current practice Signed-off-by: Bridget Kromhout <bridget@kromhout.org> * Fix : Prints empty list in json/yaml is no repositories are present (helm#7949) * Prints empty repolist in json/yaml if there are no repos and output format is given as json/yaml rather that printing the error msg "no repositories to show" Signed-off-by: Anshul Verma <anshulvermapatel@gmail.com> * Prints empty repolist in json/yaml if there are no repos and output format is given as json/yaml rather that printing the error msg "no repositories to show" Signed-off-by: Anshul Verma <anshulvermapatel@gmail.com> * feat: lint the names of templated resources (helm#8011) Signed-off-by: Matt Butcher <matt.butcher@microsoft.com> * Add support for using OCI references as the dependency repository Signed-off-by: Leo Sjöberg <leo.sjoberg@gmail.com> * fix accidental bug in chart saving and fix linting Signed-off-by: Leo Sjöberg <leo.sjoberg@gmail.com> * add tests and URL fallback handling Signed-off-by: Leo Sjöberg <leo.sjoberg@gmail.com> * extract oci to its own constant Signed-off-by: Leo Sjöberg <leo.sjoberg@gmail.com> * refactor to a GetWithDetails method Signed-off-by: Leo Sjöberg <leo.sjoberg@gmail.com> * fix GetWithDetails tests Signed-off-by: Leo Sjöberg <leo.sjoberg@gmail.com> * fix tests Signed-off-by: Leo Sjöberg <leo.sjoberg@gmail.com> * rename ChartContent to Content and use naked return Signed-off-by: Leo Sjöberg <leo.sjoberg@gmail.com> * move URL and filename handling into the downloader Signed-off-by: Leo Sjöberg <leo.sjoberg@gmail.com> * fix imports Signed-off-by: Leo Sjöberg <leo.sjoberg@gmail.com> * use old filepath.join behavior to avoid calling it twice Signed-off-by: Leo Sjöberg <leo.sjoberg@gmail.com> Co-authored-by: James McElwain <jmcelwain@gmail.com> Co-authored-by: Pavel Macík <pavel.macik@gmail.com> Co-authored-by: Guangwen Feng <fenggw-fnst@cn.fujitsu.com> Co-authored-by: Naseem <naseemkullah@gmail.com> Co-authored-by: Dao Cong Tien <tiendc@vn.fujitsu.com> Co-authored-by: Daniel Poelzleithner <git@poelzi.org> Co-authored-by: Daniel Strobusch <1847260+dastrobu@users.noreply.github.com> Co-authored-by: Florian Hopfensperger <f.hopfensperger@gmail.com> Co-authored-by: Federico Bevione <f.bevione@cognitio.it> Co-authored-by: Matthew Fisher <matt.fisher@microsoft.com> Co-authored-by: Matt Butcher <matt.butcher@microsoft.com> Co-authored-by: Reinhard Naegele <unguiculus@gmail.com> Co-authored-by: Hidde Beydals <hello@hidde.co> Co-authored-by: Vibhav Bobade <vibhav.bobde@gmail.com> Co-authored-by: Matt Farina <matt@mattfarina.com> Co-authored-by: Rui Chen <chenrui333@gmail.com> Co-authored-by: ylvmw <yngliu@vmware.com> Co-authored-by: Zhou Hao <zhouhao@cn.fujitsu.com> Co-authored-by: Martin Hickey <martin.hickey@ie.ibm.com> Co-authored-by: Song Shukun <song.shukun@fujitsu.com> Co-authored-by: Taylor Thomas <taylor.thomas@microsoft.com> Co-authored-by: Adam Reese <adamreese@users.noreply.github.com> Co-authored-by: Paul Czarkowski <username.taken@gmail.com> Co-authored-by: Marc Khouzam <marc.khouzam@montreal.ca> Co-authored-by: Jacob LeGrone <git@jacob.work> Co-authored-by: akash-gautam <gautam.akash04@gmail.com> Co-authored-by: Tomáš Kohout <tomas.kohout@leveris.com> Co-authored-by: Adam Reese <adam@reese.io> Co-authored-by: Xiangxuan Liu <xiangxuan.liu@rightcapital.com> Co-authored-by: Evgenii Iablokov <eyablokov@me.com> Co-authored-by: Matt Morrissette <yinzara@gmail.com> Co-authored-by: Dong Gang <dong.gang@daocloud.io> Co-authored-by: Matthias Riegler <me@xvzf.tech> Co-authored-by: Mikhail Gusarov <misha@ridge.co> Co-authored-by: Anshul Verma <ansverma@localhost.localdomain> Co-authored-by: Bridget Kromhout <bridget@kromhout.org> Co-authored-by: tiendc <tiendc@gmail.com> Co-authored-by: Kim Bao Long <longkb@vn.fujitsu.com> Co-authored-by: Erik Sundell <erik.i.sundell@gmail.com> Co-authored-by: Ihor Dvoretskyi <ihor@linux.com> Co-authored-by: Jon Leonard <jgleonard@gmail.com> Co-authored-by: Tuan <me@nmtuan.space> Co-authored-by: Hu Shuai <hus.fnst@cn.fujitsu.com> Co-authored-by: Mario Valderrama <15158349+avorima@users.noreply.github.com> Co-authored-by: lnattrass <liam.d.nattrass@gmail.com> Co-authored-by: Andrey Voronkov <voronkovaa@gmail.com> Co-authored-by: Naseem <naseem@transit.app> Co-authored-by: Andreas Lindhé <lindhe@users.noreply.github.com> Co-authored-by: Eric Bailey <yurrriq@users.noreply.github.com> Co-authored-by: Lu Fengqi <lufq.fnst@cn.fujitsu.com> Co-authored-by: Riccardo Piccoli <riccardo.piccoli@gmail.com> Co-authored-by: Riccardo Piccoli <riccardo.piccoli@softonic.com> Co-authored-by: Scott Rigby <scott@r6by.com> Co-authored-by: ZouYu <zouy.fnst@cn.fujitsu.com> Co-authored-by: uzxmx <mingxiangxue@gmail.com> Co-authored-by: Elliot Maincourt <e.maincourt@gmail.com> Co-authored-by: Sebastian Pöhn <sebastian.poehn@gmail.com> Co-authored-by: Predrag Knezevic <pknezevi@redhat.com> Co-authored-by: Andre Sencioles <asenci@gmail.com> Co-authored-by: David Pait <DP19@users.noreply.github.com> Co-authored-by: Thomas FREYSS <thomas.freyss@gmail.com> Co-authored-by: Josh Dolitsky <393494+jdolitsky@users.noreply.github.com> Co-authored-by: Raphaël <rabadin@cisco.com> Co-authored-by: rabadin <rvbadin@gmail.com> Co-authored-by: Anshul Verma <anshulvermapatel@gmail.com> Co-authored-by: Leo Sjöberg <leo.sjoberg@gmail.com>
|
I am planning to roll out a new version before I do just wanted to make sure I am doing the right way. I am always running below command in our ci-cd pipeline if deploying the first time or upgrading. sh "helm upgrade --install ${appName} ${chartPath} --namespace=${namespace} --debug --version ${chartVersion} --set=deployment.image=${image} -f overridesFile.json --history-max=5 --kubeconfig=${kubeConfig} --wait" Are we expecting above the fix should work for above? Or do I need to change the above command and split based on based if it is installed or upgrades? something like if first time install sh "helm install ${appName} ${chartPath} --namespace=${namespace} --debug --version ${chartVersion} --set=deployment.image=${image} -f overridesFile.json --history-max=5 --kubeconfig=${kubeConfig} --wait" And if upgrade then sh "helm upgrade ${appName} ${chartPath} --namespace=${namespace} --debug --version ${chartVersion} --set=deployment.image=${image} -f overridesFile.json --history-max=5 --kubeconfig=${kubeConfig} --wait" |
Signed-off-by: Matt Morrissette <yinzara@gmail.com> (cherry picked from commit 1911870)
|
Does this MR allow us to use :
If I do : And then I try to upgrae/install it again: |
|
No. For that, use |
Signed-off-by: Matt Morrissette <yinzara@gmail.com> Signed-off-by: Matheus Hunsche <matheus.hunsche@ifood.com.br>
This PR changes the behavior of "helm upgrade" command when there have been no successful and at least one failed release. This allows the user to recover from a failed install (or a partially failed install) without uninstalling the release. The user can now decide to decide to ignore the "no deployed releases" errors and allow it to upgrade a failed release.
There were no test cases that tested the failure scenarios of upgrading a failed/missing release so I added them and also added additional test cases that test the new functionality.
Fixes issue #5595
Signed-off-by: Matt Morrissete yinzara@gmail.com