-
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
Open
dvaumoron
wants to merge
22
commits into
buildpacks:main
Choose a base branch
from
dvaumoron:main
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+110
−0
Open
Changes from all commits
Commits
Show all changes
22 commits
Select commit
Hold shift + click to select a range
2a66f40
first draft of pack direct podman call rfc
dvaumoron 00c4cb6
pack direct podman call rfc, motivation part
dvaumoron c7ecea3
pack direct podman call rfc, how it works part
dvaumoron fa4142e
Update text/0000-pack-direct-podman-call.md
dvaumoron 5d573ef
pack direct podman call rfc, flag name change
dvaumoron f2b0d73
pack direct podman call rfc, how it works part
dvaumoron e023641
pack direct podman call rfc, how it works part
dvaumoron ef69023
pack direct podman call rfc, how it works part
dvaumoron 955cf39
pack direct podman call rfc, how it works part
dvaumoron fcd8787
Update text/0000-pack-direct-podman-call.md
dvaumoron b3214ee
Update text/0000-pack-direct-podman-call.md
dvaumoron d395684
pack direct podman call rfc, definitions part
dvaumoron add5b44
pack direct podman call rfc, motivation part (fix)
dvaumoron 9cac6e1
Merge branch 'main' of https://github.com/dvaumoron/CNB-rfcs
dvaumoron 724d08f
Merge branch 'buildpacks:main' into main
dvaumoron 835b363
Merge branch 'buildpacks:main' into main
dvaumoron 6d15cf7
first draft of pack direct podman call rfc (spelling)
dvaumoron cf6cb21
first draft of pack direct podman call rfc (fix)
dvaumoron 7fcbe58
first draft of pack direct podman call rfc, how it work part
dvaumoron 129c740
Update text/0000-pack-direct-podman-call.md
dvaumoron c4dfc18
first draft of pack direct podman call rfc, motivation part
dvaumoron 5ab6ea8
Merge branch 'buildpacks:main' into main
dvaumoron File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,110 @@ | ||
# 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 interacts with a container manager through an API service. Both `docker` and `podman` can run the daemon service, which provides a compatible [socket API](https://docs.docker.com/engine/api/v1.24/). This RFC proposes an alternative "daemonless" mode with direct calls to `podman` without opening up a local socket or running API service. This makes `pack` an userspace, standalone application (currently only on Linux, as Windows and macOS need `podman` machine). | ||
|
||
# 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 | ||
|
||
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)). | ||
|
||
In order to ensure correctness, options passed to the lifecycle will probably need to be adapted too (however, it does not seem necessary to change the lifecycle implementation because it already have alternatives to work without the docker socket). | ||
|
||
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 difficulties 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? | ||
---> |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
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 comment
The 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 comment
The 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 comment
The 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 comment
The 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 comment
The 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 comment
The reason will be displayed to describe this comment to others. Learn more.
1 & 2 have been detailed in the RFC document
3 layer are cached with a "VolumeCache" mecanism, which rely on the DockerClient interface
4 podman store local image in user space (/home/dvaumoron/.local/share/containers on my computer), and docker store them in common space using root right (/var/lib/containers/storage on my computer)
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.
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
shouldConnectSock
will be false in a daemonless scenario as pack will not mount the socket. But don't we need to specify other options to enable the podman wrapper (can you elaborate how this would be done - via some new input to the lifecycle or something else)? Or are you expecting to use the lifecycle in "OCI layout mode" and handle the processing in pack?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.
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 comment
The reason will be displayed to describe this comment to others. Learn more.
I added precisions in the RFC corresponding to my previous comment