-
Notifications
You must be signed in to change notification settings - Fork 71
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
first draft of pack direct podman call rfc #288
base: main
Are you sure you want to change the base?
Changes from 15 commits
2a66f40
00c4cb6
c7ecea3
fa4142e
5d573ef
f2b0d73
e023641
ef69023
955cf39
fcd8787
b3214ee
d395684
add5b44
9cac6e1
724d08f
835b363
6d15cf7
cf6cb21
7fcbe58
129c740
c4dfc18
5ab6ea8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,108 @@ | ||
# Meta | ||
[meta]: #meta | ||
- Name: Pack direct podman call | ||
- Start Date: 2023-06-16 | ||
- Author(s): dvaumoron | ||
- Status: Draft | ||
- RFC Pull Request: (leave blank) | ||
- CNB Pull Request: (leave blank) | ||
- CNB Issue: (leave blank) | ||
- Supersedes: "N/A" | ||
|
||
# Summary | ||
[summary]: #summary | ||
|
||
A flag `--daemonless` will be added to the `pack` CLI, the flag presence will switch from the current functioning to one where podman is called directly (without needing a daemon backed socket) | ||
|
||
# Definitions | ||
[definitions]: #definitions | ||
|
||
Container Manager Client API: an API such as Docker's `CommonAPIClient` or Podman's `ContainerEngine` allowing interaction with container and OCI image | ||
|
||
# Motivation | ||
[motivation]: #motivation | ||
|
||
`pack` currently requires a container manager client API available via a socket. Both `docker` and `podman` are compatible with the [socket API](https://docs.docker.com/engine/api/v1.24/). This RFC proposes that `pack` is extended to support a "daemonless" mode. In a "daemonless" mode `pack` will directly use a container manager client API. This avoids running a `docker` or `podman` daemon and avoids opening up a local socket. This will allow to use pack as a full-userspace, standalone application on the OS supported by podman (currently Linux, as Windows and macOS need podman machine). | ||
dvaumoron marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
# What it is | ||
[what-it-is]: #what-it-is | ||
|
||
Adding `--daemonless` to the `pack` command which currently need docker call would allow them to work without docker installed. | ||
|
||
# How it Works | ||
[how-it-works]: #how-it-works | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we need to expand on this section? I'd like a
I expect that we're proposing here to use an OCI registry for the layer cache and for output images. But I think we need to make all this explict to RFC readers. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In order to store locally OCI images with the new mode, change in the lifecycle tool will be needed too There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I have added a short code sample to show how an adapter could be done to answer 2 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I have digged deeper into the caching part of pack and lifecycle, it don't seem needed to modify those part (volume caching store in local host by mounting it in the container using the interface DockerClient, image caching use registries (licefycle use github.com/google/go-containerregistry via imgutil)) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Could you talk about what these changes would look like? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
As stated in the following comment, after a deeper check, that part doesn't need change, it relies on the DockerClient interface or an independent library no tied to the Docker socket There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
1 & 2 have been detailed in the RFC document There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Here's where we initialize the docker client in the lifecycle: https://github.com/buildpacks/lifecycle/blob/367b451eb708dff1aa6e5ba179e1c88c4b9917e2/priv/docker.go#L16 I'm assuming There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. After some more check, depending on which lifecycle command is launched the fallback when the flag use daemon is false, will be the layout version or the remote version, so the options passed to lifecycle should be adapted to ensure correctness. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I added precisions in the RFC corresponding to my previous comment |
||
The flag will change the initialization of the CommonAPIClient passed to call docker (interface defined in "github.com/docker/docker/client") by an adapter conforming to the used subset [DockerClient](https://github.com/buildpacks/pack/blob/main/pkg/client/docker.go#L14) to call podman as a library (forwarding calls to an initialized instance of [ContainerEngine](https://github.com/containers/podman/blob/main/pkg/domain/entities/engine_container.go#L16)). | ||
|
||
The adapter will look like : | ||
```Go | ||
type podmanAdapter struct { | ||
inner entities.ContainerEngine | ||
} | ||
|
||
func MakePodmanAdapter() DockerClient { | ||
// initialization of engine | ||
return podmanAdapter{inner: engine} | ||
} | ||
|
||
func (a podmanAdapter) ContainerCreate(ctx context.Context, config *containertypes.Config, hostConfig *containertypes.HostConfig, networkingConfig *networktypes.NetworkingConfig, platform *specs.Platform, containerName string) (containertypes.CreateResponse, error) { | ||
// initialization of specGenerator from the different configs | ||
report, err := a.inner.ContainerCreate(ctx, specGenerator) | ||
if err != nil { | ||
return containertypes.CreateResponse{}, err | ||
} | ||
// initialization of adaptedReport from the report | ||
return adaptedReport, nil | ||
} | ||
``` | ||
|
||
# Migration | ||
[migration]: #migration | ||
|
||
The current mode of `pack` operation is the default mode of operation. The proposed `--daemonless` mode of operation is an optional mode of execution. We will need to document how to switch between operation modes and the motivations for choosing an appropriate mode of operation. | ||
|
||
# Drawbacks | ||
[drawbacks]: #drawbacks | ||
|
||
pack contributor could need to update the podman dependencies in go.mod for bugfixes | ||
dvaumoron marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
# Alternatives | ||
[alternatives]: #alternatives | ||
|
||
pack user could have dificulties to set up docker or podman to work with the pack CLI | ||
|
||
# Prior Art | ||
[prior-art]: #prior-art | ||
|
||
N/A | ||
|
||
# Unresolved Questions | ||
[unresolved-questions]: #unresolved-questions | ||
|
||
N/A | ||
|
||
# Spec. Changes (OPTIONAL) | ||
[spec-changes]: #spec-changes | ||
|
||
N/A | ||
|
||
# History | ||
[history]: #history | ||
|
||
<!-- | ||
## Amended | ||
### Meta | ||
[meta-1]: #meta-1 | ||
- Name: (fill in the amendment name: Variable Rename) | ||
- Start Date: (fill in today's date: YYYY-MM-DD) | ||
- Author(s): (Github usernames) | ||
- Amendment Pull Request: (leave blank) | ||
|
||
### Summary | ||
|
||
A brief description of the changes. | ||
|
||
### Motivation | ||
|
||
Why was this amendment necessary? | ||
---> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pack
uses the docker daemon both to spin up containers in which the lifecycle executes, as well as an export target in which to store application images. Does this RFC support both use cases? I assume so, but thought it best to clarify.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It depend of what kind of export we are talking about, if you mean export to a registry, pack use the github.com/google/go-containerregistry library to do it, and do not depend on Docker for that.
If you talk about local storage, pack call the Docker socket via the DockerClient interface, changing this implementation will change the place where image will be stored, however i don't think this is a problem.
Users using the new flag to avoid installing Docker will have podman if they need to run locally containers (without having to use the podman compatibility socket), or no tools at all because pack will be usable as a standalone.
User with several tool including Docker, would be able to use them to "send" images to Docker if they prefer to use the flag (podman or buildah (which use the same image storage) allow to do that).
User with only Docker have no reason to use the flag, if they use it and want to run the generated images locally without installing more tools, they will need to export them to a registry and retrieve them from it. Maybe adding a warning when the flag is used will avoid this kind of mistake.