add a doc for prefetchfiles-nri-plugin #524

billie60 · 2023-08-29T05:41:18Z

This document mainly introduces the early configuration and usage of prefetch-nri-plugin.

changweige · 2023-08-31T02:59:56Z

docs/prefetchfiles-nri-plugin.md

@@ -0,0 +1,156 @@
+# User self-defined nydus image files prefetch
+To improve the flexibility of nydus image files prefetch, for the k8s scenario, we can specify a prefetch files list when create a nydus daemon. The prefetch files list is user self-defined. Nydus-snapshotter has implemented a containerd NRI plugin to transmit the path of prefetch files list to nydus-snapshotter. The prefetch plugin requires NRI 2.0, which is available in containerd (>=v1.7.0). The prefetch plugin subscribes pod creation event, fetches the path of prefetch files list and forwards it to nydus-snapshotter. Then when `nydusd` starts, it will pull the files defined in the prefetch files list through lazy loading. This allows the pull of the prefetch files to be done during container creation rather than image convert, improving the flexibility of file prefetching.


Do you mean "the list of prefetch file paths"?

Thanks for your comment. What is saved in the prefetch file is indeed the list of prefetch file paths. But the --prefetch-files parameter of nydusd needs to specify a path that points to the file containing the prefetch files (maybe a txt file). So I mean the path of prefetch files list, which is the actual parameter of --prefetch-files

changweige · 2023-08-31T03:02:33Z

docs/prefetchfiles-nri-plugin.md

+## Workflow
+
+1. Add information such as image reference and prefetch list path to annotations in pod configuration file.
+2. Run the prefetch plugin to monitoring RunPodSandbox events.


Bad syntax, should be "to monitor"

changweige · 2023-08-31T03:03:26Z

docs/prefetchfiles-nri-plugin.md

+
+1. Add information such as image reference and prefetch list path to annotations in pod configuration file.
+2. Run the prefetch plugin to monitoring RunPodSandbox events.
+3. The prefetch plugin fetches image reference and prefetch files path and forwards them to nydus-snapshotter.


changweige · 2023-08-31T03:04:07Z

docs/prefetchfiles-nri-plugin.md

+3. The prefetch plugin fetches image reference and prefetch files path and forwards them to nydus-snapshotter.
+4. Nydus-snapshotter specifies the prefetch list when starting nydus daemon.
+5. Nydusd completes the mounting of the nydus image.
+<!-- 4. Run the prefetch plugin to fetch those information and forward them to nydus-snapshotter when the RunPodSandbox event occurs.  -->


What is this line for?

changweige · 2023-08-31T03:10:17Z

docs/prefetchfiles-nri-plugin.md

+
+
+
+Note that the naming of keys in annotations is fixed, and the values in annotations is user self-defined.


are user self-defined.

changweige · 2023-08-31T03:10:31Z

docs/prefetchfiles-nri-plugin.md

+
+
+Note that the naming of keys in annotations is fixed, and the values in annotations is user self-defined.
+After create a pod, image reference and path of prefetch list will received by nydus-snapshotter.


After creating a pod

changweige · 2023-08-31T03:13:04Z

docs/prefetchfiles-nri-plugin.md

