Use overlay for cached #104

udnay · 2025-02-05T21:35:04Z

Use an overlay for dateilager's cached service. Instead of giving each sandbox pod its own hard linked copy of the dl_cache we will mount an overlayfs of the cached's dl_cache

In the pod that's mounting to overlay we can see that we can hardlink between the 2 directories and that the pod can write to the cache and corrupt it.

however on the cached pod the contents are still fine

and additionally if we mount a second pod, it too sees the clean copy.

The pods are configured to use their ephemeral space so any writes to dl_cache will be cleaned up when the pod is killed.

The cached pod stores the "golden cache" at /var/lib/kubelet/dateilager_cache

For a simple example pod lets look at

apiVersion: v1
kind: Pod
metadata:
  name: busybox-csi
spec:
  containers:
  - name: busybox
    image: busybox
    command: 
      - "/bin/sh"
      - "-c"
      - "sleep infinity"
    volumeMounts:
    - name: gadget
      mountPath: /gadget
      subPath: gadget
  volumes:
  - name: gadget
    csi:
      driver: com.gadget.dateilager.cached
      volumeAttributes:
        placeCacheAtPath: "dl_cache"
        mountCache: "true"
      readOnly: false

A pod that is mounting a volume managed by the dateilager-csi will get a new ephemeral dir (like emptyDir) that is on the host /var/lib/kubelet/pods/88064d87-6870-4da5-8ba1-d9ac62b3f3ff/volumes/kubernetes.io~csi/gadget/mount this is the targetPath that comes in on the csi.NodePublishVolumeRequest

The directory structure that we create inside of the target dir is like this

/var/lib/kubelet/pods/
   | -- .../
      | -- gadget/mount/
          | -- gadget/
          | -- gadget_writeable/
          | -- work/

gadget is the target mount point for the overlay. gadget_writeable is the upperdir and work is the workdir

The readonly lowerdir is /var/lib/kubelet/dateilager_cache which contains dl_cache.

airhorns · 2025-02-10T14:18:33Z

pkg/api/cached.go

+			gadgetDir,
+			"-n",
+			"-o",
+			fmt.Sprintf("lowerdir=%s,upperdir=%s,workdir=%s", c.StagingPath, upperdir, workdir),


We should consider what value we want for the redirect_dir option. When I was working with overlayfs for making the gadget CI bits that reset the filesystem faster, I had to set it to on, but that's not necessarily true here. https://docs.kernel.org/filesystems/overlayfs.html#redirect-dir

See https://github.com/gadget-inc/gadget/pull/11070/files#diff-37490d715785dc29d4323daef03e84518b414f5fdca4d258c9f415c936080d4f if you are curious

TIL.
We can set it, I'm not sure how often we'll change the cache but it'll be nice to have the option.

What do you mean by

CI bits that reset the filesystem faster

As in, overlay was faster or something else?

udnay · 2025-02-10T14:38:11Z

@airhorns https://github.com/gadget-inc/gadget/pull/14578 CI is passing with the overlay running. I think this PR is ready for review.

pkg/api/cached.go

airhorns · 2025-02-10T15:20:20Z

pkg/api/cached.go

+	// Perform an overlay mount if the mountCache attribute is true
+	// Mount the root `StagingPath` at `targetPath/gadget` making `targetPath/gadget` `0777`
+	if mountCache, exists := volumeAttributes["mountCache"]; exists && mountCache == "true" {
+		upperdir := path.Join(targetPath, "gadget_writeable")


We gonna try to avoid this extra subfolder if we can after talking IRL

udnay · 2025-02-11T21:23:55Z

.github/workflows/test-integration.yml

@@ -0,0 +1,31 @@
+name: Test Integration


Added this because overlay won't run on mac's so bumping the csi-tests out to be more integration sanity tests.

udnay · 2025-02-11T21:25:52Z

pkg/api/cached.go

@@ -287,3 +355,13 @@ func getFolderSize(path string) (int64, error) {
 	})
 	return totalSize, err
 }
