diff options
author | Vikram bir Singh <vikrambir.singh@docker.com> | 2019-09-03 19:11:21 +0000 |
---|---|---|
committer | Vikram bir Singh <vikrambir.singh@docker.com> | 2019-09-06 01:50:20 +0000 |
commit | ebf12dbda08633375ab12387255d3f617ee9be38 (patch) | |
tree | 1853b7c202055375337098cdcfff1e9781e42bd4 /layer/filestore.go | |
parent | f505abb6a7efbaf22ba9adbf31db3d750c26537f (diff) | |
download | docker-ebf12dbda08633375ab12387255d3f617ee9be38.tar.gz |
Reimplement iteration over fileInfos in getOrphan.
1. Reduce complexity due to nested if blocks by using early
return/continue
2. Improve logging
Changes suggested as a part of code review comments in 39748
Signed-off-by: Vikram bir Singh <vikrambir.singh@docker.com>
Diffstat (limited to 'layer/filestore.go')
-rw-r--r-- | layer/filestore.go | 54 |
1 files changed, 31 insertions, 23 deletions
diff --git a/layer/filestore.go b/layer/filestore.go index 37a927022a..a1726a5a40 100644 --- a/layer/filestore.go +++ b/layer/filestore.go @@ -14,7 +14,7 @@ import ( "github.com/docker/distribution" "github.com/docker/docker/pkg/ioutils" - "github.com/opencontainers/go-digest" + digest "github.com/opencontainers/go-digest" "github.com/pkg/errors" "github.com/sirupsen/logrus" ) @@ -317,32 +317,40 @@ func (fms *fileMetadataStore) getOrphan() ([]roLayer, error) { } for _, fi := range fileInfos { - if fi.IsDir() && strings.Contains(fi.Name(), "-removing") { - nameSplit := strings.Split(fi.Name(), "-") - dgst := digest.NewDigestFromEncoded(algorithm, nameSplit[0]) - if err := dgst.Validate(); err != nil { - logrus.Debugf("Ignoring invalid digest %s:%s", algorithm, nameSplit[0]) - } else { - chainID := ChainID(dgst) - chainFile := filepath.Join(fms.root, string(algorithm), fi.Name(), "cache-id") - contentBytes, err := ioutil.ReadFile(chainFile) - if err != nil { - logrus.WithError(err).WithField("digest", dgst).Error("cannot get cache ID") - } - cacheID := strings.TrimSpace(string(contentBytes)) - if cacheID == "" { - logrus.Errorf("invalid cache id value") - } - - l := &roLayer{ - chainID: chainID, - cacheID: cacheID, - } - orphanLayers = append(orphanLayers, *l) + if !fi.IsDir() || !strings.HasSuffix(fi.Name(), "-removing") { + continue + } + // At this stage, fi.Name value looks like <digest>-<random>-removing + // Split on '-' to get the digest value. + nameSplit := strings.Split(fi.Name(), "-") + dgst := digest.NewDigestFromEncoded(algorithm, nameSplit[0]) + if err := dgst.Validate(); err != nil { + logrus.WithError(err).WithField("digest", string(algorithm)+":"+nameSplit[0]).Debug("ignoring invalid digest") + continue + } + + chainFile := filepath.Join(fms.root, string(algorithm), fi.Name(), "cache-id") + contentBytes, err := ioutil.ReadFile(chainFile) + if err != nil { + if !os.IsNotExist(err) { + logrus.WithError(err).WithField("digest", dgst).Error("failed to read cache ID") } + continue + } + cacheID := strings.TrimSpace(string(contentBytes)) + if cacheID == "" { + logrus.Error("invalid cache ID") + continue } + + l := &roLayer{ + chainID: ChainID(dgst), + cacheID: cacheID, + } + orphanLayers = append(orphanLayers, *l) } } + return orphanLayers, nil } |