From 1bdd4c3d3a280b2c438cac97b89ded52271dbfa9 Mon Sep 17 00:00:00 2001 From: Xuehao Xu Date: Thu, 30 Jul 2026 18:18:37 +0800 Subject: [PATCH 1/8] fix(kubernetes): clean up snapshot registry images --- docs/guides/pause-resume.md | 2 +- docs/kubernetes/index.md | 4 +- .../charts/opensandbox-controller/values.yaml | 2 +- kubernetes/cmd/controller/main.go | 2 +- kubernetes/go.mod | 15 +- kubernetes/go.sum | 34 ++++- .../controller/registry_image_deleter.go | 128 ++++++++++++++++++ .../controller/registry_image_deleter_test.go | 90 ++++++++++++ .../controller/sandboxsnapshot_controller.go | 4 + .../sandboxsnapshot_controller_test.go | 91 +++++++++++++ .../controller/sandboxsnapshot_lifecycle.go | 47 ++++++- 11 files changed, 406 insertions(+), 13 deletions(-) create mode 100644 kubernetes/internal/controller/registry_image_deleter.go create mode 100644 kubernetes/internal/controller/registry_image_deleter_test.go diff --git a/docs/guides/pause-resume.md b/docs/guides/pause-resume.md index 8f71f824d..57c79e1c7 100644 --- a/docs/guides/pause-resume.md +++ b/docs/guides/pause-resume.md @@ -152,7 +152,7 @@ Configure the controller manager deployment with snapshot flags: |-----|------|---------|-------------| | `--snapshot-registry` | string | `""` | **Required.** OCI registry prefix. Images are stored as `/-:snap-gen`. | | `--snapshot-registry-insecure` | bool | `false` | Enables insecure registry mode for snapshot push operations. Use only for HTTP or self-signed local registries. | -| `--snapshot-push-secret` | string | `""` | Kubernetes Secret name for pushing snapshots. Must be `kubernetes.io/dockerconfigjson` type. | +| `--snapshot-push-secret` | string | `""` | Kubernetes Secret name for pushing and deleting snapshot images. Must be `kubernetes.io/dockerconfigjson` type, and the credentials must permit manifest deletion. | | `--resume-pull-secret` | string | `""` | Kubernetes Secret name injected into resumed sandboxes for pulling snapshot images. Can be the same as push secret. | | `--image-committer-image` | string | `"image-committer:dev"` | Image used by commit Jobs. | | `--commit-job-timeout` | duration | `"10m"` | Timeout for commit Jobs. | diff --git a/docs/kubernetes/index.md b/docs/kubernetes/index.md index 567a0c646..aaae1b266 100644 --- a/docs/kubernetes/index.md +++ b/docs/kubernetes/index.md @@ -137,7 +137,7 @@ The snapshot controller supports the following command-line flags: | Flag | Default | Description | |------|---------|-------------| | `--snapshot-registry` | `""` | OCI registry prefix used for snapshot images | -| `--snapshot-push-secret` | `""` | Secret name used by commit Jobs to push snapshots | +| `--snapshot-push-secret` | `""` | Secret name used to push and delete snapshot images | | `--resume-pull-secret` | `""` | Secret name injected into resumed sandboxes for image pulls | | `--image-committer-image` | `image-committer:dev` | Image used for commit operations (must contain `nerdctl` tool) | | `--commit-job-timeout` | `10m` | Timeout duration for commit jobs | @@ -171,7 +171,7 @@ Then configure the controller manager with: ``` ::: info -Snapshot image retention is registry-managed. Deleting a `SandboxSnapshot` removes the Kubernetes commit/unpause Jobs, but it does not delete pushed OCI images from the registry. Configure registry retention/GC for tags such as `snap-gen` according to your environment. +Deleting a `SandboxSnapshot` also deletes its pushed OCI images before the controller removes the finalizer. Keep the configured `--snapshot-push-secret` available and grant its registry credentials permission to pull and delete manifests. Registry garbage collection may still be required to reclaim unreferenced blob storage, depending on the registry implementation. ::: ## Getting Started diff --git a/kubernetes/charts/opensandbox-controller/values.yaml b/kubernetes/charts/opensandbox-controller/values.yaml index cdb1432e3..536196ef1 100644 --- a/kubernetes/charts/opensandbox-controller/values.yaml +++ b/kubernetes/charts/opensandbox-controller/values.yaml @@ -68,7 +68,7 @@ controller: registry: "" # -- Use insecure registry mode when pushing snapshot images. registryInsecure: false - # -- Secret name used by commit Jobs to push snapshot images. + # -- Secret name used to push and delete snapshot images. snapshotPushSecret: "" # -- Secret name for pulling the image-committer image in commit Jobs. # Required when imageCommitterImage is stored in a private registry. diff --git a/kubernetes/cmd/controller/main.go b/kubernetes/cmd/controller/main.go index 4482fbd19..1c97d1bb0 100644 --- a/kubernetes/cmd/controller/main.go +++ b/kubernetes/cmd/controller/main.go @@ -218,7 +218,7 @@ func main() { flag.BoolVar(&snapshotRegistryInsecure, "snapshot-registry-insecure", false, "Use insecure registry mode when pushing snapshot images.") var snapshotPushSecret string - flag.StringVar(&snapshotPushSecret, "snapshot-push-secret", "", "K8s Secret name for pushing snapshots to registry.") + flag.StringVar(&snapshotPushSecret, "snapshot-push-secret", "", "K8s Secret name for pushing and deleting snapshots in the registry.") var imageCommitterPullSecret string flag.StringVar(&imageCommitterPullSecret, "image-committer-pull-secret", "", "K8s Secret name for pulling the image-committer image in commit Jobs. Required when imageCommitterImage is in a private registry.") diff --git a/kubernetes/go.mod b/kubernetes/go.mod index 18f3948db..7c8d1a755 100644 --- a/kubernetes/go.mod +++ b/kubernetes/go.mod @@ -4,6 +4,7 @@ go 1.25.0 require ( github.com/golang/mock v1.6.0 + github.com/google/go-containerregistry v0.20.6 github.com/onsi/ginkgo/v2 v2.22.0 github.com/onsi/gomega v1.36.1 github.com/stretchr/testify v1.11.1 @@ -18,9 +19,19 @@ require ( require github.com/cenkalti/backoff/v5 v5.0.3 // indirect require ( + github.com/containerd/stargz-snapshotter/estargz v0.16.3 // indirect + github.com/docker/cli v28.2.2+incompatible // indirect + github.com/docker/distribution v2.8.3+incompatible // indirect + github.com/docker/docker-credential-helpers v0.9.3 // indirect github.com/gorilla/websocket v1.5.4-0.20250319132907-e064f32e3674 // indirect + github.com/klauspost/compress v1.18.0 // indirect + github.com/mitchellh/go-homedir v1.1.0 // indirect github.com/moby/spdystream v0.5.1 // indirect github.com/mxk/go-flowrate v0.0.0-20140419014527-cca7078d478f // indirect + github.com/opencontainers/go-digest v1.0.0 // indirect + github.com/opencontainers/image-spec v1.1.1 // indirect + github.com/sirupsen/logrus v1.9.3 // indirect + github.com/vbatts/tar-split v0.12.1 // indirect go.yaml.in/yaml/v3 v3.0.4 // indirect ) @@ -65,8 +76,8 @@ require ( github.com/prometheus/client_model v0.6.1 // indirect github.com/prometheus/common v0.62.0 // indirect github.com/prometheus/procfs v0.15.1 // indirect - github.com/spf13/cobra v1.8.1 // indirect - github.com/spf13/pflag v1.0.5 // indirect + github.com/spf13/cobra v1.9.1 // indirect + github.com/spf13/pflag v1.0.6 // indirect github.com/x448/float16 v0.8.4 // indirect go.opentelemetry.io/auto/sdk v1.2.1 // indirect go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.63.0 // indirect diff --git a/kubernetes/go.sum b/kubernetes/go.sum index 6e9cb736d..b703834df 100644 --- a/kubernetes/go.sum +++ b/kubernetes/go.sum @@ -12,11 +12,19 @@ github.com/cenkalti/backoff/v5 v5.0.3 h1:ZN+IMa753KfX5hd8vVaMixjnqRZ3y8CuJKRKj1x github.com/cenkalti/backoff/v5 v5.0.3/go.mod h1:rkhZdG3JZukswDf7f0cwqPNk4K0sa+F97BxZthm/crw= github.com/cespare/xxhash/v2 v2.3.0 h1:UL815xU9SqsFlibzuggzjXhog7bL6oX9BbNZnL2UFvs= github.com/cespare/xxhash/v2 v2.3.0/go.mod h1:VGX0DQ3Q6kWi7AoAeZDth3/j3BFtOZR5XLFGgcrjCOs= -github.com/cpuguy83/go-md2man/v2 v2.0.4/go.mod h1:tgQtvFlXSQOSOSIRvRPT7W67SCa46tRHOmNcaadrF8o= +github.com/containerd/stargz-snapshotter/estargz v0.16.3 h1:7evrXtoh1mSbGj/pfRccTampEyKpjpOnS3CyiV1Ebr8= +github.com/containerd/stargz-snapshotter/estargz v0.16.3/go.mod h1:uyr4BfYfOj3G9WBVE8cOlQmXAbPN9VEQpBBeJIuOipU= +github.com/cpuguy83/go-md2man/v2 v2.0.6/go.mod h1:oOW0eioCTA6cOiMLiUPZOpcVxMig6NIQQ7OS05n1F4g= github.com/creack/pty v1.1.9/go.mod h1:oKZEueFk5CKHvIhNR5MUki03XCEU+Q6VDXinZuGJ33E= github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= +github.com/docker/cli v28.2.2+incompatible h1:qzx5BNUDFqlvyq4AHzdNB7gSyVTmU4cgsyN9SdInc1A= +github.com/docker/cli v28.2.2+incompatible/go.mod h1:JLrzqnKDaYBop7H2jaqPtU4hHvMKP+vjCwu2uszcLI8= +github.com/docker/distribution v2.8.3+incompatible h1:AtKxIZ36LoNK51+Z6RpzLpddBirtxJnzDrHLEKxTAYk= +github.com/docker/distribution v2.8.3+incompatible/go.mod h1:J2gT2udsDAN96Uj4KfcMRqY0/ypR+oyYUYmja8H+y+w= +github.com/docker/docker-credential-helpers v0.9.3 h1:gAm/VtF9wgqJMoxzT3Gj5p4AqIjCBS4wrsOh9yRqcz8= +github.com/docker/docker-credential-helpers v0.9.3/go.mod h1:x+4Gbw9aGmChi3qTLZj8Dfn0TD20M/fuWy0E5+WDeCo= github.com/emicklei/go-restful/v3 v3.11.0 h1:rAQeMHw1c7zTmncogyy8VvRZwtkmkZ4FxERmMY4rD+g= github.com/emicklei/go-restful/v3 v3.11.0/go.mod h1:6n3XBCmQQb25CM2LCACGz8ukIrRry+4bhvbpWn3mrbc= github.com/evanphx/json-patch v4.12.0+incompatible h1:4onqiflcdA9EOZ4RxV643DvftH5pOlLGNtQ5lPWQu84= @@ -61,6 +69,8 @@ github.com/google/gnostic-models v0.6.9/go.mod h1:CiWsm0s6BSQd1hRn8/QmxqB6BesYcb github.com/google/go-cmp v0.5.9/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY= github.com/google/go-cmp v0.7.0 h1:wk8382ETsv4JYUZwIsn6YpYiWiBsYLSJiTsyBybVuN8= github.com/google/go-cmp v0.7.0/go.mod h1:pXiqmnSA92OHEEa9HXL2W4E7lf9JzCmGVUdgjX3N/iU= +github.com/google/go-containerregistry v0.20.6 h1:cvWX87UxxLgaH76b4hIvya6Dzz9qHB31qAwjAohdSTU= +github.com/google/go-containerregistry v0.20.6/go.mod h1:T0x8MuoAoKX/873bkeSfLD2FAkwCDf9/HZgsFJ02E2Y= github.com/google/gofuzz v1.0.0/go.mod h1:dBl0BpW6vV/+mYPU4Po3pmUjxk6FQPldtuIdl/M65Eg= github.com/google/gofuzz v1.2.0 h1:xRy4A+RhZaiKjJ1bPfwQ8sedCA+YS2YcCHW6ec7JMi0= github.com/google/gofuzz v1.2.0/go.mod h1:dBl0BpW6vV/+mYPU4Po3pmUjxk6FQPldtuIdl/M65Eg= @@ -93,6 +103,8 @@ github.com/kylelemons/godebug v1.1.0 h1:RPNrshWIDI6G2gRW9EHilWtl7Z6Sb1BR0xunSBf0 github.com/kylelemons/godebug v1.1.0/go.mod h1:9/0rRGxNHcop5bhtWyNeEfOS8JIWk580+fNqagV/RAw= github.com/mailru/easyjson v0.7.7 h1:UGYAvKxe3sBsEDzO8ZeWOSlIQfWFlxbzLZe7hwFURr0= github.com/mailru/easyjson v0.7.7/go.mod h1:xzfreul335JAWq5oZzymOObrkdz5UnU4kGfJJLY9Nlc= +github.com/mitchellh/go-homedir v1.1.0 h1:lukF9ziXFxDFPkA1vsr5zpc1XuPDn/wFntq5mG+4E0Y= +github.com/mitchellh/go-homedir v1.1.0/go.mod h1:SfyaCUpYCn1Vlf4IUYiD9fPX4A5wJrkLzIz1N1q0pr0= github.com/moby/spdystream v0.5.1 h1:9sNYeYZUcci9R6/w7KDaFWEWeV4LStVG78Mpyq/Zm/Y= github.com/moby/spdystream v0.5.1/go.mod h1:xBAYlnt/ay+11ShkdFKNAG7LsyK/tmNBVvVOwrfMgdI= github.com/modern-go/concurrent v0.0.0-20180228061459-e0a39a4cb421/go.mod h1:6dJC0mAP4ikYIbvyc7fijjWJddQyLn8Ig3JB5CqoB9Q= @@ -108,6 +120,10 @@ github.com/onsi/ginkgo/v2 v2.22.0 h1:Yed107/8DjTr0lKCNt7Dn8yQ6ybuDRQoMGrNFKzMfHg github.com/onsi/ginkgo/v2 v2.22.0/go.mod h1:7Du3c42kxCUegi0IImZ1wUQzMBVecgIHjR1C+NkhLQo= github.com/onsi/gomega v1.36.1 h1:bJDPBO7ibjxcbHMgSCoo4Yj18UWbKDlLwX1x9sybDcw= github.com/onsi/gomega v1.36.1/go.mod h1:PvZbdDc8J6XJEpDK4HCuRBm8a6Fzp9/DmhC9C7yFlog= +github.com/opencontainers/go-digest v1.0.0 h1:apOUWs51W5PlhuyGyz9FCeeBIOUDA/6nW8Oi/yOhh5U= +github.com/opencontainers/go-digest v1.0.0/go.mod h1:0JzlMkj0TRzQZfJkVvzbP0HBR3IKzErnv2BNG4W4MAM= +github.com/opencontainers/image-spec v1.1.1 h1:y0fUlFfIZhPF1W537XOLg0/fcx6zcHCJwooC2xJA040= +github.com/opencontainers/image-spec v1.1.1/go.mod h1:qpqAh3Dmcf36wStyyWU+kCeDgrGnAve2nCC8+7h8Q0M= github.com/pkg/errors v0.9.1 h1:FEBLx1zS214owpjy7qsBeixbURkuhQAwrK5UwLGTwt4= github.com/pkg/errors v0.9.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= @@ -123,21 +139,26 @@ github.com/prometheus/procfs v0.15.1/go.mod h1:fB45yRUv8NstnjriLhBQLuOUt+WW4BsoG github.com/rogpeppe/go-internal v1.14.1 h1:UQB4HGPB6osV0SQTLymcB4TgvyWu6ZyliaW0tI/otEQ= github.com/rogpeppe/go-internal v1.14.1/go.mod h1:MaRKkUm5W0goXpeCfT7UZI6fk/L7L7so1lCWt35ZSgc= github.com/russross/blackfriday/v2 v2.1.0/go.mod h1:+Rmxgy9KzJVeS9/2gXHxylqXiyQDYRxCVz55jmeOWTM= -github.com/spf13/cobra v1.8.1 h1:e5/vxKd/rZsfSJMUX1agtjeTDf+qv1/JdBF8gg5k9ZM= -github.com/spf13/cobra v1.8.1/go.mod h1:wHxEcudfqmLYa8iTfL+OuZPbBZkmvliBWKIezN3kD9Y= -github.com/spf13/pflag v1.0.5 h1:iy+VFUOCP1a+8yFto/drg2CJ5u0yRoB7fZw3DKv/JXA= -github.com/spf13/pflag v1.0.5/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An2Bg= +github.com/sirupsen/logrus v1.9.3 h1:dueUQJ1C2q9oE3F7wvmSGAaVtTmUizReu6fjN8uqzbQ= +github.com/sirupsen/logrus v1.9.3/go.mod h1:naHLuLoDiP4jHNo9R0sCBMtWGeIprob74mVsIT4qYEQ= +github.com/spf13/cobra v1.9.1 h1:CXSaggrXdbHK9CF+8ywj8Amf7PBRmPCOJugH954Nnlo= +github.com/spf13/cobra v1.9.1/go.mod h1:nDyEzZ8ogv936Cinf6g1RU9MRY64Ir93oCnqb9wxYW0= +github.com/spf13/pflag v1.0.6 h1:jFzHGLGAlb3ruxLB8MhbI6A8+AQX/2eW4qeyNZXNp2o= +github.com/spf13/pflag v1.0.6/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An2Bg= github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= github.com/stretchr/objx v0.4.0/go.mod h1:YvHI0jy2hoMjB+UWwv71VJQ9isScKT/TqJzVSSt89Yw= github.com/stretchr/objx v0.5.0/go.mod h1:Yh+to48EsGEfYuaHDzXPcE3xhTkx73EhmCGUpEOglKo= github.com/stretchr/objx v0.5.2 h1:xuMeJ0Sdp5ZMRXx/aWO6RZxdr3beISkG5/G/aIRr3pY= github.com/stretchr/objx v0.5.2/go.mod h1:FRsXN1f5AsAjCGJKqEizvkpNtU+EGNCLh3NxZ/8L+MA= github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI= +github.com/stretchr/testify v1.7.0/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= github.com/stretchr/testify v1.7.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= github.com/stretchr/testify v1.8.0/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO+kdMU+MU= github.com/stretchr/testify v1.8.1/go.mod h1:w2LPCIKwWwSfY2zedu0+kehJoqGctiVI29o6fzry7u4= github.com/stretchr/testify v1.11.1 h1:7s2iGBzp5EwR7/aIZr8ao5+dra3wiQyKjjFuvgVKu7U= github.com/stretchr/testify v1.11.1/go.mod h1:wZwfW3scLgRK+23gO65QZefKpKQRnfz6sD981Nm4B6U= +github.com/vbatts/tar-split v0.12.1 h1:CqKoORW7BUWBe7UL/iqTVvkTBOF8UvOMKOIZykxnnbo= +github.com/vbatts/tar-split v0.12.1/go.mod h1:eF6B6i6ftWQcDqEn3/iGFRFRo8cBIMSJVOpnNdfTMFA= github.com/x448/float16 v0.8.4 h1:qLwI1I70+NjRFUR3zs1JPUCgaCXSh3SW62uAKT1mSBM= github.com/x448/float16 v0.8.4/go.mod h1:14CWIYCyZA/cWjXOioeEpHeN/83MdbZDRQHoFcYsOfg= github.com/yuin/goldmark v1.1.27/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74= @@ -200,6 +221,7 @@ golang.org/x/sys v0.0.0-20200930185726-fdedc70b468f/go.mod h1:h1NjWce9XRLGQEsW7w golang.org/x/sys v0.0.0-20201119102817-f84b799fce68/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20210330210617-4fbd30eecc44/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20210510120138-977fb7262007/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= +golang.org/x/sys v0.0.0-20220715151400-c0bba94af5f8/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.45.0 h1:dO4czNzziLiiXplLQgBCEpCvXQ3dnkn0SdaZSYdQ+FY= golang.org/x/sys v0.45.0/go.mod h1:4GL1E5IUh+htKOUEOaiffhrAeqysfVGipDYzABqnCmw= golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo= @@ -246,6 +268,8 @@ gopkg.in/natefinch/lumberjack.v2 v2.2.1/go.mod h1:YD8tP3GAjkrDg1eZH7EGmyESg/lsYs gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= +gotest.tools/v3 v3.0.3 h1:4AuOwCGf4lLR9u3YOe2awrHygurzhO/HeQ6laiA6Sx0= +gotest.tools/v3 v3.0.3/go.mod h1:Z7Lb0S5l+klDB31fvDQX8ss/FlKDxtlFlw3Oa8Ymbl8= k8s.io/api v0.33.0 h1:yTgZVn1XEe6opVpP1FylmNrIFWuDqe2H0V8CT5gxfIU= k8s.io/api v0.33.0/go.mod h1:CTO61ECK/KU7haa3qq8sarQ0biLq2ju405IZAd9zsiM= k8s.io/apiextensions-apiserver v0.33.0 h1:d2qpYL7Mngbsc1taA4IjJPRJ9ilnsXIrndH+r9IimOs= diff --git a/kubernetes/internal/controller/registry_image_deleter.go b/kubernetes/internal/controller/registry_image_deleter.go new file mode 100644 index 000000000..7a8c3cca6 --- /dev/null +++ b/kubernetes/internal/controller/registry_image_deleter.go @@ -0,0 +1,128 @@ +// Copyright 2025 Alibaba Group Holding Ltd. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package controller + +import ( + "context" + "encoding/json" + "errors" + "fmt" + "net/http" + "strings" + + "github.com/google/go-containerregistry/pkg/authn" + "github.com/google/go-containerregistry/pkg/name" + "github.com/google/go-containerregistry/pkg/v1/remote" + "github.com/google/go-containerregistry/pkg/v1/remote/transport" + corev1 "k8s.io/api/core/v1" +) + +type registryImageDeleter interface { + Delete(ctx context.Context, imageReference string, registrySecret *corev1.Secret, insecure bool) error +} + +type remoteRegistryImageDeleter struct{} + +func (remoteRegistryImageDeleter) Delete( + ctx context.Context, + imageReference string, + registrySecret *corev1.Secret, + insecure bool, +) error { + nameOptions := []name.Option{name.StrictValidation} + if insecure { + nameOptions = append(nameOptions, name.Insecure) + } + + ref, err := name.ParseReference(imageReference, nameOptions...) + if err != nil { + return fmt.Errorf("parse image reference %q: %w", imageReference, err) + } + + authenticator, err := registryAuthenticator(registrySecret, ref.Context().RegistryStr()) + if err != nil { + return err + } + remoteOptions := []remote.Option{remote.WithContext(ctx), remote.WithAuth(authenticator)} + + // Registry implementations commonly require deletion by digest. Resolve + // legacy snapshots that do not have imageDigest recorded before deleting. + if _, ok := ref.(name.Digest); !ok { + descriptor, headErr := remote.Head(ref, remoteOptions...) + if isRegistryNotFound(headErr) { + return nil + } + if headErr != nil { + return fmt.Errorf("resolve image digest for %q: %w", imageReference, headErr) + } + ref = ref.Context().Digest(descriptor.Digest.String()) + } + + if err := remote.Delete(ref, remoteOptions...); err != nil && !isRegistryNotFound(err) { + return fmt.Errorf("delete image %q: %w", imageReference, err) + } + return nil +} + +func registryAuthenticator(secret *corev1.Secret, registry string) (authn.Authenticator, error) { + if secret == nil { + return authn.Anonymous, nil + } + + var auths map[string]authn.AuthConfig + switch { + case len(secret.Data[corev1.DockerConfigJsonKey]) > 0: + var config struct { + Auths map[string]authn.AuthConfig `json:"auths"` + } + if err := json.Unmarshal(secret.Data[corev1.DockerConfigJsonKey], &config); err != nil { + return nil, fmt.Errorf("parse registry secret %s/%s: %w", secret.Namespace, secret.Name, err) + } + auths = config.Auths + case len(secret.Data[corev1.DockerConfigKey]) > 0: + if err := json.Unmarshal(secret.Data[corev1.DockerConfigKey], &auths); err != nil { + return nil, fmt.Errorf("parse registry secret %s/%s: %w", secret.Namespace, secret.Name, err) + } + default: + return nil, fmt.Errorf("registry secret %s/%s has neither %s nor %s", secret.Namespace, secret.Name, corev1.DockerConfigJsonKey, corev1.DockerConfigKey) + } + + for server, config := range auths { + if normalizeRegistry(server) == normalizeRegistry(registry) { + return authn.FromConfig(config), nil + } + } + return nil, fmt.Errorf("registry secret %s/%s has no credentials for %s", secret.Namespace, secret.Name, registry) +} + +func normalizeRegistry(registry string) string { + registry = strings.TrimPrefix(registry, "https://") + registry = strings.TrimPrefix(registry, "http://") + registry = strings.TrimSuffix(registry, "/") + registry = strings.TrimSuffix(registry, "/v1") + registry = strings.TrimSuffix(registry, "/v2") + if registry == "index.docker.io" { + return name.DefaultRegistry + } + return registry +} + +func isRegistryNotFound(err error) bool { + if err == nil { + return false + } + var transportErr *transport.Error + return errors.As(err, &transportErr) && transportErr.StatusCode == http.StatusNotFound +} diff --git a/kubernetes/internal/controller/registry_image_deleter_test.go b/kubernetes/internal/controller/registry_image_deleter_test.go new file mode 100644 index 000000000..7697eb988 --- /dev/null +++ b/kubernetes/internal/controller/registry_image_deleter_test.go @@ -0,0 +1,90 @@ +// Copyright 2025 Alibaba Group Holding Ltd. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package controller + +import ( + "context" + "net/http" + "net/http/httptest" + "strings" + "testing" + + "github.com/google/go-containerregistry/pkg/authn" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +func TestRemoteRegistryImageDeleter_ResolvesTagAndDeletesDigest(t *testing.T) { + const digest = "sha256:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" + deletedPath := make(chan string, 1) + registry := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, request *http.Request) { + switch { + case request.URL.Path == "/v2/": + w.WriteHeader(http.StatusOK) + case request.Method == http.MethodHead && request.URL.Path == "/v2/snapshots/test/manifests/tag": + w.Header().Set("Docker-Content-Digest", digest) + w.Header().Set("Content-Type", "application/vnd.oci.image.manifest.v1+json") + w.Header().Set("Content-Length", "2") + w.WriteHeader(http.StatusOK) + case request.Method == http.MethodDelete: + deletedPath <- request.URL.Path + w.WriteHeader(http.StatusAccepted) + default: + http.NotFound(w, request) + } + })) + defer registry.Close() + + imageReference := strings.TrimPrefix(registry.URL, "http://") + "/snapshots/test:tag" + err := (remoteRegistryImageDeleter{}).Delete(context.Background(), imageReference, nil, true) + require.NoError(t, err) + assert.Equal(t, "/v2/snapshots/test/manifests/"+digest, <-deletedPath) +} + +func TestRegistryAuthenticator_ReadsDockerConfigJSON(t *testing.T) { + secret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{Name: "registry-secret", Namespace: "default"}, + Data: map[string][]byte{ + corev1.DockerConfigJsonKey: []byte(`{"auths":{"https://registry.example.com/v1/":{"username":"user","password":"pass"}}}`), + }, + } + + authenticator, err := registryAuthenticator(secret, "registry.example.com") + require.NoError(t, err) + config, err := authn.Authorization(context.Background(), authenticator) + require.NoError(t, err) + assert.Equal(t, "user", config.Username) + assert.Equal(t, "pass", config.Password) +} + +func TestRegistryAuthenticator_RejectsSecretWithoutMatchingRegistry(t *testing.T) { + secret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{Name: "registry-secret", Namespace: "default"}, + Data: map[string][]byte{ + corev1.DockerConfigJsonKey: []byte(`{"auths":{"other.example.com":{"auth":"dXNlcjpwYXNz"}}}`), + }, + } + + _, err := registryAuthenticator(secret, "registry.example.com") + require.ErrorContains(t, err, "has no credentials for registry.example.com") +} + +func TestRegistryAuthenticator_AllowsAnonymousRegistry(t *testing.T) { + authenticator, err := registryAuthenticator(nil, "registry.example.com") + require.NoError(t, err) + assert.Equal(t, authn.Anonymous, authenticator) +} diff --git a/kubernetes/internal/controller/sandboxsnapshot_controller.go b/kubernetes/internal/controller/sandboxsnapshot_controller.go index b79769cdb..10d8fcbb2 100644 --- a/kubernetes/internal/controller/sandboxsnapshot_controller.go +++ b/kubernetes/internal/controller/sandboxsnapshot_controller.go @@ -89,6 +89,10 @@ type SandboxSnapshotReconciler struct { // SnapshotRegistryInsecure controls whether image-committer uses insecure registry mode. SnapshotRegistryInsecure bool + + // registryImageDeleter performs remote manifest cleanup. When nil, the + // reconciler uses remoteRegistryImageDeleter. + registryImageDeleter registryImageDeleter } // +kubebuilder:rbac:groups=sandbox.opensandbox.io,resources=sandboxsnapshots,verbs=get;list;watch;create;update;patch;delete diff --git a/kubernetes/internal/controller/sandboxsnapshot_controller_test.go b/kubernetes/internal/controller/sandboxsnapshot_controller_test.go index 31ada010a..b45413862 100644 --- a/kubernetes/internal/controller/sandboxsnapshot_controller_test.go +++ b/kubernetes/internal/controller/sandboxsnapshot_controller_test.go @@ -16,6 +16,7 @@ package controller import ( "context" + "errors" "fmt" "testing" "time" @@ -36,6 +37,35 @@ import ( sandboxv1alpha1 "github.com/alibaba/OpenSandbox/sandbox-k8s/apis/sandbox/v1alpha1" ) +type registryDeleteCall struct { + imageReference string + secretName string + insecure bool +} + +type recordingRegistryImageDeleter struct { + calls []registryDeleteCall + err error +} + +func (d *recordingRegistryImageDeleter) Delete( + _ context.Context, + imageReference string, + secret *corev1.Secret, + insecure bool, +) error { + secretName := "" + if secret != nil { + secretName = secret.Name + } + d.calls = append(d.calls, registryDeleteCall{ + imageReference: imageReference, + secretName: secretName, + insecure: insecure, + }) + return d.err +} + func newTestSnapshotReconciler(objs ...client.Object) *SandboxSnapshotReconciler { scheme := k8sruntime.NewScheme() utilruntime.Must(corev1.AddToScheme(scheme)) @@ -55,6 +85,67 @@ func newTestSnapshotReconciler(objs ...client.Object) *SandboxSnapshotReconciler } } +func TestSandboxSnapshotHandleDeletion_DeletesRegistryImagesBeforeRemovingFinalizer(t *testing.T) { + snapshot := &sandboxv1alpha1.SandboxSnapshot{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-snapshot", + Namespace: "default", + Finalizers: []string{SandboxSnapshotFinalizer}, + }, + Status: sandboxv1alpha1.SandboxSnapshotStatus{ + Containers: []sandboxv1alpha1.ContainerSnapshot{ + {ContainerName: "main", ImageURI: "registry.example.com/snapshots/main:tag", ImageDigest: "sha256:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"}, + {ContainerName: "main-duplicate", ImageURI: "registry.example.com/snapshots/main:tag", ImageDigest: "sha256:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"}, + {ContainerName: "sidecar", ImageURI: "registry.example.com/snapshots/sidecar:tag"}, + }, + }, + } + secret := &corev1.Secret{ObjectMeta: metav1.ObjectMeta{Name: "registry-secret", Namespace: "default"}} + deleter := &recordingRegistryImageDeleter{} + r := newTestSnapshotReconciler(snapshot, secret) + r.SnapshotPushSecret = secret.Name + r.SnapshotRegistryInsecure = true + r.registryImageDeleter = deleter + + result, err := r.handleDeletion(context.Background(), snapshot) + require.NoError(t, err) + assert.Equal(t, ctrl.Result{}, result) + require.Len(t, deleter.calls, 2) + assert.Equal(t, "registry.example.com/snapshots/main@sha256:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", deleter.calls[0].imageReference) + assert.Equal(t, "registry.example.com/snapshots/sidecar:tag", deleter.calls[1].imageReference) + assert.Equal(t, "registry-secret", deleter.calls[0].secretName) + assert.True(t, deleter.calls[0].insecure) + + updated := &sandboxv1alpha1.SandboxSnapshot{} + require.NoError(t, r.Get(context.Background(), types.NamespacedName{Name: snapshot.Name, Namespace: snapshot.Namespace}, updated)) + assert.NotContains(t, updated.Finalizers, SandboxSnapshotFinalizer) +} + +func TestSandboxSnapshotHandleDeletion_KeepsFinalizerWhenRegistryDeleteFails(t *testing.T) { + snapshot := &sandboxv1alpha1.SandboxSnapshot{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-snapshot", + Namespace: "default", + Finalizers: []string{SandboxSnapshotFinalizer}, + }, + Status: sandboxv1alpha1.SandboxSnapshotStatus{ + Containers: []sandboxv1alpha1.ContainerSnapshot{ + {ContainerName: "main", ImageURI: "registry.example.com/snapshots/main:tag"}, + }, + }, + } + deleter := &recordingRegistryImageDeleter{err: errors.New("registry unavailable")} + r := newTestSnapshotReconciler(snapshot) + r.registryImageDeleter = deleter + + _, err := r.handleDeletion(context.Background(), snapshot) + require.ErrorContains(t, err, "registry unavailable") + + updated := &sandboxv1alpha1.SandboxSnapshot{} + require.NoError(t, r.Get(context.Background(), types.NamespacedName{Name: snapshot.Name, Namespace: snapshot.Namespace}, updated)) + assert.Contains(t, updated.Finalizers, SandboxSnapshotFinalizer) +} + func TestSandboxSnapshotHandleCommitting_SetsSucceedReadyCondition(t *testing.T) { snapshot := &sandboxv1alpha1.SandboxSnapshot{ ObjectMeta: metav1.ObjectMeta{ diff --git a/kubernetes/internal/controller/sandboxsnapshot_lifecycle.go b/kubernetes/internal/controller/sandboxsnapshot_lifecycle.go index eaf41596c..de6298f4e 100644 --- a/kubernetes/internal/controller/sandboxsnapshot_lifecycle.go +++ b/kubernetes/internal/controller/sandboxsnapshot_lifecycle.go @@ -174,10 +174,14 @@ func findJobCondition(conditions []batchv1.JobCondition, conditionType batchv1.J return nil } -// handleDeletion cleans up the commit job and removes the finalizer. +// handleDeletion cleans up snapshot images and jobs, then removes the finalizer. func (r *SandboxSnapshotReconciler) handleDeletion(ctx context.Context, snapshot *sandboxv1alpha1.SandboxSnapshot) (ctrl.Result, error) { log := logf.FromContext(ctx) + if err := r.deleteSnapshotImages(ctx, snapshot); err != nil { + return ctrl.Result{}, err + } + jobName := r.getJobName(snapshot) job := &batchv1.Job{} if err := r.Get(ctx, types.NamespacedName{Namespace: snapshot.Namespace, Name: jobName}, job); err == nil { @@ -204,6 +208,47 @@ func (r *SandboxSnapshotReconciler) handleDeletion(ctx context.Context, snapshot return ctrl.Result{}, nil } +func (r *SandboxSnapshotReconciler) deleteSnapshotImages(ctx context.Context, snapshot *sandboxv1alpha1.SandboxSnapshot) error { + if len(snapshot.Status.Containers) == 0 { + return nil + } + + var registrySecret *corev1.Secret + if r.SnapshotPushSecret != "" { + registrySecret = &corev1.Secret{} + if err := r.Get(ctx, types.NamespacedName{Namespace: snapshot.Namespace, Name: r.SnapshotPushSecret}, registrySecret); err != nil { + return fmt.Errorf("get snapshot registry secret %s/%s: %w", snapshot.Namespace, r.SnapshotPushSecret, err) + } + } + + deleter := r.registryImageDeleter + if deleter == nil { + deleter = remoteRegistryImageDeleter{} + } + deleted := make(map[string]struct{}, len(snapshot.Status.Containers)) + for _, container := range snapshot.Status.Containers { + if container.ImageURI == "" { + continue + } + imageReference := container.ImageURI + if container.ImageDigest != "" { + imageReference = strings.SplitN(container.ImageURI, "@", 2)[0] + if tagIndex := strings.LastIndex(imageReference, ":"); tagIndex > strings.LastIndex(imageReference, "/") { + imageReference = imageReference[:tagIndex] + } + imageReference += "@" + container.ImageDigest + } + if _, exists := deleted[imageReference]; exists { + continue + } + if err := deleter.Delete(ctx, imageReference, registrySecret, r.SnapshotRegistryInsecure); err != nil { + return fmt.Errorf("delete snapshot image for container %s: %w", container.ContainerName, err) + } + deleted[imageReference] = struct{}{} + } + return nil +} + // findPodForSandbox finds the running pod belonging to a BatchSandbox. func (r *SandboxSnapshotReconciler) findPodForSandbox(ctx context.Context, bs *sandboxv1alpha1.BatchSandbox, namespace string) (*corev1.Pod, error) { alloc, err := parseSandboxAllocation(bs) From acf2fdf6e3fa1d91f13e9375a774a3c116bdeccd Mon Sep 17 00:00:00 2001 From: Xuehao Xu Date: Mon, 10 Aug 2026 13:14:17 +0800 Subject: [PATCH 2/8] fix(kubernetes): address snapshot cleanup review feedback --- docs/guides/pause-resume.md | 9 ++- docs/kubernetes/index.md | 4 +- .../controller/registry_image_deleter.go | 18 +++++- .../controller/registry_image_deleter_test.go | 35 ++++++++++- .../controller/sandboxsnapshot_controller.go | 2 +- .../sandboxsnapshot_controller_test.go | 46 ++++++++++++++- .../controller/sandboxsnapshot_lifecycle.go | 58 +++++++++++++------ kubernetes/test/e2e/pause_resume_test.go | 53 +++++++++++++++++ 8 files changed, 200 insertions(+), 25 deletions(-) diff --git a/docs/guides/pause-resume.md b/docs/guides/pause-resume.md index 57c79e1c7..4f63dfb3b 100644 --- a/docs/guides/pause-resume.md +++ b/docs/guides/pause-resume.md @@ -152,7 +152,7 @@ Configure the controller manager deployment with snapshot flags: |-----|------|---------|-------------| | `--snapshot-registry` | string | `""` | **Required.** OCI registry prefix. Images are stored as `/-:snap-gen`. | | `--snapshot-registry-insecure` | bool | `false` | Enables insecure registry mode for snapshot push operations. Use only for HTTP or self-signed local registries. | -| `--snapshot-push-secret` | string | `""` | Kubernetes Secret name for pushing and deleting snapshot images. Must be `kubernetes.io/dockerconfigjson` type, and the credentials must permit manifest deletion. | +| `--snapshot-push-secret` | string | `""` | Kubernetes Secret name for pushing and deleting snapshot images. Must be `kubernetes.io/dockerconfigjson` type, contain inline `auths` credentials for the registry, and permit manifest deletion. `credHelpers`/`credsStore` entries are not usable by the controller. | | `--resume-pull-secret` | string | `""` | Kubernetes Secret name injected into resumed sandboxes for pulling snapshot images. Can be the same as push secret. | | `--image-committer-image` | string | `"image-committer:dev"` | Image used by commit Jobs. | | `--commit-job-timeout` | duration | `"10m"` | Timeout for commit Jobs. | @@ -318,7 +318,12 @@ Before pausing containers, `image-committer` attempts to run `sync` inside every If the commit Job fails, the controller creates a best-effort `-unpause` Job on the same node to unpause any source containers that may have been left paused by an abrupt committer exit. -Deleting a `SandboxSnapshot` cleans up Kubernetes commit/unpause Jobs, but does not delete pushed OCI images from the registry. Repeated pause cycles create tags such as `snap-gen`; configure registry retention or garbage collection externally. +Deleting a `SandboxSnapshot` stops its commit/unpause Jobs, deletes pushed OCI manifests, and then removes the Kubernetes finalizer. Registry garbage collection may still be required to reclaim unreferenced blob storage. If the registry or credentials are permanently unavailable, remove the finalizer manually only after accepting that the image may need separate registry cleanup: + +```bash +kubectl patch sandboxsnapshot -n --type=merge \ + -p '{"metadata":{"finalizers":[]}}' +``` ### Monitoring diff --git a/docs/kubernetes/index.md b/docs/kubernetes/index.md index aaae1b266..c9429e452 100644 --- a/docs/kubernetes/index.md +++ b/docs/kubernetes/index.md @@ -137,7 +137,7 @@ The snapshot controller supports the following command-line flags: | Flag | Default | Description | |------|---------|-------------| | `--snapshot-registry` | `""` | OCI registry prefix used for snapshot images | -| `--snapshot-push-secret` | `""` | Secret name used to push and delete snapshot images | +| `--snapshot-push-secret` | `""` | Secret name used to push and delete snapshot images; must contain inline `auths` credentials with manifest delete permission | | `--resume-pull-secret` | `""` | Secret name injected into resumed sandboxes for image pulls | | `--image-committer-image` | `image-committer:dev` | Image used for commit operations (must contain `nerdctl` tool) | | `--commit-job-timeout` | `10m` | Timeout duration for commit jobs | @@ -171,7 +171,7 @@ Then configure the controller manager with: ``` ::: info -Deleting a `SandboxSnapshot` also deletes its pushed OCI images before the controller removes the finalizer. Keep the configured `--snapshot-push-secret` available and grant its registry credentials permission to pull and delete manifests. Registry garbage collection may still be required to reclaim unreferenced blob storage, depending on the registry implementation. +Deleting a `SandboxSnapshot` stops its commit/unpause Jobs and deletes its pushed OCI images before the controller removes the finalizer. Keep the configured `--snapshot-push-secret` available during cleanup. If the registry is permanently unavailable, remove the finalizer manually with `kubectl patch sandboxsnapshot -n --type=merge -p '{"metadata":{"finalizers":[]}}'`, then clean up the registry image separately. Registry garbage collection may still be required to reclaim unreferenced blob storage. ::: ## Getting Started diff --git a/kubernetes/internal/controller/registry_image_deleter.go b/kubernetes/internal/controller/registry_image_deleter.go index 7a8c3cca6..83ed0640a 100644 --- a/kubernetes/internal/controller/registry_image_deleter.go +++ b/kubernetes/internal/controller/registry_image_deleter.go @@ -59,6 +59,8 @@ func (remoteRegistryImageDeleter) Delete( // Registry implementations commonly require deletion by digest. Resolve // legacy snapshots that do not have imageDigest recorded before deleting. + // This compatibility path can only delete the manifest currently behind the + // tag; snapshots with a recorded digest are protected from tag reuse. if _, ok := ref.(name.Digest); !ok { descriptor, headErr := remote.Head(ref, remoteOptions...) if isRegistryNotFound(headErr) { @@ -82,15 +84,21 @@ func registryAuthenticator(secret *corev1.Secret, registry string) (authn.Authen } var auths map[string]authn.AuthConfig + var credentialHelpers map[string]string + var credentialStore string switch { case len(secret.Data[corev1.DockerConfigJsonKey]) > 0: var config struct { - Auths map[string]authn.AuthConfig `json:"auths"` + Auths map[string]authn.AuthConfig `json:"auths"` + CredHelpers map[string]string `json:"credHelpers"` + CredsStore string `json:"credsStore"` } if err := json.Unmarshal(secret.Data[corev1.DockerConfigJsonKey], &config); err != nil { return nil, fmt.Errorf("parse registry secret %s/%s: %w", secret.Namespace, secret.Name, err) } auths = config.Auths + credentialHelpers = config.CredHelpers + credentialStore = config.CredsStore case len(secret.Data[corev1.DockerConfigKey]) > 0: if err := json.Unmarshal(secret.Data[corev1.DockerConfigKey], &auths); err != nil { return nil, fmt.Errorf("parse registry secret %s/%s: %w", secret.Namespace, secret.Name, err) @@ -104,6 +112,14 @@ func registryAuthenticator(secret *corev1.Secret, registry string) (authn.Authen return authn.FromConfig(config), nil } } + for server, helper := range credentialHelpers { + if normalizeRegistry(server) == normalizeRegistry(registry) { + return nil, fmt.Errorf("registry secret %s/%s uses credential helper %q for %s; controller requires inline auths credentials", secret.Namespace, secret.Name, helper, registry) + } + } + if credentialStore != "" && len(auths) == 0 { + return nil, fmt.Errorf("registry secret %s/%s uses credential store %q; controller requires inline auths credentials", secret.Namespace, secret.Name, credentialStore) + } return nil, fmt.Errorf("registry secret %s/%s has no credentials for %s", secret.Namespace, secret.Name, registry) } diff --git a/kubernetes/internal/controller/registry_image_deleter_test.go b/kubernetes/internal/controller/registry_image_deleter_test.go index 7697eb988..dbae7d8cd 100644 --- a/kubernetes/internal/controller/registry_image_deleter_test.go +++ b/kubernetes/internal/controller/registry_image_deleter_test.go @@ -20,6 +20,7 @@ import ( "net/http/httptest" "strings" "testing" + "time" "github.com/google/go-containerregistry/pkg/authn" "github.com/stretchr/testify/assert" @@ -52,7 +53,27 @@ func TestRemoteRegistryImageDeleter_ResolvesTagAndDeletesDigest(t *testing.T) { imageReference := strings.TrimPrefix(registry.URL, "http://") + "/snapshots/test:tag" err := (remoteRegistryImageDeleter{}).Delete(context.Background(), imageReference, nil, true) require.NoError(t, err) - assert.Equal(t, "/v2/snapshots/test/manifests/"+digest, <-deletedPath) + select { + case path := <-deletedPath: + assert.Equal(t, "/v2/snapshots/test/manifests/"+digest, path) + case <-time.After(time.Second): + t.Fatal("timed out waiting for registry DELETE request") + } +} + +func TestRemoteRegistryImageDeleter_TreatsMissingManifestAsSuccess(t *testing.T) { + registry := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, request *http.Request) { + if request.URL.Path == "/v2/" { + w.WriteHeader(http.StatusOK) + return + } + http.NotFound(w, request) + })) + defer registry.Close() + + imageReference := strings.TrimPrefix(registry.URL, "http://") + "/snapshots/missing@sha256:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" + err := (remoteRegistryImageDeleter{}).Delete(context.Background(), imageReference, nil, true) + require.NoError(t, err) } func TestRegistryAuthenticator_ReadsDockerConfigJSON(t *testing.T) { @@ -88,3 +109,15 @@ func TestRegistryAuthenticator_AllowsAnonymousRegistry(t *testing.T) { require.NoError(t, err) assert.Equal(t, authn.Anonymous, authenticator) } + +func TestRegistryAuthenticator_RejectsCredentialHelpers(t *testing.T) { + secret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{Name: "registry-secret", Namespace: "default"}, + Data: map[string][]byte{ + corev1.DockerConfigJsonKey: []byte(`{"credHelpers":{"registry.example.com":"osxkeychain"}}`), + }, + } + + _, err := registryAuthenticator(secret, "registry.example.com") + require.ErrorContains(t, err, "requires inline auths credentials") +} diff --git a/kubernetes/internal/controller/sandboxsnapshot_controller.go b/kubernetes/internal/controller/sandboxsnapshot_controller.go index 10d8fcbb2..a40bb9a53 100644 --- a/kubernetes/internal/controller/sandboxsnapshot_controller.go +++ b/kubernetes/internal/controller/sandboxsnapshot_controller.go @@ -80,7 +80,7 @@ type SandboxSnapshotReconciler struct { // SnapshotRegistry is the OCI registry for snapshot images (from Controller Manager startup params) SnapshotRegistry string - // SnapshotPushSecret is the K8s Secret name for pushing to registry (from Controller Manager startup params) + // SnapshotPushSecret is the K8s Secret name for pushing and deleting snapshot images (from Controller Manager startup params) SnapshotPushSecret string // ImageCommitterPullSecret is the K8s Secret name used to pull the image-committer image in commit Jobs. diff --git a/kubernetes/internal/controller/sandboxsnapshot_controller_test.go b/kubernetes/internal/controller/sandboxsnapshot_controller_test.go index b45413862..c70b2212b 100644 --- a/kubernetes/internal/controller/sandboxsnapshot_controller_test.go +++ b/kubernetes/internal/controller/sandboxsnapshot_controller_test.go @@ -101,14 +101,21 @@ func TestSandboxSnapshotHandleDeletion_DeletesRegistryImagesBeforeRemovingFinali }, } secret := &corev1.Secret{ObjectMeta: metav1.ObjectMeta{Name: "registry-secret", Namespace: "default"}} + commitJob := &batchv1.Job{ObjectMeta: metav1.ObjectMeta{Name: "test-snapshot-commit", Namespace: "default"}} + unpauseJob := &batchv1.Job{ObjectMeta: metav1.ObjectMeta{Name: "test-snapshot-unpause", Namespace: "default"}} deleter := &recordingRegistryImageDeleter{} - r := newTestSnapshotReconciler(snapshot, secret) + r := newTestSnapshotReconciler(snapshot, secret, commitJob, unpauseJob) r.SnapshotPushSecret = secret.Name r.SnapshotRegistryInsecure = true r.registryImageDeleter = deleter result, err := r.handleDeletion(context.Background(), snapshot) require.NoError(t, err) + assert.Equal(t, time.Second, result.RequeueAfter) + assert.Empty(t, deleter.calls, "registry cleanup must wait for jobs to terminate") + + result, err = r.handleDeletion(context.Background(), snapshot) + require.NoError(t, err) assert.Equal(t, ctrl.Result{}, result) require.Len(t, deleter.calls, 2) assert.Equal(t, "registry.example.com/snapshots/main@sha256:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", deleter.calls[0].imageReference) @@ -121,6 +128,24 @@ func TestSandboxSnapshotHandleDeletion_DeletesRegistryImagesBeforeRemovingFinali assert.NotContains(t, updated.Finalizers, SandboxSnapshotFinalizer) } +func TestSandboxSnapshotHandleDeletion_WaitsForJobPods(t *testing.T) { + now := metav1.Now() + snapshot := &sandboxv1alpha1.SandboxSnapshot{ + ObjectMeta: metav1.ObjectMeta{Name: "test-snapshot", Namespace: "default", Finalizers: []string{SandboxSnapshotFinalizer}}, + Status: sandboxv1alpha1.SandboxSnapshotStatus{Containers: []sandboxv1alpha1.ContainerSnapshot{{ContainerName: "main", ImageURI: "registry.example.com/snapshots/main:tag"}}}, + } + job := &batchv1.Job{ObjectMeta: metav1.ObjectMeta{Name: "test-snapshot-commit", Namespace: "default", DeletionTimestamp: &now, Finalizers: []string{"foregroundDeletion"}}} + pod := &corev1.Pod{ObjectMeta: metav1.ObjectMeta{Name: "test-snapshot-commit-pod", Namespace: "default", Labels: map[string]string{"job-name": "test-snapshot-commit"}}} + deleter := &recordingRegistryImageDeleter{} + r := newTestSnapshotReconciler(snapshot, job, pod) + r.registryImageDeleter = deleter + + result, err := r.handleDeletion(context.Background(), snapshot) + require.NoError(t, err) + assert.Equal(t, time.Second, result.RequeueAfter) + assert.Empty(t, deleter.calls) +} + func TestSandboxSnapshotHandleDeletion_KeepsFinalizerWhenRegistryDeleteFails(t *testing.T) { snapshot := &sandboxv1alpha1.SandboxSnapshot{ ObjectMeta: metav1.ObjectMeta{ @@ -146,6 +171,25 @@ func TestSandboxSnapshotHandleDeletion_KeepsFinalizerWhenRegistryDeleteFails(t * assert.Contains(t, updated.Finalizers, SandboxSnapshotFinalizer) } +func TestSandboxSnapshotHandleDeletion_KeepsFinalizerWhenRegistrySecretIsMissing(t *testing.T) { + snapshot := &sandboxv1alpha1.SandboxSnapshot{ + ObjectMeta: metav1.ObjectMeta{Name: "test-snapshot", Namespace: "default", Finalizers: []string{SandboxSnapshotFinalizer}}, + Status: sandboxv1alpha1.SandboxSnapshotStatus{Containers: []sandboxv1alpha1.ContainerSnapshot{{ContainerName: "main", ImageURI: "registry.example.com/snapshots/main:tag"}}}, + } + deleter := &recordingRegistryImageDeleter{} + r := newTestSnapshotReconciler(snapshot) + r.SnapshotPushSecret = "missing-registry-secret" + r.registryImageDeleter = deleter + + _, err := r.handleDeletion(context.Background(), snapshot) + require.ErrorContains(t, err, "missing-registry-secret") + assert.Empty(t, deleter.calls) + + updated := &sandboxv1alpha1.SandboxSnapshot{} + require.NoError(t, r.Get(context.Background(), types.NamespacedName{Name: snapshot.Name, Namespace: snapshot.Namespace}, updated)) + assert.Contains(t, updated.Finalizers, SandboxSnapshotFinalizer) +} + func TestSandboxSnapshotHandleCommitting_SetsSucceedReadyCondition(t *testing.T) { snapshot := &sandboxv1alpha1.SandboxSnapshot{ ObjectMeta: metav1.ObjectMeta{ diff --git a/kubernetes/internal/controller/sandboxsnapshot_lifecycle.go b/kubernetes/internal/controller/sandboxsnapshot_lifecycle.go index de6298f4e..c52b84f05 100644 --- a/kubernetes/internal/controller/sandboxsnapshot_lifecycle.go +++ b/kubernetes/internal/controller/sandboxsnapshot_lifecycle.go @@ -174,32 +174,24 @@ func findJobCondition(conditions []batchv1.JobCondition, conditionType batchv1.J return nil } -// handleDeletion cleans up snapshot images and jobs, then removes the finalizer. +// handleDeletion stops snapshot jobs, cleans up images, then removes the finalizer. func (r *SandboxSnapshotReconciler) handleDeletion(ctx context.Context, snapshot *sandboxv1alpha1.SandboxSnapshot) (ctrl.Result, error) { log := logf.FromContext(ctx) - if err := r.deleteSnapshotImages(ctx, snapshot); err != nil { + jobsPending, err := r.deleteSnapshotJobs(ctx, snapshot) + if err != nil { return ctrl.Result{}, err } - - jobName := r.getJobName(snapshot) - job := &batchv1.Job{} - if err := r.Get(ctx, types.NamespacedName{Namespace: snapshot.Namespace, Name: jobName}, job); err == nil { - if deleteErr := r.Delete(ctx, job, client.PropagationPolicy(metav1.DeletePropagationBackground)); deleteErr != nil && !errors.IsNotFound(deleteErr) { - return ctrl.Result{}, deleteErr - } - log.Info("Deleted commit job", "job", jobName) + if jobsPending { + return ctrl.Result{RequeueAfter: time.Second}, nil } - unpauseJobName := r.getUnpauseJobName(snapshot) - unpauseJob := &batchv1.Job{} - if err := r.Get(ctx, types.NamespacedName{Namespace: snapshot.Namespace, Name: unpauseJobName}, unpauseJob); err == nil { - if deleteErr := r.Delete(ctx, unpauseJob, client.PropagationPolicy(metav1.DeletePropagationBackground)); deleteErr != nil && !errors.IsNotFound(deleteErr) { - return ctrl.Result{}, deleteErr - } - log.Info("Deleted unpause job", "job", unpauseJobName) + if err := r.deleteSnapshotImages(ctx, snapshot); err != nil { + return ctrl.Result{}, err } + log.Info("Deleted snapshot registry images") + if controllerutil.ContainsFinalizer(snapshot, SandboxSnapshotFinalizer) { if err := utils.UpdateFinalizer(r.Client, snapshot, utils.RemoveFinalizerOpType, SandboxSnapshotFinalizer); err != nil { return ctrl.Result{}, err @@ -208,6 +200,38 @@ func (r *SandboxSnapshotReconciler) handleDeletion(ctx context.Context, snapshot return ctrl.Result{}, nil } +// deleteSnapshotJobs requests foreground deletion for both jobs and waits for +// their owned Pods to disappear before registry cleanup. This closes the race +// where a commit Job can finish pushing after a tag was observed as missing. +func (r *SandboxSnapshotReconciler) deleteSnapshotJobs(ctx context.Context, snapshot *sandboxv1alpha1.SandboxSnapshot) (bool, error) { + pending := false + for _, jobName := range []string{r.getJobName(snapshot), r.getUnpauseJobName(snapshot)} { + job := &batchv1.Job{} + err := r.Get(ctx, types.NamespacedName{Namespace: snapshot.Namespace, Name: jobName}, job) + switch { + case err == nil: + pending = true + if job.DeletionTimestamp.IsZero() { + if deleteErr := r.Delete(ctx, job, client.PropagationPolicy(metav1.DeletePropagationForeground)); deleteErr != nil && !errors.IsNotFound(deleteErr) { + return false, deleteErr + } + } + case errors.IsNotFound(err): + default: + return false, err + } + + pods := &corev1.PodList{} + if err := r.List(ctx, pods, client.InNamespace(snapshot.Namespace), client.MatchingLabels{"job-name": jobName}); err != nil { + return false, err + } + if len(pods.Items) > 0 { + pending = true + } + } + return pending, nil +} + func (r *SandboxSnapshotReconciler) deleteSnapshotImages(ctx context.Context, snapshot *sandboxv1alpha1.SandboxSnapshot) error { if len(snapshot.Status.Containers) == 0 { return nil diff --git a/kubernetes/test/e2e/pause_resume_test.go b/kubernetes/test/e2e/pause_resume_test.go index ebc334538..250fc9613 100644 --- a/kubernetes/test/e2e/pause_resume_test.go +++ b/kubernetes/test/e2e/pause_resume_test.go @@ -18,6 +18,8 @@ import ( "encoding/base64" "encoding/json" "fmt" + "net" + "net/http" "os" "os/exec" "path/filepath" @@ -261,6 +263,12 @@ var _ = Describe("PauseResume", Ordered, Label("PauseResume"), func() { Expect(err).NotTo(HaveOccurred()) Expect(output).To(Equal("Succeed"), "Internal pause snapshot should be ready after pause") + cmd = exec.Command("kubectl", "get", "sandboxsnapshot", sandboxName+"-pause", + "-n", pauseResumeNamespace, "-o", "jsonpath={.status.containers[0].imageUri}") + snapshotImageURI, err := utils.Run(cmd) + Expect(err).NotTo(HaveOccurred()) + Expect(snapshotImageURI).NotTo(BeEmpty()) + // --- Step 4: Resume - patch spec.pause=false --- By("triggering resume by patching spec.pause=false") cmd = exec.Command("kubectl", "patch", "batchsandbox", sandboxName, @@ -284,6 +292,9 @@ var _ = Describe("PauseResume", Ordered, Label("PauseResume"), func() { output, err = utils.Run(cmd) Expect(err).To(HaveOccurred(), "Internal pause snapshot should be deleted after successful resume") + By("verifying the deleted snapshot manifest is absent from the registry") + expectRegistryManifestMissing(snapshotImageURI) + // --- Step 5: Verify rootfs data persistence --- By("getting resumed pod name") cmd = exec.Command("kubectl", "get", "pods", "-n", pauseResumeNamespace, "-o", "json") @@ -1069,3 +1080,45 @@ func createDockerRegistrySecrets(namespace string) error { return nil } + +func expectRegistryManifestMissing(imageURI string) { + listener, err := net.Listen("tcp", "127.0.0.1:0") + Expect(err).NotTo(HaveOccurred()) + port := listener.Addr().(*net.TCPAddr).Port + Expect(listener.Close()).To(Succeed()) + + portForward := exec.Command("kubectl", "port-forward", "-n", pauseResumeNamespace, + "service/docker-registry", fmt.Sprintf("%d:5000", port)) + Expect(portForward.Start()).To(Succeed()) + defer func() { + _ = portForward.Process.Kill() + _ = portForward.Wait() + }() + + client := &http.Client{Timeout: 2 * time.Second} + registryURL := fmt.Sprintf("http://127.0.0.1:%d", port) + Eventually(func(g Gomega) { + request, requestErr := http.NewRequest(http.MethodGet, registryURL+"/v2/", nil) + g.Expect(requestErr).NotTo(HaveOccurred()) + request.SetBasicAuth(registryUsername, registryPassword) + response, requestErr := client.Do(request) + g.Expect(requestErr).NotTo(HaveOccurred()) + defer response.Body.Close() + g.Expect(response.StatusCode).To(Equal(http.StatusOK)) + }, 30*time.Second).Should(Succeed()) + + repositoryAndTag := strings.TrimPrefix(imageURI, registryServiceAddr+"/") + tagSeparator := strings.LastIndex(repositoryAndTag, ":") + Expect(tagSeparator).To(BeNumerically(">", 0), "snapshot image must include a tag") + manifestURL := fmt.Sprintf("%s/v2/%s/manifests/%s", registryURL, repositoryAndTag[:tagSeparator], repositoryAndTag[tagSeparator+1:]) + + for range 2 { + request, requestErr := http.NewRequest(http.MethodHead, manifestURL, nil) + Expect(requestErr).NotTo(HaveOccurred()) + request.SetBasicAuth(registryUsername, registryPassword) + response, requestErr := client.Do(request) + Expect(requestErr).NotTo(HaveOccurred()) + response.Body.Close() + Expect(response.StatusCode).To(Equal(http.StatusNotFound)) + } +} From 5ebb181a8916198849e80997dc694b7095c591d6 Mon Sep 17 00:00:00 2001 From: Xuehao Xu Date: Mon, 10 Aug 2026 14:30:10 +0800 Subject: [PATCH 3/8] test(kubernetes): enable registry deletes in pause e2e --- docs/guides/pause-resume.md | 4 ++++ kubernetes/test/e2e/testdata/registry-deployment.yaml | 4 +++- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/docs/guides/pause-resume.md b/docs/guides/pause-resume.md index 4f63dfb3b..99b6ca854 100644 --- a/docs/guides/pause-resume.md +++ b/docs/guides/pause-resume.md @@ -184,6 +184,7 @@ Any OCI-compatible registry works (Docker Hub, GitHub Container Registry, Harbor - **Reachable from cluster nodes** (for the commit Job to push) - **Reachable from the Kubernetes API server / kubelet** (for image pull on resume) +- **Configured to allow manifest deletion** (for snapshot cleanup) ### Step 2: Create the push secret @@ -216,6 +217,9 @@ For development with a cluster-internal `registry:2` deployment: kubectl create deployment docker-registry \ --image=registry:2 --port=5000 +kubectl set env deployment/docker-registry \ + REGISTRY_STORAGE_DELETE_ENABLED=true + kubectl expose deployment docker-registry --port=5000 # No authentication needed for internal registry diff --git a/kubernetes/test/e2e/testdata/registry-deployment.yaml b/kubernetes/test/e2e/testdata/registry-deployment.yaml index b97044312..59d82601b 100644 --- a/kubernetes/test/e2e/testdata/registry-deployment.yaml +++ b/kubernetes/test/e2e/testdata/registry-deployment.yaml @@ -25,6 +25,8 @@ spec: value: "Registry Realm" - name: REGISTRY_AUTH_HTPASSWD_PATH value: /auth/htpasswd + - name: REGISTRY_STORAGE_DELETE_ENABLED + value: "true" volumeMounts: - name: auth mountPath: /auth @@ -48,4 +50,4 @@ spec: - port: 5000 targetPort: 5000 selector: - app: docker-registry \ No newline at end of file + app: docker-registry From ca0354f8929a07fc7d4906bf11b3734fde7a9f82 Mon Sep 17 00:00:00 2001 From: Xuehao Xu Date: Tue, 11 Aug 2026 19:36:32 +0800 Subject: [PATCH 4/8] fix(kubernetes): fall back to registry digest on cleanup miss --- .../controller/registry_image_deleter.go | 44 +++++++++++++------ .../controller/registry_image_deleter_test.go | 41 ++++++++++++++++- .../sandboxsnapshot_controller_test.go | 6 ++- .../controller/sandboxsnapshot_lifecycle.go | 6 +-- 4 files changed, 76 insertions(+), 21 deletions(-) diff --git a/kubernetes/internal/controller/registry_image_deleter.go b/kubernetes/internal/controller/registry_image_deleter.go index 83ed0640a..703c5c032 100644 --- a/kubernetes/internal/controller/registry_image_deleter.go +++ b/kubernetes/internal/controller/registry_image_deleter.go @@ -30,7 +30,7 @@ import ( ) type registryImageDeleter interface { - Delete(ctx context.Context, imageReference string, registrySecret *corev1.Secret, insecure bool) error + Delete(ctx context.Context, imageReference, imageDigest string, registrySecret *corev1.Secret, insecure bool) error } type remoteRegistryImageDeleter struct{} @@ -38,6 +38,7 @@ type remoteRegistryImageDeleter struct{} func (remoteRegistryImageDeleter) Delete( ctx context.Context, imageReference string, + imageDigest string, registrySecret *corev1.Secret, insecure bool, ) error { @@ -57,23 +58,40 @@ func (remoteRegistryImageDeleter) Delete( } remoteOptions := []remote.Option{remote.WithContext(ctx), remote.WithAuth(authenticator)} - // Registry implementations commonly require deletion by digest. Resolve - // legacy snapshots that do not have imageDigest recorded before deleting. - // This compatibility path can only delete the manifest currently behind the - // tag; snapshots with a recorded digest are protected from tag reuse. - if _, ok := ref.(name.Digest); !ok { - descriptor, headErr := remote.Head(ref, remoteOptions...) - if isRegistryNotFound(headErr) { + if imageDigest != "" { + digestRef := ref.Context().Digest(imageDigest) + if err := remote.Delete(digestRef, remoteOptions...); err == nil { return nil + } else if !isRegistryNotFound(err) { + return fmt.Errorf("delete image %q by digest %q: %w", imageReference, imageDigest, err) } - if headErr != nil { - return fmt.Errorf("resolve image digest for %q: %w", imageReference, headErr) + // nerdctl can convert the pushed manifest media type, so the local + // content-store digest may differ from the registry digest. Resolve the + // tag after a digest miss and delete the registry's current manifest. + } + + if _, ok := ref.(name.Digest); ok { + if err := remote.Delete(ref, remoteOptions...); err != nil && !isRegistryNotFound(err) { + return fmt.Errorf("delete image %q: %w", imageReference, err) } - ref = ref.Context().Digest(descriptor.Digest.String()) + return nil + } + + return deleteRegistryTag(ctx, ref, remoteOptions, imageReference) +} + +func deleteRegistryTag(ctx context.Context, ref name.Reference, remoteOptions []remote.Option, imageReference string) error { + descriptor, err := remote.Head(ref, remoteOptions...) + if isRegistryNotFound(err) { + return nil + } + if err != nil { + return fmt.Errorf("resolve image digest for %q: %w", imageReference, err) } - if err := remote.Delete(ref, remoteOptions...); err != nil && !isRegistryNotFound(err) { - return fmt.Errorf("delete image %q: %w", imageReference, err) + registryRef := ref.Context().Digest(descriptor.Digest.String()) + if err := remote.Delete(registryRef, remoteOptions...); err != nil && !isRegistryNotFound(err) { + return fmt.Errorf("delete image %q by registry digest %q: %w", imageReference, descriptor.Digest, err) } return nil } diff --git a/kubernetes/internal/controller/registry_image_deleter_test.go b/kubernetes/internal/controller/registry_image_deleter_test.go index dbae7d8cd..eb6d5be9a 100644 --- a/kubernetes/internal/controller/registry_image_deleter_test.go +++ b/kubernetes/internal/controller/registry_image_deleter_test.go @@ -51,7 +51,7 @@ func TestRemoteRegistryImageDeleter_ResolvesTagAndDeletesDigest(t *testing.T) { defer registry.Close() imageReference := strings.TrimPrefix(registry.URL, "http://") + "/snapshots/test:tag" - err := (remoteRegistryImageDeleter{}).Delete(context.Background(), imageReference, nil, true) + err := (remoteRegistryImageDeleter{}).Delete(context.Background(), imageReference, "", nil, true) require.NoError(t, err) select { case path := <-deletedPath: @@ -72,10 +72,47 @@ func TestRemoteRegistryImageDeleter_TreatsMissingManifestAsSuccess(t *testing.T) defer registry.Close() imageReference := strings.TrimPrefix(registry.URL, "http://") + "/snapshots/missing@sha256:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" - err := (remoteRegistryImageDeleter{}).Delete(context.Background(), imageReference, nil, true) + err := (remoteRegistryImageDeleter{}).Delete(context.Background(), imageReference, "", nil, true) require.NoError(t, err) } +func TestRemoteRegistryImageDeleter_FallsBackToTagAfterRecordedDigestMiss(t *testing.T) { + const recordedDigest = "sha256:bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb" + const registryDigest = "sha256:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" + deletedPaths := make(chan string, 2) + registry := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, request *http.Request) { + switch { + case request.URL.Path == "/v2/": + w.WriteHeader(http.StatusOK) + case request.Method == http.MethodHead && request.URL.Path == "/v2/snapshots/test/manifests/tag": + w.Header().Set("Docker-Content-Digest", registryDigest) + w.Header().Set("Content-Type", "application/vnd.oci.image.manifest.v1+json") + w.Header().Set("Content-Length", "2") + w.WriteHeader(http.StatusOK) + case request.Method == http.MethodDelete && strings.HasSuffix(request.URL.Path, "/"+recordedDigest): + deletedPaths <- request.URL.Path + http.NotFound(w, request) + case request.Method == http.MethodDelete && strings.HasSuffix(request.URL.Path, "/"+registryDigest): + deletedPaths <- request.URL.Path + w.WriteHeader(http.StatusAccepted) + default: + http.NotFound(w, request) + } + })) + defer registry.Close() + + imageReference := strings.TrimPrefix(registry.URL, "http://") + "/snapshots/test:tag" + err := (remoteRegistryImageDeleter{}).Delete(context.Background(), imageReference, recordedDigest, nil, true) + require.NoError(t, err) + assert.Equal(t, "/v2/snapshots/test/manifests/"+recordedDigest, <-deletedPaths) + select { + case path := <-deletedPaths: + assert.Equal(t, "/v2/snapshots/test/manifests/"+registryDigest, path) + case <-time.After(time.Second): + t.Fatal("timed out waiting for fallback registry DELETE request") + } +} + func TestRegistryAuthenticator_ReadsDockerConfigJSON(t *testing.T) { secret := &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{Name: "registry-secret", Namespace: "default"}, diff --git a/kubernetes/internal/controller/sandboxsnapshot_controller_test.go b/kubernetes/internal/controller/sandboxsnapshot_controller_test.go index c70b2212b..524424519 100644 --- a/kubernetes/internal/controller/sandboxsnapshot_controller_test.go +++ b/kubernetes/internal/controller/sandboxsnapshot_controller_test.go @@ -39,6 +39,7 @@ import ( type registryDeleteCall struct { imageReference string + imageDigest string secretName string insecure bool } @@ -51,6 +52,7 @@ type recordingRegistryImageDeleter struct { func (d *recordingRegistryImageDeleter) Delete( _ context.Context, imageReference string, + imageDigest string, secret *corev1.Secret, insecure bool, ) error { @@ -60,6 +62,7 @@ func (d *recordingRegistryImageDeleter) Delete( } d.calls = append(d.calls, registryDeleteCall{ imageReference: imageReference, + imageDigest: imageDigest, secretName: secretName, insecure: insecure, }) @@ -118,7 +121,8 @@ func TestSandboxSnapshotHandleDeletion_DeletesRegistryImagesBeforeRemovingFinali require.NoError(t, err) assert.Equal(t, ctrl.Result{}, result) require.Len(t, deleter.calls, 2) - assert.Equal(t, "registry.example.com/snapshots/main@sha256:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", deleter.calls[0].imageReference) + assert.Equal(t, "registry.example.com/snapshots/main:tag", deleter.calls[0].imageReference) + assert.Equal(t, "sha256:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", deleter.calls[0].imageDigest) assert.Equal(t, "registry.example.com/snapshots/sidecar:tag", deleter.calls[1].imageReference) assert.Equal(t, "registry-secret", deleter.calls[0].secretName) assert.True(t, deleter.calls[0].insecure) diff --git a/kubernetes/internal/controller/sandboxsnapshot_lifecycle.go b/kubernetes/internal/controller/sandboxsnapshot_lifecycle.go index c52b84f05..0929aa7aa 100644 --- a/kubernetes/internal/controller/sandboxsnapshot_lifecycle.go +++ b/kubernetes/internal/controller/sandboxsnapshot_lifecycle.go @@ -256,16 +256,12 @@ func (r *SandboxSnapshotReconciler) deleteSnapshotImages(ctx context.Context, sn } imageReference := container.ImageURI if container.ImageDigest != "" { - imageReference = strings.SplitN(container.ImageURI, "@", 2)[0] - if tagIndex := strings.LastIndex(imageReference, ":"); tagIndex > strings.LastIndex(imageReference, "/") { - imageReference = imageReference[:tagIndex] - } imageReference += "@" + container.ImageDigest } if _, exists := deleted[imageReference]; exists { continue } - if err := deleter.Delete(ctx, imageReference, registrySecret, r.SnapshotRegistryInsecure); err != nil { + if err := deleter.Delete(ctx, container.ImageURI, container.ImageDigest, registrySecret, r.SnapshotRegistryInsecure); err != nil { return fmt.Errorf("delete snapshot image for container %s: %w", container.ContainerName, err) } deleted[imageReference] = struct{}{} From 179cae69ea3eabadba7ba2aee300f142143012e2 Mon Sep 17 00:00:00 2001 From: Xuehao Xu Date: Tue, 11 Aug 2026 19:38:55 +0800 Subject: [PATCH 5/8] test(kubernetes): bound registry fallback assertions --- .../internal/controller/registry_image_deleter_test.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/kubernetes/internal/controller/registry_image_deleter_test.go b/kubernetes/internal/controller/registry_image_deleter_test.go index eb6d5be9a..1e72f7a79 100644 --- a/kubernetes/internal/controller/registry_image_deleter_test.go +++ b/kubernetes/internal/controller/registry_image_deleter_test.go @@ -104,7 +104,12 @@ func TestRemoteRegistryImageDeleter_FallsBackToTagAfterRecordedDigestMiss(t *tes imageReference := strings.TrimPrefix(registry.URL, "http://") + "/snapshots/test:tag" err := (remoteRegistryImageDeleter{}).Delete(context.Background(), imageReference, recordedDigest, nil, true) require.NoError(t, err) - assert.Equal(t, "/v2/snapshots/test/manifests/"+recordedDigest, <-deletedPaths) + select { + case path := <-deletedPaths: + assert.Equal(t, "/v2/snapshots/test/manifests/"+recordedDigest, path) + case <-time.After(time.Second): + t.Fatal("timed out waiting for recorded-digest DELETE request") + } select { case path := <-deletedPaths: assert.Equal(t, "/v2/snapshots/test/manifests/"+registryDigest, path) From 85cb70a57ebcfca0fce49d095446ff1858e715d0 Mon Sep 17 00:00:00 2001 From: Xuehao Xu Date: Tue, 11 Aug 2026 20:29:35 +0800 Subject: [PATCH 6/8] fix(kubernetes): verify registry digest after cleanup --- .../controller/registry_image_deleter.go | 38 ++++++++++++++---- .../controller/registry_image_deleter_test.go | 40 +++++++++++++++++++ 2 files changed, 71 insertions(+), 7 deletions(-) diff --git a/kubernetes/internal/controller/registry_image_deleter.go b/kubernetes/internal/controller/registry_image_deleter.go index 703c5c032..654255fae 100644 --- a/kubernetes/internal/controller/registry_image_deleter.go +++ b/kubernetes/internal/controller/registry_image_deleter.go @@ -60,14 +60,20 @@ func (remoteRegistryImageDeleter) Delete( if imageDigest != "" { digestRef := ref.Context().Digest(imageDigest) - if err := remote.Delete(digestRef, remoteOptions...); err == nil { - return nil - } else if !isRegistryNotFound(err) { - return fmt.Errorf("delete image %q by digest %q: %w", imageReference, imageDigest, err) + deleteErr := remote.Delete(digestRef, remoteOptions...) + if deleteErr != nil && !isRegistryNotFound(deleteErr) { + return fmt.Errorf("delete image %q by digest %q: %w", imageReference, imageDigest, deleteErr) } - // nerdctl can convert the pushed manifest media type, so the local - // content-store digest may differ from the registry digest. Resolve the - // tag after a digest miss and delete the registry's current manifest. + // nerdctl can convert the pushed manifest media type, and some registries + // report a successful DELETE even when the supplied digest is not the + // digest currently referenced by the tag. Resolve the tag after every + // digest attempt and remove a different registry manifest when present. + if _, ok := ref.(name.Digest); !ok { + if err := deleteRegistryTagIfDifferent(ctx, ref, imageDigest, remoteOptions, imageReference); err != nil { + return err + } + } + return nil } if _, ok := ref.(name.Digest); ok { @@ -80,6 +86,24 @@ func (remoteRegistryImageDeleter) Delete( return deleteRegistryTag(ctx, ref, remoteOptions, imageReference) } +func deleteRegistryTagIfDifferent(ctx context.Context, ref name.Reference, imageDigest string, remoteOptions []remote.Option, imageReference string) error { + descriptor, err := remote.Head(ref, remoteOptions...) + if isRegistryNotFound(err) { + return nil + } + if err != nil { + return fmt.Errorf("resolve image digest for %q: %w", imageReference, err) + } + if descriptor.Digest.String() == imageDigest { + return nil + } + registryRef := ref.Context().Digest(descriptor.Digest.String()) + if err := remote.Delete(registryRef, remoteOptions...); err != nil && !isRegistryNotFound(err) { + return fmt.Errorf("delete image %q by registry digest %q: %w", imageReference, descriptor.Digest, err) + } + return nil +} + func deleteRegistryTag(ctx context.Context, ref name.Reference, remoteOptions []remote.Option, imageReference string) error { descriptor, err := remote.Head(ref, remoteOptions...) if isRegistryNotFound(err) { diff --git a/kubernetes/internal/controller/registry_image_deleter_test.go b/kubernetes/internal/controller/registry_image_deleter_test.go index 1e72f7a79..1f864f553 100644 --- a/kubernetes/internal/controller/registry_image_deleter_test.go +++ b/kubernetes/internal/controller/registry_image_deleter_test.go @@ -118,6 +118,46 @@ func TestRemoteRegistryImageDeleter_FallsBackToTagAfterRecordedDigestMiss(t *tes } } +func TestRemoteRegistryImageDeleter_RemovesTagDigestAfterRecordedDeleteAccepted(t *testing.T) { + const recordedDigest = "sha256:bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb" + const registryDigest = "sha256:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" + deletedPaths := make(chan string, 2) + registry := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, request *http.Request) { + switch { + case request.URL.Path == "/v2/": + w.WriteHeader(http.StatusOK) + case request.Method == http.MethodHead && request.URL.Path == "/v2/snapshots/test/manifests/tag": + w.Header().Set("Docker-Content-Digest", registryDigest) + w.WriteHeader(http.StatusOK) + case request.Method == http.MethodDelete && strings.HasSuffix(request.URL.Path, "/"+recordedDigest): + deletedPaths <- request.URL.Path + w.WriteHeader(http.StatusAccepted) + case request.Method == http.MethodDelete && strings.HasSuffix(request.URL.Path, "/"+registryDigest): + deletedPaths <- request.URL.Path + w.WriteHeader(http.StatusAccepted) + default: + http.NotFound(w, request) + } + })) + defer registry.Close() + + imageReference := strings.TrimPrefix(registry.URL, "http://") + "/snapshots/test:tag" + err := (remoteRegistryImageDeleter{}).Delete(context.Background(), imageReference, recordedDigest, nil, true) + require.NoError(t, err) + select { + case path := <-deletedPaths: + assert.Equal(t, "/v2/snapshots/test/manifests/"+recordedDigest, path) + case <-time.After(time.Second): + t.Fatal("timed out waiting for recorded-digest DELETE request") + } + select { + case path := <-deletedPaths: + assert.Equal(t, "/v2/snapshots/test/manifests/"+registryDigest, path) + case <-time.After(time.Second): + t.Fatal("timed out waiting for registry-digest DELETE request") + } +} + func TestRegistryAuthenticator_ReadsDockerConfigJSON(t *testing.T) { secret := &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{Name: "registry-secret", Namespace: "default"}, From b0d17af48f35584972241cf675d6b41be3468373 Mon Sep 17 00:00:00 2001 From: Xuehao Xu Date: Tue, 11 Aug 2026 20:40:28 +0800 Subject: [PATCH 7/8] test(kubernetes): complete registry manifest response fixture --- kubernetes/internal/controller/registry_image_deleter_test.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/kubernetes/internal/controller/registry_image_deleter_test.go b/kubernetes/internal/controller/registry_image_deleter_test.go index 1f864f553..a70aab9d7 100644 --- a/kubernetes/internal/controller/registry_image_deleter_test.go +++ b/kubernetes/internal/controller/registry_image_deleter_test.go @@ -128,6 +128,8 @@ func TestRemoteRegistryImageDeleter_RemovesTagDigestAfterRecordedDeleteAccepted( w.WriteHeader(http.StatusOK) case request.Method == http.MethodHead && request.URL.Path == "/v2/snapshots/test/manifests/tag": w.Header().Set("Docker-Content-Digest", registryDigest) + w.Header().Set("Content-Type", "application/vnd.oci.image.manifest.v1+json") + w.Header().Set("Content-Length", "2") w.WriteHeader(http.StatusOK) case request.Method == http.MethodDelete && strings.HasSuffix(request.URL.Path, "/"+recordedDigest): deletedPaths <- request.URL.Path From 4cd7bfcebd740c9b31045292381eedc08af44489 Mon Sep 17 00:00:00 2001 From: Xuehao Xu Date: Wed, 12 Aug 2026 10:15:11 +0800 Subject: [PATCH 8/8] test(kubernetes): bound pause e2e teardown --- kubernetes/test/e2e/pause_resume_test.go | 33 ++++++++++++++++++------ 1 file changed, 25 insertions(+), 8 deletions(-) diff --git a/kubernetes/test/e2e/pause_resume_test.go b/kubernetes/test/e2e/pause_resume_test.go index 250fc9613..8cfe64364 100644 --- a/kubernetes/test/e2e/pause_resume_test.go +++ b/kubernetes/test/e2e/pause_resume_test.go @@ -125,24 +125,41 @@ var _ = Describe("PauseResume", Ordered, Label("PauseResume"), func() { }) AfterAll(func() { - By("cleaning up Docker Registry") - cmd := exec.Command("kubectl", "delete", "deployment", "docker-registry", "-n", pauseResumeNamespace, "--ignore-not-found=true") + By("cleaning up any remaining batchsandboxes") + cmd := exec.Command("kubectl", "delete", "batchsandboxes", "--all", "-n", pauseResumeNamespace, + "--ignore-not-found=true", "--wait=false") utils.Run(cmd) - cmd = exec.Command("kubectl", "delete", "service", "docker-registry", "-n", pauseResumeNamespace, "--ignore-not-found=true") + + By("requesting cleanup of any remaining sandboxsnapshots") + cmd = exec.Command("kubectl", "delete", "sandboxsnapshots", "--all", "-n", pauseResumeNamespace, + "--ignore-not-found=true", "--wait=false") utils.Run(cmd) + // Failure-path tests can intentionally leave snapshots whose image URI + // cannot be cleaned up. Exercise the documented operator escape hatch so + // teardown cannot block forever on their strict cleanup finalizers. + By("removing finalizers from snapshots that could not be cleaned up") + cmd = exec.Command("kubectl", "get", "sandboxsnapshots", "-n", pauseResumeNamespace, + "-o", "jsonpath={range .items[*]}{.metadata.name}{'\\n'}{end}") + remainingSnapshots, err := utils.Run(cmd) + if err == nil { + for _, snapshotName := range strings.Fields(remainingSnapshots) { + cmd = exec.Command("kubectl", "patch", "sandboxsnapshot", snapshotName, "-n", pauseResumeNamespace, + "--type=merge", "-p", `{"metadata":{"finalizers":[]}}`) + utils.Run(cmd) + } + } + By("cleaning up secrets") for _, secret := range []string{"registry-auth", "registry-snapshot-push-secret", "registry-pull-secret"} { cmd = exec.Command("kubectl", "delete", "secret", secret, "-n", pauseResumeNamespace, "--ignore-not-found=true") utils.Run(cmd) } - By("cleaning up any remaining sandboxsnapshots") - cmd = exec.Command("kubectl", "delete", "sandboxsnapshots", "--all", "-n", pauseResumeNamespace, "--ignore-not-found=true") + By("cleaning up Docker Registry") + cmd = exec.Command("kubectl", "delete", "deployment", "docker-registry", "-n", pauseResumeNamespace, "--ignore-not-found=true") utils.Run(cmd) - - By("cleaning up any remaining batchsandboxes") - cmd = exec.Command("kubectl", "delete", "batchsandboxes", "--all", "-n", pauseResumeNamespace, "--ignore-not-found=true") + cmd = exec.Command("kubectl", "delete", "service", "docker-registry", "-n", pauseResumeNamespace, "--ignore-not-found=true") utils.Run(cmd) By("undeploying the controller-manager")