che: Workspace creation fails if a workspace with the same name already exist

I really hate to beat the dead horse here, but I think the n-th cycle we’re in on this issue is due to the different understanding of what devfile is.

I apologize for this to be a rather long comment but I find it necessary to paint a complete(ish) picture before I try to come to some conclusion.

On one hand, @sunix, @benoitf and others want to understand the devfile as a workspace template of sorts, e.g. use 1 devfile to create N workspaces. This is completely valid and IMHO no one argues about the utility of it being possible to create several workspaces using a single devfile. Let’s call this understanding “workspace template”.

On the other hand some other people, including me, understand the devfile as the workspace itself. There is no such thing as a “devfile object” from which a number of workspaces has been created. No, there is just a bunch of workspaces, each defined using a “recipe” that we happen to call devfile. Let’s call this understanding “workspace definition”.

Now let me spend some time explaining the “workspace definition” approach in more detail. First, because that is what I believe is a more rational understanding (while fully agreeing about the need to support the workflow that @sunix suggests) and second because I believe many of the arguments were not well taken by the “opposing party”. So let me try one more time to explain why I think this is a reasonable understanding of the devfile.

It all boils down to Kubernetes. In Kubernetes, there are objects of different kinds, each identified by its name (in given namespace). There can be no 2 objects of the same name and kind in a single namespace. If devfile was a Kubernetes object (which is something I think we’re aiming for), I believe its kind should be “Workspace”. From this, all the reasoning about the behavior of “name” and “generateName” that we argued for, follows rather directly - we’re trying to adhere to “the principle of the least astonishment” and mimic the behavior of other Kubernetes objects (name taking precedence over generateName is indeed the behavior in Kubernetes).

On the other hand the “workspace template” approach requires a different understanding of the objects involved. As alluded to already by @benoitf, it would be a logical approach if we were storing objects of kind “Devfile” and somehow from them created objects of kind “Workspace”. IMHO, this indirection doesn’t bring any value because there is no difference between “Devfile” object and “Workspace” object apart from the fact that the workspace object can be started/stopped. But what would the “Devfile” object be for?

So now, I’m going to assume that we agree on there being no need to store “Devfile” objects and that we want to only work with "Workspace"s (and please bring forward any usecases for “Devfile” objects). We want to follow the Kubernetes conventions, which dictate the name to be unique (within the kind and namespace). If we want to follow Kubernetes conventions we have no option but to require name to be unique and fail workspace creation if it isn’t or use generateName. If we encounter devfile with both name and generateName we have to use the name. This is the behavior that we actually implemented.

So now, I said that I wanted to support the workflows that @sunix advertizes in this issue but the above doesn’t exactly do that (and I understand the objections @sunix has had, because, really, we didn’t do what he asked for).

It is my belief that the “workspace template” workflow is best done using the workspace factory, which basically says “I’m going to create a workspace like that one”, as opposed to “I’m going to create this workspace” which the workspace endpoint does. I.e. /factory = “workspace template”, /workspace = “workspace definition”. This btw. means that che server currently doesn’t work like that yet - https://github.com/eclipse/che/issues/13683 is still open.

An ideal solution, in my mind, is done on the client side (as already pointed out by several others) and that’s because, in the end, it is the user’s decision what should happen with the workspace being created using the devfile - questions like “you already have a workspace called this name. Do you really want to create another one?”, “what should be the name of the new workspace?”. While the server side can implement logic to override fields of the devfile with values from query parameters for example, it is the client side that needs to inform the users about the state of the workspaces and ask for guidance.

IMHO, the attitude of “I want this to work like that because it always has” is not correct - we weren’t based on Kubernetes prior to Che 7.

What I would like to do is to do this properly - not introduce historical behavior that is going to be hard/unintuitive to revert in the future.

So the plan for me would be this:

• Add generateName so that we support the same naming logic as Kubernetes
• factory should never fail to create workspace due to name collisions - https://github.com/eclipse/che/issues/13683
• Implement overrides of devfile properties using query parameters during workspace creation
• teach chectl to use the devfile overrides during workspace creation
• (optional) make the workspace creation using chectl interactive

WDYT?

@sparkoo the PR is approved and I don’t think you need to wait any comment here to merge it.

My point of view is that #13683 is the real P1 and need to be fixed.

Making the devfile a kind of a template (a la OpenShift Templates) is a nice to have for me.

My two cents in this discussion, i think provide solution on client side more reasonable and flexible. For example, chectl got Already exists exception can be proposed several choices for user:

• recreate with same name
• start existed as is
• create new one with new name

Devfile definition is part of our API and if it have workspace name definition, API should respect it and don’t modify it without user interaction

Although pull request is already open (#14129), our team does not much like the solution and discussion now directs to generate random suffix. We would like to propose this solution: Inspired by Kubernetes (https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.14/#objectmeta-v1-meta), we would like to add a new field to the devfile-metadata - generateName. This field will be applied if and only if name is not defined. In that case, final workspace name will be generated like [generateName]-YYYYY, where Y is random [0-9a-z] character. In case that name is also defined, generateName will be ignored and name will be used and must be unique in the namespace. So devfile will look like this:

---
apiVersion: 1.0.0
metadata:
  generateName: python-
projects:
  -
    name: python-hello-world
    source:
      type: git
      location: "https://github.com/che-samples/python-hello-world.git"
components:
  -
    type: chePlugin
    id: ms-python/python/latest
    memoryLimit: 512Mi
  -
    type: dockerimage
    alias: python
    image: quay.io/eclipse/che-python-3.6:nightly
    memoryLimit: 512Mi
    mountSources: true
commands:
  -
    name: run
    actions:
      -
        type: exec
        component: python
        command: "python hello-world.py"
        workdir: ${CHE_PROJECTS_ROOT}/python-hello-world

(taken from https://github.com/eclipse/che-devfile-registry/blob/master/devfiles/python/devfile.yaml only changed in metadata-name/generateName field) and workspaces created from this devfile might look like python-123ab, python-1a2b3, …

We believe, that this solution is much more cleaner, explicit, predictable and future-proof. We would also follow Kubernetes pattern, so users that already know Kubernetes will come to familiar environment. Of course it will be more work as we would have to update our devfile specification, devfiles in devfile registry, and wsmaster code changes will be more heavyweight. However, we think that it’s worth the effort. Thoughts?