tilt icon indicating copy to clipboard operation
tilt copied to clipboard

clean up PVCs created by VolumeClaimTemplates

Open landism opened this issue 2 years ago • 6 comments

Describe the Feature You Want

e.g.:

load('ext://helm_remote', 'helm_remote')
helm_remote('mysql', repo_name='bitnami')

tilt up causes the pvc data-mysql-0 to be created. tilt down should cause it to be deleted.

Current Behavior

Tilt is unaware of this PVC and does not touch it on down. This is presumably because the helm yaml doesn't actually include the PVC, but rather specifies StatefulSet.Spec.VolumeClaimTemplates, which, IIUC, causes the PVC to be created. As a workaround, a user can put something like this in their Tiltfile:

if config.subcommand == 'down':
  local('kubectl delete pvc data-mysql-0')

which is functional, but involves 1) the user getting past what feels like a bug and 2) hardcoding the pvc names (or somehow querying for them)

Why Do You Want This?

While users often want PVCs to persist, the default behavior in Tilt is still to delete PVCs, and it feels buggy that Tilt misses this case.

Additional context Add any other context or screenshots about the feature request here.

landism avatar Apr 22 '22 15:04 landism

Our use case is that we spin up a temporary postgres statefulset which we proxy to during some migration tests for a local_resource, so we don't necessarily want it sticking around.

chiefy avatar Apr 22 '22 18:04 chiefy

I utilize the behavior of PVCs being retained in my normal flows, if it is unintentional i'd love to see some way to replicate this behavior after a fix implemented.

Dhouti avatar Apr 22 '22 19:04 Dhouti

FWIW, this behavior is built into StatefulSet. See here:

https://kubernetes.io/docs/tasks/run-application/delete-stateful-set/#persistent-volumes

though i have wondered what more controls over volume management in tilt might look like.

nicks avatar Apr 25 '22 13:04 nicks

we would love this - right now we do this in our Tiltfile:

if config.tilt_subcommand == "down":
    # make sure the PVCs are gone
    local(
        'kubectl patch pvc filestore-default-pvc --namespace=default -p \'{"metadata":{"finalizers": []}}\' --type=merge || true'
    )
    local("kubectl delete pvc -n default filestore-default-pvc || true")
    local(
        'kubectl patch pv filestore-default-pv --namespace=default -p \'{"metadata":{"finalizers": []}}\' --type=merge || true'
    )
    local("kubectl delete pv -n default filestore-default-pv || true")
    local(
        'kubectl patch pvc filestore-pools-pvc --namespace=database-pools -p \'{"metadata":{"finalizers": []}}\' --type=merge || true'
    )
    local("kubectl delete pvc -n database-pools filestore-pools-pvc || true")
    local(
        'kubectl patch pvc filestore-pools-pv --namespace=database-pools -p \'{"metadata":{"finalizers": []}}\' --type=merge || true'
    )

Which is basically a hack.

adamf avatar May 10 '22 21:05 adamf

I don't know, I feel like by definition, a PersistentVolumeClaim should persist between tilt up and tilt down. Take the case of a dev environment that has a database that takes a couple of hours to load a realistic dataset. You want to only do every once in while.

The Tilt behavior is similar to when you spin down a docker-compose project and the volumes remain.

I'm pretty sure, helm uninstall does not delete PVC either.

I can see the use case to blow the PVCs away thought but it doesn't seem like it should be the default. Maybe there should be a tilt purge or tilt down --purge?

brandondoran avatar May 11 '22 20:05 brandondoran

Any updates on this? I'm currently experiencing a problem where i tilt up a PersistentVolume and PersistentVolumeClaim but both the PVC and PV won't delete on tilt down, then on tilt up again i get an error of timeout for waiting for PVC and PV to be deleted, which will never happen since they are protected by kubernetes.io/pvc-protection. Would be nice if this option could be disabled with some kind of flag.

ofirnissim1 avatar Dec 13 '23 16:12 ofirnissim1