Skip to content
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
179 changes: 179 additions & 0 deletions proposals/004-skips/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,179 @@
# SKIP 000 -Sidecar Containers in Spin Operator

Summary: This proposal introduces sidecar container support in the Spin Operator, enabling Spin applications to run alongside Linux containers within the same pod. This allows for added functionality such as monitoring, logging, and security.

Owner: [email protected]

Impacted Projects:

- [x] spin-operator
- [ ] `spin kube` plugin
- [ ] runtime-class-manager
- [ ] containerd-shim-spin
- [ ] Governance
- [ ] Creates a new project

Created: October 16th, 2024

Updated: October 16th, 2024

## Background

The [Runwasi] project supports running Wasm workloads in the same pod as Linux containers. Currently, the SpinApp Custom Resource Definition (CRD) does not support running [Sidecar Containers]. Sidecars are important in use cases where additional services need to run in the same pod as the Spin application, enabling low-latency communication and resource sharing.

[Sidecar Containers]: (https://kubernetes.io/docs/concepts/workloads/pods/sidecar-containers/)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

[Runwasi]: (https://github.com/containerd/runwasi)
[SpinApp CRD]: (https://www.spinkube.dev/docs/reference/spin-app/)
[Spin Operators repository]: (https://github.com/spinkube/spin-operator)

### User Stories

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.


- As an user, I have a proprietary solution packaged as a binary that cannot be ported to Wasm. I would like to run this binary as a Linux container. Also, I would like to run this container in the same pod as the Spin application so that they can communicate with each other locally. In addition, I want the sidecar container to start before the Spin application.
- As a developer, I would like to run [Dapr] sidecar containers in the same pod as the Spin application so that I can leverage Dapr's capabilities, such as service invocation, state management, pub/sub, etc, in my Spin application.
- As a developer, I would like to run a [Envoy] proxy sidecar container in the same pod as the Spin application so that I can re-use the service mesh capabilities provided by Envoy, such as traffic management, security, observability, etc, in my Spin application.

[Dapr]: (https://dapr.io/)
[Envoy]: (https://www.envoyproxy.io/)

### Existing applications

Here are some existing prototypes that I have found that runs Wasm workloads side by side with Linux containers:

- [SpinKube - the first look at WebAssembly/WASI application (SpinApp) on Kubernetes](https://dev.to/thangchung/spinkube-the-first-look-at-webassemblywasi-application-spinapp-on-kubernetes-36jd) and [Coffeeshop repo](https://github.com/thangchung/coffeeshop-on-spinkube/tree/feat/global-azure-2024)
- [Istio Wasm Demo](https://github.com/keithmattix/istio-wasm-demo)


## Proposal


The main changes are

1. A new field `InitContainers` in the `spinapp_types.go` that defines a list of sidecar containers to be included in the Deployment.
```go
// InitContainers defines the list of sidecar containers to be included in the deployment.
//
// These containers will not include the main Spin App. They share the Spin App's
// environment variables and volumes.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

// +kubebuilder:validation:Optional
InitContainers []corev1.Container `json:"containers,omitempty"`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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/

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

^^
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"

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member Author

@Mossaka Mossaka Oct 25, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+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"

Copy link
Member Author

@Mossaka Mossaka Oct 29, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good question!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"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/

```

2. Adds containers to the deployment creation in `spinapp_controller.go`:
```go
var containers []corev1.Container
if len(app.Spec.InitContainers) > 0 {
for _, c := range app.Spec.InitContainers {
if c.Image == app.Spec.Image {
return nil, errors.New("container in app.Spec.InitContainers must have a different image than Spin App")
}
if c.Name == "" {
return nil, errors.New("container in app.Spec.InitContainers must have a name")
}
if c.Name == app.Name {
return nil, errors.New("container in app.Spec.InitContainers must have a different name than the Spin App")
}
c.Env = append(c.Env, env...)
c.VolumeMounts = append(c.VolumeMounts, volumeMounts...)
if c.Resources.Limits == nil && c.Resources.Requests == nil {
c.Resources = resources
}
if c.LivenessProbe == nil {
c.LivenessProbe = livenessProbe
}
if c.ReadinessProbe == nil {
c.ReadinessProbe = readinessProbe
}
containers = append(containers, c)
}
}
```

See the full diff [here](https://github.com/spinkube/spin-operator/compare/main...Mossaka:spin-operator-msk:sidecars)

[sidecar containers]: (https://kubernetes.io/docs/concepts/workloads/pods/sidecar-containers/)
[InitContainers]: (https://kubernetes.io/docs/concepts/workloads/pods/init-containers/)

### Ports

Sidecar containers must avoid using port 80, which is reserved for the Spin application. Other ports like 8080 or 8081 can be used.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why port 80? Isn't that configurable?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

relevant discussion: spinframework/spin-operator#326


### Services

Sidecar containers can communicate internally using `localhost`, but if external access is needed, users must create a separate service for the sidecar container.

### Restrictions

* Sidecar containers cannot use the same image or name as the Spin App.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

* Sidecar containers must not use port 80.

### Sample SpinApp yaml

```yaml
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"
initContainers:
- name: logshipper
image: alpine:latest
restartPolicy: Always
command: ['sh', '-c', 'tail -F /opt/logs.txt']
volumeMounts:
- name: example-volume
mountPath: "/mnt/data"
```

Note: The `InitContainers` field is intentionally designed to follow the the design of sidecar containers in Kubernetes Pods. You will need to have `SidecarContainers` feature gate enabled in your Kubernetes cluster to use this feature. See more details [here](https://kubernetes.io/docs/concepts/workloads/pods/sidecar-containers/).

### Init Containers

Since the sidecar containers are just a special case of InitContainers, I realized that the scope of this SKIP is naturally expanded to include the support of init containers in the Spin Operator.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)


Examples of init containers include:

```yaml
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"
initContainers:
- name: init-myservice
image: busybox:1.28
command: ['sh', '-c', "until nslookup myservice.$(cat /var/run/secrets/kubernetes.io/serviceaccount/namespace).svc.cluster.local; do echo waiting for myservice; sleep 2; done"]
- name: init-mydb
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"]
```

## Alternatives Considered

1. Use `Containers` instead of `InitContainers` in the SpinApp CRD.
Copy link

@jsturtevant jsturtevant Oct 30, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

2. Use `SidecarContainers` instead of `InitContainers` in the SpinApp CRD and automatically set the `RestartPolicy` to `Always` in the Deployment.
3. Automatically setting up services for sidecar containers in the operator.

---

This proposal adds sidecar container support in Spin Operator, addressing key use cases for running sidecar containers alongside Spin applications.