A more easy way to setup nydus snapshotter by DaemonSet #570

ChengyuZhu6 · 2023-12-19T17:23:38Z

Confidential Containers leverage nydus snapshotter to pull images within the guest using fs_driver=proxy and share images on the host using fs_driver=blockdev to ensure data confidentiality. However, when running related tests for Kata CI with nydus snapshotter, we faced challenges installing and uninstalling nydus snapshotter on various architectures and configuring the containerd settings. We envisioned a smoother experience, where nydus snapshotter provides a daemonset, enabling users to seamlessly run and clean up nydus snapshotter and making it more easy.

Fixes #565

Add comments for fs_driver on snapshotter config. Signed-off-by: ChengyuZhu6 <[email protected]>

ChengyuZhu6 · 2023-12-19T17:25:57Z

/cc @jiangliu @sctb512 @imeoer @adamqqqplay

adamqqqplay · 2023-12-25T03:12:33Z

@ChengyuZhu6 Thanks for your great work! It will take some time to review.

imeoer · 2023-12-25T03:36:41Z

It seems to break some tests, let's take a look first.

imeoer · 2023-12-29T01:42:23Z

Cloud we enhance the helm chart? We can provide the options to configure the snapshotter fs_driver.

ChengyuZhu6 · 2023-12-29T02:01:26Z

Cloud we enhance the helm chart? We can provide the options to configure the snapshotter fs_driver.

Yes, I plan to use helm for installing snapshotter, but it requires a different repo. So I have proposed an alternative method that does not rely on helm. Next, I will submit a PR to the helm repo.

imeoer · 2023-12-29T06:31:10Z

It seems the CI still needs to be fixed: https://github.com/containerd/nydus-snapshotter/actions/runs/7269460179/job/19807116165?pr=570 :)

wainersm · 2024-01-08T21:06:53Z

misc/snapshotter/Dockerfile

@@ -26,6 +26,14 @@ COPY containerd-nydus-grpc /usr/local/bin/
 RUN chmod +x /usr/local/bin/containerd-nydus-grpc
 COPY nydusd-config.fusedev.json /etc/nydus/config.json
 COPY nydusd-config-localfs.json /etc/nydus/localfs.json
+COPY nydusd-config.fusedev.json /etc/nydus/nydusd-fusedev.json
+COPY nydusd-config-localfs.json /etc/nydus/nydusd-localfs.json


nydusd-config-localfs.json is copied twice (likewise nydusd-config.fusedev.json);

Maybe it can be copied to /etc/nydus/nydusd-localfs.json only, then create a /etc/nydus/localfs.json symlink

wainersm · 2024-01-08T21:11:24Z

misc/snapshotter/Dockerfile

+COPY config-proxy.toml /etc/nydus/config-proxy.toml
+COPY config-fscache.toml /etc/nydus/config-fscache.toml
+COPY config-fusedev.toml /etc/nydus/config-fusedev.toml
+
 COPY entrypoint.sh /



Regarding the renaming of config-coco-host-sharing.toml → misc/snapshotter/config-blockdev.toml and ...napshotter/config-coco-guest-pulling.toml → misc/snapshotter/config-proxy.toml , would make sense to add the kata- filename prefix because they should be used with kata containers?

wainersm · 2024-01-08T21:19:06Z

misc/snapshotter/snapshotter.sh

+    fscache) SNAPSHOTTER_CONFIG="${NYDUS_CONFIG_DIR}/config-fscache.toml" ;;
+    blockdev) SNAPSHOTTER_CONFIG="${NYDUS_CONFIG_DIR}/config-blockdev.toml" ;;
+    proxy) SNAPSHOTTER_CONFIG="${NYDUS_CONFIG_DIR}/config-proxy.toml" ;;
+    *) die "invalid fs driver" ;;


I suggest to log the name of the invalid driver, e.g., invalid fs driver: ${FS_DRIVER}

wainersm · 2024-01-08T21:29:56Z

misc/snapshotter/Dockerfile


 FROM alpine:3.17.0

 WORKDIR /root/

-RUN apk add --no-cache libc6-compat
+RUN apk add --no-cache libc6-compat bash jq


