vmware-tanzu / helm-charts

Contains Helm charts for Kubernetes related open source tools
https://vmware-tanzu.github.io/helm-charts/
Apache License 2.0
254 stars 363 forks source link

Version 4.0.0 is not useable #454

Closed Ujkugri closed 1 year ago

Ujkugri commented 1 year ago

What steps did you take and what happened: When updating to helmchart version 4.0.0, the deployment of the velero server will be deleted. Here is a part of the output of my helmfile diff:

XXX-backup, velero, Deployment (apps) has been removed:
- # Source: velero/templates/deployment.yaml
- apiVersion: apps/v1
- kind: Deployment
- metadata:
...

What did you expect to happen: That the deployment gets updated.

Anything else you would like to add: This is probably because of https://github.com/vmware-tanzu/helm-charts/blob/main/charts/velero/templates/deployment.yaml#L1

With the newest version, you removed the key configuration.provider. Users will get an information like:

  Error: Failed to render chart: exit status 1: Error: execution error at (velero/templates/NOTES.txt:78:4): 
  #################################################################################
  ######   BREAKING: The config values passed contained no longer accepted    #####
  ######             options. See the messages below for more details.        #####
  ######                                                                      #####
  ######             To verify your updated config is accepted, you can use   #####
  ######             the `helm template` command.                             #####
  #################################################################################
  REMOVED: .configuration.provider has been removed, instead each backupStorageLocation and volumeSnapshotLocation has a provider configured
  Use --debug flag to render out invalid YAML
  Error: plugin "diff" exited with error

But this key is in your if-condition, so since there is no configuration.provider, helm thinks it has to "update" the deployment by deleting it.

Environment:

benedikt-bartscher commented 1 year ago

Same here, see my comment in the related PR: https://github.com/vmware-tanzu/helm-charts/pull/413#issuecomment-1523638626

larivierec commented 1 year ago

yep, same here. velero deployment is completely removed and only agents are running

benedikt-bartscher commented 1 year ago

A simple fix is to remove the breaking check in NOTES.txt: https://github.com/vmware-tanzu/helm-charts/blob/4553dc54fb871838e406b07ebe99e5c7a4d8fcb6/charts/velero/templates/NOTES.txt#L37-L39

And keep configuration.provider in your yaml alongside the new Arrays for Backup Locations and Snapshot Locations

benedikt-bartscher commented 1 year ago

Another indicator that removing the breaking check is probably the correct solution: According to the comments in values.yaml configuration.provider is still used as a default value. https://github.com/vmware-tanzu/helm-charts/blob/4553dc54fb871838e406b07ebe99e5c7a4d8fcb6/charts/velero/values.yaml#L242-L243

larivierec commented 1 year ago

Not sure that would solve the issue.

This depends how the application runs. If the application supports the use of a Go slice, the helm chart should be doing a for each over all the backup storage locations and using that provider.

If it doesn't, it should be one deployment per storage location, which to me is kind of weird.