Skip to content

Commit dad4ac9

Browse files
committed
sidecar: Add another PoC implementation
Reviewed-by: Joseph-Irving <[email protected]> Signed-off-by: Rodrigo Campos <[email protected]>
1 parent 8ab8258 commit dad4ac9

File tree

1 file changed

+29
-5
lines changed

1 file changed

+29
-5
lines changed

keps/sig-node/0753-sidecarcontainers.md

Lines changed: 29 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1043,8 +1043,8 @@ for examples). It was decided to not have a `standard` container type back then,
10431043
to avoid such issues.
10441044

10451045
We consider this weird but, as we can't do anything about it and we were okay in
1046-
the past, we propose to keep this as it is (it seems we were okay, judging from [that same
1047-
review comment][pr-kep-type-standard] and [this][apr-kep-api-ok]).
1046+
the past, we propose to keep this as it is. It seems we were okay, judging from [that same
1047+
review comment][pr-kep-type-standard] and [this][pr-kep-api-ok].
10481048

10491049
[pr-kep-type-standard]: https://github.com/kubernetes/kubernetes/pull/79649#discussion_r362658887
10501050
[pr-kep-api-ok]: https://github.com/kubernetes/kubernetes/pull/79649#issuecomment-589861200
@@ -1197,15 +1197,39 @@ This is very similar to the previous:
11971197
This semantic will just be a 1-1 translation from the proposed KEP to this
11981198
"Depends On" semantic, so it is easy for them to coexist.
11991199

1200-
#### Proof of concept implementations
1200+
### Proof of concept implementations
12011201

1202-
##### KEP implementation and Demo
1202+
#### KEP implementation and Demo
12031203

1204-
#### PoC and Demo
12051204
There is a [PR here](https://github.com/kubernetes/kubernetes/pull/75099) with a working Proof of concept for this KEP, it's just a draft but should help illustrate what these changes would look like.
12061205

12071206
Please view this [video](https://youtu.be/4hC8t6_8bTs) if you want to see what the PoC looks like in action.
12081207

1208+
#### Another implementation using pod annotations
1209+
1210+
Another implementation using pod annotations on top of Kubernetes 1.17 is
1211+
available [here][kinvolk-sidecar-branch].
1212+
1213+
There are some example yamls in the [sidecar-tests
1214+
folder][kinvolk-poc-sidecar-test], also the yaml output was captured to easily
1215+
see the behavior. [See the commit that created
1216+
them][kinvolk-poc-sidecar-test-commit] for instruction on how to run it.
1217+
1218+
Some other things worth noting about the implementation:
1219+
* It is done using pod annotations so it is easy to test for users (doesn't
1220+
modify pod spec)
1221+
* It implements the KEP proposal and not the suggested modifications, with one
1222+
exception: the podPhase is not modified.
1223+
* [This line][kinvolk-poc-sidecar-prestop] should be modified to use the
1224+
`TerminationHook` instead, if such alternative is chosen
1225+
* There is some c&p code to avoid doing refactors and just have a patch that is
1226+
simpler to cherry-pick on different Kubernetes versions.
1227+
1228+
[kinvolk-poc-sidecar-prestop]: https://github.com/kinvolk/kubernetes/blob/52a96112b3e7878740a0945ad3fc4c6d0a6c5227/pkg/kubelet/kuberuntime/kuberuntime_container.go#L851
1229+
[kinvolk-sidecar-branch]: https://github.com/kinvolk/kubernetes/tree/rata/sidecar-ordering-annotations-1.17
1230+
[kinvolk-poc-sidecar-test]: https://github.com/kinvolk/kubernetes/tree/52a96112b3e7878740a0945ad3fc4c6d0a6c5227/sidecar-tests
1231+
[kinvolk-poc-sidecar-test-commit]: https://github.com/kinvolk/kubernetes/commit/385a89d83df9c076963d2943507d1527ffa606f7
1232+
12091233
### Risks and Mitigations
12101234

12111235
You could set all containers to have `lifecycle.type: Sidecar`, this would cause strange behaviour in regards to shutting down the sidecars when all the non-sidecars have exited. To solve this the api could do a validation check that at least one container is not a sidecar.

0 commit comments

Comments
 (0)