Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Using pkg/bindings Go API should not require native dependencies to be installed #12548

Closed
strideynet opened this issue Dec 8, 2021 · 24 comments
Assignees
Labels
5.0 kind/feature Categorizes issue or PR as related to a new feature. locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments.

Comments

@strideynet
Copy link
Contributor

Is this a BUG REPORT or FEATURE REQUEST? (leave only one on its own line)

/kind feature

Description

As it stands, building an application that uses Podman via the HTTP API client (pkg/bindings) requires native dependencies (e.g libbtrfs, libgpgme etc) to be available on the machine. This provides a poor developer experience, especially to those on operating systems other than Linux where they may struggle to source the dependencies, and makes building applications that use this API much more difficult (these dependencies must also be installed as part of CI pipelines etc). As an engineer, I am unable to understand why I need these dependencies to leverage this API client, as they don't seem reasonable dependencies for an API client that makes HTTP requests (e.g libbtrfs).

We should explore options that would mean it's possible to use this API without the installation of those native dependencies. As far as I can tell, this is caused by the bindings packages relying on types defined in other packages where those dependencies are required as part of the runtime. If these types were extracted, or copied, and placed in separate packages, it should be possible to build applications using the Podman API Client without installing those native dependencies.

Many thanks,

@openshift-ci openshift-ci bot added the kind/feature Categorizes issue or PR as related to a new feature. label Dec 8, 2021
@rhatdan
Copy link
Member

rhatdan commented Dec 8, 2021

We build the remote client with these golang build tags, which should remove most of the heinous stuff you are seeing.

REMOTETAGS ?= remote exclude_graphdriver_btrfs btrfs_noversion exclude_graphdriver_devicemapper containers_image_openpgp

Sure we would love to get rid of these bindings leakage, but it is fairly difficult to do, and we have not recently had the engineering time to look into it.

@strideynet
Copy link
Contributor Author

Thanks @rhatdan for providing a work-around, that's greatly appreciated. I'll give that a go in tomorrow's working hours and see if that resolves the issue <3

Does it seem reasonable to you to keep this ticket open long term to track this until the root issue is resolved ?

@strideynet
Copy link
Contributor Author

I can confirm that applying these build flags resolves the need to install the native dependencies. It feels like it could be potentially helpful to others to include these instructions in the documentation regarding the API client.

@rhatdan
Copy link
Member

rhatdan commented Dec 9, 2021

Interested in opening a PR to fix the documentation?

@rhatdan
Copy link
Member

rhatdan commented Dec 9, 2021

BTW Which bindings are you using. If you game me a simple go program that used the bindings I could take a look at where it is pulling in containers/storage and try to fix it.

@strideynet
Copy link
Contributor Author

strideynet commented Dec 9, 2021

BTW Which bindings are you using. If you game me a simple go program that used the bindings I could take a look at where it is pulling in containers/storage and try to fix it.

