Add content gc ref labels from containers, images, and snapshots

Currently the objects which can retain content from labels
are limited. This limitation has required clients to work
around this and and in some cases add outside reference
counting (e.g. buildkit keeping content for snapshots).
Updated the logic to treat content and snapshot labels equally
and simplified the code in the process.

Signed-off-by: Derek McGowan <derek@mcgstyle.net>

metadata/gc.go

@@ -64,6 +64,18 @@ func scanRoots(ctx context.Context, tx *bolt.Tx, nc chan<- gc.Node) error {
 	// iterate through each namespace
 	v1c := v1bkt.Cursor()
 
+	// cerr indicates the scan did not successfully send all
+	// the roots. The scan does not need to be cancelled but
+	// must return error at the end.
+	var cerr error
+	fn := func(n gc.Node) {
+		select {
+		case nc <- n:
+		case <-ctx.Done():
+			cerr = ctx.Err()
+		}
+	}
+
 	for k, v := v1c.First(); k != nil; k, v = v1c.Next() {
 		if v != nil {
 			continue
@@ -92,11 +104,7 @@ func scanRoots(ctx context.Context, tx *bolt.Tx, nc chan<- gc.Node) error {
 					}
 				}
 
-				select {
+				fn(gcnode(ResourceLease, ns, string(k)))
-				case nc <- gcnode(ResourceLease, ns, string(k)):
-				case <-ctx.Done():
-					return ctx.Err()
-				}
 
 				// Emit content and snapshots as roots instead of implementing
 				// in references. Since leases cannot be referenced there is
@@ -106,11 +114,7 @@ func scanRoots(ctx context.Context, tx *bolt.Tx, nc chan<- gc.Node) error {
 				cbkt := libkt.Bucket(bucketKeyObjectContent)
 				if cbkt != nil {
 					if err := cbkt.ForEach(func(k, v []byte) error {
-						select {
+						fn(gcnode(ResourceContent, ns, string(k)))
-						case nc <- gcnode(ResourceContent, ns, string(k)):
-						case <-ctx.Done():
-							return ctx.Err()
-						}
 						return nil
 					}); err != nil {
 						return err
@@ -126,11 +130,7 @@ func scanRoots(ctx context.Context, tx *bolt.Tx, nc chan<- gc.Node) error {
 						snbkt := sbkt.Bucket(sk)
 
 						return snbkt.ForEach(func(k, v []byte) error {
-							select {
+							fn(gcnode(ResourceSnapshot, ns, fmt.Sprintf("%s/%s", sk, k)))
-							case nc <- gcnode(ResourceSnapshot, ns, fmt.Sprintf("%s/%s", sk, k)):
-							case <-ctx.Done():
-								return ctx.Err()
-							}
 							return nil
 						})
 					}); err != nil {
@@ -141,11 +141,7 @@ func scanRoots(ctx context.Context, tx *bolt.Tx, nc chan<- gc.Node) error {
 				ibkt := libkt.Bucket(bucketKeyObjectIngests)
 				if ibkt != nil {
 					if err := ibkt.ForEach(func(k, v []byte) error {
-						select {
+						fn(gcnode(ResourceIngest, ns, string(k)))
-						case nc <- gcnode(ResourceIngest, ns, string(k)):
-						case <-ctx.Done():
-							return ctx.Err()
-						}
 						return nil
 					}); err != nil {
 						return err
@@ -168,18 +164,9 @@ func scanRoots(ctx context.Context, tx *bolt.Tx, nc chan<- gc.Node) error {
 				target := ibkt.Bucket(k).Bucket(bucketKeyTarget)
 				if target != nil {
 					contentKey := string(target.Get(bucketKeyDigest))
-					select {
+					fn(gcnode(ResourceContent, ns, contentKey))
-					case nc <- gcnode(ResourceContent, ns, contentKey):
-					case <-ctx.Done():
-						return ctx.Err()
-					}
 				}
-				return sendSnapshotRefs(ns, ibkt.Bucket(k), func(n gc.Node) {
+				return sendLabelRefs(ns, ibkt.Bucket(k), fn)
-					select {
-					case nc <- n:
-					case <-ctx.Done():
-					}
-				})
 			}); err != nil {
 				return err
 			}
@@ -200,11 +187,7 @@ func scanRoots(ctx context.Context, tx *bolt.Tx, nc chan<- gc.Node) error {
 					if ea == nil || expThreshold.After(*ea) {
 						return nil
 					}
-					select {
+					fn(gcnode(ResourceIngest, ns, string(k)))
-					case nc <- gcnode(ResourceIngest, ns, string(k)):
-					case <-ctx.Done():
-						return ctx.Err()
-					}
 					return nil
 				}); err != nil {
 					return err
@@ -216,7 +199,12 @@ func scanRoots(ctx context.Context, tx *bolt.Tx, nc chan<- gc.Node) error {
 					if v != nil {
 						return nil
 					}
-					return sendRootRef(ctx, nc, gcnode(ResourceContent, ns, string(k)), cbkt.Bucket(k))
+
+					if isRootRef(cbkt.Bucket(k)) {
+						fn(gcnode(ResourceContent, ns, string(k)))
+					}
+
+					return nil
 				}); err != nil {
 					return err
 				}
@@ -229,23 +217,15 @@ func scanRoots(ctx context.Context, tx *bolt.Tx, nc chan<- gc.Node) error {
 				if v != nil {
 					return nil
 				}
-				snapshotter := string(cbkt.Bucket(k).Get(bucketKeySnapshotter))
+
+				cibkt := cbkt.Bucket(k)
+				snapshotter := string(cibkt.Get(bucketKeySnapshotter))
 				if snapshotter != "" {
-					ss := string(cbkt.Bucket(k).Get(bucketKeySnapshotKey))
+					ss := string(cibkt.Get(bucketKeySnapshotKey))
-					select {
+					fn(gcnode(ResourceSnapshot, ns, fmt.Sprintf("%s/%s", snapshotter, ss)))
-					case nc <- gcnode(ResourceSnapshot, ns, fmt.Sprintf("%s/%s", snapshotter, ss)):
-					case <-ctx.Done():
-						return ctx.Err()
-					}
 				}
 
-				// TODO: Send additional snapshot refs through labels
+				return sendLabelRefs(ns, cibkt, fn)
-				return sendSnapshotRefs(ns, cbkt.Bucket(k), func(n gc.Node) {
-					select {
-					case nc <- n:
-					case <-ctx.Done():
-					}
-				})
 			}); err != nil {
 				return err
 			}
@@ -263,15 +243,17 @@ func scanRoots(ctx context.Context, tx *bolt.Tx, nc chan<- gc.Node) error {
 					if v != nil {
 						return nil
 					}
-
+					if isRootRef(snbkt.Bucket(k)) {
-					return sendRootRef(ctx, nc, gcnode(ResourceSnapshot, ns, fmt.Sprintf("%s/%s", sk, k)), snbkt.Bucket(k))
+						fn(gcnode(ResourceSnapshot, ns, fmt.Sprintf("%s/%s", sk, k)))
+					}
+					return nil
 				})
 			}); err != nil {
 				return err
 			}
 		}
 	}
