redhat-developer / opencompose

OpenCompose - A higher level abstraction for Kubernetes Resource
Apache License 2.0
64 stars 12 forks source link

add a mandatory "name" field to define containers #164

Closed concaf closed 7 years ago

concaf commented 7 years ago

This commit adds a mandatory field "name" to the containers definition in the OpenCompose spec. Using this field, we can mention the name of the container.

Prior to this commit, a minimal OpenCompose file would look like -

version: "0.1-dev"

services:
- name: foobar
  containers:
  - image: foo/bar:tag

Now, a "name" field for every container is mandatory to be specified-

version: "0.1-dev"

services:
- name: foobar
  containers:
  - name: baz
    image: foo/bar:tag

This commit also fixes docs, updates tests for the changes, updates examples.

concaf commented 7 years ago

This is blocking #47 and is a part of #149, @cdrage @kadel PTAL :)

kadel commented 7 years ago

Would it be possible to make name only mandatory if there are more containers in a pod. If there is only one we could use service name as the default name for the container.

Otherwise it looks good

concaf commented 7 years ago

Would it be possible to make name only mandatory if there are more containers in a pod. If there is only one we could use service name as the default name for the container.

@kadel I am not very positive about this :( , I think we should keep it mandatory all the while. This brings inconsistency in the sense if someone wants to add another container, they don't think about modifying the previous container. Also, when we want to delete this unnamed container, we won't have an identifier for it, so this also brings inconsistency in the $operation: delete part (#149)

kadel commented 7 years ago

I am not very positive about this :( , I think we should keep it mandatory all the while. This brings inconsistency in the sense if someone wants to add another container, they don't think about modifying the previous container.

My problem with requiring name is that in most cases it will be single container anyway, it's quite tedious to type container name that will be in most cases the same as service name :-(

Also, when we want to delete this unnamed container, we won't have an identifier for it, so this also brings inconsistency in the $operation: delete part (#149)

It won't be unnamed, it can't be. Kubernetes requires names for containers. Service name will be used for container name. I believe that its what we are doing right now.

kadel commented 7 years ago

But I guess we can go ahead with requiring name for containers, we can add default naming for single container services, later.

concaf commented 7 years ago

I have merged this and the the discussion can be carried forward at #166