Skip to content

Commit

Permalink
driver: do not insert "platform" as driver-opt
Browse files Browse the repository at this point in the history
Addresses docker/setup-buildx-action#45

Simple repro:
```
$ buildx create --platform linux/amd64 --use
$ buildx build - <<EOF
from scratch
EOF
```

Since docker#370 a `platform` driver-opt was automatically inserted with the value specified by `--platform` flag on regardless of the type of driver, even though it was only used in the kubernetes driver. However, because the docker-container driver is pedantic about the options being passed, it errored out.

Another side-effect I suspect is that with the kubernetes driver it was now possible to specify the platforms in two different ways: `--driver-opt platform=...` and `--platform`.

This patch reverts completely the `platform` driver-opt and instead ensures the platforms information is passed onto the kubernetes driver via variables.

Signed-off-by: Tibor Vass <tibor@docker.com>
  • Loading branch information
Tibor Vass committed Dec 15, 2020
1 parent 780fad4 commit 381dc8f
Show file tree
Hide file tree
Showing 3 changed files with 7 additions and 27 deletions.
19 changes: 2 additions & 17 deletions commands/util.go
Expand Up @@ -4,7 +4,6 @@ import (
"context"
"os"
"path/filepath"
"strings"

"github.com/docker/buildx/build"
"github.com/docker/buildx/driver"
Expand Down Expand Up @@ -220,7 +219,7 @@ func driversForNodeGroup(ctx context.Context, dockerCli command.Cli, ng *store.N
}
}

d, err := driver.GetDriver(ctx, "buildx_buildkit_"+n.Name, f, dockerapi, dockerCli.ConfigFile(), kcc, n.Flags, n.ConfigFile, assignDriverOptsByDriverInfo(n.DriverOpts, di), contextPathHash)
d, err := driver.GetDriver(ctx, "buildx_buildkit_"+n.Name, f, dockerapi, dockerCli.ConfigFile(), kcc, n.Flags, n.ConfigFile, n.DriverOpts, n.Platforms, contextPathHash)
if err != nil {
di.Err = err
return nil
Expand All @@ -238,20 +237,6 @@ func driversForNodeGroup(ctx context.Context, dockerCli command.Cli, ng *store.N
return dis, nil
}

// pass platform as driver opts to provide for some drive, like kubernetes
func assignDriverOptsByDriverInfo(opts map[string]string, driveInfo build.DriverInfo) map[string]string {
m := map[string]string{}

if len(driveInfo.Platform) > 0 {
m["platform"] = strings.Join(platformutil.Format(driveInfo.Platform), ",")
}

for key := range opts {
m[key] = opts[key]
}
return m
}

// clientForEndpoint returns a docker client for an endpoint
func clientForEndpoint(dockerCli command.Cli, name string) (dockerclient.APIClient, error) {
list, err := dockerCli.ContextStore().List()
Expand Down Expand Up @@ -353,7 +338,7 @@ func getDefaultDrivers(ctx context.Context, dockerCli command.Cli, defaultOnly b
}
}

d, err := driver.GetDriver(ctx, "buildx_buildkit_default", nil, dockerCli.Client(), dockerCli.ConfigFile(), nil, nil, "", nil, contextPathHash)
d, err := driver.GetDriver(ctx, "buildx_buildkit_default", nil, dockerCli.Client(), dockerCli.ConfigFile(), nil, nil, "", nil, nil, contextPathHash)
if err != nil {
return nil, err
}
Expand Down
10 changes: 1 addition & 9 deletions driver/kubernetes/factory.go
Expand Up @@ -9,7 +9,6 @@ import (
"github.com/docker/buildx/driver/bkimage"
"github.com/docker/buildx/driver/kubernetes/manifest"
"github.com/docker/buildx/driver/kubernetes/podchooser"
"github.com/docker/buildx/util/platformutil"
dockerclient "github.com/docker/docker/client"
"github.com/pkg/errors"
"k8s.io/client-go/kubernetes"
Expand Down Expand Up @@ -71,6 +70,7 @@ func (f *factory) New(ctx context.Context, cfg driver.InitConfig) (driver.Driver
Replicas: 1,
BuildkitFlags: cfg.BuildkitFlags,
Rootless: false,
Platforms: cfg.Platforms,
}
loadbalance := LoadbalanceSticky
imageOverride := ""
Expand All @@ -91,14 +91,6 @@ func (f *factory) New(ctx context.Context, cfg driver.InitConfig) (driver.Driver
return nil, err
}
deploymentOpt.Image = bkimage.DefaultRootlessImage
case "platform":
if v != "" {
platforms, err := platformutil.Parse(strings.Split(v, ","))
if err != nil {
return nil, err
}
deploymentOpt.Platforms = platforms
}
case "nodeselector":
kvs := strings.Split(strings.Trim(v, `"`), ",")
s := map[string]string{}
Expand Down
5 changes: 4 additions & 1 deletion driver/manager.go
Expand Up @@ -11,6 +11,7 @@ import (

dockerclient "github.com/docker/docker/client"
"github.com/moby/buildkit/client"
specs "github.com/opencontainers/image-spec/specs-go/v1"
"github.com/pkg/errors"
)

Expand Down Expand Up @@ -55,6 +56,7 @@ type InitConfig struct {
ConfigFile string
DriverOpts map[string]string
Auth Auth
Platforms []specs.Platform
// ContextPathHash can be used for determining pods in the driver instance
ContextPathHash string
}
Expand Down Expand Up @@ -101,7 +103,7 @@ func GetFactory(name string, instanceRequired bool) Factory {
return nil
}

func GetDriver(ctx context.Context, name string, f Factory, api dockerclient.APIClient, auth Auth, kcc KubeClientConfig, flags []string, config string, do map[string]string, contextPathHash string) (Driver, error) {
func GetDriver(ctx context.Context, name string, f Factory, api dockerclient.APIClient, auth Auth, kcc KubeClientConfig, flags []string, config string, do map[string]string, platforms []specs.Platform, contextPathHash string) (Driver, error) {
ic := InitConfig{
DockerAPI: api,
KubeClientConfig: kcc,
Expand All @@ -110,6 +112,7 @@ func GetDriver(ctx context.Context, name string, f Factory, api dockerclient.API
ConfigFile: config,
DriverOpts: do,
Auth: auth,
Platforms: platforms,
ContextPathHash: contextPathHash,
}
if f == nil {
Expand Down

0 comments on commit 381dc8f

Please sign in to comment.