-	return nil
+	return cerr
 }
 
 func references(ctx context.Context, tx *bolt.Tx, node gc.Node, fn func(gc.Node)) error {
@@ -282,10 +264,7 @@ func references(ctx context.Context, tx *bolt.Tx, node gc.Node, fn func(gc.Node)
 			return nil
 		}
 
-		if err := sendSnapshotRefs(node.Namespace, bkt, fn); err != nil {
+		return sendLabelRefs(node.Namespace, bkt, fn)
-			return err
-		}
-		return sendContentRefs(node.Namespace, bkt, fn)
 	} else if node.Type == ResourceSnapshot {
 		parts := strings.SplitN(node.Key, "/", 2)
 		if len(parts) != 2 {
@@ -304,7 +283,7 @@ func references(ctx context.Context, tx *bolt.Tx, node gc.Node, fn func(gc.Node)
 			fn(gcnode(ResourceSnapshot, node.Namespace, fmt.Sprintf("%s/%s", ss, pv)))
 		}
 
-		return sendSnapshotRefs(node.Namespace, bkt, fn)
+		return sendLabelRefs(node.Namespace, bkt, fn)
 	} else if node.Type == ResourceIngest {
 		// Send expected value
 		bkt := getBucket(tx, bucketKeyVersion, []byte(node.Namespace), bucketKeyObjectContent, bucketKeyObjectIngests, []byte(node.Key))
@@ -456,25 +435,8 @@ func remove(ctx context.Context, tx *bolt.Tx, node gc.Node) error {
 	return nil
 }
 
-// sendSnapshotRefs sends all snapshot references referred to by the labels in the bkt
+// sendLabelRefs sends all snapshot and content references referred to by the labels in the bkt
-func sendSnapshotRefs(ns string, bkt *bolt.Bucket, fn func(gc.Node)) error {
+func sendLabelRefs(ns string, bkt *bolt.Bucket, fn func(gc.Node)) error {
-	lbkt := bkt.Bucket(bucketKeyObjectLabels)
-	if lbkt != nil {
-		lc := lbkt.Cursor()
-
-		for k, v := lc.Seek(labelGCSnapRef); k != nil && strings.HasPrefix(string(k), string(labelGCSnapRef)); k, v = lc.Next() {
-			snapshotter := k[len(labelGCSnapRef):]
-			if i := bytes.IndexByte(snapshotter, '/'); i >= 0 {
-				snapshotter = snapshotter[:i]
-			}
-			fn(gcnode(ResourceSnapshot, ns, fmt.Sprintf("%s/%s", snapshotter, v)))
-		}
-	}
-	return nil
-}
-
-// sendContentRefs sends all content references referred to by the labels in the bkt
-func sendContentRefs(ns string, bkt *bolt.Bucket, fn func(gc.Node)) error {
 	lbkt := bkt.Bucket(bucketKeyObjectLabels)
 	if lbkt != nil {
 		lc := lbkt.Cursor()
@@ -490,6 +452,15 @@ func sendContentRefs(ns string, bkt *bolt.Bucket, fn func(gc.Node)) error {
 
 			fn(gcnode(ResourceContent, ns, string(v)))
 		}
+
+		for k, v := lc.Seek(labelGCSnapRef); k != nil && strings.HasPrefix(string(k), string(labelGCSnapRef)); k, v = lc.Next() {
+			snapshotter := k[len(labelGCSnapRef):]
+			if i := bytes.IndexByte(snapshotter, '/'); i >= 0 {
+				snapshotter = snapshotter[:i]
+			}
+			fn(gcnode(ResourceSnapshot, ns, fmt.Sprintf("%s/%s", snapshotter, v)))
+		}
+
 	}
 	return nil
 }
@@ -506,17 +477,6 @@ func isRootRef(bkt *bolt.Bucket) bool {
 	return false
 }
 
-func sendRootRef(ctx context.Context, nc chan<- gc.Node, n gc.Node, bkt *bolt.Bucket) error {
-	if isRootRef(bkt) {
-		select {
-		case nc <- n:
-		case <-ctx.Done():
-			return ctx.Err()
-		}
-	}
-	return nil
-}
-
 func gcnode(t gc.ResourceType, ns, key string) gc.Node {
 	return gc.Node{
 		Type:      t,

metadata/gc_test.go

@@ -43,9 +43,16 @@ func TestGCRoots(t *testing.T) {
 	alters := []alterFunc{
 		addImage("ns1", "image1", dgst(1), nil),
 		addImage("ns1", "image2", dgst(2), labelmap(string(labelGCSnapRef)+"overlay", "sn2")),
+		addImage("ns2", "image3", dgst(10), labelmap(string(labelGCContentRef), dgst(11).String())),
 		addContainer("ns1", "container1", "overlay", "sn4", nil),
 		addContainer("ns1", "container2", "overlay", "sn5", labelmap(string(labelGCSnapRef)+"overlay", "sn6")),
-		addContainer("ns1", "container3", "overlay", "sn7", labelmap(string(labelGCSnapRef)+"overlay/anything-1", "sn8", string(labelGCSnapRef)+"overlay/anything-2", "sn9")),
+		addContainer("ns1", "container3", "overlay", "sn7", labelmap(
+			string(labelGCSnapRef)+"overlay/anything-1", "sn8",
+			string(labelGCSnapRef)+"overlay/anything-2", "sn9",
+			string(labelGCContentRef), dgst(7).String())),
+		addContainer("ns1", "container4", "", "", labelmap(
+			string(labelGCContentRef)+".0", dgst(8).String(),
+			string(labelGCContentRef)+".1", dgst(9).String())),
 		addContent("ns1", dgst(1), nil),
 		addContent("ns1", dgst(2), nil),
 		addContent("ns1", dgst(3), nil),
@@ -88,10 +95,15 @@ func TestGCRoots(t *testing.T) {
 	expected := []gc.Node{
 		gcnode(ResourceContent, "ns1", dgst(1).String()),
 		gcnode(ResourceContent, "ns1", dgst(2).String()),
+		gcnode(ResourceContent, "ns1", dgst(7).String()),
+		gcnode(ResourceContent, "ns1", dgst(8).String()),
+		gcnode(ResourceContent, "ns1", dgst(9).String()),
 		gcnode(ResourceContent, "ns2", dgst(2).String()),
 		gcnode(ResourceContent, "ns2", dgst(4).String()),
 		gcnode(ResourceContent, "ns2", dgst(5).String()),
 		gcnode(ResourceContent, "ns2", dgst(6).String()),
+		gcnode(ResourceContent, "ns2", dgst(10).String()),
+		gcnode(ResourceContent, "ns2", dgst(11).String()),
 		gcnode(ResourceSnapshot, "ns1", "overlay/sn2"),
 		gcnode(ResourceSnapshot, "ns1", "overlay/sn3"),
 		gcnode(ResourceSnapshot, "ns1", "overlay/sn4"),
@@ -253,6 +265,9 @@ func TestGCRefs(t *testing.T) {
 		addSnapshot("ns1", "btrfs", "sn1", "", nil),
 		addSnapshot("ns2", "overlay", "sn1", "", nil),
 		addSnapshot("ns2", "overlay", "sn2", "sn1", nil),
+		addSnapshot("ns2", "overlay", "sn3", "", labelmap(
+			string(labelGCContentRef), dgst(1).String(),
+			string(labelGCContentRef)+".keep-me", dgst(6).String())),
 	}
 
 	refs := map[gc.Node][]gc.Node{
@@ -293,6 +308,10 @@ func TestGCRefs(t *testing.T) {
 		gcnode(ResourceSnapshot, "ns2", "overlay/sn2"): {
 			gcnode(ResourceSnapshot, "ns2", "overlay/sn1"),
 		},
+		gcnode(ResourceSnapshot, "ns2", "overlay/sn3"): {
+			gcnode(ResourceContent, "ns2", dgst(1).String()),
+			gcnode(ResourceContent, "ns2", dgst(6).String()),
+		},
 		gcnode(ResourceIngest, "ns1", "ingest-1"): nil,
 		gcnode(ResourceIngest, "ns2", "ingest-2"): {
 			gcnode(ResourceContent, "ns2", dgst(8).String()),