buildpacks / rfcs

RFCs for Cloud Native Buildpacks
Apache License 2.0
56 stars 71 forks source link

first draft of pack direct podman call rfc #288

Open dvaumoron opened 1 year ago

dvaumoron commented 1 year ago

This PR follows my message in https://github.com/buildpacks/pack/issues/413

buildpack-bot commented 1 year ago

Maintainers,

As you review this RFC please queue up issues to be created using the following commands:

/queue-issue <repo> "<title>" [labels]...
/unqueue-issue <uid>

Issues

(none)

dvaumoron commented 1 year ago

what do you think of an option in the configuration file in addition to the flag ?

AidanDelaney commented 1 year ago

For reference, a previous discussion on replacing the daemon is at https://github.com/buildpacks/rfcs/pull/201

jjbustamante commented 10 months ago

Hi @dvaumoron.

As my understanding the proposal is to offer a new implementation of the our DockerClient interface through the Adapter. The following diagram shows the components that depends on this docker client implementation, maybe it could help us on thinking potential edges cases we need to take care of.

classDiagram

    DockerClient <|-- docker_client
    DockerClient <|-- podmanAdapter
    ImageFactory <|-- pack_imageFactory
    ImageFetcher <|-- image_Fetcher
    LifecycleExecutor <|-- build_LifecycleExecutor
    BuildpackDownloader <|-- buildpack_buildpackDownloader

    class ImageFactory {
        <<interface>>
    }

    FetchOptions <.. ImageFetcher

    class ImageFetcher {
        <<interface>>
    }

    class BlobDownloader {
        <<interface>>
    }

    class LifecycleExecutor {
        <<interface>>
    }

    build_LifecycleExecutor o-- DockerClient

    class build_LifecycleExecutor {

    }

    class BuildpackDownloader {
        <<interface>>
    }

    class buildpack_buildpackDownloader {

    }

    class pack_Client {

    }

    class FetchOptions {
        +Bool Daemon
        +String Platform
        +PullPolicy PullPolicy
        +LayoutOption LayoutOption
    }

    pack_Client o-- ImageFactory
    pack_Client o-- ImageFetcher
    pack_Client o-- DockerClient
    pack_Client o-- BlobDownloader
    pack_Client o-- LifecycleExecutor
    pack_Client o-- BuildpackDownloader
    buildpack_buildpackDownloader o-- ImageFetcher

    class DockerClient {
        <<interface>>
        ImageHistory()
        ImageInspectWithRaw()
        ImageTag() 
        ImageLoad()
        ImageSave()
        ImageRemove()
        ImagePull()
        Info()
        VolumeRemove()
        ContainerCreate()
        CopyFromContainer()
        ContainerInspect()
        ContainerRemove()
        CopyToContainer()
        ContainerWait()
        ContainerAttach()
        ContainerStart()
    }

    pack_imageFactory o-- DockerClient
    image_Fetcher o-- DockerClient

    class pack_imageFactory {

    }

    class image_Fetcher {

    }

From the diagram, I noticed:

I hope this can be helpful, I just tried to put my thoughts on paper and share them 😄 . Also can you fix the DCO error ?

dvaumoron commented 10 months ago

Hi @jjbustamante,

The methods in DockerClient interface return struct, the goal is to populate them with the data returned by the ContainerEngine implementation, the Image implementation returned by the method NewImage from "github.com/buildpacks/imgutil/local" (called at https://github.com/buildpacks/pack/blob/main/pkg/client/client.go#L271) wich use the DockerClient interface doesn't need to change (see https://github.com/buildpacks/imgutil/blob/main/local/local.go#L280 or https://github.com/buildpacks/imgutil/blob/main/local/save.go#L76 among other examples).

As earlier the Fetcher struct which implements the ImageFetcher interface use the DockerClient interface (see https://github.com/buildpacks/pack/blob/main/pkg/image/fetcher.go#L56)

The LifecycleExecutor struct use the DockerClient interface too (see https://github.com/buildpacks/pack/blob/main/internal/build/lifecycle_executor.go#L49), the lifecycle part was unclear so i have checked (reported at https://github.com/buildpacks/rfcs/pull/288#discussion_r1247989303)

All my verifications convey the same conclusion, by (a good) design the DockerClient interface is the single point of contact with the Docker API, a correct replacement with an adapter which call the Podman API instead should be enough to make the feature work.

dvaumoron commented 10 months ago

I didn't get the point with the DCO check, it doesn't accept my signed-of-by, but looking at https://github.com/buildpacks/rfcs/commit/714a7f0637d75968d1bf3938aedb3e493f39a10b, it seems possible to use your true name with an email with doesn't match, and my name associated with my github account is correct, i need to dive deeper...

dvaumoron commented 10 months ago

I rebase and reword my commit in order to have the waited signed-off-by

abitrolly commented 7 months ago

A callgraph of functions that are calling socket API while building the sample project, would greatly help to estimate the critical path that needs to be patched.

dvaumoron commented 6 months ago

go-callvis doesn't seem to help for that, it does not keep track of object passed as argument.

The diagram from @jjbustamante is more helpful than those I obtain with go-calvis

dvaumoron commented 6 months ago

To be more accurate, you are limited to path calling some part of a package (pkg/client) from another (cmd/pack), the resulting graph is not very readable