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: implement install and uninstall flow #68

Merged
merged 16 commits into from
May 23, 2024
Merged

Conversation

voigt
Copy link
Contributor

@voigt voigt commented Mar 21, 2024

Describe your changes

  1. This PR makes sure we use the node-installer for the install job.
  2. The PR adds a "shim-downloader" image. The idea is to use the downloader as initContainer: this makes the download & unzip procedure customizable to make shim downloading configurable.
  3. This PR adds the configuration required to distinguish "install" and "uninstall" jobs

Issue ticket number and link

Addresses #52

How to test

# create kind cluster with annotations
make kind

# run rcm locally, against kind cluster
CONTROLLER_NAMESPACE="default" \
SHIM_DOWNLOADER_IMAGE="ghcr.io/spinkube/shim-downloader:latest-feat-add_shim_downloader" \
SHIM_NODE_INSTALLER_IMAGE="ghcr.io/spinkube/node-installer:latest-feat-add_shim_downloader" \
go run -ldflags "${LDFLAGS}" ./cmd/rcm/main.go

Install Spin Shim

# create spin shim resource
kubectl apply -f ./config/samples/test_shim_spin.yaml

Two jobs will be scheduled on nodes with label spin=true

  • kwasm-worker-spin-v2-install
  • kwasm-worker2-spin-v2-install

After jobs will eventually succeed:

kubectl get shims
NAME      RUNTIMECLASS   READY   NODES
spin-v2   spin-v2        2       2

Check containerd config on worker and worker2:

bash docker exec -it $(docker ps --format json | jq -r '. | "\(.ID) \(.Names)"' | grep kwasm-worker2 | awk '{print $1}') cat /etc/containerd/config.toml[...] # KWASM runtime config for spin-v2 [plugins."io.containerd.grpc.v1.cri".containerd.runtimes.spin-v2] runtime_type = "/opt/kwasm/bin/containerd-shim-spin-v2"

Check spin-v2 runtime binary:

docker exec -it $(docker ps --format json | jq -r '. | "\(.ID) \(.Names)"' | grep kwasm-worker2 | awk '{print $1}') ls -lah /opt/kwasm/bin
total 40M
drwxr-xr-x 1 root root  46 May 19 20:35 .
drwxr-xr-x 1 root root  36 May 19 20:35 ..
-rwxr-xr-x 1 root root 40M May 19 20:36 containerd-shim-spin-v2

Uninstall Spin Shim

# delete spin shim resource
kubectl delete shim spin-v2

Two jobs will be scheduled on nodes with label spin=true

  • kwasm-worker2-spin-v2-uninstall
  • kwasm-worker-spin-v2-uninstall

Check containerd config of worker or worker2; directive for spin-v2 is successfully removed:
bash docker exec -it $(docker ps --format json | jq -r '. | "\(.ID) \(.Names)"' | grep kwasm-worker2 | awk '{print $1}') cat /etc/containerd/config.toml

Check for binary, which is also removed:

docker exec -it $(docker ps --format json | jq -r '. | "\(.ID) \(.Names)"' | grep kwasm-worker2 | awk '{print $1}') ls -lah /opt/kwasm/bin
total 0
drwxr-xr-x 1 root root  0 May 19 20:36 .
drwxr-xr-x 1 root root 36 May 19 20:35 ..

Known Issues

Install and uninstall Jobs spawn two pods in total: one pod successfully doing install/uninstall operations, but ending in unknown state. The other one in "completed" state, doing effectively nothing. This is not as clean as I want it to be.

Bug is documented here: #140

A fix of this bug should be done in a separate PR.

Checklist before requesting a review

  • I have performed a self-review of my code
  • If it is a core feature, I have added thorough tests. installation/uninstallation procedure should be tested in a dedicated CI job; right now a helm chart is missing
  • I tested the changes with the following distributions:
    • Kind (with rcm running locally)

@voigt voigt force-pushed the feat-add_shim_downloader branch from 14f46be to 7619510 Compare March 21, 2024 16:19
@voigt voigt force-pushed the feat-add_shim_downloader branch from 7619510 to d906ef9 Compare May 17, 2024 19:16
Copy link

github-actions bot commented May 17, 2024

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

OpenSSF Scorecard

PackageVersionScoreDetails

Scanned Manifest Files

@voigt voigt force-pushed the feat-add_shim_downloader branch from d55633e to 236ca00 Compare May 19, 2024 19:47
@voigt voigt requested review from phyrog and 0xE282B0 May 19, 2024 20:55
@voigt voigt marked this pull request as ready for review May 19, 2024 21:04
@voigt voigt force-pushed the feat-add_shim_downloader branch from 0b5da93 to cd16a63 Compare May 20, 2024 20:20
@voigt voigt force-pushed the feat-add_shim_downloader branch from c76feb2 to cd16a63 Compare May 20, 2024 21:27
@voigt voigt requested a review from flavio May 20, 2024 21:29
@voigt voigt linked an issue May 20, 2024 that may be closed by this pull request
@voigt voigt changed the title feat add shim downloader feat: implement install and uninstall flow May 21, 2024
Copy link
Contributor

@0xE282B0 0xE282B0 left a comment

Choose a reason for hiding this comment

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

Some notes:

  • SpinKube uses wasmtime-spin-v2 as default, to use the RCM example together with the SpinKube Quickstart the runtimeClassName in the SpinAppExecutor resource needs to be adapted to spin-v2 (we should choose the same for both quickstarts)
  • Every installer job on every node does a download as init step. (just a note)
  • I did not face the known issue in my setup, but we will face it more often when we run jobs in parallel that try to restart containerd.

LGTM: Install/Uninstall functionality works for me.

config/samples/test_shim_spin.yaml Show resolved Hide resolved
config/samples/test_shim_spin.yaml Show resolved Hide resolved
@voigt voigt merged commit ff8f997 into main May 23, 2024
36 checks passed
@voigt voigt deleted the feat-add_shim_downloader branch May 23, 2024 14:18
flavio pushed a commit to flavio/runtime-class-manager that referenced this pull request Sep 11, 2024
add node-installer and shim-downloader to install job
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Node-Installer job with init container
3 participants