generate accessed files list for container with NRI plugin #349

sctb512 · 2023-02-08T04:51:57Z

Generate an in-order accessed files list for container workload. The accessed files list can be set as prefetch-patterns when converting OCI images to nydus format images.

Related to dragonflyoss/nydus#807

Requirements

NRI 2.0: Which has been integrated into containerd since v1.7.0-beta.1.

Workflow

Run the optimizer as an NRI plugin to run optimizer server when the StartContainer event occurs.
Optimizer server joins container mount namespace and starts fanotify server.
Optimizer server detects fanotify event and sends the accessed file descriptor to client.
Optimizer client converts file descriptor to path.
Generate list of accessed files on local disk.
Convert OCI images with accessed files list to nydus image.

Generate an accessed files list

To install the optimizer and optimizer-server, all you need is the command:

make optimizer && make install-optimizer

Install containerd that supports NRI 2.0：

VERSION=1.7.0-beta.3

CONTAINERD_DIR=$(cat /lib/systemd/system/containerd.service | grep "ExecStart=" | awk -F= '{gsub("/containerd","",$2); print $2}')

wget https://github.com/containerd/containerd/releases/download/$VERSION/containerd-$VERSION-linux-amd64.tar.gz
tar -zxvf containerd-$VERSION-linux-amd64.tar.gz

