podman Image diff between two images #10667

infiniteregrets · 2021-06-13T11:40:24Z

This adds image diff functionality as mentioned in #10649. This was my first time writing go, so if I made any mistakes or if something is not idiomatic then please let me know and I will fix it. I couldn't find a way to accept optional arguments in go so I just added if else blocks to check for length of args.
If no image is given then the following error would occur:

[mehul@fedora bin]$ ./podman image diff
Error: accepts between 1 and 2 arg(s), received 0

If a single image is given then it is compared to the parent layer, retaining the functionality from before.

[mehul@fedora bin]$ ./podman image diff alpine:latest
A /lib
A /lib/apk
A /lib/apk/db
A /lib/apk/db/installed
A /lib/apk/db/lock
.
.

If two images are given then they are compared.

[mehul@fedora bin]$ ./podman image diff alpine:latest busybox:latest
C /etc
C /etc/group
A /etc/localtime
C /etc/network
.
.

JSON/json output is now indented.

Thanks!

Signed-off-by: Mehul Arora [email protected]

openshift-ci · 2021-06-13T11:40:26Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: infiniteregrets
To complete the pull request process, please assign jwhonce after the PR has been reviewed.
You can assign the PR to them by writing /assign @jwhonce in a comment when ready.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

infiniteregrets · 2021-06-13T12:03:47Z

I am unable to get make lint and make validate to work:

[mehul@fedora podman]$ make lint
VERSION=1.36.0 GOBIN=/home/mehul/go/bin sh ./hack/install_golangci.sh
golangci-lint has version 1.36.0 built from 6c25d06 on 2021-01-26T13:14:05Z
Using existing ./bin/golangci-lint has version 1.36.0 built from 6c25d06 on 2021-01-26T13:14:05Z
hack/golangci-lint.sh run

Running golangci-lint for tunnel
Build Tags tunnel: apparmor,seccomp,selinux,linter,remote,remoteclient
Skipped directories tunnel: pkg/api
WARN [runner] Can't run linter goanalysis_metalinter: bodyclose: failed prerequisites: [[email protected]/containers/podman/v3/pkg/api/handlers: analysis skipped: errors in package: [/home/mehul/Documents/podman/pkg/api/handlers/types.go:7:2: could not import github.com/containers/common/libimage (/home/mehul/go/pkg/mod/github.com/containers/[email protected]/libimage/copier.go:16:19: could not import github.com/containers/image/v5/storage (/home/mehul/go/pkg/mod/github.com/containers/image/[email protected]/storage/storage_image.go:25:2: could not import github.com/containers/storage (/home/mehul/go/pkg/mod/github.com/containers/[email protected]/store.go:16:4: could not import github.com/containers/storage/drivers/register (/home/mehul/go/pkg/mod/github.com/containers/[email protected]/drivers/register/register_btrfs.go:7:4: could not import github.com/containers/storage/drivers/btrfs (/home/mehul/go/pkg/mod/github.com/containers/[email protected]/drivers/btrfs/btrfs.go:15:8: could not import C (cgo preprocessing failed))))))]] 
WARN [runner] Can't run linter unused: buildir: failed to load package btrfs: could not load export data: no export data for "github.com/containers/storage/drivers/btrfs" 
ERRO Running error: buildir: failed to load package btrfs: could not load export data: no export data for "github.com/containers/storage/drivers/btrfs" 
make: *** [Makefile:227: golangci-lint] Error 3

mehul@fedora podman]$ make validate
find . -name '*.go' -type f \
        -not \( \
                -name '.golangci.yml' -o \
                -name 'Makefile' -o \
                -path './vendor/*' -prune -o \
                -path './contrib/*' -prune \
        \) -exec gofmt -d -e -s -w {} \+
diff -u ./cmd/podman/common/diffChanges.go.orig ./cmd/podman/common/diffChanges.go
--- ./cmd/podman/common/diffChanges.go.orig     2021-06-13 17:32:20.634126467 +0530
+++ ./cmd/podman/common/diffChanges.go  2021-06-13 17:32:20.634126467 +0530
@@ -31,7 +31,7 @@
        }
 
        // Pull in configured json library
-
+
        output, err := json.MarshalIndent(body, "", "  ")
        if err != nil {
                return err
git diff --exit-code
diff --git a/cmd/podman/common/diffChanges.go b/cmd/podman/common/diffChanges.go
index e21bcf241..e37214ac1 100644
--- a/cmd/podman/common/diffChanges.go
+++ b/cmd/podman/common/diffChanges.go
@@ -31,7 +31,7 @@ func ChangesToJSON(diffs *entities.DiffReport) error {
        }
 
        // Pull in configured json library
-
+
        output, err := json.MarshalIndent(body, "", "  ")
        if err != nil {
                return err
make: *** [Makefile:232: gofmt] Error 1

Any ideas on where I could be going wrong?

Luap99 · 2021-06-13T15:38:54Z

I am not sure why make lint is failing but validate is trying to tell you that you have a formatting error. It looks like you have an extra tab character in this line.

The JSON change must be made in https://github.com/containers/common. You cannot edit files under the vendor directory directly. Also please squash your commits into one.

infiniteregrets · 2021-06-13T17:13:30Z

Okay so looks like I might have messed up a bit. When I was rebasing to squash commits, I probably added a commit made by rhatdan previously and now it is here and it is co-authored? I am not sure how to fix it...

Luap99 · 2021-06-13T18:24:22Z

Use git rebase -i HEAD~2 and delete the commit, afterwards use git rebase master.

infiniteregrets · 2021-06-13T19:08:03Z

Thanks! But I think I made another mistake. When I was rebasing I wrote drop on that commit instead of removing it. I am not sure if that is what you meant

Luap99 · 2021-06-13T20:33:15Z

Yeah I meant drop so its correct. You need to add a test to test/e2e/diff_test.go, just use diff with two images and check if it works.

infiniteregrets · 2021-06-13T22:39:32Z

I added some tests, but some CI tests are failing idk why

Nvm! i figured out where I went wrong, I will fix it soon

Edit:
So I thought the tests were failing because I changed the function signature of DoRequest? But I did not. So not sure what is wrong

rhatdan · 2021-06-14T09:58:38Z

pkg/domain/infra/tunnel/images.go

 	options := new(images.DiffOptions)
-	changes, err := images.Diff(ir.ClientCtx, nameOrID, options)
+	changes, err := images.Diff(ir.ClientCtx, options, nameOrID, otherNameOrID)


Why not pass down the diff options from the caller?

rhatdan · 2021-06-14T10:01:15Z

Thanks @infiniteregrets
A good test for this would be to do a build based on an image and add a couple of files.

Then do a diff between the newly created image and the from image, and make sure that the new files are in the diff.

Luap99

The remote client does not work. I left some comments what you have to do on the client side. On the server side you have to parse the new option. You have to do this here https://github.com/containers/podman/blob/master/pkg/api/handlers/compat/changes.go. Take a look at https://github.com/containers/podman/blob/master/pkg/api/handlers/compat/containers_logs.go for an example how to parse options on the server.

Luap99 · 2021-06-14T16:32:36Z

pkg/bindings/images/diff.go

@@ -9,7 +9,7 @@ import (
 )

 // Diff provides the changes between two container layers
-func Diff(ctx context.Context, nameOrID string, options *DiffOptions) ([]archive.Change, error) {
+func Diff(ctx context.Context, options *DiffOptions, nameOrID string, otherNameOrID string) ([]archive.Change, error) {


We cannot change the function parameters here. This package is meant for external use and we cannot do breaking changes. You have to add the second image name to the options struct here

podman/pkg/bindings/images/types.go

Line 18 in e3dd12a

type DiffOptions struct {

After you added this, you need to run make generate-bindings.

No, you just cannot break the http api bindings. Just add the second image to the options for the bindings and api. For the cli it is fine to use two arguments.

Okay so is having a flag that would override the comparison between the parent layer and the given image a good idea?
For example: podman image diff --format json --compare-with alpine:3 alpine:latest

On it, was going to make a separate comment down regarding the same so I deleted it from here

Luap99 · 2021-06-14T16:49:51Z

pkg/bindings/images/diff.go

@@ -19,7 +19,7 @@ func Diff(ctx context.Context, nameOrID string, options *DiffOptions) ([]archive
 		return nil, err
 	}

-	response, err := conn.DoRequest(nil, http.MethodGet, "/images/%s/changes", nil, nil, nameOrID)
+	response, err := conn.DoRequest(nil, http.MethodGet, "/images/%s/changes", nil, nil, nameOrID, otherNameOrID)


This does not work, when you add the the second image to the options struct as described above. You have to add

params, err := options.ToParams() if err != nil { return nil, err } response, err := conn.DoRequest(nil, http.MethodGet, "/images/%s/changes", params, nil, nameOrID)

infiniteregrets · 2021-06-14T17:54:23Z

Thanks for the review @Luap99. I will try again and also add the tests as mentioned by rhatdan

infiniteregrets · 2021-06-15T17:18:01Z

The remote client does not work. I left some comments what you have to do on the client side. On the server side you have to parse the new option. You have to do this here https://github.com/containers/podman/blob/master/pkg/api/handlers/compat/changes.go. Take a look at https://github.com/containers/podman/blob/master/pkg/api/handlers/compat/containers_logs.go for an example how to parse options on the server.

@Luap99 I tried to work on this, but I don't think I completely understand how everything works. I really want to do all of it myself but before that I think it would be really helpful if you could give me a brief overview of how everything is laid out

When you say the remote client, are you referring to the client used for mac os? And what do you mean by client side and server side. I do understand that there are functions to interact with Podman's REST API but I am not quite sure what make generate-bindings is doing here? And the changes you suggested for: pkg/bindings/images/diff.go, I am not sure how the otherimg field in the DiffOptions struct would be used by the DoRequest method to get the diff of two images? I also don't quite understand how queryParams are used there

Sorry if this is a bit naive, but I hope I can understand everything and make better future contributions.
I will squash my commits once everything is done. Thank you!

Luap99 · 2021-06-16T08:45:56Z

So you can use podman in two ways, via cli and rest api. The podman remote client called podman-remote talks to the podman api endpoint. Unlike podman, podman-remote can be used on macOS, windows and linux since it just needs to talk to a podman service on linux.
The cmd/podman package is used for both local and remote podman. It is the entry point for the cli. This package will call into pkg/domain/infra/abi for local podman and into pkg/domain/infra/tunnel for podman remote. The pkg/bindings package is used to talk to the podman service via http. This package is used by pkg/domain/infra/tunnel and also meant to be to consumed by external users who want to easily control containers/images from their golang applications with a stable API, if you change function parameters inside this package their code will no longer compile.

The make generate-bindings is just to automatically generate some boilerplate code.

On the server side pkg/api/server package is used as entry point for the http. This calls functions in pkg/api/handlers/libpod and pkg/api/handlers/compat.

You already have the local podman working but you have to add this to the http api so that podman-remote can also use it.

To test this run bin/podman system service -t0 in one terminal window and bin/podman-remote image diff ... in the other one.

rhatdan · 2021-06-16T10:02:52Z

@Luap99 Nice write up on the design. We should add this comment to the contributors page or somewhere to help explain the design of the podman command.

infiniteregrets · 2021-06-16T19:27:23Z

Thanks @Luap99! This was really helpful.
Starting from the first comment you made, you said I need to parse the new option on the server... so I am assuming you are referring to this line https://github.com/containers/podman/blob/master/pkg/api/handlers/compat/containers_logs.go#L83 from the example you linked. But in order to do that, I will have to change the function signature of

podman/libpod/diff.go

Line 24 in b3f61ec

func (r *Runtime) GetDiff(from, to string) ([]archive.Change, error) {

So is it okay to do so?

rhatdan · 2021-06-16T20:43:44Z

Fixes: #10649

Signed-off-by: Mehul Arora <[email protected]>

infiniteregrets · 2021-06-20T07:11:29Z

Hi @Luap99, will you be able to take this over from here by any chance? I am still not completely sure about a few things.
I am still unable to get make lint to work, I have tried googling this but couldn't reach anywhere:

Running golangci-lint for tunnel
Build Tags tunnel: apparmor,seccomp,selinux,linter,remote,remoteclient
Skipped directories tunnel: pkg/api
WARN [runner] Can't run linter goanalysis_metalinter: bodyclose: failed prerequisites: [[email protected]/containers/podman/v3/pkg/api/handlers: analysis skipped: errors in package: [/home/mehul/Documents/podman/pkg/api/handlers/types.go:7:2: could not import github.com/containers/common/libimage (/home/mehul/go/pkg/mod/github.com/containers/[email protected]/libimage/copier.go:16:19: could not import github.com/containers/image/v5/storage (/home/mehul/go/pkg/mod/github.com/containers/image/[email protected]/storage/storage_image.go:25:2: could not import github.com/containers/storage (/home/mehul/go/pkg/mod/github.com/containers/[email protected]/store.go:16:4: could not import github.com/containers/storage/drivers/register (/home/mehul/go/pkg/mod/github.com/containers/[email protected]/drivers/register/register_devicemapper.go:7:4: could not import github.com/containers/storage/drivers/devmapper (/home/mehul/go/pkg/mod/github.com/containers/[email protected]/drivers/devmapper/deviceset.go:21:2: could not import github.com/containers/storage/pkg/devicemapper (/home/mehul/go/pkg/mod/github.com/containers/[email protected]/pkg/devicemapper/devmapper_log.go:5:8: could not import C (cgo preprocessing failed)))))))]] 
WARN [runner] Can't run linter unused: buildir: failed to load package devicemapper: could not load export data: no export data for "github.com/containers/storage/pkg/devicemapper" 
ERRO Running error: buildir: failed to load package devicemapper: could not load export data: no export data for "github.com/containers/storage/pkg/devicemapper" 
make: *** [Makefile:227: golangci-lint] Error 3

I really appreciate the help! Thanks!

Luap99 · 2021-06-21T12:31:29Z

Hi @Luap99, will you be able to take this over from here by any chance?

Sure, I'll take it.

infiniteregrets · 2021-06-21T13:50:52Z

Thanks! I've set this PR to be editable by maintainers if that is what's required

infiniteregrets · 2021-06-26T18:34:53Z

@Luap99 did you get time to work on this?

Luap99 · 2021-07-01T07:56:33Z

@infiniteregrets I am working on this now

infiniteregrets · 2021-07-01T09:55:34Z

Thanks!

infiniteregrets changed the title ~~Image diff~~ podman Image diff between two images Jun 13, 2021

rhatdan reviewed Jun 14, 2021

View reviewed changes

Luap99 reviewed Jun 14, 2021

View reviewed changes

implemented diff between two images

f309826

Signed-off-by: Mehul Arora <[email protected]>

rhatdan closed this Jun 30, 2021

rhatdan deleted the branch containers:master June 30, 2021 15:09

github-actions bot added the locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. label Sep 22, 2023

github-actions bot locked as resolved and limited conversation to collaborators Sep 22, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

podman Image diff between two images #10667

podman Image diff between two images #10667

infiniteregrets commented Jun 13, 2021 •

edited

Loading

openshift-ci bot commented Jun 13, 2021

infiniteregrets commented Jun 13, 2021 •

edited

Loading

Luap99 commented Jun 13, 2021

infiniteregrets commented Jun 13, 2021

Luap99 commented Jun 13, 2021

infiniteregrets commented Jun 13, 2021

Luap99 commented Jun 13, 2021

infiniteregrets commented Jun 13, 2021 •

edited

Loading

rhatdan Jun 14, 2021

rhatdan commented Jun 14, 2021

Luap99 left a comment

Luap99 Jun 14, 2021

Luap99 Jun 15, 2021

infiniteregrets Jun 15, 2021

Luap99 Jun 14, 2021

infiniteregrets commented Jun 14, 2021

infiniteregrets commented Jun 15, 2021

Luap99 commented Jun 16, 2021

rhatdan commented Jun 16, 2021

infiniteregrets commented Jun 16, 2021

rhatdan commented Jun 16, 2021

infiniteregrets commented Jun 20, 2021

Luap99 commented Jun 21, 2021

infiniteregrets commented Jun 21, 2021

infiniteregrets commented Jun 26, 2021

Luap99 commented Jul 1, 2021

infiniteregrets commented Jul 1, 2021

podman Image diff between two images #10667

podman Image diff between two images #10667

Conversation

infiniteregrets commented Jun 13, 2021 • edited Loading

openshift-ci bot commented Jun 13, 2021

infiniteregrets commented Jun 13, 2021 • edited Loading

Luap99 commented Jun 13, 2021

infiniteregrets commented Jun 13, 2021

Luap99 commented Jun 13, 2021

infiniteregrets commented Jun 13, 2021

Luap99 commented Jun 13, 2021

infiniteregrets commented Jun 13, 2021 • edited Loading

rhatdan Jun 14, 2021

Choose a reason for hiding this comment

rhatdan commented Jun 14, 2021

Luap99 left a comment

Choose a reason for hiding this comment

Luap99 Jun 14, 2021

Choose a reason for hiding this comment

Luap99 Jun 15, 2021

Choose a reason for hiding this comment

infiniteregrets Jun 15, 2021

Choose a reason for hiding this comment

Luap99 Jun 14, 2021

Choose a reason for hiding this comment

infiniteregrets commented Jun 14, 2021

infiniteregrets commented Jun 15, 2021

Luap99 commented Jun 16, 2021

rhatdan commented Jun 16, 2021

infiniteregrets commented Jun 16, 2021

rhatdan commented Jun 16, 2021

infiniteregrets commented Jun 20, 2021

Luap99 commented Jun 21, 2021

infiniteregrets commented Jun 21, 2021

infiniteregrets commented Jun 26, 2021

Luap99 commented Jul 1, 2021

infiniteregrets commented Jul 1, 2021

infiniteregrets commented Jun 13, 2021 •

edited

Loading

infiniteregrets commented Jun 13, 2021 •

edited

Loading

infiniteregrets commented Jun 13, 2021 •

edited

Loading