+annotations:
+ containerd.io/nydus-prefetch: |
+    [
+    {"image": "dockerhub.kubekey.local/dfns/wordpress:nydus_latest", "prefetch": "/path/to/wordpress_prefetch.txt"}


Um. If the value is a file path, I am concerning how can we distribute the file to each K8s node?

Maybe add an comment: specify the prefetch value with an URL is coming. 😉

THANKS very much! Other comments have been modified, and I will also correct this comment in the next version commit

sctb512 · 2023-09-11T14:28:35Z

docs/prefetchfiles-nri-plugin.md

+socket_address = "/run/containerd-nydus/system.sock"
+EOF
+```
+Also, when manually starting the prefetch NRI plugin, the socket address can also be modified through the command line parameter `socket-addr`. Note that the priority of configuration file parameter passing is higher than that of command line parameter passing. 


Perhaps command line parameters have higher priority would make more sense.

sctb512 · 2023-09-11T14:31:29Z

docs/prefetchfiles-nri-plugin.md

+```
+Add a prefetch configuration file to specify the correct socket address. Note that this configuration file is designed for starting the NRI plugin in `pre-connection` mode. In this mode, the NRI plugin and containerd will start simultaneously.
+```console
+sudo tee /etc/nydus/prefetchConfig.toml <<- EOF


Is /etc/nydus/prefetchConfig.toml the right path? In default, containerd read NRI's configuration file from /etc/nri/conf.d.

sctb512 · 2023-09-11T14:35:26Z

docs/prefetchfiles-nri-plugin.md

+annotations:
+ containerd.io/nydus-prefetch: |
+    [
+    {"image": "dockerhub.kubekey.local/dfns/wordpress:nydus_latest", "prefetch": "/path/to/wordpress_prefetch.txt"}


Maybe add an comment: specify the prefetch value with an URL is coming. 😉

imeoer · 2023-09-26T01:43:49Z

The .idea directory should be removed.

sctb512 · 2023-09-26T02:05:42Z

misc/example/prefetchfiles-nri-plugin.conf

+socket_address = "/run/containerd-nydus/system.sock"
+# The events that containerd subscribes to.
+# Do not change this element.
+events = ["RunPodSandbox"]


There is no need to export this entry.

sctb512 · 2023-09-26T02:15:37Z

cmd/prefetchfiles-nri-plugin/main.go

-				}
-			} else {
-				globalSocket = flags.Args.SocketAddress
+			if globalSocket == "" {


I think we should verify if the flags.Args.Config.SocketAddr is nil.

if flags.Args.Config.SocketAddr != nil { globalSocket = flags.Args.Config.SocketAddr } else { globalSocket = defaultSystemControllerAddress }

sctb512 · 2023-09-26T02:16:47Z

cmd/prefetchfiles-nri-plugin/main.go

 	nydusPrefetchAnnotation        = "containerd.io/nydus-prefetch"
 )

+type PluginConfig struct {
+	SocketAddr string   `toml:"socket_address"`
+	Events     []string `toml:"events"`


We can remove it because it is unused. The defaultEvents is enough.

sctb512

Others, LGTM

sctb512 · 2023-10-24T10:57:29Z

.github/workflows/ci.yml

@@ -82,7 +82,8 @@ jobs:
      - name: Build
        run: |
          # Download nydus components
-          NYDUS_VER=v$(curl --header 'authorization: Bearer ${{ secrets.GITHUB_TOKEN }}' -s "https://api.github.com/repos/dragonflyoss/image-service/releases/latest" | jq -r .tag_name | sed 's/^v//')
+          # NYDUS_VER=v$(curl --header 'authorization: Bearer ${{ secrets.GITHUB_TOKEN }}' -s "https://api.github.com/repos/dragonflyoss/image-service/releases/latest" | jq -r .tag_name | sed 's/^v//')


Please remove the dead code.

sctb512 · 2023-10-24T11:36:46Z

cmd/prefetchfiles-nri-plugin/main.go

@@ -62,8 +64,8 @@ func buildFlags(args *PluginArgs) []cli.Flag {
 		&cli.StringFlag{
 			Name:        "socket-addr",
 			Value:       defaultSystemControllerAddress,
-			Usage:       "unix domain socket address. If defined in the configuration file, there is no need to add ",
-			Destination: &args.SocketAddress,
+			Usage:       "unix domain socket address.",


Maybe we need more information. UNIX domain socket address for connection to the nydus-snapshotter API.?

sctb512 · 2023-10-24T11:41:04Z

cmd/prefetchfiles-nri-plugin/main.go

 			} else {
-				globalSocket = flags.Args.SocketAddress
+				globalSocket = defaultSystemControllerAddress


No need to set defaultSystemControllerAddress for globalSocket because we have specify defaultSystemControllerAddress as the default value for SocketAddr flag. IOW, the flags.Args.Config.SocketAddr must not be nil.

sctb512 · 2023-10-24T11:44:14Z

docs/prefetchfiles-nri-plugin.md

+```console
+sudo systemctl restart containerd
+```
+If you want to start the plugin using `pre-connection` mode, that is, `disableConnections: true`. You need to write a configuration file and place the plugin's binary file and configuration file in the correct directories. Here is an example of configuration file:


The pre-connection mode is not related to disableConnections, they can work at the same time.

sctb512 · 2023-10-24T11:48:12Z

docs/prefetchfiles-nri-plugin.md

+
+crictl runp pod.yaml
+```
+The files that require prefetching are written in a URL, and `nydus-snapshoter` will read the prefetch list based on the URL address and transfer it to a local text file. The specific content of the prefetch files can be customized by the user.


The list of files to be prefetched is written in a URL, and `nydus-snapshoter` will read the prefetch list based on the URL address and transfer it to a local file.

sctb512 · 2023-10-24T11:55:24Z

misc/example/prefetchfiles-nri-plugin.conf

@@ -0,0 +1,3 @@
+# unix domain socket address


sctb512 · 2023-10-24T12:00:18Z

integration/prefetch.sh

Is it necessary to create a new script for integration?
Morever, entrypoint.sh is used as the ENTRYPOINT of image, but this script integration/prefetch.sh is not used by make integrate command.

sctb512 · 2023-10-24T12:02:52Z

pkg/prefetch/prefetch.go

 )

