-
Notifications
You must be signed in to change notification settings - Fork 40
[spec change] Make containers a subkey #119
Comments
@cdrage so I'm not looking at it from "making containers a subkey" angle, maybe more like the discussions we are having at the Stateful Sets issue - #110. So, maybe having controllers at the top level of the spec, like "deployment", "statefulSets", "jobs" at the top level, and everything inside them, including services. For instance, |
@containscafeine This is getting confusing between Kubernetes services and our definition of "services". Perhaps we need to draw a line to say that these are Kubernetes services and not a "services" definition. But yeah, that's an alternative way that we could do it. |
@cdrage yep, the services definition we have is the Kubernetes services, we don't have a services definition (meaning microservices or some such) (AFAIK) |
Actually these kedge services are Kubernetes services. At root level we have: Services []ServiceSpecMod `json:"services,omitempty"` Where type ServiceSpecMod struct {
api_v1.ServiceSpec `json:",inline"`
Name string `json:"name,omitempty"`
Ports []ServicePortMod `json:"ports"`
} see the line: api_v1.ServiceSpec `json:",inline"` It is actually a Kubernetes service spec embedded in it. |
As discussed in here #110 (comment) there are two approaches here either add a root level key for each controller: First approachfor deployment: deployment:
containers:
- image: centos/httpd for job job:
containers:
- image: gitcrawler vs Second approachfor deployment: containers:
- image: centos/httpd for job: kind: job
containers:
- image: gitcrawler Speaking for myself I am inclined towards the second approach. Because it is less indentation, also if user does not provide the Also in second approach we can do something similar to how kubernetes does the detection based on the kind field. So that we don't need to populate the app struct with everything, it will rather be a clean segregated structs depending on the kind everything will be happening. |
Thinking about it, there are 2 approaches -
deployment:
name: httpd
containers:
- image: centos/httpd
services:
- name: httpd
type: NodePort
ports:
- port: 8080
job:
name: piJob
containers:
- image: perl
name: pival
command: ["perl", "-Mbignum=bpi", "-wle", "print bpi(2000)"]
restartPolicy: Never
parallelism: 3
name: httpd
kind: deployment # optional, defaults to deployment
containers:
- image: centos/httpd
services:
- name: httpd
type: NodePort
ports:
- port: 8080
---
kind: job
containers:
- image: perl
name: pival
command: ["perl", "-Mbignum=bpi", "-wle", "print bpi(2000)"]
restartPolicy: Never
parallelism: 3 |
Let's keep this thread/issue for discussing the approach of how we gonna enable multiple controllers |
@containscafeine you did not suggest what approach you wanna go with? |
I'm inclining towards the second one, i.e. introducing a ping @kadel @jstrachan would love your feedback on this :) |
Seems so messy, that's the problem :( People will have to memorize that if it is a "job" or anything that's not a service, it's not going to have the "root" key of For @containscafeine 's suggestion 👍 for the first approach, the second approach get's confusing as suddenly you have "services" but in "job" you don't (everything is a root key? for restartPolicy nad parrallelism? what?) As how @surajssd described, the |
For @containscafeine I believe this is wrong:
It should be formatted like this: deployment:
name: httpd
containers:
- image: centos/httpd
services:
name: httpd
containers:
- image: centos/httpd
type: NodePort
ports:
- port: 8080
job:
name: piJob
containers:
- image: perl
name: pival
command: ["perl", "-Mbignum=bpi", "-wle", "print bpi(2000)"]
restartPolicy: Never
parallelism: 3s Considering the first approach is to device them into separate root-level keys ^^. Of course, with this example, we're manually deploying |
As a rule we decided that one file (or one section of the file) is going to describe one application. In that sense, one application will always be just one Deployment or one StatefullSet. We don't need a way to define both in the same file. I would like to keep it is without making containers subkey |
@kadel How do you think we should implement the different kinds? If we want separate files, ideally i'd be:
then. |
I got to this discussion later, so I'm still a bit confused but why we can't do jobs just like list of JobSpec under jobs key? jobs:
- name: job1
template:
metadata:
name: pi
spec:
containers:
- name: pi
image: perl
command: ["perl", "-Mbignum=bpi", "-wle", "print bpi(2000)"]
restartPolicy: Never this will also solve #114 |
@kadel Because it's confusing having containers as a root key as well as a subkey. Basically, if you defining a It should be the same between the two. Moving |
I'm more and more confused :-( |
@kadel Yeah, they do. And like how you're confused, it confuses me too: name: web
containers:
- image: centos/httpd
services:
- name: httpd
type: NodePort
ports:
- port: 8080 Should be: name: web
services:
- name: httpd
containers:
- image: centos/httpd
type: NodePort
ports:
- port: 8080 |
No. The second one is wrong. |
Ah okay. Hence the confusion because we have two definition of "services" within Kedge at the moment. One is similar to how it's the same as Docker Compose (App Services) and the other being Kubernetes (Services, networking like you said). What implementation do you think we should go through with? @containscafeine 's or @surajssd 's? |
There should be only one service in Kedge, and that is Kubernetes Service.
In most cases, users will define Deployments. I would like to keep it as simple as possible for deployments. Because of that I like what is @surajssd proposing as the second approach in #119 (comment) But I don't think we need "kind" filed for Jobs. I would use this only for "StatefulSets" |
@kadel There are other use-cases such as DaemonSets / Jobs / CronJobs which this would be useful for. |
Jobs and CronJobs are a little bit different. They are not usually standalone applications, they complement application that is represented by Deployment. This is also why Jobs have separate root level key as they are proposed right now. DoeamonSets are really specific, they are used for cluster wide things, as such they are rarely used by regular developers. DaemonSets are almost exclusively used by cluster managers. I don't think we need some extra support for it beyond what is described in #157 |
@kadel I believe adding kind would be beneficial to create extensive examples for deployments. StatefulSets are becoming more popular as it's beginning to be the go-to for databases. Why not add Limiting Kedge to only Services and Deployments would be limiting to what people can do with Kedge. Sure, we can add |
I'm not saying that Kedge shouldn't do StatefulSets. Only that I wouldn't worry about it for now. As you are saying its used for databases. Developer's don't deploy databases that often, and if they do its one-time thing. Its OK to force people to learn Kubernetes artifacts. Kedge is using Kubernetes artifacts everywhere. There is no field that is redefined its all just Kubernetes primitives. Simplification is there in the form that users don't have to write everything, a lot of fields can be computed or deduced from others. |
I would like to see If we consider making Also we should not assume Also all the controllers use the same pod spec, this also allows us to reuse the So for the sake of consistency on the user side, we should add root level string field |
I feel that this will go away from the whole idea of defining an application, and stepping back closer to Kubernetes OPs view.
I'm not saying that this will be always the case, but its most common.
|
If we really add Kind object that we can go back to Kubernetes resources and just provide some additional shortcuts but keep the structure of the object intacted. Otherwise we are creating some strange hybrid that will just be more and more confusing for everyone. |
I'm against the deployment:
name: httpd
containers:
- image: centos/httpd
services:
name: httpd
containers:
- image: centos/httpd
type: NodePort
ports:
- port: 8080
job:
name: piJob
containers:
- image: perl
name: pival
command: ["perl", "-Mbignum=bpi", "-wle", "print bpi(2000)"]
restartPolicy: Never
parallelism: 3s Thus avoiding confusion of a weird "hybrid" between Kedge and Kubernetes artifacts. |
@cdrage @kadel the more I think and talk about Jobs, it seems that they are generally not tied to one part of the application, in fact they are tied to multiple parts. e.g., populate the "database", so "web" can use it; train a "model", so the "API gateway" can use it. So use cases for jobs are mostly tied to >=2 parts of the broader application, and it makes more sense to me to go the "kind: Job" or "controller: Job" way in a separate file. Also, IMHO, we are going in circles with this issue since everyone has a "different" opinion on the implementation and nothing is right or wrong, we need to decide on this. Should we ping more folks, ask for opinion, or maybe get into a meeting tomorrow and decide this? |
Okay, so we are not able to reach a decision on this. Ping @bparees @jstrachan, would love your opinion on this, thanks! Problem Statement: Proposed Solutions:
name: httpd
containers:
- image: centos/httpd
services:
- name: httpd
type: NodePort
ports:
- port: 8080
jobs:
- containers:
- image: perl
name: pival
command: ["perl", "-Mbignum=bpi", "-wle", "print bpi(2000)"]
restartPolicy: Never
parallelism: 3 Cons:
name: httpd
deployment:
containers:
- image: centos/httpd
services:
- name: httpd
type: NodePort
ports:
- port: 8080
job:
containers:
- image: perl
name: pival
command: ["perl", "-Mbignum=bpi", "-wle", "print bpi(2000)"]
restartPolicy: Never
parallelism: 3 Cons:
httpd.yaml - name: httpd
controller: deployment # optional, defaults to deployment
containers:
- image: centos/httpd
services:
- name: httpd
type: NodePort
ports:
- port: 8080 job.yaml - controller: job
containers:
- image: perl
name: pival
command: ["perl", "-Mbignum=bpi", "-wle", "print bpi(2000)"]
restartPolicy: Never
parallelism: 3 Cons:
|
@containscafeine To note, this also reflects other implementations such as StatefulSets, DaemonSets, CronJobs when they are implemented in the future, for example: statefulSet:
- containers:
- image: mysql
name: mysql
replicas: 3 |
an application is what you want it to be. if you don't want your job to be part of an existing app because it's more "global" than that, define it in a second application. ie option 1. this is assuming you are committed to the "application as a top level config container" concept for your DSL. |
@bparees -
do you mean option 3? |
no I mean option 1. I would then envision having, for example, 3 files:
|
We've had a lot of conversation about this issue, both here and at our Slack https://kedgeproject.slack.com/. There were 3 options and all of those were "different", there was no right or wrong. The most pressing argument against Option 1 was that it brings about inconsistency in the spec, in the sense that Kubernetes' Deployment controller is given special treatment compared to other controllers like StatefulSets and Jobs, because a deployment can be defined at the root level and not other controllers, those are supposed to have their own field. As for Option 3, it might not make sense when a job has to be tied with a deployment, since both the definitions have to be def In the end, we decided to vote, since everyone was very, very divided on which way to go forward with. The vote count was four for Option 3 and three for Option 1. So, for now, we are going forward with Option 3, and time bounding it for 6 weeks till we get some feedback when we deploy real world applications. If it such happens that the UX does not make sense for Option 3, we will reiterate on this issue. |
Closed by #195 |
We're running into confusion as per: #116 as well as #110 which causes issues such as these to occur:
Which IMO is really confusing.
I propose we make
containers
a subkey. So instead, we have this:Thus, we can also implement #110 (for example), by having separate root keys:
The text was updated successfully, but these errors were encountered: