Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

support setting warnings from frontends #2482

Merged
merged 1 commit into from
Dec 1, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
525 changes: 426 additions & 99 deletions api/services/control/control.pb.go

Large diffs are not rendered by default.

7 changes: 7 additions & 0 deletions api/services/control/control.proto
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,7 @@ message StatusResponse {
repeated Vertex vertexes = 1;
repeated VertexStatus statuses = 2;
repeated VertexLog logs = 3;
repeated VertexWarning warnings = 4;
}

message Vertex {
Expand Down Expand Up @@ -134,6 +135,12 @@ message VertexLog {
bytes msg = 4;
}

message VertexWarning {
string vertex = 1 [(gogoproto.customtype) = "github.com/opencontainers/go-digest.Digest", (gogoproto.nullable) = false];
int64 level = 2;
bytes msg = 3;
}

message BytesMessage {
bytes data = 1;
}
Expand Down
5 changes: 5 additions & 0 deletions client/build.go
Original file line number Diff line number Diff line change
Expand Up @@ -148,3 +148,8 @@ func (g *gatewayClientForBuild) ExecProcess(ctx context.Context, opts ...grpc.Ca
ctx = buildid.AppendToOutgoingContext(ctx, g.buildID)
return g.gateway.ExecProcess(ctx, opts...)
}

func (g *gatewayClientForBuild) Warn(ctx context.Context, in *gatewayapi.WarnRequest, opts ...grpc.CallOption) (*gatewayapi.WarnResponse, error) {
ctx = buildid.AppendToOutgoingContext(ctx, g.buildID)
return g.gateway.Warn(ctx, in)
}
86 changes: 86 additions & 0 deletions client/build_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import (
utilsystem "github.com/moby/buildkit/util/system"
"github.com/moby/buildkit/util/testutil/echoserver"
"github.com/moby/buildkit/util/testutil/integration"
digest "github.com/opencontainers/go-digest"
"github.com/pkg/errors"
"github.com/stretchr/testify/require"
"golang.org/x/crypto/ssh/agent"
Expand All @@ -51,6 +52,7 @@ func TestClientGatewayIntegration(t *testing.T) {
testClientGatewaySlowCacheExecError,
testClientGatewayExecFileActionError,
testClientGatewayContainerExtraHosts,
testWarnings,
}, integration.WithMirroredImages(integration.OfficialImages("busybox:latest")))

integration.Run(t, []integration.Test{
Expand Down Expand Up @@ -151,6 +153,90 @@ func testClientGatewaySolve(t *testing.T, sb integration.Sandbox) {
checkAllReleasable(t, c, sb, true)
}

func testWarnings(t *testing.T, sb integration.Sandbox) {
requiresLinux(t)

ctx := sb.Context()

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.Scratch().File(llb.Mkfile("/dummy", 0600, []byte("foo")))

def, err := st.Marshal(ctx)
if err != nil {
return nil, errors.Wrap(err, "failed to marshal state")
}

dgst, _, _, _, err := st.Output().Vertex(ctx, def.Constraints).Marshal(ctx, def.Constraints)
if err != nil {
return nil, err
}

r, err := c.Solve(ctx, client.SolveRequest{
Definition: def.ToPB(),
})
if err != nil {
return nil, errors.Wrap(err, "failed to solve")
}

require.NoError(t, c.Warn(ctx, dgst, "this is warning"))
return r, nil
}

status := make(chan *SolveStatus)
statusDone := make(chan struct{})
done := make(chan struct{})

var warnings []*VertexWarning
vertexes := map[digest.Digest]struct{}{}

go func() {
defer close(statusDone)
for {
select {
case st, ok := <-status:
if !ok {
return
}
for _, s := range st.Vertexes {
vertexes[s.Digest] = struct{}{}
}
warnings = append(warnings, st.Warnings...)
case <-done:
return
}
}
}()

_, err = c.Build(ctx, SolveOpt{}, product, b, status)
require.NoError(t, err)

select {
case <-statusDone:
case <-time.After(10 * time.Second):
close(done)
}

<-statusDone

require.Equal(t, 1, len(vertexes))
require.Equal(t, 1, len(warnings))

w := warnings[0]

require.Equal(t, "this is warning", string(w.Message))
require.Equal(t, 1, w.Level)
_, ok := vertexes[w.Vertex]
require.True(t, ok)

checkAllReleasable(t, c, sb, true)
}

func testClientGatewayFailedSolve(t *testing.T, sb integration.Sandbox) {
requiresLinux(t)

Expand Down
7 changes: 7 additions & 0 deletions client/graph.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,10 +34,17 @@ type VertexLog struct {
Timestamp time.Time
}

type VertexWarning struct {
Vertex digest.Digest
Level int
Message []byte
}

type SolveStatus struct {
Vertexes []*Vertex
Statuses []*VertexStatus
Logs []*VertexLog
Warnings []*VertexWarning
}

type SolveResponse struct {
Expand Down
7 changes: 4 additions & 3 deletions client/llb/marshal.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,10 @@ import (
// Definition is the LLB definition structure with per-vertex metadata entries
// Corresponds to the Definition structure defined in solver/pb.Definition.
type Definition struct {
Def [][]byte
Metadata map[digest.Digest]pb.OpMetadata
Source *pb.Source
Def [][]byte
Metadata map[digest.Digest]pb.OpMetadata
Source *pb.Source
Constraints *Constraints
}

func (def *Definition) ToPB() *pb.Definition {
Expand Down
24 changes: 15 additions & 9 deletions client/llb/state.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,18 @@ type Vertex interface {
Inputs() []Output
}

func NewConstraints(co ...ConstraintsOpt) *Constraints {
defaultPlatform := platforms.Normalize(platforms.DefaultSpec())
c := &Constraints{
Platform: &defaultPlatform,
LocalUniqueID: identity.NewID(),
}
for _, o := range co {
o.SetConstraintsOption(c)
}
return c
}

func NewState(o Output) State {
s := State{
out: o,
Expand Down Expand Up @@ -112,18 +124,12 @@ func (s State) SetMarshalDefaults(co ...ConstraintsOpt) State {
}

func (s State) Marshal(ctx context.Context, co ...ConstraintsOpt) (*Definition, error) {
c := NewConstraints(append(s.opts, co...)...)
def := &Definition{
Metadata: make(map[digest.Digest]pb.OpMetadata, 0),
Metadata: make(map[digest.Digest]pb.OpMetadata, 0),
Constraints: c,
}

defaultPlatform := platforms.Normalize(platforms.DefaultSpec())
c := &Constraints{
Platform: &defaultPlatform,
LocalUniqueID: identity.NewID(),
}
for _, o := range append(s.opts, co...) {
o.SetConstraintsOption(c)
}
if s.Output() == nil || s.Output().Vertex(ctx, c) == nil {
return def, nil
}
Expand Down
7 changes: 7 additions & 0 deletions client/solve.go
Original file line number Diff line number Diff line change
Expand Up @@ -292,6 +292,13 @@ func (c *Client) solve(ctx context.Context, def *llb.Definition, runGateway runG
Timestamp: v.Timestamp,
})
}
for _, v := range resp.Warnings {
s.Warnings = append(s.Warnings, &VertexWarning{
Vertex: v.Vertex,
Level: int(v.Level),
Message: v.Msg,
})
}
if statusChan != nil {
statusChan <- &s
}
Expand Down
7 changes: 7 additions & 0 deletions control/control.go
Original file line number Diff line number Diff line change
Expand Up @@ -376,6 +376,13 @@ func (c *Controller) Status(req *controlapi.StatusRequest, stream controlapi.Con
break
}
}
for _, v := range ss.Warnings {
sr.Warnings = append(sr.Warnings, &controlapi.VertexWarning{
Vertex: v.Vertex,
Level: int64(v.Level),
Msg: v.Message,
})
}
if err := stream.SendMsg(&sr); err != nil {
return err
}
Expand Down
8 changes: 8 additions & 0 deletions control/gateway/gateway.go
Original file line number Diff line number Diff line change
Expand Up @@ -176,3 +176,11 @@ func (gwf *GatewayForwarder) ExecProcess(srv gwapi.LLBBridge_ExecProcessServer)
}
return fwd.ExecProcess(srv)
}

func (gwf *GatewayForwarder) Warn(ctx context.Context, req *gwapi.WarnRequest) (*gwapi.WarnResponse, error) {
fwd, err := gwf.lookupForwarder(ctx)
if err != nil {
return nil, errors.Wrap(err, "forwarding Warn")
}
return fwd.Warn(ctx, req)
}
1 change: 1 addition & 0 deletions frontend/frontend.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ type Frontend interface {
type FrontendLLBBridge interface {
Solve(ctx context.Context, req SolveRequest, sid string) (*Result, error)
ResolveImageConfig(ctx context.Context, ref string, opt llb.ResolveImageConfigOpt) (digest.Digest, []byte, error)
Warn(ctx context.Context, dgst digest.Digest, level int, msg string) error
}

type SolveRequest = gw.SolveRequest
Expand Down
1 change: 1 addition & 0 deletions frontend/gateway/client/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ type Client interface {
BuildOpts() BuildOpts
Inputs(ctx context.Context) (map[string]llb.State, error)
NewContainer(ctx context.Context, req NewContainerRequest) (Container, error)
Warn(ctx context.Context, dgst digest.Digest, msg string) error
}

// NewContainerRequest encapsulates the requirements for a client to define a
Expand Down
5 changes: 5 additions & 0 deletions frontend/gateway/forwarder/forward.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
opspb "github.com/moby/buildkit/solver/pb"
"github.com/moby/buildkit/util/apicaps"
"github.com/moby/buildkit/worker"
digest "github.com/opencontainers/go-digest"
"github.com/pkg/errors"
fstypes "github.com/tonistiigi/fsutil/types"
"golang.org/x/sync/errgroup"
Expand Down Expand Up @@ -216,6 +217,10 @@ func (c *bridgeClient) discard(err error) {
}
}

func (c *bridgeClient) Warn(ctx context.Context, dgst digest.Digest, msg string) error {
return c.FrontendLLBBridge.Warn(ctx, dgst, 1, msg)
}

func (c *bridgeClient) NewContainer(ctx context.Context, req client.NewContainerRequest) (client.Container, error) {
ctrReq := gateway.NewContainerRequest{
ContainerID: identity.NewID(),
Expand Down
8 changes: 8 additions & 0 deletions frontend/gateway/gateway.go
Original file line number Diff line number Diff line change
Expand Up @@ -993,6 +993,14 @@ func (lbf *llbBridgeForwarder) ReleaseContainer(ctx context.Context, in *pb.Rele
return &pb.ReleaseContainerResponse{}, stack.Enable(err)
}

func (lbf *llbBridgeForwarder) Warn(ctx context.Context, in *pb.WarnRequest) (*pb.WarnResponse, error) {
err := lbf.llbBridge.Warn(ctx, in.Digest, int(in.Level), string(in.Message))
if err != nil {
return nil, err
}
return &pb.WarnResponse{}, nil
}

type processIO struct {
id string
mu sync.Mutex
Expand Down
9 changes: 9 additions & 0 deletions frontend/gateway/grpcclient/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -298,6 +298,15 @@ func (c *grpcClient) requestForRef(ref client.Reference) (*pb.SolveRequest, erro
return req, nil
}

func (c *grpcClient) Warn(ctx context.Context, dgst digest.Digest, msg string) error {
_, err := c.client.Warn(ctx, &pb.WarnRequest{
Digest: dgst,
Level: 1,
Message: []byte(msg),
})
return err
}

func (c *grpcClient) Solve(ctx context.Context, creq client.SolveRequest) (res *client.Result, err error) {
if creq.Definition != nil {
for _, md := range creq.Definition.Metadata {
Expand Down
10 changes: 10 additions & 0 deletions frontend/gateway/pb/caps.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,9 @@ const (
// results. This is generally used by the client to return and handle solve
// errors.
CapGatewayEvaluateSolve apicaps.CapID = "gateway.solve.evaluate"

// CapGatewayWarnings is the capability to log warnings from frontend
CapGatewayWarnings apicaps.CapID = "gateway.warnings"
)

func init() {
Expand Down Expand Up @@ -180,4 +183,11 @@ func init() {
Enabled: true,
Status: apicaps.CapStatusExperimental,
})

Caps.Init(apicaps.Cap{
ID: CapGatewayWarnings,
Name: "logging warnings",
Enabled: true,
Status: apicaps.CapStatusExperimental,
})
}
Loading