From 8de3e7604b276c0194e819d644b39be1c2015f8d Mon Sep 17 00:00:00 2001 From: Changwei Ge Date: Fri, 7 Jan 2022 10:50:53 +0800 Subject: [PATCH 1/2] nydusify: utilize golangci-lint to perform static check Add golangci-lint rules definition into. Signed-off-by: Changwei Ge --- contrib/nydusify/Makefile | 1 + 1 file changed, 1 insertion(+) diff --git a/contrib/nydusify/Makefile b/contrib/nydusify/Makefile index 9b063ee45a6..370a6cbeaea 100644 --- a/contrib/nydusify/Makefile +++ b/contrib/nydusify/Makefile @@ -19,6 +19,7 @@ static-release: .PHONY: test test: build build-smoke @go vet $(PACKAGES) + golangci-lint run @go test -count=1 -v -timeout 20m -race ./pkg/... build-smoke: From c6d43845cd4298bf04dce7018a27e8715215e127 Mon Sep 17 00:00:00 2001 From: Changwei Ge Date: Thu, 6 Jan 2022 21:06:03 +0800 Subject: [PATCH 2/2] nydusify: silent golangci-lint complaints As golangci-lint reports: pkg/checker/tool/builder.go:35:2: S1021: should merge variable declaration with assignment on next line (gosimple) var args []string ^ pkg/checker/tool/nydusd.go:160:2: S1021: should merge variable declaration with assignment on next line (gosimple) var args []string ^ pkg/backend/backend.go:38:27: var-naming: func parameter blobId should be blobID (revive) func blobDesc(size int64, blobId string) ocispec.Descriptor { ^ pkg/backend/backend.go:31:6: exported: type name will be used as backend.BackendType by other packages, and that stutters; consider calling this Type (revive) type BackendType = int ^ pkg/backend/oss.go:123:21: var-declaration: should omit type bool from declaration of var needMultiparts; it will be inferred from the right-hand side (revive) var needMultiparts bool = false ^ pkg/backend/oss.go:130:21: var-declaration: should drop = 0 from declaration of var crc64; it is the zero value (revive) var crc64 uint64 = 0 ^ pkg/backend/oss.go:240:1: receiver-naming: receiver name r should be consistent with previous receiver name b for OSSBackend (revive) func (r *OSSBackend) Type() BackendType { return OssBackend } pkg/metrics/file_exporter/file_exporter.go:5:1: var-naming: don't use an underscore in package name (revive) package file_exporter import ( "github.com/prometheus/client_golang/prometheus" "github.com/dragonflyoss/image-service/contrib/nydusify/pkg/metrics" ) type FileExporter struct{ name string } func New(name string) *FileExporter { return &FileExporter{ name: name, } } func (exp *FileExporter) Export() { prometheus.WriteToTextfile(exp.name, metrics.Registry) } cmd/nydusify.go:314:14: ineffectual assignment to err (ineffassign) _, arch, err := provider.ExtractOsArch(c.String("platform")) ^ pkg/cache/spec.go:12:6: exported: type name will be used as cache.CacheManifest by other packages, and that stutters; consider calling this Manifest (revive) type CacheManifest struct { ^ pkg/cache/spec.go:17:6: exported: type name will be used as cache.CacheRecord by other packages, and that stutters; consider calling this Record (revive) type CacheRecord struct { ^ pkg/converter/cache.go:81:50: `diffrent` is a misspelling of `different` (misspell) // because the blob data is not shared between diffrent namespaces in registry, ^ tests/registry.go:51:27: `Destory` is a misspelling of `Destroy` (misspell) func (registry *Registry) Destory(t *testing.T) { ^ pkg/parser/parser.go:167:1: receiver-naming: receiver name p should be consistent with previous receiver name parser for Parser (revive) func (p *Parser) matchImagePlatform(desc *ocispec.Descriptor) bool { if p.interestedArch == desc.Platform.Architecture && desc.Platform.OS == "linux" { return true } return false Signed-off-by: Changwei Ge --- contrib/nydusify/.golangci.yml | 23 +++++++++++++ contrib/nydusify/cmd/nydusify.go | 7 ++-- contrib/nydusify/pkg/backend/backend.go | 10 +++--- contrib/nydusify/pkg/backend/oss.go | 6 ++-- contrib/nydusify/pkg/backend/registry.go | 2 +- contrib/nydusify/pkg/cache/cache.go | 32 +++++++++---------- contrib/nydusify/pkg/cache/cache_test.go | 8 ++--- contrib/nydusify/pkg/cache/spec.go | 4 +-- contrib/nydusify/pkg/checker/tool/builder.go | 3 +- contrib/nydusify/pkg/checker/tool/nydusd.go | 3 +- contrib/nydusify/pkg/converter/cache.go | 8 ++--- contrib/nydusify/pkg/converter/layer.go | 6 ++-- .../fileexporter.go} | 2 +- contrib/nydusify/pkg/parser/parser.go | 4 +-- contrib/nydusify/tests/e2e_test.go | 4 +-- contrib/nydusify/tests/image_test.go | 2 +- contrib/nydusify/tests/registry.go | 2 +- 17 files changed, 75 insertions(+), 51 deletions(-) create mode 100644 contrib/nydusify/.golangci.yml rename contrib/nydusify/pkg/metrics/{file_exporter/file_exporter.go => fileexporter/fileexporter.go} (95%) diff --git a/contrib/nydusify/.golangci.yml b/contrib/nydusify/.golangci.yml new file mode 100644 index 00000000000..90172836751 --- /dev/null +++ b/contrib/nydusify/.golangci.yml @@ -0,0 +1,23 @@ +# https://golangci-lint.run/usage/configuration#config-file + +linters: + enable: + - structcheck + - varcheck + - staticcheck + - unconvert + - gofmt + - goimports + - revive + - ineffassign + - vet + - unused + - misspell + disable: + - errcheck + +run: + deadline: 4m + skip-dirs: + - misc + diff --git a/contrib/nydusify/cmd/nydusify.go b/contrib/nydusify/cmd/nydusify.go index d02ddf0750d..e30005024e5 100644 --- a/contrib/nydusify/cmd/nydusify.go +++ b/contrib/nydusify/cmd/nydusify.go @@ -25,7 +25,7 @@ import ( "github.com/dragonflyoss/image-service/contrib/nydusify/pkg/converter" "github.com/dragonflyoss/image-service/contrib/nydusify/pkg/converter/provider" "github.com/dragonflyoss/image-service/contrib/nydusify/pkg/metrics" - "github.com/dragonflyoss/image-service/contrib/nydusify/pkg/metrics/file_exporter" + "github.com/dragonflyoss/image-service/contrib/nydusify/pkg/metrics/fileexporter" "github.com/dragonflyoss/image-service/contrib/nydusify/pkg/remote" "github.com/dragonflyoss/image-service/contrib/nydusify/pkg/utils" ) @@ -273,7 +273,7 @@ func main() { return err } - metrics.Register(file_exporter.New(filepath.Join(opt.WorkDir, "conversion_metrics.prom"))) + metrics.Register(fileexporter.New(filepath.Join(opt.WorkDir, "conversion_metrics.prom"))) defer metrics.Export() return cvt.Convert(context.Background()) @@ -312,6 +312,9 @@ func main() { } _, arch, err := provider.ExtractOsArch(c.String("platform")) + if err != nil { + return err + } checker, err := checker.New(checker.Opt{ WorkDir: c.String("work-dir"), diff --git a/contrib/nydusify/pkg/backend/backend.go b/contrib/nydusify/pkg/backend/backend.go index c2fe6ac338d..eed3f7f9f93 100644 --- a/contrib/nydusify/pkg/backend/backend.go +++ b/contrib/nydusify/pkg/backend/backend.go @@ -23,20 +23,20 @@ type Backend interface { // file handle and file path. Upload(ctx context.Context, blobID, blobPath string, blobSize int64, forcePush bool) (*ocispec.Descriptor, error) Check(blobID string) (bool, error) - Type() BackendType + Type() Type } // TODO: Directly forward blob data to storage backend -type BackendType = int +type Type = int const ( - OssBackend BackendType = iota + OssBackend Type = iota RegistryBackend ) -func blobDesc(size int64, blobId string) ocispec.Descriptor { - blobDigest := digest.NewDigestFromEncoded(digest.SHA256, blobId) +func blobDesc(size int64, blobID string) ocispec.Descriptor { + blobDigest := digest.NewDigestFromEncoded(digest.SHA256, blobID) desc := ocispec.Descriptor{ Digest: blobDigest, Size: size, diff --git a/contrib/nydusify/pkg/backend/oss.go b/contrib/nydusify/pkg/backend/oss.go index 7741ac46b4e..5a65cb3f932 100644 --- a/contrib/nydusify/pkg/backend/oss.go +++ b/contrib/nydusify/pkg/backend/oss.go @@ -120,14 +120,14 @@ func (b *OSSBackend) Upload(ctx context.Context, blobID, blobPath string, size i } blobSize := stat.Size() - var needMultiparts bool = false + var needMultiparts = false // Blob size bigger than 100MB, apply multiparts upload. if blobSize >= multipartsUploadThreshold { needMultiparts = true } start := time.Now() - var crc64 uint64 = 0 + var crc64 uint64 crc64ErrChan := make(chan error, 1) go func() { var e error @@ -237,6 +237,6 @@ func (b *OSSBackend) Check(blobID string) (bool, error) { return b.bucket.IsObjectExist(blobID) } -func (r *OSSBackend) Type() BackendType { +func (b *OSSBackend) Type() Type { return OssBackend } diff --git a/contrib/nydusify/pkg/backend/registry.go b/contrib/nydusify/pkg/backend/registry.go index 4d64d0f8d40..7624bcca2b2 100644 --- a/contrib/nydusify/pkg/backend/registry.go +++ b/contrib/nydusify/pkg/backend/registry.go @@ -40,7 +40,7 @@ func (r *Registry) Check(blobID string) (bool, error) { return true, nil } -func (r *Registry) Type() BackendType { +func (r *Registry) Type() Type { return RegistryBackend } diff --git a/contrib/nydusify/pkg/cache/cache.go b/contrib/nydusify/pkg/cache/cache.go index 76ba9102d34..e0aeec082d5 100644 --- a/contrib/nydusify/pkg/cache/cache.go +++ b/contrib/nydusify/pkg/cache/cache.go @@ -56,9 +56,9 @@ type Cache struct { // Remote is responsible for pulling & pushing cache image remote *remote.Remote // Store the pulled records from registry - pulledRecords map[digest.Digest]*CacheRecord + pulledRecords map[digest.Digest]*Record // Store the records prepared to push to registry - pushedRecords []*CacheRecord + pushedRecords []*Record } // New creates Nydus cache instance, @@ -67,14 +67,14 @@ func New(remote *remote.Remote, opt Opt) (*Cache, error) { opt: opt, remote: remote, // source_layer_chain_id -> cache_record - pulledRecords: make(map[digest.Digest]*CacheRecord), - pushedRecords: []*CacheRecord{}, + pulledRecords: make(map[digest.Digest]*Record), + pushedRecords: []*Record{}, } return cache, nil } -func (cache *Cache) recordToLayer(record *CacheRecord) (*ocispec.Descriptor, *ocispec.Descriptor) { +func (cache *Cache) recordToLayer(record *Record) (*ocispec.Descriptor, *ocispec.Descriptor) { bootstrapCacheMediaType := ocispec.MediaTypeImageLayerGzip if cache.opt.DockerV2Format { bootstrapCacheMediaType = images.MediaTypeDockerSchema2LayerGzip @@ -129,7 +129,7 @@ func (cache *Cache) exportRecordsToLayers() []ocispec.Descriptor { return layers } -func (cache *Cache) layerToRecord(layer *ocispec.Descriptor) *CacheRecord { +func (cache *Cache) layerToRecord(layer *ocispec.Descriptor) *Record { sourceChainIDStr, ok := layer.Annotations[utils.LayerAnnotationNydusSourceChainID] if !ok { return nil @@ -181,7 +181,7 @@ func (cache *Cache) layerToRecord(layer *ocispec.Descriptor) *CacheRecord { }, } } - return &CacheRecord{ + return &Record{ SourceChainID: sourceChainID, NydusBootstrapDesc: &bootstrapDesc, NydusBlobDesc: nydusBlobDesc, @@ -199,7 +199,7 @@ func (cache *Cache) layerToRecord(layer *ocispec.Descriptor) *CacheRecord { utils.LayerAnnotationNydusBlob: "true", }, } - return &CacheRecord{ + return &Record{ SourceChainID: sourceChainID, NydusBlobDesc: nydusBlobDesc, } @@ -208,9 +208,9 @@ func (cache *Cache) layerToRecord(layer *ocispec.Descriptor) *CacheRecord { return nil } -func mergeRecord(old, new *CacheRecord) *CacheRecord { +func mergeRecord(old, new *Record) *Record { if old == nil { - old = &CacheRecord{ + old = &Record{ SourceChainID: new.SourceChainID, } } @@ -228,8 +228,8 @@ func mergeRecord(old, new *CacheRecord) *CacheRecord { } func (cache *Cache) importRecordsFromLayers(layers []ocispec.Descriptor) { - pulledRecords := make(map[digest.Digest]*CacheRecord) - pushedRecords := []*CacheRecord{} + pulledRecords := make(map[digest.Digest]*Record) + pushedRecords := []*Record{} for _, layer := range layers { record := cache.layerToRecord(&layer) @@ -308,7 +308,7 @@ func (cache *Cache) Export(ctx context.Context) error { mediaType = images.MediaTypeDockerSchema2Manifest } - manifest := CacheManifest{ + manifest := Manifest{ MediaType: mediaType, Manifest: ocispec.Manifest{ Versioned: specs.Versioned{ @@ -355,7 +355,7 @@ func (cache *Cache) Import(ctx context.Context) error { return errors.Wrap(err, "Read cache manifest") } - var manifest CacheManifest + var manifest Manifest if err := json.Unmarshal(manifestBytes, &manifest); err != nil { return errors.Wrap(err, "Unmarshal cache manifest") } @@ -374,7 +374,7 @@ func (cache *Cache) Import(ctx context.Context) error { } // Check checks bootstrap & blob layer exists in registry or storage backend -func (cache *Cache) Check(ctx context.Context, layerChainID digest.Digest) (*CacheRecord, io.ReadCloser, io.ReadCloser, error) { +func (cache *Cache) Check(ctx context.Context, layerChainID digest.Digest) (*Record, io.ReadCloser, io.ReadCloser, error) { record, ok := cache.pulledRecords[layerChainID] if !ok { return nil, nil, nil, nil @@ -409,7 +409,7 @@ func (cache *Cache) Check(ctx context.Context, layerChainID digest.Digest) (*Cac } // Record puts new bootstrap & blob layer to cache record, it's a limited queue. -func (cache *Cache) Record(records []*CacheRecord) { +func (cache *Cache) Record(records []*Record) { moveFront := map[digest.Digest]bool{} for _, record := range records { moveFront[record.SourceChainID] = true diff --git a/contrib/nydusify/pkg/cache/cache_test.go b/contrib/nydusify/pkg/cache/cache_test.go index 6bc331d5a34..8d5a7f0249e 100644 --- a/contrib/nydusify/pkg/cache/cache_test.go +++ b/contrib/nydusify/pkg/cache/cache_test.go @@ -17,7 +17,7 @@ import ( "github.com/dragonflyoss/image-service/contrib/nydusify/pkg/utils" ) -func makeRecord(id int64, hashBlob bool) *CacheRecord { +func makeRecord(id int64, hashBlob bool) *Record { var blobDesc *ocispec.Descriptor idStr := strconv.FormatInt(id, 10) if hashBlob { @@ -27,7 +27,7 @@ func makeRecord(id int64, hashBlob bool) *CacheRecord { Size: id, } } - return &CacheRecord{ + return &Record{ SourceChainID: digest.FromString("chain-" + idStr), NydusBootstrapDesc: &ocispec.Descriptor{ MediaType: ocispec.MediaTypeImageLayerGzip, @@ -79,7 +79,7 @@ func testWithBackend(t *testing.T, _backend backend.Backend) { }) assert.Nil(t, err) - exported := []*CacheRecord{ + exported := []*Record{ makeRecord(1, true), makeRecord(2, true), makeRecord(3, false), @@ -105,7 +105,7 @@ func testWithBackend(t *testing.T, _backend backend.Backend) { } cache.importRecordsFromLayers(layers) - cache.Record([]*CacheRecord{ + cache.Record([]*Record{ makeRecord(4, true), makeRecord(5, true), }) diff --git a/contrib/nydusify/pkg/cache/spec.go b/contrib/nydusify/pkg/cache/spec.go index 90114629469..e96906624ef 100644 --- a/contrib/nydusify/pkg/cache/spec.go +++ b/contrib/nydusify/pkg/cache/spec.go @@ -9,12 +9,12 @@ import ( ocispec "github.com/opencontainers/image-spec/specs-go/v1" ) -type CacheManifest struct { +type Manifest struct { MediaType string `json:"mediaType,omitempty"` ocispec.Manifest } -type CacheRecord struct { +type Record struct { SourceChainID digest.Digest NydusBlobDesc *ocispec.Descriptor NydusBootstrapDesc *ocispec.Descriptor diff --git a/contrib/nydusify/pkg/checker/tool/builder.go b/contrib/nydusify/pkg/checker/tool/builder.go index 991deadb04d..7f921290372 100644 --- a/contrib/nydusify/pkg/checker/tool/builder.go +++ b/contrib/nydusify/pkg/checker/tool/builder.go @@ -32,8 +32,7 @@ func NewBuilder(binaryPath string) *Builder { // Check calls `nydus-image check` to parse Nydus bootstrap // and output debug information to specified JSON file. func (builder *Builder) Check(option BuilderOption) error { - var args []string - args = []string{ + args := []string{ "check", "--bootstrap", option.BootstrapPath, diff --git a/contrib/nydusify/pkg/checker/tool/nydusd.go b/contrib/nydusify/pkg/checker/tool/nydusd.go index 94c217c3e29..f7bfd00bdbb 100644 --- a/contrib/nydusify/pkg/checker/tool/nydusd.go +++ b/contrib/nydusify/pkg/checker/tool/nydusd.go @@ -157,8 +157,7 @@ func NewNydusd(conf NydusdConfig) (*Nydusd, error) { func (nydusd *Nydusd) Mount() error { nydusd.Umount() - var args []string - args = []string{ + args := []string{ "--config", nydusd.ConfigPath, "--mountpoint", diff --git a/contrib/nydusify/pkg/converter/cache.go b/contrib/nydusify/pkg/converter/cache.go index d7abcfbe400..fc953abdb83 100644 --- a/contrib/nydusify/pkg/converter/cache.go +++ b/contrib/nydusify/pkg/converter/cache.go @@ -63,12 +63,12 @@ func newCacheGlue( func (cg *cacheGlue) Pull( ctx context.Context, sourceLayerChainID digest.Digest, -) (*cache.CacheRecord, error) { +) (*cache.Record, error) { if cg.cache == nil { return nil, nil } - var cacheRecord *cache.CacheRecord + var cacheRecord *cache.Record // Using ChainID to ensure we can find corresponding overlayed // Nydus blob/bootstrap layer in cache records. @@ -78,7 +78,7 @@ func (cg *cacheGlue) Pull( "ChainID": sourceLayerChainID, }) // Pull the cached layer from cache image, then push to target namespace/repo, - // because the blob data is not shared between diffrent namespaces in registry, + // because the blob data is not shared between different namespaces in registry, // this operation ensures that Nydus image owns these layers. cacheRecord = _cacheRecord defer bootstrapReader.Close() @@ -176,7 +176,7 @@ func (cg *cacheGlue) Export( // conversion progress as much as possible cg.cache.Import(ctx) - cacheRecords := []*cache.CacheRecord{} + cacheRecords := []*cache.Record{} for _, layer := range buildLayers { record := layer.GetCacheRecord() cacheRecords = append(cacheRecords, &record) diff --git a/contrib/nydusify/pkg/converter/layer.go b/contrib/nydusify/pkg/converter/layer.go index 02b69d2ef5e..6486696ad31 100644 --- a/contrib/nydusify/pkg/converter/layer.go +++ b/contrib/nydusify/pkg/converter/layer.go @@ -44,7 +44,7 @@ type buildLayer struct { bootstrapsDir string dockerV2Format bool - cacheRecord *cache.CacheRecord + cacheRecord *cache.Record blobDesc *ocispec.Descriptor bootstrapDesc *ocispec.Descriptor bootstrapDiffID *digest.Digest @@ -304,11 +304,11 @@ func (layer *buildLayer) Build(ctx context.Context) error { return buildDone(nil) } -func (layer *buildLayer) GetCacheRecord() cache.CacheRecord { +func (layer *buildLayer) GetCacheRecord() cache.Record { if layer.cacheRecord != nil { return *layer.cacheRecord } - return cache.CacheRecord{ + return cache.Record{ SourceChainID: layer.source.ChainID(), NydusBlobDesc: layer.blobDesc, NydusBootstrapDesc: layer.bootstrapDesc, diff --git a/contrib/nydusify/pkg/metrics/file_exporter/file_exporter.go b/contrib/nydusify/pkg/metrics/fileexporter/fileexporter.go similarity index 95% rename from contrib/nydusify/pkg/metrics/file_exporter/file_exporter.go rename to contrib/nydusify/pkg/metrics/fileexporter/fileexporter.go index b9737c6862d..2940308f0ad 100644 --- a/contrib/nydusify/pkg/metrics/file_exporter/file_exporter.go +++ b/contrib/nydusify/pkg/metrics/fileexporter/fileexporter.go @@ -2,7 +2,7 @@ // // SPDX-License-Identifier: Apache-2.0 -package file_exporter +package fileexporter import ( "github.com/prometheus/client_golang/prometheus" diff --git a/contrib/nydusify/pkg/parser/parser.go b/contrib/nydusify/pkg/parser/parser.go index 1897b488494..535e83c7605 100644 --- a/contrib/nydusify/pkg/parser/parser.go +++ b/contrib/nydusify/pkg/parser/parser.go @@ -164,8 +164,8 @@ func (parser *Parser) PullNydusBootstrap(ctx context.Context, image *Image) (io. return reader, nil } -func (p *Parser) matchImagePlatform(desc *ocispec.Descriptor) bool { - if p.interestedArch == desc.Platform.Architecture && desc.Platform.OS == "linux" { +func (parser *Parser) matchImagePlatform(desc *ocispec.Descriptor) bool { + if parser.interestedArch == desc.Platform.Architecture && desc.Platform.OS == "linux" { return true } return false diff --git a/contrib/nydusify/tests/e2e_test.go b/contrib/nydusify/tests/e2e_test.go index d4081a53eef..d14057538eb 100644 --- a/contrib/nydusify/tests/e2e_test.go +++ b/contrib/nydusify/tests/e2e_test.go @@ -10,7 +10,7 @@ import ( func testBasicConvert(t *testing.T) { registry := NewRegistry(t) - defer registry.Destory(t) + defer registry.Destroy(t) registry.Build(t, "image-basic") nydusify := NewNydusify(registry, "image-basic", "image-basic-nydus", "") @@ -20,7 +20,7 @@ func testBasicConvert(t *testing.T) { func testConvertWithCache(t *testing.T) { registry := NewRegistry(t) - defer registry.Destory(t) + defer registry.Destroy(t) registry.Build(t, "image-basic") nydusify1 := NewNydusify(registry, "image-basic", "image-basic-nydus-1", "cache:v1") diff --git a/contrib/nydusify/tests/image_test.go b/contrib/nydusify/tests/image_test.go index a516bf6e7da..812fca27259 100644 --- a/contrib/nydusify/tests/image_test.go +++ b/contrib/nydusify/tests/image_test.go @@ -36,7 +36,7 @@ func transfer(t *testing.T, ref string) { func convert(t *testing.T, ref string) { registry := NewRegistry(t) - defer registry.Destory(t) + defer registry.Destroy(t) transfer(t, ref) nydusify := NewNydusify(registry, ref, fmt.Sprintf("%s-nydus", ref), "") nydusify.Convert(t) diff --git a/contrib/nydusify/tests/registry.go b/contrib/nydusify/tests/registry.go index 27f06782f8a..81e5fb776a9 100644 --- a/contrib/nydusify/tests/registry.go +++ b/contrib/nydusify/tests/registry.go @@ -48,7 +48,7 @@ func NewRegistry(t *testing.T) *Registry { } } -func (registry *Registry) Destory(t *testing.T) { +func (registry *Registry) Destroy(t *testing.T) { run(t, fmt.Sprintf("docker rm -f %s", registry.id), true) }