guerzon / vaultwarden

Helm chart for Vaultwarden, the (unofficial) Bitwarden-compatible server written in Rust, formerly known as bitwarden_rs
MIT License
153 stars 73 forks source link

v0.31.0 removes data volume from deployment #135

Open cterence opened 2 days ago

cterence commented 2 days ago

Hi ! Trying to update to this new version removes my existing volume definitions in .spec.volumes in the deployment. My updated values (I just added the new storage key, an empty existingVolumeClaim key and moved my existing data and attachments keys in it):

storage:
  ## @param existingVolumeClaim If defined, the values here will be used for the data and
  ## attachments PV's. The custom values for data and attachments will be ignored if
  ## a value is set here
  ##
  existingVolumeClaim:
    {}
    # claimName: "vaultwarden-pvc"
    # dataPath: "/data"
    # attachmentsPath: /data/attachments

  ## @param data Data directory configuration, refer to values.yaml for parameters.
  ##
  data:
    name: "vaultwarden-data"
    size: "5Gi"
    class: ""
    path: "/data"
    accessMode: "ReadWriteOnce"
    keepPvc: true

  ## @param attachments Attachments directory configuration, refer to values.yaml for parameters.
  ## By default, attachments/ is located inside the data directory.
  ##
  attachments:
    name: "vaultwarden-files"
    size: "10Gi"
    class: ""
    path: /files
    accessMode: "ReadWriteOnce"
    keepPvc: true

My diff in ArgoCD (deployment manifest):

image

@paimonsoror could you please help since you were the author of this change please ?

paimonsoror commented 2 days ago

Hey there! taking a look now. Super odd, so is it only removing your vaultwarden-data volume?

cterence commented 1 day ago

Yes this is the only change, if you want to take a look at my helm chart/values, it's over here.

paimonsoror commented 1 day ago

Yes this is the only change, if you want to take a look at my helm chart/values, it's over here.

Can you confirm that's the right values file?

https://github.com/cterence/homelab-gitops/blob/70d37df1d88f6c0cdcd10b72ffbc2ae4d2182661/k8s-apps/vaultwarden/values.yaml#L236

This and attachments should be nested under storage

cterence commented 1 day ago

Yes this is the correct one. It's on the previous chart version here sorry.

Here's my PR which targets the new v0.31.0 version with the updated values. This is what I previously merged and got me the ArgoCD error.

paimonsoror commented 1 day ago

Ok cool. I'll text your values once I get home. Thanks for the details!

Mawarii commented 1 day ago

I am not sure, but maybe this has to do with the changes made here: image

I have also a problem with 0.31.0, because if I template it volumes: will be defined twice and therefore the deployment.yaml is not valid. image

I don't know much about ArgoCD, but I could imagine that the Argo diff removes the first volumes block and therefore removes the data volume.

santiagon610 commented 1 day ago

I don't know much about ArgoCD, but I could imagine that the Argo diff removes the first volumes block and therefore removes the data volume.

Good point. If memory serves me correctly, YAML is "last one defined wins," which means that the volumes with name: vaultwarden-files should win and the volumes with vaultwarden-data would be ignored.

paimonsoror commented 1 day ago

yup! i see it - ok let me work on a fix here

paimonsoror commented 1 day ago

got a fix ready - rebasing my local against main and then staging a PR