Che: Make Devfile syntax K8s-compliant

[davidfestal]

Description

In the context of the work on Che Workspace CRDs (https://github.com/eclipse/che/issues/12933), Devfile content is becoming used in Kubernetes Custom Resources. More precisely it is a Custom Resource itself, as well as part of the Workspace Custom Resource as show on this example:

Workspace Custom Resource Example

apiVersion: workspace.che.eclipse.org/v1beta1
kind: Workspace
metadata:
  labels:
    controller-tools.k8s.io: "1.0"
  name: workspace-sample
  namespace: crds
spec:
  started: true
  devfile:
    apiVersion: 0.0.1
    metadata:
      name: chectl 
    projects:
      - name: chectl
        source:
          type: git
          location: 'https://github.com/che-incubator/chectl.git'
    components:
      - name: theia-ide
        type: cheEditor
        id: org.eclipse.che.editor.theia:next
      - name: exec-plugin
        type: chePlugin
        id: che-machine-exec-plugin:0.0.1
      - name: mvn-stack
        type: dockerimage
        image: maven:3.5.4-jdk-8
        command: ['/bin/sh', '-c']
        args: ['tail -f /dev/null']
    commands:

Based on this let me do 2 remarks

Name and version

To be a CR and inside a CR, the devfile in the POC had to be be slightly modified to provide a name and version that would be kubernetes-compliant:

apiVersion: 0.0.1
metadata:
    name: chectl 

instead of:

specVersion: 0.0.1
name: chectl

Would it be possible to include this change in the devfile format and json schema in upstream Che ? Since it seems a lightweight change, I assume it would be something to do before GA ?

Of course this doesn't answer all the question, since I assume that to be fully compliant, especially if Devfile CRs were to be used standalone, outside of a Workspace CR. In this case I imagine that the version would be more of the form workspace.che.eclipse.org/v1beta1, which is indeed the version of the K8s API group that contains both Workspace and Devfile CRs. In fact a standalone devfile CR would be defined like that:

Devfile standalone Custom Resource Example

apiVersion: workspace.che.eclipse.org/v1beta1
kind: DevFileSpec
metadata:
  name: chectl 
projects:
  - name: chectl
    source:
      type: git
      location: 'https://github.com/che-incubator/chectl.git'
components:
  - name: theia-ide
    type: cheEditor
    id: org.eclipse.che.editor.theia:next
  - name: exec-plugin
    type: chePlugin
    id: che-machine-exec-plugin:0.0.1
  - name: mvn-stack
    type: dockerimage
    image: maven:3.5.4-jdk-8
    command: ['/bin/sh', '-c']
    args: ['tail -f /dev/null']
commands:

Note the additional k8s-standard kind attribute there.
So the question of how we manage the Devile version in case a Devfile is both a general-purpose format and a Kubernetes object should probably be discussed.

Polymorphic Components

Currently, the Devfile Json schema allows having several types of components that have differents types of attributes according to the type of component.
On the Kubernetes side, custom resources are defined in a Go-first way, that, obviously doesn't allow such polymorphic lists.
So in the current definition of the Devfile CR, the ComponentSpec Go Type includes all the attributes of all the component types, and has to make most of them optional, as you can see here: https://github.com/che-incubator/che-workspace-crd-controller/blob/33d853f975d5dca7ac7fd93786c2217fcfe236b8/pkg/apis/workspace/v1beta1/devfile.go#L42

This obviously makes things less "typesafe".

2 solutions for this:

• Keep it as it is now and add, in the future, a validation web hook that implements the additional rules contained in both the Json Schema and in the addition Che server logic
• Change the Devfile syntax to introduce several sub-lists of components: one for each type of component, such as:

components:
    cheEditor:
        - name: theia-ide
          id: org.eclipse.che.editor.theia:next
    chePlugin:
        - name: exec-plugin
          id: che-machine-exec-plugin:0.0.1
    dockerimage:
        - name: mvn-stack
          image: maven:3.5.4-jdk-8
          command: ['/bin/sh', '-c']
          args: ['tail -f /dev/null']
etc ...

@l0rd Should we decide on this before GA ?

• [ ] [Devfile] Make specVersion and name more K8s-like https://github.com/eclipse/che/issues/13336

[l0rd]

Thank you @davidfestal nice explanation

@skabashnyuk @mshaposhnik @sleshchenko @metlos I would like to discuss these topic with you next week

[amisevsk]

FWIW I think it's a good idea if it can be done. If nothing else it's using a format a developer might be more familiar with.

[l0rd]

@gorkem it would be nice if you could have a look at @davidfestal proposal

[l0rd]

Some remarks after a discussion we had internally:

• For Che 7 GA we are not going to change the components section format (and probably we will never do it because it's harder to read/write). We should change devfile specification to be CRD compatible though.
• The proposed devfile format doesn't have a spec section as usually other k8s resources have: shouldn't it have one?
• What about setting a relation between workspace and devfile resources similar to PV and PVC resources?More decoupled with respect to the pod/deployment relation.
• We have recently added attributes in devfile, if we move to this new model we may want to use metadata/annotations instead
• If the devfile can be created without a workspace shouldn't be the type Devfile instead of DevfileSpec?

[garagatyi]

My 0.05c:

• I like idea of being able to do kubectl apply devfile.yaml, thus I'm +1 to change format of components as David suggested. In my opinion it doesn't complicate devfile writing. And, probably, this would significantly simplify both CRD Go code and WSmaster Java code
• taking into account previous point I would like to make devfile root object. This would also help to promote devfile format for kubernetes native dev workspaces without making users stick to Che and use just devfile format and kubectl
• +1 to proposals about Devfile vs DevfileSpec, spec field, metadata/annotations, and version in specified in the kubernetes way

[skabashnyuk]

@l0rd can we remove GA label from this issue in favor of https://github.com/eclipse/che/issues/13336

[l0rd]

@skabashnyuk ok

[davidfestal]

Some remarks after a discussion we had internally:

• For Che 7 GA we are not going to change the components section format (and probably we will never do it because it's harder to read/write).

Yes, probably the solution with using a validation webhook would be acceptable, until the native k8s CRD OpenAPI support would be enriched to support more of the Devfile JSON schema constraints.

We should change devfile specification to be CRD compatible though.

• The proposed devfile format doesn't have a spec section as usually other k8s resources have: shouldn't it have one?

I assume it could yes: that would make it more k8S-like, even though that doesn't seem a requirement since, even when having standalone devfile CRs, it would mainly be data objects, without any underlying K8s objects, which makes the distinction between Status and Spec a bit meaningless in this case.

• What about setting a relation between workspace and devfile resources similar to PV and PVC resources?More decoupled with respect to the pod/deployment relation.

I'm OK with that. That would also make it easier to support workspaces created from an initial devfile, but then modified during development. In such a case, the effective workspace definition would be stored in the workspace status.

• We have recently added attributes in devfile, if we move to this new model we may want to use metadata/annotations instead

• If the devfile can be created without a workspace shouldn't be the type Devfile instead of DevfileSpec?

Well I assume that if we add a Spec section (see above), then we would have Devfile and DevfileSpec.

[l0rd]

@davidfestal ok so if it's not required I would rather not add the spec attribute (and DevfileSpec).

[skabashnyuk]

@l0rd @davidfestal I propose to close this issue. As I understand it would be a goal of Devfile 2.0 epic.

[davidfestal]

Sure. I think we can close it