+const prefetchlistPath string = "/opt/PrefetchFiles"


Is this configurable?

sctb512 · 2023-10-24T12:05:13Z

pkg/prefetch/prefetch.go

+	defer resp.Body.Close()
+
+	if resp.StatusCode != http.StatusOK {
+		errMsg := fmt.Sprintf("HTTP response status code is not 200 OK: %s", resp.Status)


"failed to GET %s, response status $v", url, resp.Status

sctb512 · 2023-10-24T12:10:26Z

pkg/prefetch/prefetch.go

+	tokenizer := html.NewTokenizer(resp.Body)
+	inBody := false
+
+	for {


I don't understand why we need this loop. May have a more concise implementation?
Could you take a look at this? cc @imeoer

Yes, we can't assume the server side returns such html format.

codecov · 2023-10-31T09:01:57Z

Codecov Report

Attention: 20 lines in your changes are missing coverage. Please review.

Comparison is base (47d4311) 33.52% compared to head (c822caa) 33.49%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #524      +/-   ##
==========================================
- Coverage   33.52%   33.49%   -0.04%     
==========================================
  Files          65       65              
  Lines        8287     8298      +11     
==========================================
+ Hits         2778     2779       +1     
- Misses       5194     5204      +10     
  Partials      315      315

Files	Coverage Δ
pkg/daemon/daemon.go	`0.00% <ø> (ø)`
pkg/system/system.go	`4.79% <0.00%> (ø)`
config/global.go	`23.21% <33.33%> (+0.27%)`	⬆️
pkg/manager/daemon_adaptor.go	`0.00% <0.00%> (ø)`
pkg/manager/manager.go	`0.00% <0.00%> (ø)`
pkg/daemon/config.go	`0.00% <0.00%> (ø)`

imeoer · 2023-11-10T01:44:25Z

docs/prefetchfiles-nri-plugin.md

@@ -0,0 +1,127 @@
+# User self-defined nydus image files prefetch
+To improve the flexibility of nydus image files prefetch, for the k8s scenario, we can specify a prefetch files list when create a nydus daemon. The prefetch files list is user self-defined. Nydus-snapshotter has implemented a containerd NRI plugin to transmit the path of prefetch files list to nydus-snapshotter. The prefetch plugin requires NRI 2.0, which is available in containerd (>=v1.7.0). The prefetch plugin subscribes pod creation event, obtains the URL address containing the content of the files need to be prefetched, and forwards it to `nydus-snapshotter`. The `nydus-snapshotter` reads the data through the URL and stores it locally. Then when `nydusd` starts, it will pull the files defined in the prefetch files list through lazy loading. This allows the pull of the prefetch files to be done during container creation rather than image convert, improving the flexibility of file prefetching.


First should explain why we need the prefetch, then say the flexibility of prefetch on runtime.

imeoer · 2023-11-10T01:44:50Z

docs/prefetchfiles-nri-plugin.md

+
+## Requirements
+
+- [NRI 2.0](https://github.com/containerd/nri): Which has been integrated into containerd since [v1.7.0](https://github.com/containerd/containerd/tree/v1.7.0-beta.1).


Should be https://github.com/containerd/containerd/tree/v1.7.0 ?

imeoer · 2023-11-10T01:45:11Z

docs/prefetchfiles-nri-plugin.md

+
+
+
+


Remove multiple spaces before this.

imeoer · 2023-11-10T02:03:09Z

docs/prefetchfiles-nri-plugin.md

+log_directory: /tmp
+annotations:
+ containerd.io/nydus-prefetch: |
+    [


Format it:

[ { "image": "ghcr.io/dragonflyoss/image-service/wordpress:nydus-nightly-v6", "prefetch": "http://example.com/api/v1/resource/wordpress", } ]

Does the annotation work with multiple line value?

yes, if there are multiple images and prefetchlists in annotations, the code works correctly.

imeoer · 2023-11-10T02:14:30Z

go.mod

+	github.com/godbus/dbus/v5 v5.1.0 // indirect
+)
+
+require github.com/freddierice/go-losetup v0.0.0-20220711213114-2a14873012db


Please merge require section.

imeoer · 2023-11-10T02:15:20Z

misc/prefetch/crictl.yaml

+runtime-endpoint: unix:///run/containerd/containerd.sock
+image-endpoint: unix:///run/containerd/containerd.sock
+timeout: 10
+debug: true


Add blank line here.

imeoer · 2023-11-10T02:15:50Z

misc/prefetch/prefetch_server.go

+	"net/http"
+)
+
+func wordpress_prefetchlist(w http.ResponseWriter, req *http.Request) {


getWordpressPrefetchList

imeoer · 2023-11-10T02:15:59Z

misc/prefetch/prefetch_server.go

+	fmt.Fprintf(w, responseText)
+}
+
+func tomcat_prefetchlist(w http.ResponseWriter, req *http.Request) {


getTomcatPrefetchList

imeoer · 2023-11-10T02:16:30Z

misc/snapshotter/config.toml

@@ -63,6 +63,9 @@ address = ":9110"
 [remote]
 convert_vpc_registry = false

+[prefetch]
+dir = "/opt/prefetch"


How do we remove the useless prefetch list file in this dir?

imeoer · 2023-11-10T02:17:12Z

pkg/prefetch/prefetch.go

+type prefetchlist struct {
+	PrefetchFiles []string `json:"prefetchFiles"`
+}
+


type prefetchList struct { Files []string `json:"files"` }

imeoer · 2023-11-10T02:17:52Z

pkg/prefetch/prefetch.go

 		p.prefetchMap[image] = prefetchfiles
 	}

 	log.L.Infof("received prefetch list from nri plugin: %v ", p.prefetchMap)
 	return nil
 }

+// Obtain the prefetch files through the URL and transfer them to local files().
+func (p *prefetchInfo) getprefetchConfigFile(url, image string) (string, error) {


getPrefetchListPath

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

1. specify the prefetch value with an URL 2. Modify the reading mode of configuration file 3. add prefetchlist path in config.toml and so on Signed-off-by: billie60 <[email protected]>

Signed-off-by: billie60 <[email protected]> crictl 1.29.0 cni v1.4.0 dockerfile all newest version

changweige reviewed Aug 31, 2023

View reviewed changes

sctb512 reviewed Sep 11, 2023

View reviewed changes

billie60 force-pushed the prefetchnri-doc branch 4 times, most recently from e2f26c6 to 87b8fa0 Compare September 25, 2023 14:24

sctb512 reviewed Sep 26, 2023

View reviewed changes

billie60 force-pushed the prefetchnri-doc branch 8 times, most recently from 5ac4580 to 0bae30a Compare September 30, 2023 12:20

billie60 force-pushed the prefetchnri-doc branch 6 times, most recently from 21cb39c to d585766 Compare October 24, 2023 08:25

sctb512 reviewed Oct 24, 2023

View reviewed changes

billie60 force-pushed the prefetchnri-doc branch 5 times, most recently from 03a0a2e to 1589530 Compare October 31, 2023 08:49

billie60 force-pushed the prefetchnri-doc branch from 1589530 to f585f28 Compare October 31, 2023 10:52

imeoer reviewed Nov 10, 2023

View reviewed changes

docs/prefetchfiles-nri-plugin.md Outdated

Copy link

Collaborator

imeoer Nov 10, 2023

Choose a reason for hiding this comment

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

Remove multiple spaces before this.

imeoer reviewed Nov 10, 2023

View reviewed changes

billie60 force-pushed the prefetchnri-doc branch 5 times, most recently from 392de38 to eb73d5d Compare November 15, 2023 09:01

billie60 force-pushed the prefetchnri-doc branch from eb73d5d to 8f21a90 Compare December 5, 2023 03:51

billie60 force-pushed the prefetchnri-doc branch 5 times, most recently from 34808aa to 9d3ff61 Compare December 22, 2023 07:51

billie60 added 4 commits December 28, 2023 17:04

add .idea for GoLand

c4ac0c9

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

add a doc for prefetch-nri-plugin

2209cc0

1. specify the prefetch value with an URL 2. Modify the reading mode of configuration file 3. add prefetchlist path in config.toml and so on Signed-off-by: billie60 <[email protected]>

add prefetch integration test

1f3917f

Signed-off-by: billie60 <[email protected]> crictl 1.29.0 cni v1.4.0 dockerfile all newest version

Modify Makefile to automatically build prefetch plugin

013eec9

billie60 force-pushed the prefetchnri-doc branch 2 times, most recently from a418ce6 to c822caa Compare December 29, 2023 04:48

http server demo

c822caa

billie60 closed this Mar 13, 2024

		@@ -0,0 +1,156 @@
		# User self-defined nydus image files prefetch
		To improve the flexibility of nydus image files prefetch, for the k8s scenario, we can specify a prefetch files list when create a nydus daemon. The prefetch files list is user self-defined. Nydus-snapshotter has implemented a containerd NRI plugin to transmit the path of prefetch files list to nydus-snapshotter. The prefetch plugin requires NRI 2.0, which is available in containerd (>=v1.7.0). The prefetch plugin subscribes pod creation event, fetches the path of prefetch files list and forwards it to nydus-snapshotter. Then when `nydusd` starts, it will pull the files defined in the prefetch files list through lazy loading. This allows the pull of the prefetch files to be done during container creation rather than image convert, improving the flexibility of file prefetching.




		Note that the naming of keys in annotations is fixed, and the values in annotations is user self-defined.



		Note that the naming of keys in annotations is fixed, and the values in annotations is user self-defined.
		After create a pod, image reference and path of prefetch list will received by nydus-snapshotter.

		@@ -0,0 +1,127 @@
		# User self-defined nydus image files prefetch
		To improve the flexibility of nydus image files prefetch, for the k8s scenario, we can specify a prefetch files list when create a nydus daemon. The prefetch files list is user self-defined. Nydus-snapshotter has implemented a containerd NRI plugin to transmit the path of prefetch files list to nydus-snapshotter. The prefetch plugin requires NRI 2.0, which is available in containerd (>=v1.7.0). The prefetch plugin subscribes pod creation event, obtains the URL address containing the content of the files need to be prefetched, and forwards it to `nydus-snapshotter`. The `nydus-snapshotter` reads the data through the URL and stores it locally. Then when `nydusd` starts, it will pull the files defined in the prefetch files list through lazy loading. This allows the pull of the prefetch files to be done during container creation rather than image convert, improving the flexibility of file prefetching.


		## Requirements

		- [NRI 2.0](https://github.com/containerd/nri): Which has been integrated into containerd since [v1.7.0](https://github.com/containerd/containerd/tree/v1.7.0-beta.1).

add a doc for prefetchfiles-nri-plugin #524

add a doc for prefetchfiles-nri-plugin #524

Conversation

billie60 commented Aug 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

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 Sep 26, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sctb512 Sep 26, 2023 • edited Loading

Choose a reason for hiding this comment

sctb512 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Oct 31, 2023 • edited Loading

Codecov Report

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 Nov 10, 2023 • edited Loading

Choose a reason for hiding this comment

imeoer Nov 10, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sctb512 Sep 26, 2023 •

edited

Loading

codecov bot commented Oct 31, 2023 •

edited

Loading

imeoer Nov 10, 2023 •

edited

Loading

imeoer Nov 10, 2023 •

edited

Loading