diff --git a/docs/guides/pause-resume.md b/docs/guides/pause-resume.md index 6bb6e891a..4afeeab84 100644 --- a/docs/guides/pause-resume.md +++ b/docs/guides/pause-resume.md @@ -156,7 +156,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, contain inline `auths` credentials for the registry, and permit manifest deletion. `credHelpers`/`credsStore` entries are not usable by the controller. | | `--image-committer-pod-template-file` | string | `""` | Path to a PodTemplateSpec overlay for commit Job Pods. | | `--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. | @@ -190,6 +190,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 @@ -222,6 +223,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 @@ -328,7 +332,12 @@ Any ServiceAccount or admission configuration referenced by the template must ex 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 a1104c65a..7f069fe89 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; must contain inline `auths` credentials with manifest delete permission | | `--image-committer-pod-template-file` | `""` | Path to a PodTemplateSpec overlay for image-committer commit Job Pods | | `--resume-pull-secret` | `""` | Secret name injected into resumed sandboxes for image pulls | | `--image-committer-image` | `image-committer:dev` | Image used for commit operations | @@ -174,7 +174,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` 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/charts/opensandbox-controller/values.yaml b/kubernetes/charts/opensandbox-controller/values.yaml index f0b499876..988898c24 100644 --- a/kubernetes/charts/opensandbox-controller/values.yaml +++ b/kubernetes/charts/opensandbox-controller/values.yaml @@ -70,7 +70,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 35e7d426b..01231d334 100644 --- a/kubernetes/cmd/controller/main.go +++ b/kubernetes/cmd/controller/main.go @@ -223,7 +223,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 ab7af617a..d4e2551d1 100644 --- a/kubernetes/go.mod +++ b/kubernetes/go.mod @@ -4,9 +4,10 @@ go 1.25.0 require ( github.com/containerd/containerd v1.7.33 - github.com/containerd/errdefs v0.3.0 + github.com/containerd/errdefs v1.0.0 github.com/distribution/reference v0.6.0 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/opencontainers/go-digest v1.0.0 @@ -33,13 +34,18 @@ require ( github.com/containerd/fifo v1.1.0 // indirect github.com/containerd/log v0.1.0 // indirect github.com/containerd/platforms v0.2.1 // indirect + github.com/containerd/stargz-snapshotter/estargz v0.16.3 // indirect github.com/containerd/ttrpc v1.2.7 // indirect github.com/containerd/typeurl/v2 v2.1.1 // indirect github.com/cyphar/filepath-securejoin v0.5.1 // 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/docker/go-events v0.0.0-20190806004212-e31b211e4f1c // indirect github.com/golang/groupcache v0.0.0-20210331224755-41bb18bfe9da // 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/locker v1.0.1 // indirect github.com/moby/spdystream v0.5.1 // indirect github.com/moby/sys/mountinfo v0.6.2 // indirect @@ -51,6 +57,7 @@ require ( github.com/opencontainers/runtime-spec v1.1.0 // indirect github.com/opencontainers/selinux v1.13.1 // indirect github.com/sirupsen/logrus v1.9.3 // indirect + github.com/vbatts/tar-split v0.12.1 // indirect go.opencensus.io v0.24.0 // indirect go.yaml.in/yaml/v3 v3.0.4 // indirect google.golang.org/genproto v0.0.0-20240123012728-ef4313101c80 // indirect @@ -97,8 +104,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 f3303b640..1165fa1d4 100644 --- a/kubernetes/go.sum +++ b/kubernetes/go.sum @@ -33,19 +33,21 @@ github.com/containerd/containerd/api v1.8.0 h1:hVTNJKR8fMc/2Tiw60ZRijntNMd1U+JVM github.com/containerd/containerd/api v1.8.0/go.mod h1:dFv4lt6S20wTu/hMcP4350RL87qPWLVa/OHOwmmdnYc= github.com/containerd/continuity v0.4.4 h1:/fNVfTJ7wIl/YPMHjf+5H32uFhl63JucB34PlCpMKII= github.com/containerd/continuity v0.4.4/go.mod h1:/lNJvtJKUQStBzpVQ1+rasXO1LAWtUQssk28EZvJ3nE= -github.com/containerd/errdefs v0.3.0 h1:FSZgGOeK4yuT/+DnF07/Olde/q4KBoMsaamhXxIMDp4= -github.com/containerd/errdefs v0.3.0/go.mod h1:+YBYIdtsnF4Iw6nWZhJcqGSg/dwvV7tyJ/kCkyJ2k+M= +github.com/containerd/errdefs v1.0.0 h1:tg5yIfIlQIrxYtu9ajqY42W3lpS19XqdxRQeEwYG8PI= +github.com/containerd/errdefs v1.0.0/go.mod h1:+YBYIdtsnF4Iw6nWZhJcqGSg/dwvV7tyJ/kCkyJ2k+M= github.com/containerd/fifo v1.1.0 h1:4I2mbh5stb1u6ycIABlBw9zgtlK8viPI9QkQNRQEEmY= github.com/containerd/fifo v1.1.0/go.mod h1:bmC4NWMbXlt2EZ0Hc7Fx7QzTFxgPID13eH0Qu+MAb2o= github.com/containerd/log v0.1.0 h1:TCJt7ioM2cr/tfR8GPbGf9/VRAX8D2B4PjzCpfX540I= github.com/containerd/log v0.1.0/go.mod h1:VRRf09a7mHDIRezVKTRCrOq78v577GXq3bSa3EhrzVo= github.com/containerd/platforms v0.2.1 h1:zvwtM3rz2YHPQsF2CHYM8+KtB5dvhISiXh5ZpSBQv6A= github.com/containerd/platforms v0.2.1/go.mod h1:XHCb+2/hzowdiut9rkudds9bE5yJ7npe7dG/wG+uFPw= +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/containerd/ttrpc v1.2.7 h1:qIrroQvuOL9HQ1X6KHe2ohc7p+HP/0VE6XPU7elJRqQ= github.com/containerd/ttrpc v1.2.7/go.mod h1:YCXHsb32f+Sq5/72xHubdiJRQY9inL4a4ZQrAbN1q9o= github.com/containerd/typeurl/v2 v2.1.1 h1:3Q4Pt7i8nYwy2KmQWIw2+1hTvwTE/6w9FqcttATPO/4= github.com/containerd/typeurl/v2 v2.1.1/go.mod h1:IDp2JFvbwZ31H8dQbEIY7sDl2L3o3HZj1hsSQlywkQ0= -github.com/cpuguy83/go-md2man/v2 v2.0.4/go.mod h1:tgQtvFlXSQOSOSIRvRPT7W67SCa46tRHOmNcaadrF8o= +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/cyphar/filepath-securejoin v0.5.1 h1:eYgfMq5yryL4fbWfkLpFFy2ukSELzaJOTaUTuh+oF48= github.com/cyphar/filepath-securejoin v0.5.1/go.mod h1:Sdj7gXlvMcPZsbhwhQ33GguGLDGQL7h7bg04C/+u9jI= @@ -54,6 +56,12 @@ 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/distribution/reference v0.6.0 h1:0IXCQ5g4/QMHHkarYzh5l+u8T3t73zM5QvfrDyIgxBk= github.com/distribution/reference v0.6.0/go.mod h1:BbU0aIcezP1/5jX/8MP0YiH4SdvB5Y4f/wlDRiLyi3E= +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/docker/go-events v0.0.0-20190806004212-e31b211e4f1c h1:+pKlWGMw7gf6bQ+oDZB4KHQFypsfjYlq/C4rfL7D3g8= github.com/docker/go-events v0.0.0-20190806004212-e31b211e4f1c/go.mod h1:Uw6UezgYA44ePAFQYUehOuCzmy5zmg/+nl2ZfMWGkpA= github.com/emicklei/go-restful/v3 v3.11.0 h1:rAQeMHw1c7zTmncogyy8VvRZwtkmkZ4FxERmMY4rD+g= @@ -126,6 +134,8 @@ github.com/google/go-cmp v0.5.5/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/ 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= @@ -159,6 +169,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/locker v1.0.1 h1:fOXqR41zeveg4fFODix+1Ch4mj/gT0NE1XJbp/epuBg= github.com/moby/locker v1.0.1/go.mod h1:S7SDdo5zpBK84bzzVlKr2V0hz+7x9hWbYC/kq7oQppc= github.com/moby/spdystream v0.5.1 h1:9sNYeYZUcci9R6/w7KDaFWEWeV4LStVG78Mpyq/Zm/Y= @@ -212,10 +224,10 @@ github.com/rogpeppe/go-internal v1.14.1/go.mod h1:MaRKkUm5W0goXpeCfT7UZI6fk/L7L7 github.com/russross/blackfriday/v2 v2.1.0/go.mod h1:+Rmxgy9KzJVeS9/2gXHxylqXiyQDYRxCVz55jmeOWTM= 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.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/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= @@ -228,6 +240,8 @@ github.com/stretchr/testify v1.8.0/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO 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= @@ -381,6 +395,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= honnef.co/go/tools v0.0.0-20190102054323-c2f93a96b099/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4= honnef.co/go/tools v0.0.0-20190523083050-ea95bdfd59fc/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4= k8s.io/api v0.33.0 h1:yTgZVn1XEe6opVpP1FylmNrIFWuDqe2H0V8CT5gxfIU= diff --git a/kubernetes/internal/controller/registry_image_deleter.go b/kubernetes/internal/controller/registry_image_deleter.go new file mode 100644 index 000000000..654255fae --- /dev/null +++ b/kubernetes/internal/controller/registry_image_deleter.go @@ -0,0 +1,186 @@ +// 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, imageDigest string, registrySecret *corev1.Secret, insecure bool) error +} + +type remoteRegistryImageDeleter struct{} + +func (remoteRegistryImageDeleter) Delete( + ctx context.Context, + imageReference string, + imageDigest 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)} + + if imageDigest != "" { + digestRef := ref.Context().Digest(imageDigest) + 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, 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 { + if err := remote.Delete(ref, remoteOptions...); err != nil && !isRegistryNotFound(err) { + return fmt.Errorf("delete image %q: %w", imageReference, err) + } + return nil + } + + 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) { + return nil + } + if err != nil { + return fmt.Errorf("resolve image digest for %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 +} + +func registryAuthenticator(secret *corev1.Secret, registry string) (authn.Authenticator, error) { + if secret == nil { + return authn.Anonymous, nil + } + + 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"` + 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) + } + 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 + } + } + 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) +} + +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..a70aab9d7 --- /dev/null +++ b/kubernetes/internal/controller/registry_image_deleter_test.go @@ -0,0 +1,207 @@ +// 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" + "time" + + "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) + 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 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) + 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 fallback registry DELETE request") + } +} + +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.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 + 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"}, + 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) +} + +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 792261ebb..ae88c6275 100644 --- a/kubernetes/internal/controller/sandboxsnapshot_controller.go +++ b/kubernetes/internal/controller/sandboxsnapshot_controller.go @@ -81,7 +81,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. @@ -93,6 +93,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 b0f035cc6..c292ca993 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,38 @@ import ( sandboxv1alpha1 "github.com/alibaba/OpenSandbox/sandbox-k8s/apis/sandbox/v1alpha1" ) +type registryDeleteCall struct { + imageReference string + imageDigest string + secretName string + insecure bool +} + +type recordingRegistryImageDeleter struct { + calls []registryDeleteCall + err error +} + +func (d *recordingRegistryImageDeleter) Delete( + _ context.Context, + imageReference string, + imageDigest string, + secret *corev1.Secret, + insecure bool, +) error { + secretName := "" + if secret != nil { + secretName = secret.Name + } + d.calls = append(d.calls, registryDeleteCall{ + imageReference: imageReference, + imageDigest: imageDigest, + secretName: secretName, + insecure: insecure, + }) + return d.err +} + func newTestSnapshotReconciler(objs ...client.Object) *SandboxSnapshotReconciler { scheme := k8sruntime.NewScheme() utilruntime.Must(corev1.AddToScheme(scheme)) @@ -55,6 +88,112 @@ 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"}} + 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, 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: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) + + 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_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{ + 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 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 abf6e8251..94a94ed22 100644 --- a/kubernetes/internal/controller/sandboxsnapshot_lifecycle.go +++ b/kubernetes/internal/controller/sandboxsnapshot_lifecycle.go @@ -174,28 +174,24 @@ func findJobCondition(conditions []batchv1.JobCondition, conditionType batchv1.J return nil } -// handleDeletion cleans up the commit job and 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) - 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) + jobsPending, err := r.deleteSnapshotJobs(ctx, snapshot) + if err != nil { + return ctrl.Result{}, err + } + 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 @@ -204,6 +200,75 @@ 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 + } + + 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 += "@" + container.ImageDigest + } + if _, exists := deleted[imageReference]; exists { + continue + } + 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{}{} + } + 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) diff --git a/kubernetes/pkg/imagecommitter/registry_containerd.go b/kubernetes/pkg/imagecommitter/registry_containerd.go index dc9c95e94..2ba056e27 100644 --- a/kubernetes/pkg/imagecommitter/registry_containerd.go +++ b/kubernetes/pkg/imagecommitter/registry_containerd.go @@ -82,12 +82,12 @@ func (p DockerConfigCredentialProvider) Credential(_ context.Context, registryHo decoded, err := base64.StdEncoding.DecodeString(entry.Auth) if err != nil { p.warn("decode registry auth for %s: %v", registryHost, err) - return RegistryCredential{}, nil + return RegistryCredential{}, fmt.Errorf("decode registry auth for %s: %w", registryHost, err) } parts := strings.SplitN(string(decoded), ":", 2) if len(parts) != 2 { p.warn("invalid registry auth for %s", registryHost) - return RegistryCredential{}, nil + return RegistryCredential{}, fmt.Errorf("invalid registry auth for %s", registryHost) } credential.Username, credential.Password = parts[0], parts[1] } diff --git a/kubernetes/pkg/imagecommitter/registry_containerd_test.go b/kubernetes/pkg/imagecommitter/registry_containerd_test.go index 709e3fe5c..5f4a33a14 100644 --- a/kubernetes/pkg/imagecommitter/registry_containerd_test.go +++ b/kubernetes/pkg/imagecommitter/registry_containerd_test.go @@ -23,6 +23,7 @@ import ( "net/http" "os" "path/filepath" + "strings" "syscall" "testing" @@ -72,6 +73,22 @@ func TestDockerConfigCredentialProviderFallsBackToAnonymousOnInvalidConfig(t *te } } +func TestDockerConfigCredentialProviderRejectsMalformedMatchingAuth(t *testing.T) { + path := filepath.Join(t.TempDir(), "config.json") + data := []byte(`{"auths":{"registry.example.com":{"auth":"not-base64"}}}`) + if err := os.WriteFile(path, data, 0o600); err != nil { + t.Fatalf("write config: %v", err) + } + + _, err := (DockerConfigCredentialProvider{Path: path}).Credential(context.Background(), "registry.example.com") + if err == nil { + t.Fatal("expected malformed matching auth to fail") + } + if !strings.Contains(err.Error(), "decode registry auth") { + t.Fatalf("unexpected error: %v", err) + } +} + func TestDockerConfigCredentialProviderSupportsIdentityToken(t *testing.T) { path := filepath.Join(t.TempDir(), "config.json") if err := os.WriteFile(path, []byte(`{"auths":{"registry.example.com":{"identitytoken":"token"}}}`), 0o600); err != nil { diff --git a/kubernetes/test/e2e/pause_resume_test.go b/kubernetes/test/e2e/pause_resume_test.go index ebc334538..dc3eb81e4 100644 --- a/kubernetes/test/e2e/pause_resume_test.go +++ b/kubernetes/test/e2e/pause_resume_test.go @@ -15,9 +15,10 @@ package e2e import ( - "encoding/base64" "encoding/json" "fmt" + "net" + "net/http" "os" "os/exec" "path/filepath" @@ -123,24 +124,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") @@ -261,6 +279,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, @@ -279,10 +303,15 @@ var _ = Describe("PauseResume", Ordered, Label("PauseResume"), func() { }, 2*time.Minute).Should(Succeed()) By("verifying the reserved internal SandboxSnapshot is deleted after successful resume") - cmd = exec.Command("kubectl", "get", "sandboxsnapshot", sandboxName+"-pause", - "-n", pauseResumeNamespace, "-o", "name") - output, err = utils.Run(cmd) - Expect(err).To(HaveOccurred(), "Internal pause snapshot should be deleted after successful resume") + Eventually(func(g Gomega) { + cmd := exec.Command("kubectl", "get", "sandboxsnapshot", sandboxName+"-pause", + "-n", pauseResumeNamespace, "-o", "name") + _, err := utils.Run(cmd) + g.Expect(err).To(HaveOccurred(), "Internal pause snapshot should be deleted after successful resume") + }, 2*time.Minute).Should(Succeed()) + + By("verifying the deleted snapshot manifest is absent from the registry") + expectRegistryManifestMissing(snapshotImageURI) // --- Step 5: Verify rootfs data persistence --- By("getting resumed pod name") @@ -811,7 +840,7 @@ var _ = Describe("PauseResume", Ordered, Label("PauseResume"), func() { utils.Run(cmd) }) - It("should set Phase=Succeed+PauseFailed when commit/push fails with invalid registry", func() { + It("should set Phase=Succeed+PauseFailed when the snapshot registry is unavailable", func() { const sandboxName = "test-pause-commit-fail" By("creating BatchSandbox with template") @@ -841,17 +870,51 @@ var _ = Describe("PauseResume", Ordered, Label("PauseResume"), func() { g.Expect(output).To(Equal("Succeed")) }, 2*time.Minute).Should(Succeed()) - By("patching registry push secret to invalid value (invalid docker config)") - // Create an invalid docker config JSON (base64 encoded) - invalidConfig := `{"auths":{"docker-registry.default.svc.cluster.local:5000":{"username":"invalid","password":"wrong","auth":"aW52YWxpZDp3cm9uZw=="}}}` - encoded := base64.StdEncoding.EncodeToString([]byte(invalidConfig)) - patchData := fmt.Sprintf(`{"data":{".dockerconfigjson":"%s"}}`, encoded) - cmd = exec.Command("kubectl", "patch", "secret", "registry-snapshot-push-secret", "-n", pauseResumeNamespace, - "--type=merge", "-p", patchData) + By("making the snapshot registry unavailable") + cmd = exec.Command("kubectl", "scale", "deployment", "docker-registry", + "-n", pauseResumeNamespace, "--replicas=0") _, err = utils.Run(cmd) Expect(err).NotTo(HaveOccurred()) - By("triggering pause with invalid registry config") + By("waiting for the snapshot registry to have no available replicas") + Eventually(func(g Gomega) { + cmd := exec.Command("kubectl", "get", "deployment", "docker-registry", "-n", pauseResumeNamespace, "-o", "json") + output, err := utils.Run(cmd) + g.Expect(err).NotTo(HaveOccurred()) + var deployment struct { + Status struct { + AvailableReplicas int `json:"availableReplicas"` + } `json:"status"` + } + g.Expect(json.Unmarshal([]byte(output), &deployment)).To(Succeed()) + g.Expect(deployment.Status.AvailableReplicas).To(Equal(0)) + + cmd = exec.Command("kubectl", "get", "endpointslice", "-l", "kubernetes.io/service-name=docker-registry", + "-n", pauseResumeNamespace, "-o", "json") + output, err = utils.Run(cmd) + g.Expect(err).NotTo(HaveOccurred()) + var endpointSlices struct { + Items []struct { + Endpoints []struct { + Conditions struct { + Ready *bool `json:"ready"` + } `json:"conditions"` + } `json:"endpoints"` + } `json:"items"` + } + g.Expect(json.Unmarshal([]byte(output), &endpointSlices)).To(Succeed()) + readyCount := 0 + for _, endpointSlice := range endpointSlices.Items { + for _, endpoint := range endpointSlice.Endpoints { + if endpoint.Conditions.Ready == nil || *endpoint.Conditions.Ready { + readyCount++ + } + } + } + g.Expect(readyCount).To(Equal(0)) + }, 2*time.Minute).Should(Succeed()) + + By("triggering pause while the snapshot registry is unavailable") cmd = exec.Command("kubectl", "patch", "batchsandbox", sandboxName, "-n", pauseResumeNamespace, "--type=merge", "-p", `{"spec":{"pause":true}}`) @@ -878,6 +941,53 @@ var _ = Describe("PauseResume", Ordered, Label("PauseResume"), func() { cmd = exec.Command("kubectl", "delete", "batchsandbox", sandboxName, "-n", pauseResumeNamespace, "--ignore-not-found=true") utils.Run(cmd) + By("restoring the snapshot registry") + cmd = exec.Command("kubectl", "scale", "deployment", "docker-registry", + "-n", pauseResumeNamespace, "--replicas=1") + _, err = utils.Run(cmd) + Expect(err).NotTo(HaveOccurred()) + + By("waiting for the snapshot registry to become available") + Eventually(func(g Gomega) { + cmd := exec.Command("kubectl", "get", "deployment", "docker-registry", "-n", pauseResumeNamespace, "-o", "json") + output, err := utils.Run(cmd) + g.Expect(err).NotTo(HaveOccurred()) + var deployment struct { + Status struct { + AvailableReplicas int `json:"availableReplicas"` + } `json:"status"` + } + g.Expect(json.Unmarshal([]byte(output), &deployment)).To(Succeed()) + g.Expect(deployment.Status.AvailableReplicas).To(Equal(1)) + }, 2*time.Minute).Should(Succeed()) + + By("waiting for the snapshot registry endpoint to become ready") + Eventually(func(g Gomega) { + cmd := exec.Command("kubectl", "get", "endpointslice", "-l", "kubernetes.io/service-name=docker-registry", + "-n", pauseResumeNamespace, "-o", "json") + output, err := utils.Run(cmd) + g.Expect(err).NotTo(HaveOccurred()) + var endpointSlices struct { + Items []struct { + Endpoints []struct { + Conditions struct { + Ready *bool `json:"ready"` + } `json:"conditions"` + } `json:"endpoints"` + } `json:"items"` + } + g.Expect(json.Unmarshal([]byte(output), &endpointSlices)).To(Succeed()) + readyCount := 0 + for _, endpointSlice := range endpointSlices.Items { + for _, endpoint := range endpointSlice.Endpoints { + if endpoint.Conditions.Ready == nil || *endpoint.Conditions.Ready { + readyCount++ + } + } + } + g.Expect(readyCount).To(BeNumerically(">=", 1)) + }, 2*time.Minute).Should(Succeed()) + By("restoring registry push secret to valid credentials") err = createDockerRegistrySecrets(pauseResumeNamespace) Expect(err).NotTo(HaveOccurred()) @@ -1069,3 +1179,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)) + } +} 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