ociruntime: replace tar extraction subprocess with in-process

[sluongng]

Address the TODO comment.

[bduffany]

I was holding off on this TODO because I wanted to make sure the performance was on-par. Any chance you could run some quick tests with larger images with several layers (on a fast network connection, clearing cache between runs) to make sure it's not significantly slower and CPU usage is not significantly higher?

[sluongng]

I applied this patch on top of master

diff --git a/enterprise/server/remote_execution/containers/ociruntime/BUILD b/enterprise/server/remote_execution/containers/ociruntime/BUILD
index 5d70c3ab1d..4c3b386f12 100644
--- a/enterprise/server/remote_execution/containers/ociruntime/BUILD
+++ b/enterprise/server/remote_execution/containers/ociruntime/BUILD
@@ -48,6 +48,7 @@ go_test(
         "test.workload-isolation-type": "firecracker",
         "test.container-image": "docker://gcr.io/flame-public/net-tools@sha256:ac701954d2c522d0d2b5296323127cacaaf77627e69db848a8d6ecb53149d344",
         "test.EstimatedComputeUnits": "2",
+        "test.EstimatedFreeDiskBytes": "4.5GB",
     },
     # NOTE: if testing locally, use --test_sharding_strategy=disabled to work
     # around the networking package not supporting cross-process locks.
diff --git a/enterprise/server/remote_execution/containers/ociruntime/ociruntime_test.go b/enterprise/server/remote_execution/containers/ociruntime/ociruntime_test.go
index 4d307ad96d..278dd16af3 100644
--- a/enterprise/server/remote_execution/containers/ociruntime/ociruntime_test.go
+++ b/enterprise/server/remote_execution/containers/ociruntime/ociruntime_test.go
@@ -1119,3 +1119,33 @@ func hasMountPermissions(t *testing.T) bool {
        require.NoError(t, err, "unmount")
        return true
 }
+
+func TestPullImage(t *testing.T) {
+       for _, tc := range []struct {
+               name  string
+               image string
+       }{
+               {
+                       name:  "dockerhub_busybox",
+                       image: "busybox:latest",
+               },
+               {
+                       name:  "ghcr_nix",
+                       image: "ghcr.io/avdv/nix-build@sha256:5f731adacf7290352fed6c1960dfb56ec3fdb31a376d0f2170961fbc96944d50",
+               },
+               {
+                       name:  "executor_image",
+                       image: "gcr.io/flame-public/buildbuddy-executor-enterprise:latest",
+               },
+       } {
+               t.Run(tc.name, func(t *testing.T) {
+                       layerDir := t.TempDir()
+                       imgStore := ociruntime.NewImageStore(layerDir)
+
+                       ctx := context.Background()
+                       img, err := imgStore.Pull(ctx, tc.image, oci.Credentials{})
+                       require.NoError(t, err)
+                       require.NotNil(t, img)
+               })
+       }
+}

Then ran it with

bazel test --config=remote --test_tag_filters=+docker enterprise/server/remote_execution/containers/ociruntime/... --test_filter=TestPullImage/executor_image --runs_per_test=10 --test_sharding_strategy=disabled

Repeated the same test with this PR on top.

• Before: https://buildbuddy.buildbuddy.io/invocation/4caca435-a067-470b-b414-ca88ef686951?executionFilter=test#execution

• After: https://buildbuddy.buildbuddy.io/invocation/41f236d6-e5bb-4945-b777-4c059ec7c33e?executionFilter=test#execution

I think the Go approach could be slightly slower and higher memory vs pipe to tar. With this small sample size the Executions, I think they are negligible. I suspect if the Go app is saturated, gc will kick in a bit more eagerly though.

[bduffany]

LGTM assuming the TestFileOwnership test passes after you rebase (we aren't passing --no-same-owner anymore)

[sluongng]

I learned about https://github.com/google/safeopen/blob/main/safeopen_linux.go over the weekend as well as golang/go#67002 (comment) (coming to Go 1.24). But I think these could be added later in a different PR?

Thoughts on merging the current PR as-is @fmeum @bduffany ?

[sluongng]

Also learned about https://pkg.go.dev/github.com/landlock-lsm/go-landlock/landlock today which could be useful if we want to do these in a subprocess in the future.