change to sidecar field in container spec

keps/sig-apps/0008-sidecarcontainer.md

@@ -6,6 +6,7 @@ authors:
 owning-sig: sig-apps
 participating-sigs:
   - sig-apps
+  - sig-node
 reviewers:
   - TBD
 approvers:
@@ -34,11 +35,11 @@ status: provisional
 
 ## Summary
 
-To solve the issue of sidecars not being well supported we can create a new class of container in the Pod Spec called sidecarContainers which are handled like normal containers but with the caveat that a normal container's status could take precedence over the sidecar's status. This will solve the problem of Jobs not completing/failing when a sidecar container never exits due to it having no awareness of the 'primary' container.
+To solve the issue of sidecars not being well supported we can create a new type of container called a sidecar which are handled like normal containers but with the caveat that a normal container's status could take precedence over the sidecar's status. This will solve the problem of Jobs not completing/failing when a sidecar container never exits due to it having no awareness of the 'primary' container.
 
 ## Motivation
 
-SideCar containers have always been used in some ways but just not formally identified as such, with the introduction of Jobs and Cronjobs it became clear that they're not handled very well. If you have a Job with two containers one of which is actually doing the main processing of the job and the other is just facilitating it, you encounter a problem when the main process finishes; your sidecar container will carry on running so the job will never finish. This only way around this is to build some kind of logic into the sidecar so that it will exit when the other container exits. This can be quite difficult if you're using a sidecar provided by a third party and results in some nasty hacks being implemented to achieve this.
+SideCar containers have always been used in some ways but just not formally identified as such, with the introduction of Jobs and Cronjobs it became clear that they're not handled very well. If you have a Job with two containers one of which is actually doing the main processing of the job and the other is just facilitating it, you encounter a problem when the main process finishes; your sidecar container will carry on running so the job will never finish. The only way around this is to build some kind of logic into the sidecar so that it will exit when the other container exits. This can be quite difficult if you're using a sidecar provided by a third party and results in some nasty hacks being implemented to achieve this.
 
 ### Goals
 
@@ -50,8 +51,8 @@ Changing the way jobs are handled
 
 ## Proposal
 
-Create a new class of containers called sidecarContainers, this will be an additional field to the Pod Spec.
-e.g
+Create a way to define containers as sidecars, this will be an additional field to the Container Spec: `sidecar: true`.
+e.g:
 ```yaml
 apiVersion: v1
 kind: Pod
@@ -64,17 +65,17 @@ spec:
   - name: myapp
     image: myapp
     command: ['do something']
-  sidecarContainers:
   - name: sidecar
     image: sidecar-image
+    sidecar: true
     command: ["do something to help my app"]
 
 ```
-These sidecar containers will be handled the same way as a normal container is almost all respects apart from the fact that in certain situations their status will be ignored:
+These sidecar containers will be handled the same way as a normal container in almost all respects apart from the fact that in certain situations their status will be ignored:
 * When all normal containers are in `Completed` State
 * When all normal containers are in `Error` State
 In a job for example this means that you can have as many sidecars as you like and they will all get terminated when the normal containers all Complete or if all normal containers error.
-This model assumes that the sidecars are not intended to reliant on your other containers running and your app will only pass readiness check if the sidecar is successfully running.
+This model assumes that the sidecars are not intended to be reliant on your other containers running and your app will only pass readiness check if the sidecar is successfully running.
 
 
 ### Implementation Details/Notes/Constraints [optional]
@@ -83,7 +84,8 @@ This model assumes that the sidecars are not intended to reliant on your other c
 
 ### Risks and Mitigations
 
-//WIP
+You could set all containers to be `sidecar: true`, this seems wrong, so maybe the api should do a validation check that at least one container is not a sidecar.
+Init containers would be able to have `sidecar: true` applied to them as it's an additional field to the container spec, there's no situation where this would make sense as init containers are executed sequentially. We could get around this by having the api throw a validation error if you try to use this field on an init container.
 
 ## Graduation Criteria
 
@@ -106,5 +108,5 @@ Major milestones might include
 
 ## Alternatives
 
-One alternative would be to have a new field in the Container Spec of `sidecar: true` however this could be quite inefficient if you had a lot of sidecars.
+One alternative would be to have a new field in the Pod Spec of `sidecarContainers:` where you could define a list of sidecar containers, however this would require a lot more work in terms of updating tooling to support this.
 Another alternative would be to change the Job Spec to have a `primaryContainer` field to tell it which containers are important. However I feel this is perhaps too specific to job when this Sidecar concept could be useful in other scenarios.