004: Add proposal for sidecar containers in Spin Operator

[Mossaka]

CC @michelleN @endocrimes @kate-goldenring @calebschoepp

[rajatjindal]

I think we may want not want to share all environment variables and volumes by default. e.g. app may have some creds that we may not want to expose to init containers

[calebschoepp]

I'm sold on the use case for this.

My main concern is the amount of support burden this opens us up to. Supporting all of init/sidecar containers and the whole corev1.Container type is a lot. That being said my hunch is it would be worth it.

I definitely think we need to have more discussion about this.

[calebschoepp]

Doesn't the term init container imply that the container is short lived and for setup — not long lived and running as a sidecar? https://kubernetes.io/docs/concepts/workloads/pods/init-containers/

[calebschoepp]

Kubernetes implements sidecar containers as a special case of init containers; sidecar containers remain running after Pod startup. This document uses the term regular init containers to clearly refer to containers that only run during Pod startup.

Saw this later

[ThorstenHans]

^^
Although there is reasoning why InitContainers would be the correct term, I would suggest using two dedicated terms instead: e.g. Initializers and Sidecars.

I would also prefer embedding the corev1.Container struct into a higher level struct, allowing users to specify the desired runtime class. This would unlock the capability to run either:

• Spin Apps (e.g. using the command trigger) or traditional containers during initialization
• Spin Apps or traditional containers as sidecars.

apiVersion: core.spinoperator.dev/v1alpha1
kind: SpinApp
metadata:
  name: simple-spinapp
spec:
  image: "ttl.sh/spinapp:44h"
  replicas: 1
  executor: containerd-shim-spin
  volumes:
    - name: example-volume
      persistentVolumeClaim:
        claimName: example-pv-claim
  volumeMounts:
    - name: example-volume
      mountPath: "/mnt/data"
  initializers:
    - name: my-init-container
      runtimeClass: default # (traditional container) could also be omitted
      spec:
        image: busybox:1.28
        command: ['sh', '-c', "until nslookup mydb.$(cat /var/run/secrets/kubernetes.io/serviceaccount/namespace).svc.cluster.local; do echo waiting for mydb; sleep 2; done"]
  sidecars:
    - name: spin-sidecar
      runtimeClass: wasmtime-spin-v2
      spec:
        image: "ttl.sh/spin-logshipper:44h"

[calebschoepp]

I like the idea of splitting init containers and sidecar containers.

I feel a bit weird about directly specifiying the runtimeClass on the init/sidecar containers because we don't do that for the root SpinApp, rather we refer to an executor which references the runtimeClass. This would logically lead us to specifying executor for the init/sidecar containers which quickly falls apart because we need all these to run in the same pod.

I guess as I think about it I think we should just force init/sidecar containers to use the same runtime class as the root spin app. I believe the shim is smart enough to just do that.

[Mossaka]

I am not sure if I understand the motivation to specify runtimeClass for Sidecars and initializers here. AFAIU, RuntimeClass should be specified to a Pod, instead of individual containers. Also, as @calebschoepp described, the SpinApp has a executor that maps to a RuntimeClass in the Pod.

Regarding splitting init containers and sidecar containers, I am concerned that it will increase the learning curve for users if they are already familiar with the sidecar container concept in Kubernetes, which use the initContainers syntax.

[kate-goldenring]

+1 to using initContainers terminology. @Mossaka is there any case where a k8s init container could not be run on the shim? I am guessing no. Want to make sure we can say "anything that is an init container (and doesnt use port 80) cna go here"

[Mossaka]

The expectation for the shim is that it can run any containers that comform to the OCI Spec, including the init containers. If in pactice we find there are init containers that cannot be run in the Spin shim, we should regard it as a bug.

[calebschoepp]

Can sidecar containers be wasm applications or only Linux images? Do we not care?

[Mossaka]

We don't care as the executor can execute both Spin application and Linux containers side-by-side.

[endocrimes]

this actually solves the database migration problem for us too, which is nice (at least when folks don't have ✨ other tools ✨ to do them out of band of app deployments)

[kate-goldenring]

LGTM! Adding init containers would be a great feature add to SpinKube

[kate-goldenring]

@Mossaka does kubelet handle ensuring init containers are executed first?

[calebschoepp]

Good question!

[Mossaka]

"Init containers must run to completion before the Pod can be ready; sidecar containers continue running during a Pod's lifetime, and do support some probes."

from https://kubernetes.io/docs/concepts/workloads/pods/init-containers/

[calebschoepp]

I still have some DX questions like what should we name this feature in the CRD, but I'm on board that this would be good for SpinKube and unlock a lot of use cases. I figure these DX nits can be addressed in the actual PR.

LGTM!

[jsturtevant]

could you add a little detail on why these ideas are were considered and then discarded? (pros/cons)

[jsturtevant]

This Kubernetes feature was new to me. I was thinking about the sidecar pattern and didn't realize it was an actual feature. In the description above could you add sentence on the difference and why Sidecar feature (capital S) was chosen?

[jsturtevant]

Overall I am in favor of being able to deploy sidecars via the SpinApp 🚀

My concern would be trying to keep up with the Kubernetes Pod spec and how we ensure we are providing value add through the SpinApp CRD and not just implementing the pod spec in a different format.

Mainly my concern with adding the Sidecar feature is whether it meets the needs of apps using the sidecar pattern? If we add this but users actually want the sidecar pattern (multiple containers in a pod) then do we also need to add support for multiple containers? Or is there another way to make this a bit more adjustable so we could meet either scenario? Maybe we don't need to do either but would love to see a bit more discussion on the reasoning for those choices in the SKIP documentation.

[jsturtevant]

Is there a use case where sidecars can be another Wasm app?

[Mossaka]

There are use cases for Wasm app as init containers, but not as sidecars AFAIK so far. In theory, Wasm could be a great alternative to Linux sidecar due to it's advantage in memory usage and artifact size.

[jsturtevant]

Why port 80? Isn't that configurable?

[Mossaka]

relevant discussion: spinkube/spin-operator#326