Hi @ChengyuZhu6 ! I'm not sure if you are aware of but we have found an issue while using toml-cli on kata containers that was corrupting the containerd's configuration file. @fidencio gave some details in kata-containers/kata-containers#8679 (comment) but it turns out you must ensure jq > 1.7 is installed. Here is how it was solved on kata side: kata-containers/kata-containers@41320c5

Hey @wainersm. Thanks for your review. Here we use toml-cli , not tomlq.

oh, I was wondering why the binary is named toml rather than tomlq and that explains :)

wainersm · 2024-01-09T12:52:17Z

misc/snapshotter/snapshotter.sh

+    local key="$1"
+    local value="$2"
+    local file="${3:-$SNAPSHOTTER_CONFIG}"
+    toml set --overwrite "$file" "$key" $value


wainersm · 2024-01-09T12:56:02Z

misc/snapshotter/snapshotter.sh

+        update_toml_config "experimental.tarfs.mount_tarfs_on_host" "$MOUNT_TARFS_ON_HOST"
+        update_toml_config "experimental.tarfs.export_mode" "$EXPORT_MODE"
+        ;;
+    *) die "invalid fs driver in options_handler" ;;


Here again print the $FS_DRIVER value

wainersm · 2024-01-09T13:01:44Z

misc/snapshotter/snapshotter.sh

+        elif toml check $CONTAINER_RUNTIME_CONFIG "proxy_plugins.nydus" >/dev/null; then
+            echo "the config has configured the nydus proxy plugin!"
+        else
+            die "there is another proxy plugin in the containerd!"


Can't it be multiple proxy plugins?

Thanks for your reminder, I have fixed it.

wainersm · 2024-01-09T13:10:48Z

misc/snapshotter/snapshotter.sh

+    if toml check $CONTAINER_RUNTIME_CONFIG "proxy_plugins" >/dev/null; then
+        if [ "$(toml get $CONTAINER_RUNTIME_CONFIG proxy_plugins | jq length)" == "0" ]; then
+            echo "there is no proxy plugin!"
+            cat <<EOF >>"$CONTAINER_RUNTIME_CONFIG"


is it possible to add the proxy_plugins entire section with toml instead of cat?

Maybe have a variable to hold /run/containerd-nydus/containerd-nydus-grpc.sock?

Maybe have a variable to hold /run/containerd-nydus/containerd-nydus-grpc.sock?
Fixed.

is it possible to add the proxy_plugins entire section with toml instead of cat?

I don't think that's possible due to a limitation on the toml-cli.

wainersm · 2024-01-09T13:16:12Z

misc/snapshotter/snapshotter.sh

+EOF
+    fi
+
+    update_toml_config "plugins.\"io.containerd.grpc.v1.cri\".containerd.discard_unpacked_layers" false "${CONTAINER_RUNTIME_CONFIG}"


Mind to document why nydus-snapshotter needs those two configurations enabled?

wainersm · 2024-01-09T13:21:33Z

misc/snapshotter/snapshotter.sh

+        update_toml_config "plugins.\"io.containerd.grpc.v1.cri\".containerd.snapshotter" "nydus" "${CONTAINER_RUNTIME_CONFIG}"
+    fi
+
+    nsenter -t 1 -m systemctl -- restart containerd.service


Should it wait containerd to restart then check it is running properly again?

Yes, please, I second Wainer's request on this, as we faced issues where containerd would simply be off after some changes.

wainersm · 2024-01-09T13:32:47Z

misc/snapshotter/snapshotter.sh

@@ -154,3 +154,21 @@ function deploy_snapshotter() {
    configure_snapshotter
    ${COMMANDLINE} &
 }
