From 157b0b30dbf2f5af5c5b121bee60c1951af71fb5 Mon Sep 17 00:00:00 2001 From: Tonis Tiigi Date: Wed, 23 May 2018 15:53:14 -0700 Subject: [PATCH] builder: lint fixes Signed-off-by: Tonis Tiigi --- api/server/backend/build/backend.go | 1 + api/server/router/system/system_routes.go | 5 +-- api/types/client.go | 6 ++-- .../adapters/containerimage/pull.go | 2 ++ .../builder-next/adapters/snapshot/layer.go | 5 +-- .../adapters/snapshot/snapshot.go | 15 ++++---- builder/builder-next/builder.go | 13 ++++--- builder/builder-next/controller.go | 2 +- builder/builder-next/exporter/export.go | 3 ++ builder/builder-next/exporter/writer.go | 7 ++-- builder/builder-next/reqbodyhandler.go | 5 --- builder/builder-next/worker/worker.go | 36 ++++++++++++------- daemon/daemon.go | 1 + daemon/images/service.go | 2 ++ 14 files changed, 58 insertions(+), 45 deletions(-) diff --git a/api/server/backend/build/backend.go b/api/server/backend/build/backend.go index 043c9e84b7..904944a962 100644 --- a/api/server/backend/build/backend.go +++ b/api/server/backend/build/backend.go @@ -118,6 +118,7 @@ func (b *Backend) PruneCache(ctx context.Context) (*types.BuildCachePruneReport, return &types.BuildCachePruneReport{SpaceReclaimed: fsCacheSize + uint64(buildCacheSize)}, nil } +// Cancel cancels the build by ID func (b *Backend) Cancel(ctx context.Context, id string) error { return b.buildkit.Cancel(ctx, id) } diff --git a/api/server/router/system/system_routes.go b/api/server/router/system/system_routes.go index 4787458a35..82b649e98f 100644 --- a/api/server/router/system/system_routes.go +++ b/api/server/router/system/system_routes.go @@ -76,10 +76,7 @@ func (s *systemRouter) getDiskUsage(ctx context.Context, w http.ResponseWriter, eg.Go(func() error { var err error du, err = s.backend.SystemDiskUsage(ctx) - if err != nil { - return err - } - return nil + return err }) var builderSize int64 // legacy diff --git a/api/types/client.go b/api/types/client.go index d825e5ae5c..a7d8ab932b 100644 --- a/api/types/client.go +++ b/api/types/client.go @@ -189,8 +189,10 @@ type ImageBuildOptions struct { type BuilderVersion string const ( - BuilderV1 BuilderVersion = "1" - BuilderBuildKit = "2" + // BuilderV1 is the first generation builder in docker daemon + BuilderV1 BuilderVersion = "1" + // BuilderBuildKit is builder based on moby/buildkit project + BuilderBuildKit = "2" ) // ImageBuildResponse holds information diff --git a/builder/builder-next/adapters/containerimage/pull.go b/builder/builder-next/adapters/containerimage/pull.go index bdb347e329..aa283a4f37 100644 --- a/builder/builder-next/adapters/containerimage/pull.go +++ b/builder/builder-next/adapters/containerimage/pull.go @@ -43,6 +43,7 @@ import ( const preferLocal = true // FIXME: make this optional from the op +// SourceOpt is options for creating the image source type SourceOpt struct { SessionManager *session.Manager ContentStore content.Store @@ -58,6 +59,7 @@ type imageSource struct { g flightcontrol.Group } +// NewSource creates a new image source func NewSource(opt SourceOpt) (source.Source, error) { is := &imageSource{ SourceOpt: opt, diff --git a/builder/builder-next/adapters/snapshot/layer.go b/builder/builder-next/adapters/snapshot/layer.go index f2c6758316..d0aa6f28fa 100644 --- a/builder/builder-next/adapters/snapshot/layer.go +++ b/builder/builder-next/adapters/snapshot/layer.go @@ -69,10 +69,7 @@ func (s *snapshotter) EnsureLayer(ctx context.Context, key string) ([]layer.Diff } } diffID, size, err = s.reg.ChecksumForGraphID(id, parent, "", tarSplitPath) - if err != nil { - return err - } - return nil + return err }) if err := eg.Wait(); err != nil { diff --git a/builder/builder-next/adapters/snapshot/snapshot.go b/builder/builder-next/adapters/snapshot/snapshot.go index 11dea47968..9934c8ae3a 100644 --- a/builder/builder-next/adapters/snapshot/snapshot.go +++ b/builder/builder-next/adapters/snapshot/snapshot.go @@ -23,6 +23,7 @@ var keyCommitted = []byte("committed") var keyChainID = []byte("chainid") var keySize = []byte("size") +// Opt defines options for creating the snapshotter type Opt struct { GraphDriver graphdriver.Driver LayerStore layer.Store @@ -50,6 +51,7 @@ type snapshotter struct { var _ snapshot.SnapshotterBase = &snapshotter{} +// NewSnapshotter creates a new snapshotter func NewSnapshotter(opt Opt) (snapshot.SnapshotterBase, error) { dbPath := filepath.Join(opt.Root, "snapshots.db") db, err := bolt.Open(dbPath, 0600, nil) @@ -196,7 +198,7 @@ func (s *snapshotter) Stat(ctx context.Context, key string) (snapshots.Info, err inf.Parent = p.ChainID().String() } inf.Kind = snapshots.KindCommitted - inf.Name = string(key) + inf.Name = key return inf, nil } @@ -215,7 +217,7 @@ func (s *snapshotter) Stat(ctx context.Context, key string) (snapshots.Info, err if b == nil && l == nil { return errors.Errorf("snapshot %s not found", id) // TODO: typed } - inf.Name = string(key) + inf.Name = key if b != nil { v := b.Get(keyParent) if v != nil { @@ -322,7 +324,7 @@ func (s *snapshotter) Remove(ctx context.Context, key string) error { } func (s *snapshotter) Commit(ctx context.Context, name, key string, opts ...snapshots.Opt) error { - if err := s.db.Update(func(tx *bolt.Tx) error { + return s.db.Update(func(tx *bolt.Tx) error { b, err := tx.CreateBucketIfNotExists([]byte(name)) if err != nil { return err @@ -331,10 +333,7 @@ func (s *snapshotter) Commit(ctx context.Context, name, key string, opts ...snap return err } return nil - }); err != nil { - return err - } - return nil + }) } func (s *snapshotter) View(ctx context.Context, key, parent string, opts ...snapshots.Opt) (snapshot.Mountable, error) { @@ -421,7 +420,7 @@ func (s *snapshotter) Usage(ctx context.Context, key string) (us snapshots.Usage }); err != nil { return usage, err } - usage.Size = int64(diffSize) + usage.Size = diffSize return usage, nil } diff --git a/builder/builder-next/builder.go b/builder/builder-next/builder.go index 6ac5b226c2..9e8c1431b7 100644 --- a/builder/builder-next/builder.go +++ b/builder/builder-next/builder.go @@ -24,12 +24,14 @@ import ( grpcmetadata "google.golang.org/grpc/metadata" ) +// Opt is option struct required for creating the builder type Opt struct { SessionManager *session.Manager Root string Dist images.DistributionServices } +// Builder can build using BuildKit backend type Builder struct { controller *control.Controller reqBodyHandler *reqBodyHandler @@ -38,6 +40,7 @@ type Builder struct { jobs map[string]*buildJob } +// New creates a new builder func New(opt Opt) (*Builder, error) { reqHandler := newReqBodyHandler(tracing.DefaultTransport) @@ -53,6 +56,7 @@ func New(opt Opt) (*Builder, error) { return b, nil } +// Cancel cancels a build using ID func (b *Builder) Cancel(ctx context.Context, id string) error { b.mu.Lock() if j, ok := b.jobs[id]; ok && j.cancel != nil { @@ -62,6 +66,7 @@ func (b *Builder) Cancel(ctx context.Context, id string) error { return nil } +// DiskUsage returns a report about space used by build cache func (b *Builder) DiskUsage(ctx context.Context) ([]*types.BuildCache, error) { duResp, err := b.controller.DiskUsage(ctx, &controlapi.DiskUsageRequest{}) if err != nil { @@ -86,6 +91,7 @@ func (b *Builder) DiskUsage(ctx context.Context) ([]*types.BuildCache, error) { return items, nil } +// Prune clears all reclaimable build cache func (b *Builder) Prune(ctx context.Context) (int64, error) { ch := make(chan *controlapi.UsageRecord) @@ -114,6 +120,7 @@ func (b *Builder) Prune(ctx context.Context) (int64, error) { return size, nil } +// Build executes a build request func (b *Builder) Build(ctx context.Context, opt backend.BuildConfig) (*builder.Result, error) { var rc = opt.Source @@ -181,10 +188,8 @@ func (b *Builder) Build(ctx context.Context, opt backend.BuildConfig) (*builder. frontendAttrs["context"] = url } - var cacheFrom []string - for _, v := range opt.Options.CacheFrom { - cacheFrom = append(cacheFrom, v) - } + cacheFrom := append([]string{}, opt.Options.CacheFrom...) + frontendAttrs["cache-from"] = strings.Join(cacheFrom, ",") for k, v := range opt.Options.BuildArgs { diff --git a/builder/builder-next/controller.go b/builder/builder-next/controller.go index 86c32863fa..2956affa79 100644 --- a/builder/builder-next/controller.go +++ b/builder/builder-next/controller.go @@ -117,7 +117,7 @@ func newController(rt http.RoundTripper, opt Opt) (*control.Controller, error) { frontends["dockerfile.v0"] = dockerfile.NewDockerfileFrontend() frontends["gateway.v0"] = gateway.NewGatewayFrontend() - wopt := mobyworker.WorkerOpt{ + wopt := mobyworker.Opt{ ID: "moby", SessionManager: opt.SessionManager, MetadataStore: md, diff --git a/builder/builder-next/exporter/export.go b/builder/builder-next/exporter/export.go index f8b061febd..ddc36f07e6 100644 --- a/builder/builder-next/exporter/export.go +++ b/builder/builder-next/exporter/export.go @@ -19,10 +19,12 @@ const ( exporterImageConfig = "containerimage.config" ) +// Differ can make a moby layer from a snapshot type Differ interface { EnsureLayer(ctx context.Context, key string) ([]layer.DiffID, error) } +// Opt defines a struct for creating new exporter type Opt struct { ImageStore image.Store ReferenceStore reference.Store @@ -33,6 +35,7 @@ type imageExporter struct { opt Opt } +// New creates a new moby imagestore exporter func New(opt Opt) (exporter.Exporter, error) { im := &imageExporter{opt: opt} return im, nil diff --git a/builder/builder-next/exporter/writer.go b/builder/builder-next/exporter/writer.go index 09a25e906e..f8d9175d96 100644 --- a/builder/builder-next/exporter/writer.go +++ b/builder/builder-next/exporter/writer.go @@ -49,9 +49,8 @@ func patchImageConfig(dt []byte, dps []digest.Digest, history []ocispec.History) var rootFS ocispec.RootFS rootFS.Type = "layers" - for _, dp := range dps { - rootFS.DiffIDs = append(rootFS.DiffIDs, dp) - } + rootFS.DiffIDs = append(rootFS.DiffIDs, dps...) + dt, err := json.Marshal(rootFS) if err != nil { return nil, errors.Wrap(err, "failed to marshal rootfs") @@ -87,7 +86,7 @@ func normalizeLayersAndHistory(diffs []digest.Digest, history []ocispec.History, var historyLayers int for _, h := range history { if !h.EmptyLayer { - historyLayers += 1 + historyLayers++ } } if historyLayers > len(diffs) { diff --git a/builder/builder-next/reqbodyhandler.go b/builder/builder-next/reqbodyhandler.go index d4b5ead883..48433908fb 100644 --- a/builder/builder-next/reqbodyhandler.go +++ b/builder/builder-next/reqbodyhandler.go @@ -65,8 +65,3 @@ func (h *reqBodyHandler) RoundTrip(req *http.Request) (*http.Response, error) { } return h.rt.RoundTrip(req) } - -type readCloser struct { - io.Reader - io.Closer -} diff --git a/builder/builder-next/worker/worker.go b/builder/builder-next/worker/worker.go index 6aa72c5e5e..ccc7dc8222 100644 --- a/builder/builder-next/worker/worker.go +++ b/builder/builder-next/worker/worker.go @@ -39,9 +39,8 @@ import ( "github.com/pkg/errors" ) -// WorkerOpt is specific to a worker. -// See also CommonOpt. -type WorkerOpt struct { +// Opt defines a structure for creating a worker. +type Opt struct { ID string Labels map[string]string SessionManager *session.Manager @@ -60,12 +59,12 @@ type WorkerOpt struct { // Worker is a local worker instance with dedicated snapshotter, cache, and so on. // TODO: s/Worker/OpWorker/g ? type Worker struct { - WorkerOpt + Opt SourceManager *source.Manager } // NewWorker instantiates a local worker -func NewWorker(opt WorkerOpt) (*Worker, error) { +func NewWorker(opt Opt) (*Worker, error) { sm, err := source.NewManager() if err != nil { return nil, err @@ -106,23 +105,27 @@ func NewWorker(opt WorkerOpt) (*Worker, error) { sm.Register(ss) return &Worker{ - WorkerOpt: opt, + Opt: opt, SourceManager: sm, }, nil } +// ID returns worker ID func (w *Worker) ID() string { - return w.WorkerOpt.ID + return w.Opt.ID } +// Labels returns map of all worker labels func (w *Worker) Labels() map[string]string { - return w.WorkerOpt.Labels + return w.Opt.Labels } +// LoadRef loads a reference by ID func (w *Worker) LoadRef(id string) (cache.ImmutableRef, error) { return w.CacheManager.Get(context.TODO(), id) } +// ResolveOp converts a LLB vertex into a LLB operation func (w *Worker) ResolveOp(v solver.Vertex, s frontend.FrontendLLBBridge) (solver.Op, error) { switch op := v.Sys().(type) { case *pb.Op_Source: @@ -136,6 +139,7 @@ func (w *Worker) ResolveOp(v solver.Vertex, s frontend.FrontendLLBBridge) (solve } } +// ResolveImageConfig returns image config for an image func (w *Worker) ResolveImageConfig(ctx context.Context, ref string) (digest.Digest, []byte, error) { // ImageSource is typically source/containerimage resolveImageConfig, ok := w.ImageSource.(resolveImageConfig) @@ -145,10 +149,7 @@ func (w *Worker) ResolveImageConfig(ctx context.Context, ref string) (digest.Dig return resolveImageConfig.ResolveImageConfig(ctx, ref) } -type resolveImageConfig interface { - ResolveImageConfig(ctx context.Context, ref string) (digest.Digest, []byte, error) -} - +// Exec executes a process directly on a worker func (w *Worker) Exec(ctx context.Context, meta executor.Meta, rootFS cache.ImmutableRef, stdin io.ReadCloser, stdout, stderr io.WriteCloser) error { active, err := w.CacheManager.New(ctx, rootFS) if err != nil { @@ -158,14 +159,17 @@ func (w *Worker) Exec(ctx context.Context, meta executor.Meta, rootFS cache.Immu return w.Executor.Exec(ctx, meta, active, nil, stdin, stdout, stderr) } +// DiskUsage returns disk usage report func (w *Worker) DiskUsage(ctx context.Context, opt client.DiskUsageInfo) ([]*client.UsageInfo, error) { return w.CacheManager.DiskUsage(ctx, opt) } +// Prune deletes reclaimable build cache func (w *Worker) Prune(ctx context.Context, ch chan client.UsageInfo) error { return w.CacheManager.Prune(ctx, ch) } +// Exporter returns exporter by name func (w *Worker) Exporter(name string) (exporter.Exporter, error) { exp, ok := w.Exporters[name] if !ok { @@ -174,10 +178,12 @@ func (w *Worker) Exporter(name string) (exporter.Exporter, error) { return exp, nil } +// GetRemote returns a remote snapshot reference for a local one func (w *Worker) GetRemote(ctx context.Context, ref cache.ImmutableRef, createIfNeeded bool) (*solver.Remote, error) { return nil, errors.Errorf("getremote not implemented") } +// FromRemote converts a remote snapshot reference to a local one func (w *Worker) FromRemote(ctx context.Context, remote *solver.Remote) (cache.ImmutableRef, error) { rootfs, err := getLayers(ctx, remote.Descriptors) if err != nil { @@ -219,7 +225,7 @@ func (w *Worker) FromRemote(ctx context.Context, remote *solver.Remote) (cache.I type discardProgress struct{} -func (_ *discardProgress) WriteProgress(_ pkgprogress.Progress) error { +func (*discardProgress) WriteProgress(_ pkgprogress.Progress) error { return nil } @@ -309,3 +315,7 @@ func oneOffProgress(ctx context.Context, id string) func(err error) error { return err } } + +type resolveImageConfig interface { + ResolveImageConfig(ctx context.Context, ref string) (digest.Digest, []byte, error) +} diff --git a/daemon/daemon.go b/daemon/daemon.go index 0b636aa3ee..5e5f586ae0 100644 --- a/daemon/daemon.go +++ b/daemon/daemon.go @@ -922,6 +922,7 @@ func NewDaemon(config *config.Config, registryService registry.Service, containe return d, nil } +// DistributionServices returns services controlling daemon storage func (daemon *Daemon) DistributionServices() images.DistributionServices { return daemon.imageService.DistributionServices() } diff --git a/daemon/images/service.go b/daemon/images/service.go index 6842760359..263217dccd 100644 --- a/daemon/images/service.go +++ b/daemon/images/service.go @@ -76,6 +76,7 @@ type ImageService struct { uploadManager *xfer.LayerUploadManager } +// DistributionServices provides daemon image storage services type DistributionServices struct { DownloadManager distribution.RootFSDownloadManager V2MetadataService metadata.V2MetadataService @@ -84,6 +85,7 @@ type DistributionServices struct { ReferenceStore dockerreference.Store } +// DistributionServices return services controlling daemon image storage func (i *ImageService) DistributionServices() DistributionServices { return DistributionServices{ DownloadManager: i.downloadManager,