fix: Allow to delete images when names of images are short digest ids… #3519

haytok · 2024-10-08T14:34:27Z

… of another images.

"nerdctl rmi " can be run to delete the target images.

However, at the current implementation, the deletion fails when images names are the short digest ids of another images.

The specific behavior is described below, which is how it works in the current implementation.

First, suppose there are alpine and busybox images.

```
> nerdctl images
REPOSITORY    TAG       IMAGE ID        CREATED           PLATFORM       SIZE       BLOB SIZE
busybox       latest    768e5c6f5cb6    3 seconds ago     linux/arm64    4.092MB    1.845MB
alpine        latest    beefdbd8a1da    11 seconds ago    linux/arm64    10.46MB    4.09MB
```

Then, we tag the alpine image using digest id of the busybox image.

```
> nerdctl tag alpine $(dn inspect busybox | jq -rc .[0].RepoDigests[0] | awk -F':' '{print substr($2, 1, 8)}')

> nerdctl images
REPOSITORY    TAG       IMAGE ID        CREATED          PLATFORM       SIZE       BLOB SIZE
768e5c6f      latest    beefdbd8a1da    4 seconds ago    linux/arm64    10.46MB    4.09MB
busybox       latest    768e5c6f5cb6    22 hours ago     linux/arm64    4.092MB    1.845MB
alpine        latest    beefdbd8a1da    22 hours ago     linux/arm64    10.46MB    4.09MB
```

In this situation, running 'nerdctl rmi "$(dn inspect busybox | jq -rc .[0].RepoDigests[0] | awk -F':' '{print substr($2, 1, 8)}')"' will fail to remove the image.

The details of the error are as follows.

```
> nerdctl rmi "$(dn inspect busybox | jq -rc .[0].RepoDigests[0] | awk -F':' '{print substr($2, 1, 8)}')"
FATA[0000] 1 errors:
multiple IDs found with provided prefix: 768e5c6f
```

This issue is reported in the following issue.

- https://github.com/containerd/nerdctl/issues/3016

Therefore, this pull request modifies this so that images can be deleted with "nerdctl rmi " when images names are the short digest ids of another images.

pkg/idutil/imagewalker/imagewalker.go

apostasie · 2024-10-08T19:10:16Z

I think the problem runs deep.
I did work earlier on inspect and fixed a bunch of issues, but it is still buggy (my bad obviously).
Also, anything else using the imagewalker seems to have issues as well.

image history is clearly broken.

GetExistingImage might be fishy as well.

A number of other cases just assume that whatever is the first result returned by ImageService().List will be right (with a filter that contains both name=X and digest=^X.* - clearly problematic)

