Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Devfile should have a schemaVersion attribute #7

Closed
l0rd opened this issue Feb 5, 2020 · 10 comments
Closed

Devfile should have a schemaVersion attribute #7

l0rd opened this issue Feb 5, 2020 · 10 comments
Labels
area/api Enhancement or issue related to the api/devfile specification kind/enhancement New feature or request
Milestone

Comments

@l0rd
Copy link
Contributor

l0rd commented Feb 5, 2020

In previous version we used to have a apiVersion attribute. We would like to rename it schemaVersion. The main reason is because the devfile format versioning doesn't adhere to the Kubernetes versioning scheme and calling it schemaVersion will avoid confusions.

@l0rd l0rd added area/api Enhancement or issue related to the api/devfile specification kind/enhancement New feature or request labels Feb 5, 2020
@l0rd l0rd mentioned this issue Feb 5, 2020
28 tasks
@davidfestal
Copy link
Collaborator

I already implemented a proposal of this in the current devfile schema on the master branch.

I pulled the name property back to the top-level
and changed the old apiVersion (which was falsly K8S-like) to schemaVersion, since here we're interested in the version of the schema itself, not the K8S API.

This gives:

name: "devfile example"
schemaVersion: "2.0.0"
projects:
components: 
...

Of course naming can be changed for something better.

@davidfestal
Copy link
Collaborator

duplicate of #10 ?

@amisevsk
Copy link
Contributor

@davidfestal are we dropping generateName from the spec? Pulling name out to the top level may be confusing if it's pseudo-optional.

@davidfestal
Copy link
Collaborator

@amisevsk I'm not sure generateName really makes sense here. Let me explain my initial reasoning:

During the discussions that happened in the context of devfile 1.0 some months ago, about possibly adding the workspace target namespace as a devfile attribute, it was finally concluded that:

  • the devfile is mainly the description of the structure of the workspace, what the workspace contains,
  • and so we should avoid adding devfile attributes for information about where and how related workspace instances will be used.

So in the context of Che, target namespace, as well as debug mode more recently, were implemented on the Che-server side as parameters of the Start Workspace API endpoint.

It seems to me that generateName somehow falls into the same category: it is mainly a matter of how a given implementation (Che server for example) will name workspace instances from the workspace structure definition provided as a devfile. Not sure it requires a distinct generateName field for that, especially knowing that we don't expect the Devfile 2.0 syntax to be a pure Kubernetes resource at the top-level anymore.

But I might be missing some point, and if a generateName (or similar) field is really meaningful and useful, let's change the currently implemented proposal.

@metlos
Copy link

metlos commented Feb 25, 2020

In the same vein, should the devfile schema really have a name? If I simplify the picture, we could consider the devfile a spec of a workspace k8s object. As far as I am aware specs don't have a name of their own.

@amisevsk
Copy link
Contributor

I agree, I tend to lean towards either no name or name and/or generateName. How is the name field in a devfile intended to be consumed?

@davidfestal
Copy link
Collaborator

In the same vein, should the devfile schema really have a name? If I simplify the picture, we could consider the devfile a spec of a workspace k8s object. As far as I am aware specs don't have a name of their own.

Well, the content of the devfile (apart from name and version) is the spec of the workspace structure. In fact the part of the devfile schema is the same (or derived from) the DevWorkspaceTemplateSpec Go struct.

When this workspace template spec is used in the context of a K8S CRD, it is referred in both a DevWorkspace (workspace instance) or a DevWorkspaceTemplate (workspace template stored for further use/reference). But, as native K8S CRD objects, both have metadata/name and apiVersion fields in addition to the DevWorkspaceTemplateSpec struct itself.

Similarly on the Devfile side, the same workspace template spec schema also defines most of the devfile content. But this doesn't mean that the complete devfile schema doesn't need a name and a version. The main difference is that, in the case of the devfile, unlike K8S CRDs, we can add name and version fields wherever we want inside the overall devfile 2.0.0 schema.

How is the name field in a devfile intended to be consumed?

How the name would be used is the responsibility of the implementation that uses the format I assume.
But a typical example is the way the existing Che server works: it would derive the name of the workspace instance it creates, from the name of the devfile (which is fact is a workspace template), according to rules that are Che-specific.
But if the devfile has no name at all, it seems to me that we are removing a quite important aspect of the workspace template semantic which is the identity of the template.

To summarize, it seems to me that:

  • we should keep the identity of the workspace template provided by the devfile => name,
  • but could remove the additional instruction about how workspace instances should be created from a devfile => generateName

@metlos
Copy link

metlos commented Mar 2, 2020

I am not sure I understand the need to identify the devfile by its name. I think we're actually talking about 3 things in the discussion above:

  1. A standalone devfile.yaml in a VCS. Even here, I don't necessarily see a hard reason for the devfile to have a name, because its identity is tied to the repo it lives in.
  2. A workspace object in k8s - IMHO here devfile is akin to PodSpec - while a pod does have a name, PodSpec doesn't.
  3. A workspace template as an object in k8s - again, the identity stems from the k8s object, not from the spec.

Now if you wanted to assert whether the 3 things above are actually the same structurally - I think it is much safer to actually compare the devfiles in them structurally than to rely on a "name". After all 2 independent authors of 2 standalone devfiles may choose the same name for a completely different structure and it would be an error to consider them the same.

Coming back to the issue of devfile version - here I like @davidfestal's schemaVersion because clearly conveys the purpose of that value - it is the version of the devfile schema, not that of the devfile itself.

@l0rd l0rd changed the title Devfile should have a version attribute Devfile should have a schemaVersion attribute Mar 10, 2020
@l0rd
Copy link
Contributor Author

l0rd commented Mar 10, 2020

Reviewed and agreed

@l0rd l0rd added this to the 2.0.0 milestone Jun 16, 2020
@l0rd
Copy link
Contributor Author

l0rd commented Jun 24, 2020

Closing this as it has been implemented in the spec.

@l0rd l0rd closed this as completed Jun 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/api Enhancement or issue related to the api/devfile specification kind/enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants