Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(shim): support archive layers #121

Merged
merged 2 commits into from
Jun 25, 2024
Merged
Show file tree
Hide file tree
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
47 changes: 38 additions & 9 deletions containerd-shim-spin/src/engine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,10 +40,6 @@ const SPIN_HTTP_LISTEN_ADDR_ENV: &str = "SPIN_HTTP_LISTEN_ADDR";
const RUNTIME_CONFIG_PATH: &str = "/runtime-config.toml";
/// Describes an OCI layer with Wasm content
const OCI_LAYER_MEDIA_TYPE_WASM: &str = "application/vnd.wasm.content.layer.v1+wasm";
Copy link
Collaborator

Choose a reason for hiding this comment

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

suggestion: make spin_oci::client::WASM_LAYER_MEDIA_TYPE public and use it in the Spin Shim.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In fact, Spin itself is waiting on the canonical type/string upstream: https://github.com/fermyon/spin/blob/main/crates/oci/src/client.rs#L34-L35. To be defined in https://github.com/bytecodealliance/registry perhaps? Therefore expecting both Spin and the shim would pull this from the upstream crate.

/// Describes an OCI layer with data content
const OCI_LAYER_MEDIA_TYPE_DATA: &str = "application/vnd.wasm.content.layer.v1+data";
/// Describes an OCI layer containing a Spin application config
const OCI_LAYER_MEDIA_TYPE_SPIN_CONFIG: &str = "application/vnd.fermyon.spin.application.v1+config";
/// Expected location of the Spin manifest when loading from a file rather than
/// an OCI image
const SPIN_MANIFEST_FILE_PATH: &str = "/spin.toml";
Expand Down Expand Up @@ -136,7 +132,7 @@ impl SpinEngine {
.write_wasm(&artifact.layer, &artifact.config.digest())
.await?;
}
MediaType::Other(name) if name == OCI_LAYER_MEDIA_TYPE_DATA => {
MediaType::Other(name) if name == spin_oci::client::DATA_MEDIATYPE => {
log::debug!(
"<<< writing data layer to cache, near {:?}",
cache.manifests_dir()
Expand All @@ -145,6 +141,19 @@ impl SpinEngine {
.write_data(&artifact.layer, &artifact.config.digest())
.await?;
}
MediaType::Other(name) if name == spin_oci::client::ARCHIVE_MEDIATYPE => {
log::debug!(
"<<< writing archive layer and unpacking contents to cache, near {:?}",
cache.manifests_dir()
);
self.handle_archive_layer(
cache,
&artifact.layer,
&artifact.config.digest(),
)
.await
.context("unable to unpack archive layer")?;
}
_ => {
log::debug!(
"<<< unknown media type {:?}",
Expand Down Expand Up @@ -367,6 +376,25 @@ impl SpinEngine {
}
None
}

async fn handle_archive_layer(
&self,
cache: &Cache,
bytes: impl AsRef<[u8]>,
digest: impl AsRef<str>,
) -> Result<()> {
// spin_oci::client::unpack_archive_layer attempts to create a tempdir via tempfile::tempdir()
// which will fall back to /tmp if TMPDIR is not set. /tmp is either not found or not accessible
// in the shim environment, hence setting to current working directory.
if env::var("TMPDIR").is_err() {
log::debug!(
Copy link
Collaborator

Choose a reason for hiding this comment

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

When do we expect TMPDIR to be set? I am wondering if this log is necessary.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point... it appears it isn't ever set, currently. Glad to remove this debug log if you'd like.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think we would want to leave it, otherwise we might overwrite it. I don't know what scenario it would be set in, but if it was set and we overwrite I don't think that would be an expected behavior?

"<<< TMPDIR is not set; setting to current working directory for unpacking archive layer"
);
env::set_var("TMPDIR", env::current_dir().unwrap_or(".".into()));
}

spin_oci::client::unpack_archive_layer(cache, bytes, digest).await
}
}

impl Engine for SpinEngine {
Expand Down Expand Up @@ -405,8 +433,9 @@ impl Engine for SpinEngine {
fn supported_layers_types() -> &'static [&'static str] {
&[
OCI_LAYER_MEDIA_TYPE_WASM,
OCI_LAYER_MEDIA_TYPE_DATA,
OCI_LAYER_MEDIA_TYPE_SPIN_CONFIG,
spin_oci::client::ARCHIVE_MEDIATYPE,
spin_oci::client::DATA_MEDIATYPE,
spin_oci::client::SPIN_APPLICATION_MEDIA_TYPE,
]
}

Expand Down Expand Up @@ -529,7 +558,7 @@ mod tests {
let data_content = WasmLayer {
layer: vec![],
config: oci_spec::image::Descriptor::new(
MediaType::Other(OCI_LAYER_MEDIA_TYPE_DATA.to_string()),
MediaType::Other(spin_oci::client::DATA_MEDIATYPE.to_string()),
1024,
"sha256:1234",
),
Expand Down Expand Up @@ -569,7 +598,7 @@ mod tests {
WasmLayer {
layer: vec![],
config: oci_spec::image::Descriptor::new(
MediaType::Other(OCI_LAYER_MEDIA_TYPE_DATA.to_string()),
MediaType::Other(spin_oci::client::DATA_MEDIATYPE.to_string()),
1024,
"sha256:1234",
),
Expand Down
1 change: 1 addition & 0 deletions images/.gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
**/.spin
3 changes: 3 additions & 0 deletions images/spin-static-assets/Dockerfile
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
FROM --platform=${BUILDPLATFORM} alpine
COPY assets ./assets
COPY spin.toml .
34 changes: 34 additions & 0 deletions images/spin-static-assets/assets/jabberwocky.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
'Twas brillig, and the slithy toves
Did gyre and gimble in the wabe:
All mimsy were the borogoves,
And the mome raths outgrabe.

"Beware the Jabberwock, my son!
The jaws that bite, the claws that catch!
Beware the Jubjub bird, and shun
The frumious Bandersnatch!"

He took his vorpal sword in hand;
Long time the manxome foe he sought-
So rested he by the Tumtum tree
And stood awhile in thought.

And, as in uffish thought he stood,
The Jabberwock, with eyes of flame,
Came whiffling through the tulgey wood,
And burbled as it came!

One, two! One, two! And through and through
The vorpal blade went snicker-snack!
He left it dead, and with its head
He went galumphing back.

"And hast thou slain the Jabberwock?
Come to my arms, my beamish boy!
O frabjous day! Callooh! Callay!"
He chortled in his joy.

'Twas brillig, and the slithy toves
Did gyre and gimble in the wabe:
All mimsy were the borogoves,
And the mome raths outgrabe.
18 changes: 18 additions & 0 deletions images/spin-static-assets/spin.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
spin_manifest_version = 2

[application]
name = "spin-static-assets"
version = "0.1.0"
authors = ["SpinKube Engineering Team"]

[application.trigger.http]
base = "/"

[[trigger.http]]
id = "trigger-static-fileserver"
component = "static-fileserver"
route = "/..."

[component.static-fileserver]
source = { url = "https://github.com/fermyon/spin-fileserver/releases/download/v0.3.0/spin_static_fs.wasm", digest = "sha256:ef88708817e107bf49985c7cefe4dd1f199bf26f6727819183d5c996baa3d148" }
files = [{ source = "assets", destination = "/" }]
2 changes: 0 additions & 2 deletions scripts/run-integration-tests.sh
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,6 @@ else
make deploy-workloads-pushed-using-docker-build-push
fi



## Verify pods can be terminated successfully
make pod-terminates-test

Expand Down
10 changes: 7 additions & 3 deletions scripts/up.sh
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,9 @@ set -euo pipefail
cluster_name="test-cluster" # name of the k3d cluster
dockerfile_path="deployments/k3d" # path to the Dockerfile

DOCKER_IMAGES=("spin" "spin-keyvalue" "spin-outbound-redis" "spin-multi-trigger-app")
OUT_DIRS=("test/out_spin" "test/out_spin_keyvalue" "test/out_spin_outbound_redis" "test/out_spin_multi_trigger_app")
IMAGES=("spin-hello-world" "spin-keyvalue" "spin-outbound-redis" "spin-multi-trigger-app")
DOCKER_IMAGES=("spin" "spin-keyvalue" "spin-outbound-redis" "spin-multi-trigger-app" "spin-static-assets")
OUT_DIRS=("test/out_spin" "test/out_spin_keyvalue" "test/out_spin_outbound_redis" "test/out_spin_multi_trigger_app" "test/out_spin_static_assets")
IMAGES=("spin-hello-world" "spin-keyvalue" "spin-outbound-redis" "spin-multi-trigger-app" "spin-static-assets")

# build the Docker image for the k3d cluster
docker build -t k3d-shim-test "$dockerfile_path"
Expand All @@ -29,6 +29,10 @@ for i in "${!DOCKER_IMAGES[@]}"; do
## images pushed as localhost:5000/<namespace>/<app>:<version>
## can be pulled as registry:5000/<namespace>/<app>:<version> from within k3d cluster
spin build -f "./images/${DOCKER_IMAGES[$i]}/spin.toml"
## For the spin-static-assets app, use archive layers to test this functionality in the shim
if [ "${i}" == "spin-static-assets" ]; then
export SPIN_OCI_ARCHIVE_LAYERS=1
fi
spin registry push "localhost:5000/spin-registry-push/${IMAGES[$i]}:latest" -f "./images/${DOCKER_IMAGES[$i]}/spin.toml" -k
done

Expand Down
23 changes: 23 additions & 0 deletions tests/src/integration_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,29 @@ mod test {
Ok(())
}

#[tokio::test]
async fn spin_static_assets_test() -> Result<()> {
let host_port = 8082;

// curl for static asset
println!(
" >>> curl http://localhost:{}/static-assets/jabberwocky.txt",
host_port
);
let res = retry_get(
&format!(
"http://localhost:{}/static-assets/jabberwocky.txt",
host_port
),
RETRY_TIMES,
INTERVAL_IN_SECS,
)
.await?;
assert!(String::from_utf8_lossy(&res).contains("'Twas brillig, and the slithy toves"));

Ok(())
}

async fn is_kubectl_installed() -> anyhow::Result<bool> {
let output: Result<std::process::Output, std::io::Error> = Command::new("kubectl")
.arg("version")
Expand Down
46 changes: 45 additions & 1 deletion tests/workloads-pushed-using-docker-build-push/workloads.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,7 @@ spec:
- /spin
- /outboundredis
- /keyvalue
- /static-assets
---
apiVersion: networking.k8s.io/v1
kind: Ingress
Expand Down Expand Up @@ -174,6 +175,13 @@ spec:
name: spin-multi-trigger-app
port:
number: 80
- path: /static-assets
pathType: Prefix
backend:
service:
name: spin-static-assets
port:
number: 80
---
apiVersion: apps/v1
kind: Deployment
Expand Down Expand Up @@ -209,4 +217,40 @@ spec:
port: 80
targetPort: 80
selector:
app: spin-multi-trigger-app
app: spin-multi-trigger-app
---
apiVersion: apps/v1
kind: Deployment
metadata:
name: spin-static-assets
spec:
replicas: 1
selector:
matchLabels:
app: spin-static-assets
template:
metadata:
labels:
app: spin-static-assets
spec:
runtimeClassName: wasmtime-spin
containers:
- name: spin-static-assets
image: docker.io/library/spin-static-assets:latest
imagePullPolicy: IfNotPresent
command: ["/"]
ports:
- containerPort: 80
---
apiVersion: v1
kind: Service
metadata:
name: spin-static-assets
spec:
type: LoadBalancer
ports:
- protocol: TCP
port: 80
targetPort: 80
selector:
app: spin-static-assets
46 changes: 45 additions & 1 deletion tests/workloads-pushed-using-spin-registry-push/workloads.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,7 @@ spec:
- /spin
- /outboundredis
- /keyvalue
- /static-assets
---
apiVersion: networking.k8s.io/v1
kind: Ingress
Expand Down Expand Up @@ -174,6 +175,13 @@ spec:
name: spin-multi-trigger-app
port:
number: 80
- path: /static-assets
pathType: Prefix
backend:
service:
name: spin-static-assets
port:
number: 80
---
apiVersion: apps/v1
kind: Deployment
Expand Down Expand Up @@ -209,4 +217,40 @@ spec:
port: 80
targetPort: 80
selector:
app: spin-multi-trigger-app
app: spin-multi-trigger-app
---
apiVersion: apps/v1
kind: Deployment
metadata:
name: spin-static-assets
spec:
replicas: 1
selector:
matchLabels:
app: spin-static-assets
template:
metadata:
labels:
app: spin-static-assets
spec:
runtimeClassName: wasmtime-spin
containers:
- name: spin-static-assets
image: test-registry:5000/spin-registry-push/spin-static-assets:latest
imagePullPolicy: IfNotPresent
command: ["/"]
ports:
- containerPort: 80
---
apiVersion: v1
kind: Service
metadata:
name: spin-static-assets
spec:
type: LoadBalancer
ports:
- protocol: TCP
port: 80
targetPort: 80
selector:
app: spin-static-assets