+
+func execCommand(cmdName string, args ...string) error {


In github actions we're not running as root, in our deployment we run as root in a privileged container so this is how we can call mount/umount during our tests. If there's a more elegant way to do this let me know.

I dont know of one 🤷

udnay · 2025-02-11T21:27:08Z

pkg/api/cached.go

-	if err := os.MkdirAll(targetPath, targetPermissions); err != nil {
-		return nil, fmt.Errorf("failed to create target directory %s: %s", targetPath, err)
+	workdir := path.Join(volumePath, WORK_DIR)
+	err = os.MkdirAll(workdir, 0777)


This did not seem to set the dir to 0777 so I had to add the explicit chmod below.

Terrifying -- maybe because its a mkdirall, the new perms only apply if it is creating new dirs, but it leaves existing ones alone?

udnay · 2025-02-11T21:27:50Z

@airhorns I made the changes, I still have some clean up to do but I think this is ready for a quick second pass.

airhorns

LGTM, or definitely good enough to canary

pkg/api/cached.go

airhorns · 2025-02-11T23:12:41Z

pkg/api/cached.go

-	if err := os.MkdirAll(targetPath, targetPermissions); err != nil {
-		return nil, fmt.Errorf("failed to create target directory %s: %s", targetPath, err)
+	workdir := path.Join(volumePath, WORK_DIR)
+	err = os.MkdirAll(workdir, 0777)


Terrifying -- maybe because its a mkdirall, the new perms only apply if it is creating new dirs, but it leaves existing ones alone?

airhorns · 2025-02-11T23:13:22Z

pkg/api/cached.go

@@ -287,3 +355,13 @@ func getFolderSize(path string) (int64, error) {
 	})
 	return totalSize, err
 }
+
+func execCommand(cmdName string, args ...string) error {


I dont know of one 🤷

airhorns · 2025-02-12T16:04:32Z

pkg/api/cached.go

+		"overlay",
+		"-n",
+		"--options",
+		fmt.Sprintf("redirect_dir=on,lowerdir=%s,upperdir=%s,workdir=%s", c.StagingPath, upperdir, workdir),


Did redirect_dir end up being necessary?

Not entirely, it seems to be useful if there are changes to the lower dir that files don't disappear which seems good in general.

Fix up mount permissions Make the cache dir not writeable by the sandbox (or non-DL user) Remove wonky integration test

Fixup overlay command and rename things to be less gadget centric

Use sudo Fixup tests Use sudo if env variable set Test tweaks

udnay requested review from airhorns and scott-rc February 5, 2025 21:35

Add overlay mount option to cached, target dl_cache

31f17ec

udnay force-pushed the yandu/use-overlay-for-cached branch from 7de191e to 9589736 Compare February 10, 2025 13:56

airhorns reviewed Feb 10, 2025

View reviewed changes

udnay marked this pull request as ready for review February 10, 2025 14:38

airhorns reviewed Feb 10, 2025

View reviewed changes

udnay commented Feb 11, 2025

View reviewed changes

airhorns approved these changes Feb 12, 2025

View reviewed changes

scott-rc approved these changes Feb 12, 2025

View reviewed changes

udnay added 8 commits February 12, 2025 12:06

local k8s helpers for testing

1cc035a

Fix up mount permissions Make the cache dir not writeable by the sandbox (or non-DL user) Remove wonky integration test

Always do overlay

3e65ad1

Unmount overlay when we unpublish

b7d43d3

Fixup overlay command and rename things to be less gadget centric

Cleanup work and upper dir

05debef

Add integration test workflow so that we can run mount

0a0d889

Use sudo Fixup tests Use sudo if env variable set Test tweaks

Remove redundant test

d0dffeb

Remove deprecated lint checker

bad7972

PR Review fixes

f366531

udnay force-pushed the yandu/use-overlay-for-cached branch from 7cf5d2e to f366531 Compare February 12, 2025 17:06

udnay changed the title ~~[WIP] use overlay for cached~~ Use overlay for cached Feb 12, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use overlay for cached #104

Use overlay for cached #104

udnay commented Feb 5, 2025 •

edited

Loading

airhorns Feb 10, 2025

udnay Feb 10, 2025

udnay commented Feb 10, 2025

airhorns Feb 10, 2025

udnay Feb 11, 2025

udnay Feb 11, 2025

airhorns Feb 11, 2025

udnay Feb 11, 2025

airhorns Feb 11, 2025

udnay commented Feb 11, 2025

airhorns left a comment

airhorns Feb 11, 2025

airhorns Feb 11, 2025

airhorns Feb 12, 2025

udnay Feb 12, 2025

Use overlay for cached #104

Are you sure you want to change the base?

Use overlay for cached #104

Conversation

udnay commented Feb 5, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

udnay commented Feb 10, 2025

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

udnay commented Feb 11, 2025

airhorns 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

udnay commented Feb 5, 2025 •

edited

Loading