Improving down to handle Volumes

[dustymabe]

In 7349dc9 i'm proposing adding --emptyvols to the up/down operations. This raises a bigger question on whether we should enable passing the other convert options so that they can be used with up/down - this can be useful, but maybe not something we want to do.

[ngtuna]

@dustymabe I think kompose down doesn't need to have --emptyvols option

$ $ kompose down --help
NAME:
   kompose down - Delete instantiated services/deployments from kubernetes

USAGE:
   kompose down [command options] [arguments...]

OPTIONS:
   --emptyvols  Use Empty Volumes. Don't generate PVCs

[kadel]

There is a bigger question.
If you are deleting object for kompose down, how you know what objects should deleted?

If user deployed using kompose up that there is PersisntentVolumeClaim and if --emptyvols was used than there is no PVC.

Easiest solution in the case of PVC might be to always try delete PVC (basicaly always assume down --emptyvol) and don't report error if there is no PVC. But this assumes that user won't create PVC with same name convention that is Kompose using, otherwise we delete PVC that wasn't created by Kompose.

[ngtuna]

Right. It's because there is no clue in docker-compose file that lets us know if pvc was created or not. From UX perspective kompose down --emptyvols looks a bit of weird.

Should we automatically/transparently add a kompose label for pvc to the original docker-compose file once user chooses to create pvc object ? OR should we update the strategy of generating pvc by adding label ?

[kadel]

From UX perspective kompose down --emptyvols looks a bit of weird.

Agreed it is weird. We should force user to think about such things.

Should we automatically/transparently add a kompose label for pvc to the original docker-compose file once user chooses to create pvc object ? OR should we update the strategy of generating pvc by adding label ?

Annotating controller with additional information about what was used for conversion might be in general great idea. Other think that we could do is to actually inspect controller object and find out if it uses PVC or not. But i don't know how complicated this might be

[dustymabe]

for kompose down we could use labels that were applied to each element that was created during "up" to bring the application down. so it's possible we don't have to do a "conversion" at all, just inspect for labels and delete everything that has that label

[kadel]

for kompose down we could use labels that were applied to each element that was created during "up" to bring the application down. so it's possible we don't have to do a "conversion" at all, just inspect for labels and delete everything that has that label

I was thinking about this also, but what if user deploys two different application (docker-compose.yml files) to one namespace?

[dustymabe]

we could:

1 - try to use a unique name
2 - collision avoidance - if you try to "up" and there is a label conflict with the label you are going to use and a label that already exists, then error out and tell the user.

[surajssd]

to improve doing down, we should label artifacts at creation time saying that those artifacts were created using kompose, then while deleting we only delete things that has those label?

that label could look like this:

kompose-project-name: <project_name>
or something better than above label so that we can query things that were created using kompose and we delete only artifacts for current project and not other project kompose generated objects.

[kadel]

@surajssd what is <project_name>?

How about we do it like this:
Lets use io.kompose.service: <service_name> label to label all objects that are related to given service.
Use this also for PVC.
During down, delete all object related to given service using this label as selector.

[procrypt]

@kadel I agree on using io.kompose.service: <service_name> and then we delete the object using that label. There is cli command for doing that kubectl delete <object> --selector=io.kompose.label=<service_name>, still need to figure out the api calls.

[surajssd]

@kadel with <project_name> I was thinking of having a single label on all resources. As opposed to labeling service wise. So that we can delete everything in one go!

With each service having different labels we will have to loop over the service names to delete the artifacts.

[procrypt]

Had a discussion with @kadel and we found out that there is a function SelectorFromSet()
in kubernetes which returns a Selector (or Label) which will match exactly the given Set (in our case it is io.kompose.service), making use of that we can find all the objects associated with the given Selector (or Label) and can further do the delete operation.

[cdrage]

Was going to type this in the PR but here goes:

@kadel @procrypt

So I think this is awesome and amazing code @procrypt

What I'd like to discuss though, is should we really be putting these labels in when we convert?

Honestly, most of our users simply convert, look at the conversion and then kubectl create/apply it..

With this PR, we're adding selectors as well as a metadata section that includes creationTimeStamp as well as a label, making an already-long Kubernetes yaml file even longer.

The point of this PR is to implement kompose down so it deletes based on the label.

What I'm getting at is should we "hide" these labels and implement them ONLY when the user kompose up's?