I do have strong feelings against imagewalker tbh (see #3502 ) - and maybe it is time to get done with it.

I appreciate this might be out of scope for this PR though - but for a larger refactor, I would suggest starting fresh with a clearer algorithm, and listing all possible cases, and removing imagewalker in favor of a simpler method that returns a slice.

IMHO image lookup algorithm should do the following:

first call ImageService().List to find an image with that exact name
if above returned nothing, try again ImageService().List, matching the prefix of the digest with the provided needle

An interesting case that is not covered anywhere is when querying using "X", where X is a prefix that actually matches two distinct RepoDigests.

eg:

query using ab

with

image1 sha256:ab19575368743875438
image2 sha256:abef89478445

This is hard to reproduce ^ as you need two distinct images with the first two chars of the digest identical, but it is doable.

... and obviously we should add a boatload of tests.

Anyhow.

apostasie · 2024-10-08T19:21:59Z

pkg/idutil/imagewalker/imagewalker.go

 	// to handle the `rmi -f` case where returned images are different but
 	// have the same short prefix.
 	uniqueImages := make(map[digest.Digest]bool)
 	for _, image := range images {
 		uniqueImages[image.Target.Digest] = true
 	}

+	// Allow to nerdctl rmi <short digest ids of another images> to remove images.
+	if len(uniqueImages) > 1 {


Is it working as intended?

There could be a case where you have multiple images with different digests, which digest start with the same few characters.
In that case, you would have multiple uniqueImages, and this would be fine, we should untag them all.

Thanks for comments!!!

The following if statement is necesary.

if len(uniqueImages) > 1 { ... }

Without this if statement, running nerdctl rmi "IMAGE ID" fails.

haytok · 2024-10-09T15:43:52Z

Hi, @apostasie

Thanks for many advices!!!

First, I respond to apostasie's comments ,and then I would like to discuss of my future works.

Afrer reviewing the issue and the related PR that apostasie pointed out, I understood the necessity of refactoring imagewalker.ImageWalker to address concurrency issues.

Specifically, I understand that the refactoring should similar to how ListNetworksMatch() is called within network.Remove(), as seen in the following PR.

pkg/cmd/network/remove.go : https://github.com/containerd/nerdctl/pull/3508/files

Future Directions

I think the following is a good policy for the future, is this correct?

Refactor cmd/nerdctl/image/image_remove_linux_test.go (Not Using base.Cmd() etc. Using nerdtest etc.)
Add tests for some scenarios to cmd/nerdctl/image/image_remove_linux_test.go
Refactor imagewalker.ImageWalker based on apostasie's advices 1 and 2

If this is correct, this PR can resolve the issue of removing images with short digest IDs using the nerdctl rmi. However, should I close this PR and focus on addressing points 1 to 3 mentioned above instead?

Questions for comments

I didn't fully understand the following comments, could you please explain them in more details? 🙏

When running nerdctl rmi with a query using "ab", I believe that images like sha256:ab19575368743875438 or sha256:abef89478445
would not match because the regular expressions used with digest IDs in nerdctl rmi are ^sha256:<input id>.*$ and ^<input id>.*$..

An interesting case that is not covered anywhere is when querying using "X", where X is a prefix that actually matches two distinct RepoDigests.
...

apostasie · 2024-10-10T20:56:21Z

If this is correct, this PR can resolve the issue of removing images with short digest IDs using the nerdctl rmi. However, should I close this PR and focus on addressing points 1 to 3 mentioned above instead?

Hey @haytok !

You are welcome.

I think your future direction sounds good.

Nevertheless, feel free to just keep things simple and if your first step is to fix some symptoms, just go for it, it is fine.

Questions for comments

I didn't fully understand the following comments, could you please explain them in more details? 🙏

When running nerdctl rmi with a query using "ab", I believe that images like sha256:ab19575368743875438 or sha256:abef89478445 would not match because the regular expressions used with digest IDs in nerdctl rmi are ^sha256:<input id>.*$ and ^<input id>.*$..

An interesting case that is not covered anywhere is when querying using "X", where X is a prefix that actually matches two distinct RepoDigests.
...

nerdctl pull --platform linux/arm64 alpine
nerdctl pull --platform linux/arm64 ubuntu

nerdctl images | grep -E " b"

ubuntu          latest    b359f1067efa    2 days ago    linux/arm64    110.3MB    28.89MB
alpine          latest    beefdbd8a1da    2 days ago    linux/arm64    9.486MB    4.09MB

So, in that case, what will your code do?

nerdctl rmi b

haytok · 2024-10-11T17:36:58Z

I think your future direction sounds good.

Nevertheless, feel free to just keep things simple and if your first step is to fix some symptoms, just go for it, it is fine.

Thans !!! OK :)

So, in that case, what will your code do?

Thanks, I can understand what you explained.

nerdctl in my code behaves below.

> _output/nerdctl images
REPOSITORY    TAG       IMAGE ID        CREATED         PLATFORM       SIZE       BLOB SIZE
ubuntu        latest    b359f1067efa    15 hours ago    linux/arm64    108.3MB    28.89MB
alpine        latest    beefdbd8a1da    15 hours ago    linux/arm64    10.46MB    4.09MB

> _output/nerdctl rmi b
FATA[0000] 1 errors:
no such image: b

However, this modification is considered incorrect as the upstream nerdctl behaves as follows.

> nerdctl rmi b
FATA[0000] 1 errors:
multiple IDs found with provided prefix: b

So, as previously advised, it would be better to implement the following.

first call ImageService().List to find an image with that exact name
if above returned nothing, try again ImageService().List, matching the prefix of the digest with the provided needle

haytok · 2024-10-17T14:47:41Z

Hi, @apostasie

Based on your advice, I updated the logic.
So, could you review when you have time ?

When running nerdctl rmi b in the current implementation, the following actions are performed.

Details

> _output/nerdctl images
REPOSITORY    TAG       IMAGE ID        CREATED               PLATFORM       SIZE       BLOB SIZE
768e5c6f      latest    beefdbd8a1da    2 seconds ago         linux/arm64    10.46MB    4.09MB
alpine        latest    beefdbd8a1da    About a minute ago    linux/arm64    10.46MB    4.09MB
nginx         latest    d2eb56950b84    53 minutes ago        linux/arm64    212.1MB    69.59MB
busybox       latest    768e5c6f5cb6    4 days ago            linux/arm64    4.092MB    1.845MB
ubuntu        latest    d4f6f70979d0    4 days ago            linux/arm64    108.3MB    28.89MB

When there is more than one image with prefix d (d2eb56950b84 and d4f6f70979d0), the following error occurs
I think this behavior is expected.

> _output/nerdctl rmi d
FATA[0000] 1 errors:
multiple IDs found with provided prefix: d

On the other hand, an image with short digest id of another image can be deleted.

> _output/nerdctl rmi 768e5c6f
Untagged: docker.io/library/768e5c6f:latest@sha256:beefdbd8a1da6d2915566fde36db9db0b524eb737fc57cd1367effd16dc0d06d
Deleted: sha256:16113d51b7181f20135f51e8ffbaead20a7671cd783017601f198748ce8a8ebf

> _output/nerdctl images
REPOSITORY    TAG       IMAGE ID        CREATED           PLATFORM       SIZE       BLOB SIZE
alpine        latest    beefdbd8a1da    7 minutes ago     linux/arm64    10.46MB    4.09MB
nginx         latest    d2eb56950b84    59 minutes ago    linux/arm64    212.1MB    69.59MB
busybox       latest    768e5c6f5cb6    4 days ago        linux/arm64    4.092MB    1.845MB
ubuntu        latest    d4f6f70979d0    4 days ago        linux/arm64    108.3MB    28.89MB

cmd/nerdctl/image/image_remove_test.go

apostasie · 2024-10-17T18:04:10Z

Hi, @apostasie

Based on your advice, I updated the logic. So, could you review when you have time ?

When running nerdctl rmi b in the current implementation, the following actions are performed.

Details

> _output/nerdctl images
REPOSITORY    TAG       IMAGE ID        CREATED               PLATFORM       SIZE       BLOB SIZE
768e5c6f      latest    beefdbd8a1da    2 seconds ago         linux/arm64    10.46MB    4.09MB
alpine        latest    beefdbd8a1da    About a minute ago    linux/arm64    10.46MB    4.09MB
nginx         latest    d2eb56950b84    53 minutes ago        linux/arm64    212.1MB    69.59MB
busybox       latest    768e5c6f5cb6    4 days ago            linux/arm64    4.092MB    1.845MB
ubuntu        latest    d4f6f70979d0    4 days ago            linux/arm64    108.3MB    28.89MB

When there is more than one image with prefix d (d2eb56950b84 and d4f6f70979d0), the following error occurs I think this behavior is expected.

> _output/nerdctl rmi d
FATA[0000] 1 errors:
multiple IDs found with provided prefix: d

On the other hand, an image with short digest id of another image can be deleted.

> _output/nerdctl rmi 768e5c6f
Untagged: docker.io/library/768e5c6f:latest@sha256:beefdbd8a1da6d2915566fde36db9db0b524eb737fc57cd1367effd16dc0d06d
Deleted: sha256:16113d51b7181f20135f51e8ffbaead20a7671cd783017601f198748ce8a8ebf

> _output/nerdctl images
REPOSITORY    TAG       IMAGE ID        CREATED           PLATFORM       SIZE       BLOB SIZE
alpine        latest    beefdbd8a1da    7 minutes ago     linux/arm64    10.46MB    4.09MB
nginx         latest    d2eb56950b84    59 minutes ago    linux/arm64    212.1MB    69.59MB
busybox       latest    768e5c6f5cb6    4 days ago        linux/arm64    4.092MB    1.845MB
ubuntu        latest    d4f6f70979d0    4 days ago        linux/arm64    108.3MB    28.89MB

Hey @haytok

I left of few comments for the test part.

Apart from that, I think that is a good first step, and probably all we can do short of a full rewrite of imagewalker.

+1

haytok · 2024-10-18T15:56:53Z

Hi, @apostasie

Thanks for review !!!

I have updated, so could you re-reiview?
If there are no problems, I'll squash.

Apart from that, I think that is a good first step, and probably all we can do short of a full rewrite of imagewalker.

Thank you! Working on this issue has made me read the imagewalker source code, so I will work on fixing imagewalker if no one else is working on it.

apostasie · 2024-10-18T16:54:16Z

Hi, @apostasie

Thanks for review !!!

I have updated, so could you re-reiview? If there are no problems, I'll squash.

+1

You need maintainers review. Tagging @AkihiroSuda @djdongjin @ktock

Side-note: to avoid having to squash, you can just amend and force push your commits instead.

cmd/nerdctl/image/image_remove_test.go

djdongjin · 2024-10-18T20:37:37Z

pkg/idutil/imagewalker/imagewalker.go

 		uniqueImages[image.Target.Digest] = true
+		// to get target image index for `nerdctl rmi <short digest ids of another images>`.
+		if (parsedReferenceStr != "" && image.Name == parsedReferenceStr) || image.Name == req {
+			nameMatchIndex = i


is it possible name match happens on multiple images?

Thank you for your feedback!

Yes, name matching can occur for multiple images if they share a similar prefix.
For example, when there are several alpine images with different tags, running nerdctl rmi alpine will specifically remove alpine:latest by default:

> nerdctl images | grep alpine alpine latest beefdbd8a1da About a minute ago linux/arm64 10.46MB 4.09MB alpine 20190408 8b6b8c0f71e8 16 minutes ago linux/arm64 6.844MB 2.708MB alpine 20190228 6199d795f07e 21 minutes ago linux/arm64 6.844MB 2.706MB ghcr.io/stargz-containers/alpine 3.13-org ec14c7992a97 19 hours ago linux/arm64 6.865MB 2.714MB > nerdctl rmi alpine Untagged: docker.io/library/alpine:latest@sha256:beefdbd8a1da6d2915566fde36db9db0b524eb737fc57cd1367effd16dc0d06d Deleted: sha256:16113d51b7181f20135f51e8ffbaead20a7671cd783017601f198748ce8a8ebf > nerdctl images | grep alpine alpine 20190408 8b6b8c0f71e8 22 minutes ago linux/arm64 6.844MB 2.708MB alpine 20190228 6199d795f07e 27 minutes ago linux/arm64 6.844MB 2.706MB ghcr.io/stargz-containers/alpine 3.13-org ec14c7992a97 19 hours ago linux/arm64 6.865MB 2.714MB

However, if an image matches the given prefix, such as alpine:custom, the image must be removed by specifying alpine:<tag name>.

Does this example and explanation address your concern?

haytok · 2024-10-19T08:07:54Z

Hey @apostasie

I updated code, but the test / unit | windows failed bacase of timeout ...
Do you have any knowledge on how to deal with this?

djdongjin

LGTM, thanks

AkihiroSuda

Please squash the commits

haytok · 2024-10-19T18:48:16Z

I have squashed commits, but a test in test-integration-docker-compatibility failed, so I'm checking ...

Details

=== NAME  TestImagesFilter/since=ghcr.io/stargz-containers/alpine:3.13-org_ghcr.io/stargz-containers/alpine:3.13-org
    command.go:112: assertion failed: expect.ExitCode is not result.ExitCode: Expected exit code: 0
        
        Command:  /usr/bin/docker images --filter since=ghcr.io/stargz-containers/alpine:3.13-org ghcr.io/stargz-containers/alpine:3.13-org
        ExitCode: 1
        Error:    exit status 1
        Stdout:   
        Stderr:   Error response from daemon: No such image: ghcr.io/stargz-containers/alpine:3.13-org
        
        Env:
        LANG=C.UTF-8
        PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:/snap/bin
        MAIL=/var/mail/root
        LOGNAME=root
        USER=root
        HOME=/root
        SHELL=/bin/bash
        TERM=unknown
        SUDO_COMMAND=/tmp/go-build1499803690/b845/image.test -test.paniconexit0 -test.timeout=20m0s -test.v=true -test.target=docker -test.allow-kill-daemon
        SUDO_USER=runner
        SUDO_UID=1001
        SUDO_GID=118
        ImageVersion=20241016.1.0
        ImageOS=ubuntu24
        ACCEPT_EULA=Y
        XDG_CONFIG_HOME=/home/runner/.config
        AGENT_TOOLSDIRECTORY=/opt/hostedtoolcache
        RUNNER_TOOL_CACHE=/opt/hostedtoolcache
        ACTIONS_RUNNER_ACTION_ARCHIVE_CACHE=/opt/actionarchivecache
        AZURE_EXTENSION_DIR=/opt/az/azcliextensions
        SWIFT_PATH=/usr/share/swift/usr/bin
        DOTNET_SKIP_FIRST_TIME_EXPERIENCE=1
        DOTNET_NOLOGO=1
        DOTNET_MULTILEVEL_LOOKUP=0
        EDGEWEBDRIVER=/usr/local/share/edge_driver
        GECKOWEBDRIVER=/usr/local/share/gecko_driver
        CHROME_BIN=/usr/bin/google-chrome
        CHROMEWEBDRIVER=/usr/local/share/chromedriver-linux64
        BOOTSTRAP_HASKELL_NONINTERACTIVE=1
        GHCUP_INSTALL_BASE_PREFIX=/usr/local
        JAVA_HOME_8_X64=/usr/lib/jvm/temurin-8-jdk-amd64
        JAVA_HOME_11_X64=/usr/lib/jvm/temurin-11-jdk-amd64
        JAVA_HOME=/usr/lib/jvm/temurin-17-jdk-amd64
        JAVA_HOME_17_X64=/usr/lib/jvm/temurin-17-jdk-amd64
        JAVA_HOME_21_X64=/usr/lib/jvm/temurin-21-jdk-amd64
        ANT_HOME=/usr/share/ant
        GRADLE_HOME=/usr/share/gradle-8.10.2
        CONDA=/usr/share/miniconda
        SELENIUM_JAR_PATH=/usr/share/java/selenium-server.jar
        VCPKG_INSTALLATION_ROOT=/usr/local/share/vcpkg
        DEBIAN_FRONTEND=noninteractive
        ANDROID_SDK_ROOT=/usr/local/lib/android/sdk
        ANDROID_HOME=/usr/local/lib/android/sdk
        ANDROID_NDK=/usr/local/lib/android/sdk/ndk/27.1.12297006
        ANDROID_NDK_HOME=/usr/local/lib/android/sdk/ndk/27.1.12297006
        ANDROID_NDK_ROOT=/usr/local/lib/android/sdk/ndk/27.1.12297006
        ANDROID_NDK_LATEST_HOME=/usr/local/lib/android/sdk/ndk/27.1.12297006
        PIPX_BIN_DIR=/opt/pipx_bin
        PIPX_HOME=/opt/pipx
        GOROOT_1_21_X64=/opt/hostedtoolcache/go/1.21.13/x64
        GOROOT_1_22_X64=/opt/hostedtoolcache/go/1.22.8/x64
        GOROOT_1_23_X64=/opt/hostedtoolcache/go/1.23.2/x64
        HOMEBREW_NO_AUTO_UPDATE=1
        HOMEBREW_CLEANUP_PERIODIC_FULL_DAYS=3650
        RUNNER_USER=runner
        DEPLOYMENT_BASEPATH=/opt/runner
        PERFLOG_LOCATION_SETTING=RUNNER_PERFLOG
        POWERSHELL_DISTRIBUTION_CHANNEL=GitHub-Actions-ubuntu24
        XDG_RUNTIME_DIR=/run/user/1001
        DOCKER_CONFIG=/tmp/TestImagesFiltersince=ghcr.iostargz-containersalpine3.13-org_ghcr.iostargz-containersalpine3.13-org1174145695/001
    case.go:164: ======================== Post-test cleanup ========================
...
=== NAME  TestImagesFilter/since=ghcr.io/stargz-containers/alpine:3.13-org
    command.go:112: assertion failed: expect.ExitCode is not result.ExitCode: Expected exit code: 0
        
        Command:  /usr/bin/docker images --filter since=ghcr.io/stargz-containers/alpine:3.13-org
        ExitCode: 1
        Error:    exit status 1
        Stdout:   
        Stderr:   Error response from daemon: No such image: ghcr.io/stargz-containers/alpine:3.13-org
        
        Env:
        LANG=C.UTF-8
        PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:/snap/bin
        MAIL=/var/mail/root
        LOGNAME=root
        USER=root
        HOME=/root
        SHELL=/bin/bash
        TERM=unknown
        SUDO_COMMAND=/tmp/go-build1499803690/b845/image.test -test.paniconexit0 -test.timeout=20m0s -test.v=true -test.target=docker -test.allow-kill-daemon
        SUDO_USER=runner
        SUDO_UID=1001
        SUDO_GID=118
        ImageVersion=20241016.1.0
        ImageOS=ubuntu24
        ACCEPT_EULA=Y
        XDG_CONFIG_HOME=/home/runner/.config
        AGENT_TOOLSDIRECTORY=/opt/hostedtoolcache
        RUNNER_TOOL_CACHE=/opt/hostedtoolcache
        ACTIONS_RUNNER_ACTION_ARCHIVE_CACHE=/opt/actionarchivecache
        AZURE_EXTENSION_DIR=/opt/az/azcliextensions
        SWIFT_PATH=/usr/share/swift/usr/bin
        DOTNET_SKIP_FIRST_TIME_EXPERIENCE=1
        DOTNET_NOLOGO=1
        DOTNET_MULTILEVEL_LOOKUP=0
        EDGEWEBDRIVER=/usr/local/share/edge_driver
        GECKOWEBDRIVER=/usr/local/share/gecko_driver
        CHROME_BIN=/usr/bin/google-chrome
        CHROMEWEBDRIVER=/usr/local/share/chromedriver-linux64
        BOOTSTRAP_HASKELL_NONINTERACTIVE=1
        GHCUP_INSTALL_BASE_PREFIX=/usr/local
        JAVA_HOME_8_X64=/usr/lib/jvm/temurin-8-jdk-amd64
        JAVA_HOME_11_X64=/usr/lib/jvm/temurin-11-jdk-amd64
        JAVA_HOME=/usr/lib/jvm/temurin-17-jdk-amd64
        JAVA_HOME_17_X64=/usr/lib/jvm/temurin-17-jdk-amd64
        JAVA_HOME_21_X64=/usr/lib/jvm/temurin-21-jdk-amd64
        ANT_HOME=/usr/share/ant
        GRADLE_HOME=/usr/share/gradle-8.10.2
        CONDA=/usr/share/miniconda
        SELENIUM_JAR_PATH=/usr/share/java/selenium-server.jar
        VCPKG_INSTALLATION_ROOT=/usr/local/share/vcpkg
        DEBIAN_FRONTEND=noninteractive
        ANDROID_SDK_ROOT=/usr/local/lib/android/sdk
        ANDROID_HOME=/usr/local/lib/android/sdk
        ANDROID_NDK=/usr/local/lib/android/sdk/ndk/27.1.12297006
        ANDROID_NDK_HOME=/usr/local/lib/android/sdk/ndk/27.1.12297006
        ANDROID_NDK_ROOT=/usr/local/lib/android/sdk/ndk/27.1.12297006
        ANDROID_NDK_LATEST_HOME=/usr/local/lib/android/sdk/ndk/27.1.12297006
        PIPX_BIN_DIR=/opt/pipx_bin
        PIPX_HOME=/opt/pipx
        GOROOT_1_21_X64=/opt/hostedtoolcache/go/1.21.13/x64
        GOROOT_1_22_X64=/opt/hostedtoolcache/go/1.22.8/x64
        GOROOT_1_23_X64=/opt/hostedtoolcache/go/1.23.2/x64
        HOMEBREW_NO_AUTO_UPDATE=1
        HOMEBREW_CLEANUP_PERIODIC_FULL_DAYS=3650
        RUNNER_USER=runner
        DEPLOYMENT_BASEPATH=/opt/runner
        PERFLOG_LOCATION_SETTING=RUNNER_PERFLOG
        POWERSHELL_DISTRIBUTION_CHANNEL=GitHub-Actions-ubuntu24
        XDG_RUNTIME_DIR=/run/user/1001
        DOCKER_CONFIG=/tmp/TestImagesFiltersince=ghcr.iostargz-containersalpine3.13-org2364475696/001
    case.go:164: ======================== Post-test cleanup ========================
...
=== NAME  TestImagesFilter/before=ID:latest
    command.go:112: assertion failed: expression is false: strings.Contains(stdout, compare): Output does not contain: "ghcr.io/stargz-containers/alpine"
        Command:  /usr/bin/docker images --filter before=testimagesfilter-2a4e0faa:latest
        ExitCode: 0
        Stdout:   REPOSITORY                                      TAG                IMAGE ID       CREATED       SIZE
        ghcr.io/stargz-containers/cesanta/docker_auth   1.7-org            e10c17d11f85   3 years ago   33.9MB
        ghcr.io/stargz-containers/registry              2-org              ee34aa9d8ab2   3 years ago   26.2MB
        taggedimage                                     one-fragment-one   49f356fa4513   3 years ago   5.61MB
        taggedimage                                     two-fragment-two   49f356fa4513   3 years ago   5.61MB
        
        Stderr:   
        Env:
        LANG=C.UTF-8
        PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:/snap/bin
        MAIL=/var/mail/root
        LOGNAME=root
        USER=root
        HOME=/root
        SHELL=/bin/bash
        TERM=unknown
        SUDO_COMMAND=/tmp/go-build1499803690/b845/image.test -test.paniconexit0 -test.timeout=20m0s -test.v=true -test.target=docker -test.allow-kill-daemon
        SUDO_USER=runner
        SUDO_UID=1001
        SUDO_GID=118
        ImageVersion=20241016.1.0
        ImageOS=ubuntu24
        ACCEPT_EULA=Y
        XDG_CONFIG_HOME=/home/runner/.config
        AGENT_TOOLSDIRECTORY=/opt/hostedtoolcache
        RUNNER_TOOL_CACHE=/opt/hostedtoolcache
        ACTIONS_RUNNER_ACTION_ARCHIVE_CACHE=/opt/actionarchivecache
        AZURE_EXTENSION_DIR=/opt/az/azcliextensions
        SWIFT_PATH=/usr/share/swift/usr/bin
        DOTNET_SKIP_FIRST_TIME_EXPERIENCE=1
        DOTNET_NOLOGO=1
        DOTNET_MULTILEVEL_LOOKUP=0
        EDGEWEBDRIVER=/usr/local/share/edge_driver
        GECKOWEBDRIVER=/usr/local/share/gecko_driver
        CHROME_BIN=/usr/bin/google-chrome
        CHROMEWEBDRIVER=/usr/local/share/chromedriver-linux64
        BOOTSTRAP_HASKELL_NONINTERACTIVE=1
        GHCUP_INSTALL_BASE_PREFIX=/usr/local
        JAVA_HOME_8_X64=/usr/lib/jvm/temurin-8-jdk-amd64
        JAVA_HOME_11_X64=/usr/lib/jvm/temurin-11-jdk-amd64
        JAVA_HOME=/usr/lib/jvm/temurin-17-jdk-amd64
        JAVA_HOME_17_X64=/usr/lib/jvm/temurin-17-jdk-amd64
        JAVA_HOME_21_X64=/usr/lib/jvm/temurin-21-jdk-amd64
        ANT_HOME=/usr/share/ant
        GRADLE_HOME=/usr/share/gradle-8.10.2
        CONDA=/usr/share/miniconda
        SELENIUM_JAR_PATH=/usr/share/java/selenium-server.jar
        VCPKG_INSTALLATION_ROOT=/usr/local/share/vcpkg
        DEBIAN_FRONTEND=noninteractive
        ANDROID_SDK_ROOT=/usr/local/lib/android/sdk
        ANDROID_HOME=/usr/local/lib/android/sdk
        ANDROID_NDK=/usr/local/lib/android/sdk/ndk/27.1.12297006
        ANDROID_NDK_HOME=/usr/local/lib/android/sdk/ndk/27.1.12297006
        ANDROID_NDK_ROOT=/usr/local/lib/android/sdk/ndk/27.1.12297006
        ANDROID_NDK_LATEST_HOME=/usr/local/lib/android/sdk/ndk/27.1.12297006
        PIPX_BIN_DIR=/opt/pipx_bin
        PIPX_HOME=/opt/pipx
        GOROOT_1_21_X64=/opt/hostedtoolcache/go/1.21.13/x64
        GOROOT_1_22_X64=/opt/hostedtoolcache/go/1.22.8/x64
        GOROOT_1_23_X64=/opt/hostedtoolcache/go/1.23.2/x64
        HOMEBREW_NO_AUTO_UPDATE=1
        HOMEBREW_CLEANUP_PERIODIC_FULL_DAYS=3650
        RUNNER_USER=runner
        DEPLOYMENT_BASEPATH=/opt/runner
        PERFLOG_LOCATION_SETTING=RUNNER_PERFLOG
        POWERSHELL_DISTRIBUTION_CHANNEL=GitHub-Actions-ubuntu24
        XDG_RUNTIME_DIR=/run/user/1001
        DOCKER_CONFIG=/tmp/TestImagesFilterbefore=IDlatest1569974224/001

AkihiroSuda

Thanks

apostasie · 2024-10-19T22:13:10Z

cmd/nerdctl/image/image_remove_test.go

+				data.Set(tagIDKey, tagID)
+			},
+			Cleanup: func(data test.Data, helpers test.Helpers) {
+				helpers.Anyhow("rmi", "-f", testutil.CommonImage)


@haytok

You should not remove these images.
Your test is being run in parallel with other tests that might depend on these images (this is why you are failing docker tests)
It is fine to just let them in.

Thanks !!!
Now that I have made sure that the CommonImage is not deleted, let's see if the test succeeds.

apostasie · 2024-10-19T22:14:00Z

I have squashed commits, but a test in test-integration-docker-compatibility failed, so I'm checking ...

Details

@haytok see comment on not removing common images.

apostasie · 2024-10-19T22:22:21Z

I have squashed commits, but a test in test-integration-docker-compatibility failed, so I'm checking ...
Details

@haytok see comment on not removing common images.

Comment applies to both images (alpinenginx as well).

apostasie · 2024-10-19T22:24:26Z

I have squashed commits, but a test in test-integration-docker-compatibility failed, so I'm checking ...
Details

@haytok see comment on not removing common images.

Comment applies to both images (alpinenginx as well).

Generally speaking, only remove images you are creating in your test (through build or tag).

We probably need a better mechanism for that overall, but TBD.

haytok · 2024-10-19T22:26:41Z

Comment applies to both images (alpinenginx as well).

Oh, OK, I'll fix

Generally speaking, only remove images you are creating in your test (through build or tag).

Thanks !!!

… of another images. "nerdctl rmi <REPOSITORY>" can be run to delete the target images. However, at the current implementation, the deletion fails when images names are the short digest ids of another images. The specific behavior is described below, which is how it works in the current implementation. First, suppose there are alpine and busybox images. ``` > nerdctl images REPOSITORY TAG IMAGE ID CREATED PLATFORM SIZE BLOB SIZE busybox latest 768e5c6f5cb6 3 seconds ago linux/arm64 4.092MB 1.845MB alpine latest beefdbd8a1da 11 seconds ago linux/arm64 10.46MB 4.09MB ``` Then, we tag the alpine image using digest id of the busybox image. ``` > nerdctl tag alpine $(dn inspect busybox | jq -rc .[0].RepoDigests[0] | awk -F':' '{print substr($2, 1, 8)}') > nerdctl images REPOSITORY TAG IMAGE ID CREATED PLATFORM SIZE BLOB SIZE 768e5c6f latest beefdbd8a1da 4 seconds ago linux/arm64 10.46MB 4.09MB busybox latest 768e5c6f5cb6 22 hours ago linux/arm64 4.092MB 1.845MB alpine latest beefdbd8a1da 22 hours ago linux/arm64 10.46MB 4.09MB ``` In this situation, running 'nerdctl rmi "$(dn inspect busybox | jq -rc .[0].RepoDigests[0] | awk -F':' '{print substr($2, 1, 8)}')"' will fail to remove the image. The details of the error are as follows. ``` > nerdctl rmi "$(dn inspect busybox | jq -rc .[0].RepoDigests[0] | awk -F':' '{print substr($2, 1, 8)}')" FATA[0000] 1 errors: multiple IDs found with provided prefix: 768e5c6f ``` This issue is reported in the following issue. - containerd#3016 Therefore, this pull request modifies this so that images can be deleted with "nerdctl rmi <short digest id of another image>" when images names are the short digest ids of another images. Signed-off-by: Hayato Kiwata <[email protected]>

apostasie · 2024-10-20T23:42:09Z

Comment applies to both images (alpinenginx as well).

Oh, OK, I'll fix

Generally speaking, only remove images you are creating in your test (through build or tag).

Thanks !!!

Thanks @haytok.

AkihiroSuda

Thanks

apostasie reviewed Oct 8, 2024

View reviewed changes

pkg/idutil/imagewalker/imagewalker.go Outdated Show resolved Hide resolved

apostasie reviewed Oct 8, 2024

View reviewed changes

haytok force-pushed the issue_3016 branch 2 times, most recently from 4b228bb to a008261 Compare October 17, 2024 13:36

apostasie reviewed Oct 17, 2024

View reviewed changes

cmd/nerdctl/image/image_remove_test.go Outdated Show resolved Hide resolved

apostasie reviewed Oct 17, 2024

View reviewed changes

cmd/nerdctl/image/image_remove_test.go Outdated Show resolved Hide resolved

apostasie reviewed Oct 17, 2024

View reviewed changes

cmd/nerdctl/image/image_remove_test.go Outdated Show resolved Hide resolved

djdongjin reviewed Oct 18, 2024

View reviewed changes

djdongjin approved these changes Oct 19, 2024

View reviewed changes

djdongjin added this to the v2.0.0 milestone Oct 19, 2024

AkihiroSuda requested changes Oct 19, 2024

View reviewed changes

haytok force-pushed the issue_3016 branch from 5da8521 to 530b764 Compare October 19, 2024 17:34

AkihiroSuda approved these changes Oct 19, 2024

View reviewed changes

apostasie reviewed Oct 19, 2024

View reviewed changes

haytok force-pushed the issue_3016 branch from 530b764 to 94b93be Compare October 19, 2024 22:18

haytok force-pushed the issue_3016 branch from 94b93be to c3627e1 Compare October 19, 2024 22:28

AkihiroSuda requested a review from apostasie October 20, 2024 21:45

apostasie approved these changes Oct 20, 2024

View reviewed changes

AkihiroSuda approved these changes Oct 20, 2024

View reviewed changes

AkihiroSuda merged commit 07cb00b into containerd:main Oct 20, 2024
26 checks passed

haytok deleted the issue_3016 branch October 21, 2024 00:20

apostasie mentioned this pull request Oct 24, 2024

nerdctl rmi can be broken by using tag names with repo short sha #3016

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: Allow to delete images when names of images are short digest ids… #3519

fix: Allow to delete images when names of images are short digest ids… #3519

haytok commented Oct 8, 2024

apostasie commented Oct 8, 2024 •

edited

Loading

apostasie Oct 8, 2024 •

edited

Loading

haytok Oct 9, 2024 •

edited

Loading

haytok commented Oct 9, 2024 •

edited

Loading

apostasie commented Oct 10, 2024

Questions for comments

haytok commented Oct 11, 2024

haytok commented Oct 17, 2024

apostasie commented Oct 17, 2024

haytok commented Oct 18, 2024

apostasie commented Oct 18, 2024

djdongjin Oct 18, 2024

haytok Oct 19, 2024

haytok commented Oct 19, 2024

djdongjin left a comment

AkihiroSuda left a comment

haytok commented Oct 19, 2024

AkihiroSuda left a comment

apostasie Oct 19, 2024

haytok Oct 19, 2024

apostasie commented Oct 19, 2024

apostasie commented Oct 19, 2024

apostasie commented Oct 19, 2024

haytok commented Oct 19, 2024 •

edited

Loading

apostasie commented Oct 20, 2024

AkihiroSuda left a comment

fix: Allow to delete images when names of images are short digest ids… #3519

fix: Allow to delete images when names of images are short digest ids… #3519

Conversation

haytok commented Oct 8, 2024

apostasie commented Oct 8, 2024 • edited Loading

apostasie Oct 8, 2024 • edited Loading

Choose a reason for hiding this comment

haytok Oct 9, 2024 • edited Loading

Choose a reason for hiding this comment

haytok commented Oct 9, 2024 • edited Loading

Future Directions

Questions for comments

apostasie commented Oct 10, 2024

Questions for comments

haytok commented Oct 11, 2024

haytok commented Oct 17, 2024

apostasie commented Oct 17, 2024

haytok commented Oct 18, 2024

apostasie commented Oct 18, 2024

djdongjin Oct 18, 2024

Choose a reason for hiding this comment

haytok Oct 19, 2024

Choose a reason for hiding this comment

haytok commented Oct 19, 2024

djdongjin left a comment

Choose a reason for hiding this comment

AkihiroSuda left a comment

Choose a reason for hiding this comment

haytok commented Oct 19, 2024

AkihiroSuda left a comment

Choose a reason for hiding this comment

apostasie Oct 19, 2024

Choose a reason for hiding this comment

haytok Oct 19, 2024

Choose a reason for hiding this comment

apostasie commented Oct 19, 2024

apostasie commented Oct 19, 2024

apostasie commented Oct 19, 2024

haytok commented Oct 19, 2024 • edited Loading

apostasie commented Oct 20, 2024

AkihiroSuda left a comment

Choose a reason for hiding this comment

apostasie commented Oct 8, 2024 •

edited

Loading

apostasie Oct 8, 2024 •

edited

Loading

haytok Oct 9, 2024 •

edited

Loading

haytok commented Oct 9, 2024 •

edited

Loading

haytok commented Oct 19, 2024 •

edited

Loading