+
+function cleanup_snapshotter() {
+    pid=$(ps -ef | grep containerd-nydus-grpc | grep -v grep | awk '{print $1}')


I suspect that you can use $SNAPSHOTTER_BINARY. Something like:

pid=$(pidof -s "$SNAPSHOTTER_BINARY" || true)

BTW, I'm a little confused here... is the snapshotter daemon running on the host or within the daemonset container? If the former then it needs to run the command with nsenter, right?

It would running in the daemonset.

I have changed the logic so that the snapshotter runs on the host and can be as a systemd service.

wainersm · 2024-01-09T13:38:42Z

misc/snapshotter/snapshotter.sh

+    pid=$(ps -ef | grep containerd-nydus-grpc | grep -v grep | awk '{print $1}')
+    if [ ! -z "$pid" ]; then
+        for i in $(nsenter -t 1 -m ctr -n k8s.io snapshot --snapshotter nydus list | grep -v KEY | cut -d' ' -f1); do
+            nsenter -t 1 -m ctr -n k8s.io snapshot --snapshotter nydus rm $i || true


I suppose this operation can fail if a running pod has used the snapshot. Here we ignore the fail and proceed to remove the other snapshots then finally remove nydus-snapshotter completely. I'm wondering what happens to the pod that is still using the snapshot. Does it keep running normally?

wainersm · 2024-01-09T13:48:15Z

@ChengyuZhu6 I've finished my review and left a couple of comments. I didn't find any bug but I'm afraid I won't have time to give it a try. Thanks for working on this && amazing work!

beraldoleal

Hi @ChengyuZhu6 , thanks for this. Just left a few comments here.

beraldoleal · 2024-01-10T18:08:23Z

docs/setup_snapshotter_by_daemonset.md

+
+Next, we can configure access control for nydus snapshotter.
+```bash
+kubectl apply -f nydus-snapshotter-rbac.yaml


This file is not present at the nydus-snapshotter root folder:

kubectl apply -f misc/snapshotter/nydus-snapshotter-rbac.yaml

Fixed. Thanks for your review @beraldoleal !

beraldoleal · 2024-01-10T18:08:49Z

docs/setup_snapshotter_by_daemonset.md

+Afterward, we can deploy a DaemonSet for nydus snapshotter.
+
+```bash
+kubectl apply -f nydus-snapshotter.yaml


Same here:

kubectl apply -f misc/snapshotter/nydus-snapshotter.yaml

beraldoleal · 2024-01-10T19:02:50Z

docs/setup_snapshotter_by_daemonset.md

+
+```bash
+$ kubectl delete -f nydus-snapshotter.yaml 
+$ kubectl delete -f nydus-snapshotter-rbac.yaml 


beraldoleal · 2024-01-10T20:14:20Z

misc/snapshotter/Dockerfile

+RUN mkdir -p /usr/bin/ /opt/nydus/ /var/lib/containerd-nydus/cache /tmp/blobs/
+COPY --from=sourcer /nydus* /usr/bin/
+COPY --from=sourcer /toml /usr/bin/
+COPY containerd-nydus-grpc /usr/bin/


I'm not familiar with this repository, so not sure if I'm missing something (probably I am).... but I was not able to test the entire solution because I can't build the image:

"/containerd-nydus-grpc": not found

Do you mind to share what I'm missing?

I guess you should build this project binaries and copy containerd-nydus-grpc over misc/snaphotter. Thanks for giving it a try @beraldoleal !

@wainersm I tried that, but the binary was still missing because is a wrong path. @ChengyuZhu6 do you mind sharing if we need any extra step to make this passing? Or maybe a README.md here with a quick/basic instructions?

Thanks for your informations. The following are steps to build the snapshotter docker image:

$ make # build snapshotter binaries $ cp bin/* misc/snapshotter/ $ pushd misc/snapshotter/ $ docker build -t ghcr.io/containerd/nydus-snapshotter:latest . $ popd

It would be nice then to add those instructions to a README on misc/snapshotter, not fancy, just this. Alternatively, add those steps as part of a make snapshotter target.

It would be nice then to add those instructions to a README on misc/snapshotter, not fancy, just this. Alternatively, add those steps as part of a make snapshotter target.

Done.

zvonkok · 2024-01-11T14:36:19Z

misc/snapshotter/nydus-snapshotter-rbac.yaml

+subjects:
+- kind: ServiceAccount
+  name: nydus-snapshotter-sa
+  namespace: nydus-snapshotter


nit: "No newline at end of file"

zvonkok · 2024-01-11T14:36:45Z

misc/snapshotter/config-proxy.toml

+log_to_stdout = true
+# Snapshotter's log level
+level = "info"
+
 [snapshot]
 # Insert Kata volume information to `Mount.Options`
 enable_kata_volume = true


nit: "No newline at end of file"

zvonkok · 2024-01-11T14:37:02Z

misc/snapshotter/config-fscache.toml

+# Print logs to stdout rather than logging files
+log_to_stdout = true
+# Snapshotter's log level
+level = "info"


nit: "No newline at end of file"

zvonkok · 2024-01-11T14:37:08Z

misc/snapshotter/config-fusedev.toml

+# Print logs to stdout rather than logging files
+log_to_stdout = true
+# Snapshotter's log level
+level = "info"


nit: "No newline at end of file"

docs/setup_snapshotter_by_daemonset.md

misc/snapshotter/config-fscache.toml

misc/snapshotter/config-fusedev.toml

misc/snapshotter/nydus-snapshotter.yaml

tests/e2e/k8s/snapshotter-cri.yaml

tests/e2e/k8s/snapshotter-kubeconf.yaml

This can be made transparent to users, eliminating the need for them to understand how to configure snapshotter in Containerd and reducing errors caused by misconfiguration. Signed-off-by: ChengyuZhu6 <[email protected]>

This can be seamlessly handled for users, eliminating the requirement for them to restore Containerd to its state prior to running the snapshotter. This approach reduces errors caused by misconfiguration or misbehavior. Signed-off-by: ChengyuZhu6 <[email protected]>

introduce main function to handle different commandline. Signed-off-by: ChengyuZhu6 <[email protected]>

Use configmap to decouple configuration artifacts from image content to keep containerized applications portable and inject configuration files and variables to the daemonset. Signed-off-by: ChengyuZhu6 <[email protected]>

misc/snapshotter/config-blockdev.toml

docs/setup_snapshotter_by_daemonset.md

imeoer · 2024-01-26T01:47:33Z

We'd better to standardize all the binary paths to /usr/local/bin/* and all the configuration paths to /etc/nydus/*.json, others LGTM, thanks! Let's provide a helm chart to further reduce the difficulty of the deploy steps.

Signed-off-by: ChengyuZhu6 <[email protected]>

Add document to setup snapshotter by daemonset. Signed-off-by: ChengyuZhu6 <[email protected]>

We no longer need the entrypoint because snapshotter.sh handles both the deployment and the cleanup of the snapshotter. Signed-off-by: ChengyuZhu6 <[email protected]>

Check the status of the systemd service after restarting it. Signed-off-by: ChengyuZhu6 <[email protected]>

update the e2e tests. Signed-off-by: ChengyuZhu6 <[email protected]>

imeoer · 2024-01-26T06:12:09Z

Thanks @ChengyuZhu6 for the work and all reviewers @fidencio @wainersm @zvonkok @beraldoleal !

misc: add comments for fs_driver on snapshotter config

bfa2d36

Add comments for fs_driver on snapshotter config. Signed-off-by: ChengyuZhu6 <[email protected]>

ChengyuZhu6 force-pushed the payload branch from 2210061 to 7bf636a Compare December 20, 2023 00:51

ChengyuZhu6 changed the title ~~A more generic way to setup nydus snapshotter by DaemonSet~~ A more easy way to setup nydus snapshotter by DaemonSet Dec 20, 2023

wainersm reviewed Jan 8, 2024

View reviewed changes

wainersm reviewed Jan 9, 2024

View reviewed changes

beraldoleal reviewed Jan 10, 2024

View reviewed changes

ChengyuZhu6 force-pushed the payload branch 3 times, most recently from 5060b74 to 045a661 Compare January 11, 2024 08:27

zvonkok reviewed Jan 11, 2024

View reviewed changes

imeoer reviewed Jan 25, 2024

View reviewed changes

docs/setup_snapshotter_by_daemonset.md Show resolved Hide resolved

imeoer reviewed Jan 25, 2024

View reviewed changes

misc/snapshotter/config-fscache.toml Outdated Show resolved Hide resolved

imeoer reviewed Jan 25, 2024

View reviewed changes

misc/snapshotter/config-fusedev.toml Outdated Show resolved Hide resolved

imeoer reviewed Jan 25, 2024

View reviewed changes

misc/snapshotter/nydus-snapshotter.yaml Outdated Show resolved Hide resolved

imeoer reviewed Jan 25, 2024

View reviewed changes

misc/snapshotter/nydus-snapshotter.yaml Outdated Show resolved Hide resolved

imeoer reviewed Jan 25, 2024

View reviewed changes

tests/e2e/k8s/snapshotter-cri.yaml Outdated Show resolved Hide resolved

imeoer reviewed Jan 25, 2024

View reviewed changes

tests/e2e/k8s/snapshotter-kubeconf.yaml Outdated Show resolved Hide resolved

fidencio mentioned this pull request Jan 25, 2024

cli tool to change containerd config confidential-containers/operator#313

Closed

ChengyuZhu6 force-pushed the payload branch 2 times, most recently from ca3e040 to cded377 Compare January 25, 2024 13:21

ChengyuZhu6 added 4 commits January 25, 2024 21:37

misc: support to configure snapshotter in containerd config

8baf2e6

This can be made transparent to users, eliminating the need for them to understand how to configure snapshotter in Containerd and reducing errors caused by misconfiguration. Signed-off-by: ChengyuZhu6 <[email protected]>

misc: introduce main function to handle

6b6c923

introduce main function to handle different commandline. Signed-off-by: ChengyuZhu6 <[email protected]>

misc: support to configmap to env and volume

0380827

Use configmap to decouple configuration artifacts from image content to keep containerized applications portable and inject configuration files and variables to the daemonset. Signed-off-by: ChengyuZhu6 <[email protected]>

ChengyuZhu6 force-pushed the payload branch 2 times, most recently from 7daf963 to 9c02cc2 Compare January 25, 2024 13:48

ChengyuZhu6 mentioned this pull request Jan 25, 2024

gha: Setup nydus snapshotter for CoCo tests kata-containers/kata-containers#8585

Merged

imeoer reviewed Jan 26, 2024

View reviewed changes

misc/snapshotter/config-blockdev.toml Outdated Show resolved Hide resolved

docs/setup_snapshotter_by_daemonset.md Outdated Show resolved Hide resolved

ChengyuZhu6 added 4 commits January 26, 2024 13:22

misc: support to run snapshotter as systemd service

d564321

Signed-off-by: ChengyuZhu6 <[email protected]>

docs: Add document to setup snapshotter by daemonset

ce51815

Add document to setup snapshotter by daemonset. Signed-off-by: ChengyuZhu6 <[email protected]>

misc: remove entrypoint

492e578

We no longer need the entrypoint because snapshotter.sh handles both the deployment and the cleanup of the snapshotter. Signed-off-by: ChengyuZhu6 <[email protected]>

misc: check the status after restarting the systemd service

ad4299e

Check the status of the systemd service after restarting it. Signed-off-by: ChengyuZhu6 <[email protected]>

ChengyuZhu6 force-pushed the payload branch 2 times, most recently from b5822db to 518d37c Compare January 26, 2024 05:23

tests: update the e2e tests

e0ba512

update the e2e tests. Signed-off-by: ChengyuZhu6 <[email protected]>

ChengyuZhu6 force-pushed the payload branch from 518d37c to e0ba512 Compare January 26, 2024 05:28

imeoer merged commit 918ca3b into containerd:main Jan 26, 2024
16 checks passed

ChengyuZhu6 deleted the payload branch January 26, 2024 06:15

A more easy way to setup nydus snapshotter by DaemonSet #570

A more easy way to setup nydus snapshotter by DaemonSet #570

Conversation

ChengyuZhu6 commented Dec 19, 2023 • edited Loading

ChengyuZhu6 commented Dec 19, 2023

adamqqqplay commented Dec 25, 2023

imeoer commented Dec 25, 2023

imeoer commented Dec 29, 2023

ChengyuZhu6 commented Dec 29, 2023 • edited Loading

imeoer commented Dec 29, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wainersm Jan 11, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wainersm commented Jan 9, 2024

beraldoleal left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

beraldoleal Jan 11, 2024 • edited Loading

Choose a reason for hiding this comment

ChengyuZhu6 Jan 12, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

imeoer commented Jan 26, 2024

imeoer commented Jan 26, 2024

ChengyuZhu6 commented Dec 19, 2023 •

edited

Loading

ChengyuZhu6 commented Dec 29, 2023 •

edited

Loading

wainersm Jan 11, 2024 •

edited

Loading

beraldoleal Jan 11, 2024 •

edited

Loading

ChengyuZhu6 Jan 12, 2024 •

edited

Loading