From 1138789f20f77cb5645ba19a59aaf9a1ed012cf1 Mon Sep 17 00:00:00 2001 From: Tonis Tiigi Date: Mon, 12 Jun 2023 17:39:09 -0700 Subject: [PATCH] avoid extra client for history API detection Signed-off-by: Tonis Tiigi --- build/build.go | 4 ++-- build/url.go | 2 +- builder/node.go | 2 +- driver/docker-container/driver.go | 7 ------- driver/docker/driver.go | 3 --- driver/driver.go | 4 ++-- driver/features.go | 2 -- driver/kubernetes/driver.go | 7 ------- driver/manager.go | 18 +++++++++++++----- driver/remote/driver.go | 7 ------- 10 files changed, 19 insertions(+), 37 deletions(-) diff --git a/build/build.go b/build/build.go index 44b3b55e..21b502a0 100644 --- a/build/build.go +++ b/build/build.go @@ -948,7 +948,7 @@ func BuildWithResultHandler(ctx context.Context, nodes []builder.Node, opt map[s } else { rr, err = c.Build(ctx, so, "buildx", buildFunc, ch) } - if node.Driver.Features(ctx)[driver.HistoryAPI] && desktop.BuildBackendEnabled() { + if desktop.BuildBackendEnabled() && node.Driver.HistoryAPISupported(ctx) { buildRef := fmt.Sprintf("%s/%s/%s", node.Builder, node.Name, so.Ref) if err != nil { return &desktop.ErrorWithBuildRef{ @@ -1262,7 +1262,7 @@ func createTempDockerfile(r io.Reader) (string, error) { return dir, err } -func LoadInputs(ctx context.Context, d driver.Driver, inp Inputs, pw progress.Writer, target *client.SolveOpt) (func(), error) { +func LoadInputs(ctx context.Context, d *driver.DriverHandle, inp Inputs, pw progress.Writer, target *client.SolveOpt) (func(), error) { if inp.ContextPath == "" { return nil, errors.New("please specify build context (e.g. \".\" for the current directory)") } diff --git a/build/url.go b/build/url.go index 6ea0c445..e1c72c68 100644 --- a/build/url.go +++ b/build/url.go @@ -13,7 +13,7 @@ import ( "github.com/pkg/errors" ) -func createTempDockerfileFromURL(ctx context.Context, d driver.Driver, url string, pw progress.Writer) (string, error) { +func createTempDockerfileFromURL(ctx context.Context, d *driver.DriverHandle, url string, pw progress.Writer) (string, error) { c, err := driver.Boot(ctx, ctx, d, pw) if err != nil { return "", err diff --git a/builder/node.go b/builder/node.go index f5f7939a..da5e1d85 100644 --- a/builder/node.go +++ b/builder/node.go @@ -22,7 +22,7 @@ import ( type Node struct { store.Node Builder string - Driver driver.Driver + Driver *driver.DriverHandle DriverInfo *driver.Info Platforms []ocispecs.Platform GCPolicy []client.PruneInfo diff --git a/driver/docker-container/driver.go b/driver/docker-container/driver.go index f2f1ad54..e06380bf 100644 --- a/driver/docker-container/driver.go +++ b/driver/docker-container/driver.go @@ -388,18 +388,11 @@ func (d *Driver) Factory() driver.Factory { } func (d *Driver) Features(ctx context.Context) map[driver.Feature]bool { - var historyAPI bool - c, err := d.Client(ctx) - if err == nil { - historyAPI = driver.HistoryAPISupported(ctx, c) - c.Close() - } return map[driver.Feature]bool{ driver.OCIExporter: true, driver.DockerExporter: true, driver.CacheExport: true, driver.MultiPlatform: true, - driver.HistoryAPI: historyAPI, } } diff --git a/driver/docker/driver.go b/driver/docker/driver.go index 3b6f16e2..64c0dd25 100644 --- a/driver/docker/driver.go +++ b/driver/docker/driver.go @@ -60,7 +60,6 @@ func (d *Driver) Client(ctx context.Context) (*client.Client, error) { func (d *Driver) Features(ctx context.Context) map[driver.Feature]bool { var useContainerdSnapshotter bool - var historyAPI bool c, err := d.Client(ctx) if err == nil { workers, _ := c.ListWorkers(ctx) @@ -69,7 +68,6 @@ func (d *Driver) Features(ctx context.Context) map[driver.Feature]bool { useContainerdSnapshotter = true } } - historyAPI = driver.HistoryAPISupported(ctx, c) c.Close() } return map[driver.Feature]bool{ @@ -77,7 +75,6 @@ func (d *Driver) Features(ctx context.Context) map[driver.Feature]bool { driver.DockerExporter: useContainerdSnapshotter, driver.CacheExport: useContainerdSnapshotter, driver.MultiPlatform: useContainerdSnapshotter, - driver.HistoryAPI: historyAPI, } } diff --git a/driver/driver.go b/driver/driver.go index 3627226c..8642a543 100644 --- a/driver/driver.go +++ b/driver/driver.go @@ -64,7 +64,7 @@ type Driver interface { Config() InitConfig } -func Boot(ctx, clientContext context.Context, d Driver, pw progress.Writer) (*client.Client, error) { +func Boot(ctx, clientContext context.Context, d *DriverHandle, pw progress.Writer) (*client.Client, error) { try := 0 for { info, err := d.Info(ctx) @@ -92,7 +92,7 @@ func Boot(ctx, clientContext context.Context, d Driver, pw progress.Writer) (*cl } } -func HistoryAPISupported(ctx context.Context, c *client.Client) bool { +func historyAPISupported(ctx context.Context, c *client.Client) bool { cl, err := c.ControlClient().ListenBuildHistory(ctx, &controlapi.BuildHistoryRequest{ ActiveOnly: true, Ref: "buildx-test-history-api-feature", // dummy ref to check if the server supports the API diff --git a/driver/features.go b/driver/features.go index a1efa567..e48d7309 100644 --- a/driver/features.go +++ b/driver/features.go @@ -7,5 +7,3 @@ const DockerExporter Feature = "Docker exporter" const CacheExport Feature = "Cache export" const MultiPlatform Feature = "Multiple platforms" - -const HistoryAPI Feature = "History API" diff --git a/driver/kubernetes/driver.go b/driver/kubernetes/driver.go index be8ef697..55c7853d 100644 --- a/driver/kubernetes/driver.go +++ b/driver/kubernetes/driver.go @@ -229,17 +229,10 @@ func (d *Driver) Factory() driver.Factory { } func (d *Driver) Features(ctx context.Context) map[driver.Feature]bool { - var historyAPI bool - c, err := d.Client(ctx) - if err == nil { - historyAPI = driver.HistoryAPISupported(ctx, c) - c.Close() - } return map[driver.Feature]bool{ driver.OCIExporter: true, driver.DockerExporter: d.DockerAPI != nil, driver.CacheExport: true, driver.MultiPlatform: true, // Untested (needs multiple Driver instances) - driver.HistoryAPI: historyAPI, } } diff --git a/driver/manager.go b/driver/manager.go index 0db8254f..6d798974 100644 --- a/driver/manager.go +++ b/driver/manager.go @@ -104,7 +104,7 @@ func GetFactory(name string, instanceRequired bool) (Factory, error) { return nil, errors.Errorf("failed to find driver %q", name) } -func GetDriver(ctx context.Context, name string, f Factory, endpointAddr string, api dockerclient.APIClient, auth Auth, kcc KubeClientConfig, flags []string, files map[string][]byte, do map[string]string, platforms []specs.Platform, contextPathHash string) (Driver, error) { +func GetDriver(ctx context.Context, name string, f Factory, endpointAddr string, api dockerclient.APIClient, auth Auth, kcc KubeClientConfig, flags []string, files map[string][]byte, do map[string]string, platforms []specs.Platform, contextPathHash string) (*DriverHandle, error) { ic := InitConfig{ EndpointAddr: endpointAddr, DockerAPI: api, @@ -128,7 +128,7 @@ func GetDriver(ctx context.Context, name string, f Factory, endpointAddr string, if err != nil { return nil, err } - return &cachedDriver{Driver: d}, nil + return &DriverHandle{Driver: d}, nil } func GetFactories(instanceRequired bool) []Factory { @@ -145,7 +145,7 @@ func GetFactories(instanceRequired bool) []Factory { return ds } -type cachedDriver struct { +type DriverHandle struct { Driver client *client.Client err error @@ -154,16 +154,24 @@ type cachedDriver struct { features map[Feature]bool } -func (d *cachedDriver) Client(ctx context.Context) (*client.Client, error) { +func (d *DriverHandle) Client(ctx context.Context) (*client.Client, error) { d.once.Do(func() { d.client, d.err = d.Driver.Client(ctx) }) return d.client, d.err } -func (d *cachedDriver) Features(ctx context.Context) map[Feature]bool { +func (d *DriverHandle) Features(ctx context.Context) map[Feature]bool { d.featuresOnce.Do(func() { d.features = d.Driver.Features(ctx) }) return d.features } + +func (d *DriverHandle) HistoryAPISupported(ctx context.Context) bool { + client, err := d.Client(ctx) + if err != nil { + return false + } + return historyAPISupported(ctx, client) +} diff --git a/driver/remote/driver.go b/driver/remote/driver.go index e27897ef..fa5cfcc5 100644 --- a/driver/remote/driver.go +++ b/driver/remote/driver.go @@ -88,18 +88,11 @@ func (d *Driver) Client(ctx context.Context) (*client.Client, error) { } func (d *Driver) Features(ctx context.Context) map[driver.Feature]bool { - var historyAPI bool - c, err := d.Client(ctx) - if err == nil { - historyAPI = driver.HistoryAPISupported(ctx, c) - c.Close() - } return map[driver.Feature]bool{ driver.OCIExporter: true, driver.DockerExporter: true, driver.CacheExport: true, driver.MultiPlatform: true, - driver.HistoryAPI: historyAPI, } }