Browse code

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>

Vikram bir Singh authored on 2019/09/04 04:11:21
Showing 1 changed files
... ...
@@ -14,7 +14,7 @@ import (
14 14
 
15 15
 	"github.com/docker/distribution"
16 16
 	"github.com/docker/docker/pkg/ioutils"
17
-	"github.com/opencontainers/go-digest"
17
+	digest "github.com/opencontainers/go-digest"
18 18
 	"github.com/pkg/errors"
19 19
 	"github.com/sirupsen/logrus"
20 20
 )
... ...
@@ -317,32 +317,40 @@ func (fms *fileMetadataStore) getOrphan() ([]roLayer, error) {
317 317
 		}
318 318
 
319 319
 		for _, fi := range fileInfos {
320
-			if fi.IsDir() && strings.Contains(fi.Name(), "-removing") {
321
-				nameSplit := strings.Split(fi.Name(), "-")
322
-				dgst := digest.NewDigestFromEncoded(algorithm, nameSplit[0])
323
-				if err := dgst.Validate(); err != nil {
324
-					logrus.Debugf("Ignoring invalid digest %s:%s", algorithm, nameSplit[0])
325
-				} else {
326
-					chainID := ChainID(dgst)
327
-					chainFile := filepath.Join(fms.root, string(algorithm), fi.Name(), "cache-id")
328
-					contentBytes, err := ioutil.ReadFile(chainFile)
329
-					if err != nil {
330
-						logrus.WithError(err).WithField("digest", dgst).Error("cannot get cache ID")
331
-					}
332
-					cacheID := strings.TrimSpace(string(contentBytes))
333
-					if cacheID == "" {
334
-						logrus.Errorf("invalid cache id value")
335
-					}
336
-
337
-					l := &roLayer{
338
-						chainID: chainID,
339
-						cacheID: cacheID,
340
-					}
341
-					orphanLayers = append(orphanLayers, *l)
320
+			if !fi.IsDir() || !strings.HasSuffix(fi.Name(), "-removing") {
321
+				continue
322
+			}
323
+			// At this stage, fi.Name value looks like <digest>-<random>-removing
324
+			// Split on '-' to get the digest value.
325
+			nameSplit := strings.Split(fi.Name(), "-")
326
+			dgst := digest.NewDigestFromEncoded(algorithm, nameSplit[0])
327
+			if err := dgst.Validate(); err != nil {
328
+				logrus.WithError(err).WithField("digest", string(algorithm)+":"+nameSplit[0]).Debug("ignoring invalid digest")
329
+				continue
330
+			}
331
+
332
+			chainFile := filepath.Join(fms.root, string(algorithm), fi.Name(), "cache-id")
333
+			contentBytes, err := ioutil.ReadFile(chainFile)
334
+			if err != nil {
335
+				if !os.IsNotExist(err) {
336
+					logrus.WithError(err).WithField("digest", dgst).Error("failed to read cache ID")
342 337
 				}
338
+				continue
339
+			}
340
+			cacheID := strings.TrimSpace(string(contentBytes))
341
+			if cacheID == "" {
342
+				logrus.Error("invalid cache ID")
343
+				continue
343 344
 			}
345
+
346
+			l := &roLayer{
347
+				chainID: ChainID(dgst),
348
+				cacheID: cacheID,
349
+			}
350
+			orphanLayers = append(orphanLayers, *l)
344 351
 		}
345 352
 	}
353
+
346 354
 	return orphanLayers, nil
347 355
 }
348 356