sudo install -D -m 755 bin/* $CONTAINERD_DIR
rm -rf bin
$CONTAINERD_DIR/containerd --version

Modify containerd's toml configuration file to enable NRI.

sudo tee -a /etc/containerd/config.toml <<- EOF
[plugins."io.containerd.nri.v1.nri"]
  config_file = "/etc/nri/nri.conf"
  disable = false
  plugin_path = "/opt/nri/plugins"
  socket_path = "/var/run/nri.sock"
EOF

sudo tee /etc/nri/nri.conf <<- EOF
disableConnections: false
EOF

sudo systemctl restart containerd

Now, just start a container workload and you will get the list of accessed files in persistDir.

sudo tee nginx.yaml <<- EOF
metadata:
  name: nginx
image:
  image: nginx:latest
log_path: nginx.0.log
linux: {}
EOF

sudo tee pod.yaml <<- EOF
metadata:
  name: nginx-sandbox
  namespace: default
  attempt: 1
  uid: hdishd83djaidwnduwk28bcsb
log_directory: /tmp
linux: {}
EOF

crictl run nginx.yaml pod.yaml

The result file for the nginx image is /opt/nri/optimiser/results/nginx:latest.

Optimize a nydus image

Nydus provides a nydusify CLI tool to convert OCI images from the source registry or local file system to nydus format and push them to the target registry.

We can install the nydusify cli tool from the nydus package.

VERSION=v2.1.3

wget https://github.com/dragonflyoss/image-service/releases/download/$VERSION/nydus-static-$VERSION-linux-amd64.tgz
tar -zxvf nydus-static-$VERSION-linux-amd64.tgz
sudo install -D -m 755 nydus-static/nydusify /usr/local/bin/nydusify
sudo install -D -m 755 nydus-static/nydus-image /usr/local/bin/nydus-image

With the --prefetch-patterns argument, we can specify the list of files to be written in the front of the nydus image and be prefetched in order when starting a container.

sudo nydusify convert --nydus-image $(which nydus-image) --source nginx --target sctb512/nginx:optimized-nydusv6 --fs-version 6 --prefetch-patterns < /opt/nri/optimizer/results/nginx:latest

Make sure the nydusd and nydus-snapshotter environments are installed correctly. Then, start a container with an optimized nydus image.

sudo nerdctl --snapshotter nydus run --rm --net host -it sctb512/nginx:optimized-nydusv6

Signed-off-by: Bin Tang tangbin.bin@bytedance.com

codecov-commenter · 2023-02-14T03:28:45Z

Codecov Report

Base: 29.26% // Head: 29.32% // Increases project coverage by +0.06% 🎉

Coverage data is based on head (969dcac) compared to base (b010060).
Patch has no changes to coverable lines.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #349      +/-   ##
==========================================
+ Coverage   29.26%   29.32%   +0.06%     
==========================================
  Files          38       38              
  Lines        3810     3812       +2     
==========================================
+ Hits         1115     1118       +3     
+ Misses       2563     2562       -1     
  Partials      132      132

Impacted Files	Coverage Δ
pkg/daemon/daemon.go	`0.00% <0.00%> (ø)`
pkg/blob/blob_manager.go	`32.65% <0.00%> (+3.06%)`	⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

imeoer · 2023-02-15T06:00:20Z

Hi @sctb512 , any doc for this? Does it work on nydus image?

sctb512 · 2023-02-15T06:03:13Z

Hi @sctb512 , any doc for this? Does it work on nydus image?

Work in progress. This NRI plugin is image format independent. And it works on nydus image.

changweige · 2023-02-15T09:25:12Z

Makefile

 .PHONY: build
 build:
 	GOOS=${GOOS} GOARCH=${GOARCH} ${PROXY} go build -ldflags "$(LDFLAGS)" -v -o bin/containerd-nydus-grpc ./cmd/containerd-nydus-grpc
+	${CARGO} fmt --manifest-path ${OPTIMIZER_SERVER_TOML} -- --check
+	${CARGO} build --release --manifest-path ${OPTIMIZER_SERVER_TOML} && cp ${OPTIMIZER_SERVER_BIN} ./bin


Could we have a new Makefile target for building the optimizer ? It will even better if we have its own Makefile

By default, we should not build the optimizer as developers may not have rust environment

Yes, this is reasonable. Fixed it by adding build-optimizer. Is it acceptable?

changweige · 2023-02-15T09:27:38Z

cmd/optimizer/main.go

+)
+
+type config struct {
+	LogFile string   `yaml:"logFile"`


Could we use the Linux naming style like log_file

I followed this style in NRI's example code: https://github.com/containerd/nri/blob/main/plugins/logger/nri-logger.go#L33. Maybe we could keep the same style with them?

Anyway. It makes sense to use the same parameter style in this project. Already fixed.

changweige · 2023-02-15T09:27:51Z

cmd/optimizer/main.go

+	Events  []string `yaml:"events"`
+
+	ServerPath string `yaml:"serverPath"`
+	PersistDir string `yaml:"persistDir"`


changweige · 2023-02-15T09:29:07Z

cmd/optimizer/main.go

+	ServerPath string `yaml:"serverPath"`
+	PersistDir string `yaml:"persistDir"`
+	Timeout    int    `yaml:"timeout"`
+	Overwrite  bool   `yaml:"overwrite"`


Moreover, toml is preferred

changweige · 2023-02-15T09:30:54Z

cmd/optimizer/main.go

+		return 0, fmt.Errorf("failed to parse events in configuration: %w", err)
+	}
+
+	if cfg.LogFile != oldCfg.LogFile {


When to close the file f

Now the optimizer server will close the file descriptor after sending the path to the client.

Emm... It is not the same issue. The file descriptor mentioned in last comment is created by optimizer server. So we close it after getting the path.

The file f should be closed before exiting this NRI plugin, i.e. in onClose event.

changweige · 2023-02-20T05:54:00Z

tools/optimizer-server/src/main.rs

@@ -0,0 +1,128 @@
+use fanotify::low_level::*;


Please add license claim here

changweige · 2023-02-20T05:55:28Z

tools/optimizer-server/src/main.rs

+use std::io;
+use std::io::Write;
+use std::os::unix::io::AsRawFd;
+use std::path::Path;


We usually put std dependencies to the top position and then the third-party dependencies and then then project internal dependenecies

changweige · 2023-02-20T06:22:08Z

Makefile

+.PHONY: build-optimizer
+build-optimizer:
+	${CARGO} fmt --manifest-path ${OPTIMIZER_SERVER_TOML} -- --check
+	${CARGO} build --release --manifest-path ${OPTIMIZER_SERVER_TOML} && cp ${OPTIMIZER_SERVER_BIN} ./bin


Could you please also add cargo clippy to lint the tool, which will give some wise suggestions.

Good suggestion. I have added this.

changweige · 2023-02-20T06:24:38Z

cmd/optimizer-nri-plugin/main.go

+)
+
+const (
+	defaultLogFile    = "/opt/nri/optimizer/optimizer.log"


Logs should be stored in /var/log or just stream it to syslog service rather than set a file as default on at /opt

Now, we redirect it to the syslog service.

changweige · 2023-02-20T06:28:40Z

cmd/optimizer-nri-plugin/main.go

+	oldCfg := cfg
+	tree, err := toml.Load(config)
+	if err != nil {
+		return 0, fmt.Errorf("failed to parse TOML: %w", err)


Can you use errors.Errorf and errors.Wrapf ? %w is deprecated now in Golang

changweige · 2023-02-20T06:34:22Z

cmd/optimizer-nri-plugin/main.go

+func (p *plugin) StartContainer(pod *api.PodSandbox, container *api.Container) error {
+	dump("StartContainer", "pod", pod, "container", container)
+
+	imageName := GetImageName(container.Annotations)


We should validate the image. Containerd already provides a package to validate image name.

changweige · 2023-02-20T06:37:48Z

cmd/optimizer-nri-plugin/main.go

+	return update, nil
+}
+
+func GetImageName(annotations map[string]string) string {


There exists a standard method comes from contanerd package to extract the tagged image name. Can you try to use it.

changweige · 2023-02-20T06:49:33Z

tools/optimizer-server/src/main.rs

+
+    let mut fds = [PollFd::new(fd.as_raw_fd(), PollFlags::POLLIN)];
+    loop {
+        let poll_num = poll(&mut fds, -1).unwrap();


Try to avoid directly unwrap unless we be sure that it's safe

Fixed it with match.

changweige · 2023-02-20T07:18:47Z

pkg/fanotify/fanotify.go

+		return err
+	}
+
+	f, err := os.OpenFile(fserver.PersistFile, os.O_CREATE|os.O_WRONLY, 0644)


Should close the file f ?

Yes, now we close it before exiting the server.

changweige · 2023-02-21T03:38:24Z

tools/optimizer-server/src/main.rs

+        FAN_MARK_ADD | FAN_MARK_MOUNT,
+        FAN_OPEN | FAN_CLOSE_WRITE,
+        AT_FDCWD,
+        path,


FAN_OPEN_EXEC
No need to monitor event FAN_CLOSE_WRITE

Signed-off-by: Bin Tang <tangbin.bin@bytedance.com>

changweige

Thanks. This is indeed a useful tool to help us build more efficient nydus images

sctb512 force-pushed the generate-accessed-files-list branch 2 times, most recently from 90deffb to a987375 Compare February 13, 2023 11:31

sctb512 changed the title ~~generate accessed files list for container with OCI Hook~~ generate accessed files list for container with NRI plugin Feb 13, 2023

sctb512 force-pushed the generate-accessed-files-list branch from a987375 to b36c4c1 Compare February 13, 2023 11:38

sctb512 marked this pull request as ready for review February 13, 2023 11:38

sctb512 force-pushed the generate-accessed-files-list branch 4 times, most recently from be14df6 to 001eb62 Compare February 14, 2023 03:27

sctb512 force-pushed the generate-accessed-files-list branch 7 times, most recently from 1e4b537 to 358b9f9 Compare February 14, 2023 10:05

sctb512 force-pushed the generate-accessed-files-list branch 6 times, most recently from 7639b1e to e0b20b3 Compare February 15, 2023 08:48

changweige reviewed Feb 15, 2023

View reviewed changes

changweige reviewed Feb 20, 2023

View reviewed changes

sctb512 force-pushed the generate-accessed-files-list branch from afc4a6b to 521fd14 Compare February 20, 2023 06:08

changweige reviewed Feb 20, 2023

View reviewed changes

sctb512 force-pushed the generate-accessed-files-list branch 2 times, most recently from 39d79ca to a317d7f Compare February 20, 2023 11:51

changweige reviewed Feb 21, 2023

View reviewed changes

sctb512 force-pushed the generate-accessed-files-list branch 3 times, most recently from ac67fb3 to 640064c Compare February 21, 2023 07:11

sctb512 added 11 commits February 24, 2023 17:43

tools: optimizer server running in target mount namespace

ad4d091

Signed-off-by: Bin Tang <tangbin.bin@bytedance.com>

Makefile: support to build optimizer server

3ec6a69

Signed-off-by: Bin Tang <tangbin.bin@bytedance.com>

pkg: add fanotify package

2eeec3d

Signed-off-by: Bin Tang <tangbin.bin@bytedance.com>

cmd: add optimizer client as a NRI plugin

1f6010c

Signed-off-by: Bin Tang <tangbin.bin@bytedance.com>

misc: add configuration example for optimizer NRI plugin

0b00e51

Signed-off-by: Bin Tang <tangbin.bin@bytedance.com>

Makefile: support to build optimizer-nri-plugin

541132d

Signed-off-by: Bin Tang <tangbin.bin@bytedance.com>

update go.mod to adapt optimizer client

e378dec

Signed-off-by: Bin Tang <tangbin.bin@bytedance.com>

Makefile: add install-optimizer subcommand for make

093bf93

Signed-off-by: Bin Tang <tangbin.bin@bytedance.com>

optimizer: kill server of subprocess completely

c3efe9c

Signed-off-by: Bin Tang <tangbin.bin@bytedance.com>

docs: add document about optimizer

5f5ea66

Signed-off-by: Bin Tang <tangbin.bin@bytedance.com>

optimizer: redirect logs to stdout as well

5abe198

Signed-off-by: Bin Tang <tangbin.bin@bytedance.com>

sctb512 force-pushed the generate-accessed-files-list branch from 640064c to 5abe198 Compare February 24, 2023 09:58

changweige approved these changes Feb 24, 2023

View reviewed changes

changweige merged commit 6876c4f into containerd:main Feb 24, 2023

generate accessed files list for container with NRI plugin #349

generate accessed files list for container with NRI plugin #349

Conversation

sctb512 commented Feb 8, 2023 • edited Loading

Requirements

Workflow

Generate an accessed files list

Optimize a nydus image

codecov-commenter commented Feb 14, 2023 • edited Loading

Codecov Report

imeoer commented Feb 15, 2023

sctb512 commented Feb 15, 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

sctb512 Feb 17, 2023 • 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

changweige Feb 20, 2023 • 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

changweige left a comment

Choose a reason for hiding this comment

sctb512 commented Feb 8, 2023 •

edited

Loading

codecov-commenter commented Feb 14, 2023 •

edited

Loading

sctb512 Feb 17, 2023 •

edited

Loading

changweige Feb 20, 2023 •

edited

Loading