Skip to content

Commit

Permalink
Merge pull request #1757 from coryb/issue-1748
Browse files Browse the repository at this point in the history
gateway exec: add platform and worker constraints to NewContainer api
  • Loading branch information
tonistiigi authored Oct 28, 2020
2 parents 5c201fa + ffd4ab2 commit a4f17f9
Show file tree
Hide file tree
Showing 7 changed files with 348 additions and 134 deletions.
72 changes: 72 additions & 0 deletions client/build_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"github.com/moby/buildkit/session/sshforward/sshprovider"
"github.com/moby/buildkit/solver/errdefs"
"github.com/moby/buildkit/solver/pb"
utilsystem "github.com/moby/buildkit/util/system"
"github.com/moby/buildkit/util/testutil/integration"
"github.com/pkg/errors"
"github.com/stretchr/testify/require"
Expand All @@ -42,6 +43,7 @@ func TestClientGatewayIntegration(t *testing.T) {
testClientGatewayContainerPID1Tty,
testClientGatewayContainerExecTty,
testClientSlowCacheRootfsRef,
testClientGatewayContainerPlatformPATH,
}, integration.WithMirroredImages(integration.OfficialImages("busybox:latest")))
}

Expand Down Expand Up @@ -1013,7 +1015,77 @@ func testClientSlowCacheRootfsRef(t *testing.T, sb integration.Sandbox) {

_, err = c.Build(ctx, SolveOpt{}, "buildkit_test", b, nil)
require.NoError(t, err)
checkAllReleasable(t, c, sb, true)
}

// testClientGatewayContainerPlatformPATH is testing the correct default PATH
// gets set for the requested platform
func testClientGatewayContainerPlatformPATH(t *testing.T, sb integration.Sandbox) {
requiresLinux(t)
ctx := context.TODO()

c, err := New(ctx, sb.Address())
require.NoError(t, err)
defer c.Close()

product := "buildkit_test"
b := func(ctx context.Context, c client.Client) (*client.Result, error) {
st := llb.Image("busybox:latest")
def, err := st.Marshal(ctx)
require.NoError(t, err)
r, err := c.Solve(ctx, client.SolveRequest{
Definition: def.ToPB(),
})
require.NoError(t, err)

tests := []struct {
Name string
Platform *pb.Platform
Expected string
}{{
"default path",
nil,
utilsystem.DefaultPathEnvUnix,
}, {
"linux path",
&pb.Platform{OS: "linux"},
utilsystem.DefaultPathEnvUnix,
}, {
"windows path",
&pb.Platform{OS: "windows"},
utilsystem.DefaultPathEnvWindows,
}}

for _, tt := range tests {
t.Run(tt.Name, func(t *testing.T) {
ctr, err := c.NewContainer(ctx, client.NewContainerRequest{
Mounts: []client.Mount{{
Dest: "/",
MountType: pb.MountType_BIND,
Ref: r.Ref,
}},
Platform: tt.Platform,
})
require.NoError(t, err)
output := bytes.NewBuffer(nil)
pid1, err := ctr.Start(ctx, client.StartRequest{
Args: []string{"/bin/sh", "-c", "echo -n $PATH"},
Stdout: &nopCloser{output},
})
require.NoError(t, err)

err = pid1.Wait()
require.NoError(t, err)
require.Equal(t, tt.Expected, output.String())
err = ctr.Release(ctx)
require.NoError(t, err)
})
}
return &client.Result{}, err
}

_, err = c.Build(ctx, SolveOpt{}, product, b, nil)
require.NoError(t, err)
checkAllReleasable(t, c, sb, true)
}

Expand Down
6 changes: 4 additions & 2 deletions frontend/gateway/client/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,10 @@ type Client interface {
// NewContainerRequest encapsulates the requirements for a client to define a
// new container, without defining the initial process.
type NewContainerRequest struct {
Mounts []Mount
NetMode pb.NetMode
Mounts []Mount
NetMode pb.NetMode
Platform *pb.Platform
Constraints *pb.WorkerConstraints
}

// Mount allows clients to specify a filesystem mount. A Reference to a
Expand Down
14 changes: 13 additions & 1 deletion frontend/gateway/container.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package gateway
import (
"context"
"fmt"
"runtime"
"sort"
"strings"
"sync"
Expand All @@ -26,6 +27,8 @@ type NewContainerRequest struct {
ContainerID string
NetMode opspb.NetMode
Mounts []Mount
Platform *opspb.Platform
Constraints *opspb.WorkerConstraints
}

// Mount used for the gateway.Container is nearly identical to the client.Mount
Expand Down Expand Up @@ -57,9 +60,17 @@ func toProtoMount(m Mount) *opspb.Mount {
func NewContainer(ctx context.Context, e executor.Executor, sm *session.Manager, g session.Group, req NewContainerRequest) (client.Container, error) {
ctx, cancel := context.WithCancel(ctx)
eg, ctx := errgroup.WithContext(ctx)
platform := opspb.Platform{
OS: runtime.GOOS,
Architecture: runtime.GOARCH,
}
if req.Platform != nil {
platform = *req.Platform
}
ctr := &gatewayContainer{
id: req.ContainerID,
netMode: req.NetMode,
platform: platform,
executor: e,
errGroup: eg,
ctx: ctx,
Expand Down Expand Up @@ -196,6 +207,7 @@ func NewContainer(ctx context.Context, e executor.Executor, sm *session.Manager,
type gatewayContainer struct {
id string
netMode opspb.NetMode
platform opspb.Platform
rootFS cache.Mountable
mounts []executor.Mount
executor executor.Executor
Expand Down Expand Up @@ -227,7 +239,7 @@ func (gwCtr *gatewayContainer) Start(ctx context.Context, req client.StartReques
if procInfo.Meta.Cwd == "" {
procInfo.Meta.Cwd = "/"
}
procInfo.Meta.Env = addDefaultEnvvar(procInfo.Meta.Env, "PATH", utilsystem.DefaultPathEnvUnix) // support windows?
procInfo.Meta.Env = addDefaultEnvvar(procInfo.Meta.Env, "PATH", utilsystem.DefaultPathEnv(gwCtr.platform.OS))
if req.Tty {
procInfo.Meta.Env = addDefaultEnvvar(procInfo.Meta.Env, "TERM", "xterm")
}
Expand Down
2 changes: 2 additions & 0 deletions frontend/gateway/gateway.go
Original file line number Diff line number Diff line change
Expand Up @@ -771,6 +771,8 @@ func (lbf *llbBridgeForwarder) NewContainer(ctx context.Context, in *pb.NewConta
ctrReq := NewContainerRequest{
ContainerID: in.ContainerID,
NetMode: in.Network,
Platform: in.Platform,
Constraints: in.Constraints,
}

for _, m := range in.Mounts {
Expand Down
2 changes: 2 additions & 0 deletions frontend/gateway/grpcclient/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -676,6 +676,8 @@ func (c *grpcClient) NewContainer(ctx context.Context, req client.NewContainerRe
_, err = c.client.NewContainer(ctx, &pb.NewContainerRequest{
ContainerID: id,
Mounts: mounts,
Platform: req.Platform,
Constraints: req.Constraints,
})
if err != nil {
return nil, err
Expand Down
Loading

0 comments on commit a4f17f9

Please sign in to comment.