Skip to content

Commit

Permalink
Merge pull request docker#2259 from namespacelabs/master
Browse files Browse the repository at this point in the history
Implement ability to load images by default in non-Docker build drivers.
  • Loading branch information
crazy-max authored Apr 5, 2024
2 parents 5c29e6e + ccc314a commit 4b3c3c8
Show file tree
Hide file tree
Showing 13 changed files with 118 additions and 31 deletions.
4 changes: 4 additions & 0 deletions build/build.go
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,10 @@ func BuildWithResultHandler(ctx context.Context, nodes []builder.Node, opt map[s
if noMobyDriver != nil && !noDefaultLoad() && noPrintFunc(opt) {
var noOutputTargets []string
for name, opt := range opt {
if noMobyDriver.Features(ctx)[driver.DefaultLoad] {
continue
}

if !opt.Linked && len(opt.Exports) == 0 {
noOutputTargets = append(noOutputTargets, name)
}
Expand Down
12 changes: 8 additions & 4 deletions build/opt.go
Original file line number Diff line number Diff line change
Expand Up @@ -162,10 +162,14 @@ func toSolveOpt(ctx context.Context, node builder.Node, multiDriver bool, opt Op
case 1:
// valid
case 0:
if nodeDriver.IsMobyDriver() && !noDefaultLoad() {
// backwards compat for docker driver only:
// this ensures the build results in a docker image.
opt.Exports = []client.ExportEntry{{Type: "image", Attrs: map[string]string{}}}
if !noDefaultLoad() {
if nodeDriver.IsMobyDriver() {
// backwards compat for docker driver only:
// this ensures the build results in a docker image.
opt.Exports = []client.ExportEntry{{Type: "image", Attrs: map[string]string{}}}
} else if nodeDriver.Features(ctx)[driver.DefaultLoad] {
opt.Exports = []client.ExportEntry{{Type: "docker", Attrs: map[string]string{}}}
}
}
default:
if err := bopts.LLBCaps.Supports(pb.CapMultipleExporters); err != nil {
Expand Down
2 changes: 2 additions & 0 deletions driver/docker-container/driver.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ type Driver struct {
cgroupParent string
restartPolicy container.RestartPolicy
env []string
defaultLoad bool
}

func (d *Driver) IsMobyDriver() bool {
Expand Down Expand Up @@ -423,6 +424,7 @@ func (d *Driver) Features(ctx context.Context) map[driver.Feature]bool {
driver.DockerExporter: true,
driver.CacheExport: true,
driver.MultiPlatform: true,
driver.DefaultLoad: d.defaultLoad,
}
}

Expand Down
5 changes: 5 additions & 0 deletions driver/docker-container/factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,11 @@ func (f *factory) New(ctx context.Context, cfg driver.InitConfig) (driver.Driver
if err != nil {
return nil, err
}
case k == "default-load":
d.defaultLoad, err = strconv.ParseBool(v)
if err != nil {
return nil, err
}
case strings.HasPrefix(k, "env."):
envName := strings.TrimPrefix(k, "env.")
if envName == "" {
Expand Down
1 change: 1 addition & 0 deletions driver/docker/driver.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@ func (d *Driver) Features(ctx context.Context) map[driver.Feature]bool {
driver.DockerExporter: useContainerdSnapshotter,
driver.CacheExport: useContainerdSnapshotter,
driver.MultiPlatform: useContainerdSnapshotter,
driver.DefaultLoad: true,
}
})
return d.features.list
Expand Down
2 changes: 2 additions & 0 deletions driver/features.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,3 +7,5 @@ const DockerExporter Feature = "Docker exporter"

const CacheExport Feature = "Cache export"
const MultiPlatform Feature = "Multi-platform build"

const DefaultLoad Feature = "Automatically load images to the Docker Engine image store"
2 changes: 2 additions & 0 deletions driver/kubernetes/driver.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ type Driver struct {
podClient clientcorev1.PodInterface
configMapClient clientcorev1.ConfigMapInterface
podChooser podchooser.PodChooser
defaultLoad bool
}

func (d *Driver) IsMobyDriver() bool {
Expand Down Expand Up @@ -229,6 +230,7 @@ func (d *Driver) Features(_ context.Context) map[driver.Feature]bool {
driver.DockerExporter: d.DockerAPI != nil,
driver.CacheExport: true,
driver.MultiPlatform: true, // Untested (needs multiple Driver instances)
driver.DefaultLoad: d.defaultLoad,
}
}

Expand Down
35 changes: 22 additions & 13 deletions driver/kubernetes/factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,11 +68,13 @@ func (f *factory) New(ctx context.Context, cfg driver.InitConfig) (driver.Driver
clientset: clientset,
}

deploymentOpt, loadbalance, namespace, err := f.processDriverOpts(deploymentName, namespace, cfg)
deploymentOpt, loadbalance, namespace, defaultLoad, err := f.processDriverOpts(deploymentName, namespace, cfg)
if nil != err {
return nil, err
}

d.defaultLoad = defaultLoad

d.deployment, d.configMaps, err = manifest.NewDeployment(deploymentOpt)
if err != nil {
return nil, err
Expand Down Expand Up @@ -100,7 +102,7 @@ func (f *factory) New(ctx context.Context, cfg driver.InitConfig) (driver.Driver
return d, nil
}

func (f *factory) processDriverOpts(deploymentName string, namespace string, cfg driver.InitConfig) (*manifest.DeploymentOpt, string, string, error) {
func (f *factory) processDriverOpts(deploymentName string, namespace string, cfg driver.InitConfig) (*manifest.DeploymentOpt, string, string, bool, error) {
deploymentOpt := &manifest.DeploymentOpt{
Name: deploymentName,
Image: bkimage.DefaultImage,
Expand All @@ -111,6 +113,8 @@ func (f *factory) processDriverOpts(deploymentName string, namespace string, cfg
ConfigFiles: cfg.Files,
}

defaultLoad := false

deploymentOpt.Qemu.Image = bkimage.QemuImage

loadbalance := LoadbalanceSticky
Expand All @@ -127,7 +131,7 @@ func (f *factory) processDriverOpts(deploymentName string, namespace string, cfg
case "replicas":
deploymentOpt.Replicas, err = strconv.Atoi(v)
if err != nil {
return nil, "", "", err
return nil, "", "", false, err
}
case "requests.cpu":
deploymentOpt.RequestsCPU = v
Expand All @@ -140,7 +144,7 @@ func (f *factory) processDriverOpts(deploymentName string, namespace string, cfg
case "rootless":
deploymentOpt.Rootless, err = strconv.ParseBool(v)
if err != nil {
return nil, "", "", err
return nil, "", "", false, err
}
if _, isImage := cfg.DriverOpts["image"]; !isImage {
deploymentOpt.Image = bkimage.DefaultRootlessImage
Expand All @@ -150,17 +154,17 @@ func (f *factory) processDriverOpts(deploymentName string, namespace string, cfg
case "nodeselector":
deploymentOpt.NodeSelector, err = splitMultiValues(v, ",", "=")
if err != nil {
return nil, "", "", errors.Wrap(err, "cannot parse node selector")
return nil, "", "", false, errors.Wrap(err, "cannot parse node selector")
}
case "annotations":
deploymentOpt.CustomAnnotations, err = splitMultiValues(v, ",", "=")
if err != nil {
return nil, "", "", errors.Wrap(err, "cannot parse annotations")
return nil, "", "", false, errors.Wrap(err, "cannot parse annotations")
}
case "labels":
deploymentOpt.CustomLabels, err = splitMultiValues(v, ",", "=")
if err != nil {
return nil, "", "", errors.Wrap(err, "cannot parse labels")
return nil, "", "", false, errors.Wrap(err, "cannot parse labels")
}
case "tolerations":
ts := strings.Split(v, ";")
Expand All @@ -185,12 +189,12 @@ func (f *factory) processDriverOpts(deploymentName string, namespace string, cfg
case "tolerationSeconds":
c, err := strconv.Atoi(kv[1])
if nil != err {
return nil, "", "", err
return nil, "", "", false, err
}
c64 := int64(c)
t.TolerationSeconds = &c64
default:
return nil, "", "", errors.Errorf("invalid tolaration %q", v)
return nil, "", "", false, errors.Errorf("invalid tolaration %q", v)
}
}
}
Expand All @@ -202,24 +206,29 @@ func (f *factory) processDriverOpts(deploymentName string, namespace string, cfg
case LoadbalanceSticky:
case LoadbalanceRandom:
default:
return nil, "", "", errors.Errorf("invalid loadbalance %q", v)
return nil, "", "", false, errors.Errorf("invalid loadbalance %q", v)
}
loadbalance = v
case "qemu.install":
deploymentOpt.Qemu.Install, err = strconv.ParseBool(v)
if err != nil {
return nil, "", "", err
return nil, "", "", false, err
}
case "qemu.image":
if v != "" {
deploymentOpt.Qemu.Image = v
}
case "default-load":
defaultLoad, err = strconv.ParseBool(v)
if err != nil {
return nil, "", "", false, err
}
default:
return nil, "", "", errors.Errorf("invalid driver option %s for driver %s", k, DriverName)
return nil, "", "", false, errors.Errorf("invalid driver option %s for driver %s", k, DriverName)
}
}

return deploymentOpt, loadbalance, namespace, nil
return deploymentOpt, loadbalance, namespace, defaultLoad, nil
}

func splitMultiValues(in string, itemsep string, kvsep string) (map[string]string, error) {
Expand Down
28 changes: 16 additions & 12 deletions driver/kubernetes/factory_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,9 @@ func TestFactory_processDriverOpts(t *testing.T) {
"loadbalance": "random",
"qemu.install": "true",
"qemu.image": "qemu:latest",
"default-load": "true",
}
r, loadbalance, ns, err := f.processDriverOpts(cfg.Name, "test", cfg)
r, loadbalance, ns, defaultLoad, err := f.processDriverOpts(cfg.Name, "test", cfg)

nodeSelectors := map[string]string{
"selector1": "value1",
Expand Down Expand Up @@ -102,14 +103,15 @@ func TestFactory_processDriverOpts(t *testing.T) {
require.Equal(t, LoadbalanceRandom, loadbalance)
require.True(t, r.Qemu.Install)
require.Equal(t, "qemu:latest", r.Qemu.Image)
require.True(t, defaultLoad)
},
)

t.Run(
"NoOptions", func(t *testing.T) {
cfg.DriverOpts = map[string]string{}

r, loadbalance, ns, err := f.processDriverOpts(cfg.Name, "test", cfg)
r, loadbalance, ns, defaultLoad, err := f.processDriverOpts(cfg.Name, "test", cfg)

require.NoError(t, err)

Expand All @@ -128,6 +130,7 @@ func TestFactory_processDriverOpts(t *testing.T) {
require.Equal(t, LoadbalanceSticky, loadbalance)
require.False(t, r.Qemu.Install)
require.Equal(t, bkimage.QemuImage, r.Qemu.Image)
require.False(t, defaultLoad)
},
)

Expand All @@ -138,7 +141,7 @@ func TestFactory_processDriverOpts(t *testing.T) {
"loadbalance": "sticky",
}

r, loadbalance, ns, err := f.processDriverOpts(cfg.Name, "test", cfg)
r, loadbalance, ns, defaultLoad, err := f.processDriverOpts(cfg.Name, "test", cfg)

require.NoError(t, err)

Expand All @@ -157,6 +160,7 @@ func TestFactory_processDriverOpts(t *testing.T) {
require.Equal(t, LoadbalanceSticky, loadbalance)
require.False(t, r.Qemu.Install)
require.Equal(t, bkimage.QemuImage, r.Qemu.Image)
require.False(t, defaultLoad)
},
)

Expand All @@ -165,7 +169,7 @@ func TestFactory_processDriverOpts(t *testing.T) {
cfg.DriverOpts = map[string]string{
"replicas": "invalid",
}
_, _, _, err := f.processDriverOpts(cfg.Name, "test", cfg)
_, _, _, _, err := f.processDriverOpts(cfg.Name, "test", cfg)
require.Error(t, err)
},
)
Expand All @@ -175,7 +179,7 @@ func TestFactory_processDriverOpts(t *testing.T) {
cfg.DriverOpts = map[string]string{
"rootless": "invalid",
}
_, _, _, err := f.processDriverOpts(cfg.Name, "test", cfg)
_, _, _, _, err := f.processDriverOpts(cfg.Name, "test", cfg)
require.Error(t, err)
},
)
Expand All @@ -185,7 +189,7 @@ func TestFactory_processDriverOpts(t *testing.T) {
cfg.DriverOpts = map[string]string{
"tolerations": "key=foo,value=bar,invalid=foo2",
}
_, _, _, err := f.processDriverOpts(cfg.Name, "test", cfg)
_, _, _, _, err := f.processDriverOpts(cfg.Name, "test", cfg)
require.Error(t, err)
},
)
Expand All @@ -195,7 +199,7 @@ func TestFactory_processDriverOpts(t *testing.T) {
cfg.DriverOpts = map[string]string{
"tolerations": "key=foo,value=bar,tolerationSeconds=invalid",
}
_, _, _, err := f.processDriverOpts(cfg.Name, "test", cfg)
_, _, _, _, err := f.processDriverOpts(cfg.Name, "test", cfg)
require.Error(t, err)
},
)
Expand All @@ -205,7 +209,7 @@ func TestFactory_processDriverOpts(t *testing.T) {
cfg.DriverOpts = map[string]string{
"annotations": "key,value",
}
_, _, _, err := f.processDriverOpts(cfg.Name, "test", cfg)
_, _, _, _, err := f.processDriverOpts(cfg.Name, "test", cfg)
require.Error(t, err)
},
)
Expand All @@ -215,7 +219,7 @@ func TestFactory_processDriverOpts(t *testing.T) {
cfg.DriverOpts = map[string]string{
"labels": "key=value=foo",
}
_, _, _, err := f.processDriverOpts(cfg.Name, "test", cfg)
_, _, _, _, err := f.processDriverOpts(cfg.Name, "test", cfg)
require.Error(t, err)
},
)
Expand All @@ -225,7 +229,7 @@ func TestFactory_processDriverOpts(t *testing.T) {
cfg.DriverOpts = map[string]string{
"loadbalance": "invalid",
}
_, _, _, err := f.processDriverOpts(cfg.Name, "test", cfg)
_, _, _, _, err := f.processDriverOpts(cfg.Name, "test", cfg)
require.Error(t, err)
},
)
Expand All @@ -235,7 +239,7 @@ func TestFactory_processDriverOpts(t *testing.T) {
cfg.DriverOpts = map[string]string{
"qemu.install": "invalid",
}
_, _, _, err := f.processDriverOpts(cfg.Name, "test", cfg)
_, _, _, _, err := f.processDriverOpts(cfg.Name, "test", cfg)
require.Error(t, err)
},
)
Expand All @@ -245,7 +249,7 @@ func TestFactory_processDriverOpts(t *testing.T) {
cfg.DriverOpts = map[string]string{
"invalid": "foo",
}
_, _, _, err := f.processDriverOpts(cfg.Name, "test", cfg)
_, _, _, _, err := f.processDriverOpts(cfg.Name, "test", cfg)
require.Error(t, err)
},
)
Expand Down
2 changes: 2 additions & 0 deletions driver/remote/driver.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ type Driver struct {
// if you add fields, remember to update docs:
// https://github.com/docker/docs/blob/main/content/build/drivers/remote.md
*tlsOpts
defaultLoad bool
}

type tlsOpts struct {
Expand Down Expand Up @@ -149,6 +150,7 @@ func (d *Driver) Features(ctx context.Context) map[driver.Feature]bool {
driver.DockerExporter: true,
driver.CacheExport: true,
driver.MultiPlatform: true,
driver.DefaultLoad: d.defaultLoad,
}
}

Expand Down
7 changes: 7 additions & 0 deletions driver/remote/factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"context"
"net/url"
"path/filepath"
"strconv"
"strings"

// import connhelpers for special url schemes
Expand Down Expand Up @@ -80,6 +81,12 @@ func (f *factory) New(ctx context.Context, cfg driver.InitConfig) (driver.Driver
}
tls.key = v
tlsEnabled = true
case "default-load":
parsed, err := strconv.ParseBool(v)
if err != nil {
return nil, err
}
d.defaultLoad = parsed
default:
return nil, errors.Errorf("invalid driver option %s for remote driver", k)
}
Expand Down
Loading

0 comments on commit 4b3c3c8

Please sign in to comment.