Add `diff` command #2626

minuk-dev · 2023-11-04T13:23:20Z

Add diff command:
- Use continuity/fs to compare filesystem.

I referred to

Lines 73 to 95 in e674fe7

    
           // NOTE: Moby uses provided rootfs to run container. It doesn't support 
        
           // to commit container created by moby. 
        
           baseImgWithoutPlatform, err := client.ImageService().Get(ctx, info.Image) 
        
           if err != nil { 
        
           	return emptyDigest, fmt.Errorf("container %q lacks image (wasn't created by nerdctl?): %w", id, err) 
        
           } 
        
           platformLabel := info.Labels[labels.Platform] 
        
           if platformLabel == "" { 
        
           	platformLabel = platforms.DefaultString() 
        
           	log.G(ctx).Warnf("Image lacks label %q, assuming the platform to be %q", labels.Platform, platformLabel) 
        
           } 
        
           ocispecPlatform, err := platforms.Parse(platformLabel) 
        
           if err != nil { 
        
           	return emptyDigest, err 
        
           } 
        
           log.G(ctx).Debugf("ocispecPlatform=%q", platforms.Format(ocispecPlatform)) 
        
           platformMC := platforms.Only(ocispecPlatform) 
        
           baseImg := containerd.NewImageWithPlatform(client, baseImgWithoutPlatform, platformMC) 
        
           baseImgConfig, _, err := imgutil.ReadImageConfig(ctx, baseImg) 
        
           if err != nil { 
        
           	return emptyDigest, err 
        
           }

:

But, I don't have any idea to test it. If you have it, please let me know. 🙇

(Updated) I found the test of diff command only works on linux, so remove its test from windows. I think I don't use a platform specific feature, but the test output is weird.:
- Q. Can I make an issue for windows and try in another pr? I'm not sure I can make it.

Signed-off-by: Min Uk Lee <minuk.dev@gmail.com>

AkihiroSuda · 2023-11-04T15:27:44Z

cmd/nerdctl/container_diff.go