If you are referring to the comment I made, and then deleted. I resolved that issue, that was just the ol race detector/macos monterey bug (golang/go#49138). If you are referring generally, we are using most of the bindings available in pkg/bindings, however, those flags resolve any issues so I'm happy with that.

Interested in opening a PR to fix the documentation?

I'd be happy to do so. I'm technically on leave for Christmas now but I'll have some spare time to whip something up this weekend.

@github-actions
Copy link

github-actions bot commented Jan 9, 2022

A friendly reminder that this issue had no activity for 30 days.

@rhatdan
Copy link
Member

rhatdan commented Jan 10, 2022

@strideynet are you going to open a PR?

BTW Do you have a simple reproducer of the error you were hitting?

@github-actions
Copy link

A friendly reminder that this issue had no activity for 30 days.

@rhatdan
Copy link
Member

rhatdan commented Feb 10, 2022

@baude after 4.0 we should take another look at this.

@github-actions
Copy link

A friendly reminder that this issue had no activity for 30 days.

@github-actions
Copy link

A friendly reminder that this issue had no activity for 30 days.

@rhatdan
Copy link
Member

rhatdan commented Apr 19, 2022

Which bindings were you using that pulled in this requirement?

@rhatdan
Copy link
Member

rhatdan commented Apr 19, 2022

I removed libbtrfs-devel package and can not get this to fail the build.

@github-actions
Copy link

A friendly reminder that this issue had no activity for 30 days.

rhatdan added a commit to rhatdan/podman that referenced this issue May 23, 2022
Working to fix containers#12548

Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>
@byarbrough
Copy link

byarbrough commented Jul 13, 2022

Hi @rhatdan I ran in to this on both Fedora and Ubuntu with Podman API v3.2.1 and v4.1.1.
Here are two examples:

  1. Simply running the https://github.com/containers/Demos/tree/main/podman_go_bindings demo requires downloading quite a few dependencies, as linked in the README

  2. Even just calling the system info binding as show below results in 95 indirect dependencies in go.mod for "just HTTP calls."

package main

import (
	"context"
	"fmt"
	"os"

	"github.com/containers/podman/v4/pkg/bindings"
	"github.com/containers/podman/v4/pkg/bindings/system"
)

func main() {
	fmt.Println("Welcome to the Podman Go bindings tutorial")

	// Get Podman socket location
	sock_dir := os.Getenv("XDG_RUNTIME_DIR")
	if sock_dir == "" {
		sock_dir = "/tmp"
	}
	socket := "unix:" + sock_dir + "/podman/podman.sock"

	// Connect to Podman socket
	ctx, err := bindings.NewConnection(context.Background(), socket)
	if err != nil {
		fmt.Println(err)
		os.Exit(1)
	}

	fmt.Println(ctx)

	// Calling this function dramatically increases the dependencies and build complexity
	info, err := system.Info(ctx, &system.InfoOptions{})
	if err != nil {
		fmt.Println(err)
		os.Exit(1)
	}

	fmt.Print(info)
}

As @strideynet mentioned above, it looks like this has to do with crossing boundaries to access structures. system.Info() makes use of the Info struct in libpod/define. Ideally, pkg bindings could stand on its own.
The podman-py project seems to do this well, likely because it is impossible to pull in things from the Go Podman core.

All that being said, I did get it working in a container with the build flags you recommended!

go build -tags exclude_graphdriver_btrfs,btrfs_noversion,exclude_graphdriver_devicemapper,containers_image_openpgp main.go

@vrothberg
Copy link
Member

Even just calling the system info binding as show below results in 95 indirect dependencies in go.mod for "just HTTP calls."

Thanks for bringing that up, @byarbrough.

It is definitely our goal to trim the (transitive) dependencies of pkg/bindings down as much as possible.

@baude @Luap99 FYI

@Luap99
Copy link
Member

Luap99 commented Jul 15, 2022

Using libpod/define is fine and expected, maintaining the same types twice is will cause more work and problems.
The problem here is that unlike libpod/define which only includes types it also uses types from other packages that also contain actual code and even c code, therefore you need the build tags to disable that.

The correct way to fix this is to move all required types into separate packages without extra code. The main works seems to be in c/storage and c/image c/buildah and not podman itself. However those libraries are stable so they must keep backwards compat. Keep that in mind when working on it.

@StarpTech
Copy link

StarpTech commented Aug 20, 2022

My few cents. Today, I tried to use the Go binding with Podman. My docker image was bloated to 1GB due to Podman Go dependencies. I couldn't build my app in a separate Stage because podman bindings are linked statically to a system dependency. For now, I have to work with the podman binary directly.

@vrothberg
Copy link
Member

@StarpTech 1 GB sounds way too much. The vendor directory of Podman is around 50MB and compiled Podman is at around 40 MB.

Did you clean up all build artifacts from your container image?

@StarpTech
Copy link

Did you clean up all build artifacts from your container image?c

What do you mean specifically?

Basically, I did.

# Install some dependencies like btrfs ...
go mod download # took some time
# after that, my image was much bigger than before

I gave up after that but I can recheck it.

@github-actions
Copy link

A friendly reminder that this issue had no activity for 30 days.

@baude baude added the 5.0 label Oct 9, 2023
giuseppe added a commit to giuseppe/libpod that referenced this issue Jan 25, 2024
Closes: containers#12548

[NO NEW TESTS NEEDED]

Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
@giuseppe
Copy link
Member

I am not sure what is the threshold to consider this fixed, since we can continue trimming dependencies and reduce the binary size. I've opened a PR to move some definitions around so we drag less stuff in the bindings: #21364

I also suggest using the remote tag since Podman is accessed through a socket.

Using the same example as in the #12548 (comment):

Using podman main:

$ go build && stat -c %s binary
37171920

Using the tag remote with podman main:

$ go build -tags remote && stat -c %s binary
18945552

Using the tag remote with the code from the PR:

$ go build -tags remote && stat -c %s binary
12229416

giuseppe added a commit to giuseppe/libpod that referenced this issue Jan 25, 2024
Closes: containers#12548

[NO NEW TESTS NEEDED]

Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
giuseppe added a commit to giuseppe/libpod that referenced this issue Jan 25, 2024
Closes: containers#12548

[NO NEW TESTS NEEDED]

Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
giuseppe added a commit to giuseppe/libpod that referenced this issue Jan 25, 2024
Closes: containers#12548

[NO NEW TESTS NEEDED]

Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
@rhatdan
Copy link
Member

rhatdan commented Jan 26, 2024

Nice shrinkage.

cgwalters pushed a commit to containers/podman-bootc that referenced this issue Apr 22, 2024
Instead of spawning a new process to run the podman CLI,
this uses the podman bindings library to interact with podman via HTTP calls.

There are a lot of dependencies added due to how the podman library
is structured. See containers/podman#12548
@github-actions 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 Apr 26, 2024
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 26, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
5.0 kind/feature Categorizes issue or PR as related to a new feature. locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments.
Projects
None yet
Development

No branches or pull requests

9 participants