+	var changes []fs.Change
+	err = mount.WithReadonlyTempMount(ctx, parent, func(lower string) error {
+		return mount.WithReadonlyTempMount(ctx, mounts, func(upper string) error {
+			fs.Changes(ctx, lower, upper, func(ck fs.ChangeKind, s string, fi os.FileInfo, err error) error {


The return value from fs.Changes shouldn’t be ignored

Fixed at 00c0d3b
Thanks a lot. 🙇

AkihiroSuda · 2023-11-04T15:28:00Z

cmd/nerdctl/container_diff.go

+}
+
+func diffShellComplete(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) {
+	// show container names (TODO: only show containers with logs)


TODO seems wrong

Fixed at d03a599 🙇‍♂️

AkihiroSuda · 2023-11-04T15:28:27Z

cmd/nerdctl/container_diff.go

+				case fs.ChangeKindModify:
+					fmt.Fprintln(cmd.OutOrStdout(), "C", change.Path)
+				case fs.ChangeKindDelete:
+					fmt.Fprintln(cmd.OutOrStdout(), "D", change.Path)


OutOrStdout should be called only once

Fixed at 3d1a5f1
Thanks a lot really. 👍

AkihiroSuda · 2023-11-04T15:30:10Z

cmd/nerdctl/main.go

@@ -251,6 +251,7 @@ Config file ($NERDCTL_TOML): %s
 		newPortCommand(),
 		newStopCommand(),
 		newStartCommand(),
+		newDiffCommand(),


Please update https://github.com/containerd/nerdctl/blob/main/docs/command-reference.md too

Added at a78b50e

Signed-off-by: Min Uk Lee <minuk.dev@gmail.com>

AkihiroSuda

Thanks

AkihiroSuda · 2023-11-06T17:32:34Z

Next time please squash commits

minuk-dev added 2 commits November 4, 2023 21:47

Add diff command

d96ad3b

Signed-off-by: Min Uk Lee <minuk.dev@gmail.com>

Add comment to diff test

2fcd3c4

Signed-off-by: Min Uk Lee <minuk.dev@gmail.com>

minuk-dev marked this pull request as draft November 4, 2023 13:33

minuk-dev changed the title ~~Add diff command~~ [WIP] Add diff command Nov 4, 2023

minuk-dev force-pushed the iss1376/diff branch from 5e9f539 to 3957c75 Compare November 4, 2023 13:49

Remove diff test from windows

da85b40

Signed-off-by: Min Uk Lee <minuk.dev@gmail.com>

minuk-dev force-pushed the iss1376/diff branch from 3957c75 to da85b40 Compare November 4, 2023 13:54

minuk-dev changed the title ~~[WIP] Add diff command~~ Add diff command Nov 4, 2023

Support diff command's autocompletion

cc52ea4

Signed-off-by: Min Uk Lee <minuk.dev@gmail.com>

minuk-dev marked this pull request as ready for review November 4, 2023 14:12

AkihiroSuda reviewed Nov 4, 2023

View reviewed changes

minuk-dev added 4 commits November 5, 2023 01:02

Fix missing error handling

00c0d3b

Signed-off-by: Min Uk Lee <minuk.dev@gmail.com>

Fix wrong comment of completion

d03a599

Signed-off-by: Min Uk Lee <minuk.dev@gmail.com>

Use prepared Stdout

3d1a5f1

Signed-off-by: Min Uk Lee <minuk.dev@gmail.com>

Add diff command to docs & fix usage message

a78b50e

Signed-off-by: Min Uk Lee <minuk.dev@gmail.com>

minuk-dev requested a review from AkihiroSuda November 4, 2023 16:32

AkihiroSuda added this to the v1.7.1 (tentative) milestone Nov 6, 2023

AkihiroSuda approved these changes Nov 6, 2023

View reviewed changes

AkihiroSuda merged commit ce2f63d into containerd:main Nov 6, 2023
22 checks passed

minuk-dev deleted the iss1376/diff branch November 14, 2023 13:38

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add `diff` command #2626

Add `diff` command #2626

minuk-dev commented Nov 4, 2023 •

edited

AkihiroSuda Nov 4, 2023

minuk-dev Nov 4, 2023 •

edited

AkihiroSuda Nov 4, 2023

minuk-dev Nov 4, 2023 •

edited

AkihiroSuda Nov 4, 2023 •

edited

minuk-dev Nov 4, 2023

AkihiroSuda Nov 4, 2023

minuk-dev Nov 4, 2023

AkihiroSuda left a comment

AkihiroSuda commented Nov 6, 2023

	// NOTE: Moby uses provided rootfs to run container. It doesn't support
	// to commit container created by moby.
	baseImgWithoutPlatform, err := client.ImageService().Get(ctx, info.Image)
	if err != nil {
	return emptyDigest, fmt.Errorf("container %q lacks image (wasn't created by nerdctl?): %w", id, err)
	}
	platformLabel := info.Labels[labels.Platform]
	if platformLabel == "" {
	platformLabel = platforms.DefaultString()
	log.G(ctx).Warnf("Image lacks label %q, assuming the platform to be %q", labels.Platform, platformLabel)
	}
	ocispecPlatform, err := platforms.Parse(platformLabel)
	if err != nil {
	return emptyDigest, err
	}
	log.G(ctx).Debugf("ocispecPlatform=%q", platforms.Format(ocispecPlatform))
	platformMC := platforms.Only(ocispecPlatform)
	baseImg := containerd.NewImageWithPlatform(client, baseImgWithoutPlatform, platformMC)

	baseImgConfig, _, err := imgutil.ReadImageConfig(ctx, baseImg)
	if err != nil {
	return emptyDigest, err
	}

Add diff command #2626

Add diff command #2626

Conversation

minuk-dev commented Nov 4, 2023 • edited

AkihiroSuda Nov 4, 2023

Choose a reason for hiding this comment

minuk-dev Nov 4, 2023 • edited

Choose a reason for hiding this comment

AkihiroSuda Nov 4, 2023

Choose a reason for hiding this comment

minuk-dev Nov 4, 2023 • edited

Choose a reason for hiding this comment

AkihiroSuda Nov 4, 2023 • edited

Choose a reason for hiding this comment

minuk-dev Nov 4, 2023

Choose a reason for hiding this comment

AkihiroSuda Nov 4, 2023

Choose a reason for hiding this comment

minuk-dev Nov 4, 2023

Choose a reason for hiding this comment

AkihiroSuda left a comment

Choose a reason for hiding this comment

AkihiroSuda commented Nov 6, 2023

Add `diff` command #2626

Add `diff` command #2626

minuk-dev commented Nov 4, 2023 •

edited

minuk-dev Nov 4, 2023 •

edited

minuk-dev Nov 4, 2023 •

edited

AkihiroSuda Nov 4, 2023 •

edited