Merge pull request #1560 from nalind/pullup-2

"pull up" images when creating them, too

Author: rhatdan

containers.go

@@ -666,7 +666,7 @@ func (r *containerStore) create(id string, names []string, image, layer string,
 	if _, idInUse := r.byid[id]; idInUse {
 		return nil, ErrDuplicateID
 	}
-	names = dedupeNames(names)
+	names = dedupeStrings(names)
 	for _, name := range names {
 		if _, nameInUse := r.byname[name]; nameInUse {
 			return nil, fmt.Errorf("the container name %q is already in use by %s. You have to remove that container to be able to reuse that name: %w", name, r.byname[name].ID, ErrDuplicateName)

images.go

@@ -703,7 +703,7 @@ func (r *imageStore) create(id string, names []string, layer string, options Ima
 	if _, idInUse := r.byid[id]; idInUse {
 		return nil, fmt.Errorf("an image with ID %q already exists: %w", id, ErrDuplicateID)
 	}
-	names = dedupeNames(names)
+	names = dedupeStrings(names)
 	for _, name := range names {
 		if image, nameInUse := r.byname[name]; nameInUse {
 			return nil, fmt.Errorf("image name %q is already associated with image %q: %w", name, image.ID, ErrDuplicateName)
@@ -712,7 +712,7 @@ func (r *imageStore) create(id string, names []string, layer string, options Ima
 	image = &Image{
 		ID:             id,
 		Digest:         options.Digest,
-		Digests:        copyDigestSlice(options.Digests),
+		Digests:        dedupeDigests(options.Digests),
 		Names:          names,
 		NamesHistory:   copyStringSlice(options.NamesHistory),
 		TopLayer:       layer,
@@ -820,7 +820,7 @@ func (r *imageStore) removeName(image *Image, name string) {
 
 // The caller must hold r.inProcessLock for writing.
 func (i *Image) addNameToHistory(name string) {
-	i.NamesHistory = dedupeNames(append([]string{name}, i.NamesHistory...))
+	i.NamesHistory = dedupeStrings(append([]string{name}, i.NamesHistory...))
 }
 
 // Requires startWriting.

layers.go

@@ -1230,7 +1230,7 @@ func (r *layerStore) create(id string, parentLayer *Layer, names []string, mount
 	if duplicateLayer, idInUse := r.byid[id]; idInUse {
 		return duplicateLayer, -1, ErrDuplicateID
 	}
-	names = dedupeNames(names)
+	names = dedupeStrings(names)
 	for _, name := range names {
 		if _, nameInUse := r.byname[name]; nameInUse {
 			return nil, -1, ErrDuplicateName

store.go

@@ -1384,26 +1384,90 @@ func (s *store) CreateImage(id string, names []string, layer, metadata string, i
 		layer = ilayer.ID
 	}
 
-	var res *Image
-	err := s.writeToImageStore(func() error {
-		var options ImageOptions
+	var options ImageOptions
+	var namesToAddAfterCreating []string
+
+	if err := s.imageStore.startWriting(); err != nil {
+		return nil, err
+	}
+	defer s.imageStore.stopWriting()
+
+	// Check if the ID refers to an image in a read-only store -- we want
+	// to allow images in read-only stores to have their names changed, so
+	// if we find one, merge the new values in with what we know about the
+	// image that's already there.
+	if id != "" {
+		for _, is := range s.roImageStores {
+			store := is
+			if err := store.startReading(); err != nil {
+				return nil, err
+			}
+			defer store.stopReading()
+			if i, err := store.Get(id); err == nil {
+				// set information about this image in "options"
+				options = ImageOptions{
+					Metadata:     i.Metadata,
+					CreationDate: i.Created,
+					Digest:       i.Digest,
+					Digests:      copyDigestSlice(i.Digests),
+					NamesHistory: copyStringSlice(i.NamesHistory),
+				}
+				for _, key := range i.BigDataNames {
+					data, err := store.BigData(id, key)
+					if err != nil {
+						return nil, err
+					}
+					dataDigest, err := store.BigDataDigest(id, key)
+					if err != nil {
+						return nil, err
+					}
+					options.BigData = append(options.BigData, ImageBigDataOption{
+						Key:    key,
+						Data:   data,
+						Digest: dataDigest,
+					})
+				}
+				namesToAddAfterCreating = dedupeStrings(append(append([]string{}, i.Names...), names...))
+				break
+			}
+		}
+	}
 
-		if iOptions != nil {
-			options = *iOptions
-			options.Digests = copyDigestSlice(iOptions.Digests)
-			options.BigData = copyImageBigDataOptionSlice(iOptions.BigData)
-			options.NamesHistory = copyStringSlice(iOptions.NamesHistory)
-			options.Flags = copyStringInterfaceMap(iOptions.Flags)
+	// merge any passed-in options into "options" as best we can
+	if iOptions != nil {
+		if !iOptions.CreationDate.IsZero() {
+			options.CreationDate = iOptions.CreationDate
 		}
-		if options.CreationDate.IsZero() {
-			options.CreationDate = time.Now().UTC()
+		if iOptions.Digest != "" {
+			options.Digest = iOptions.Digest
 		}
+		options.Digests = append(options.Digests, copyDigestSlice(iOptions.Digests)...)
+		if iOptions.Metadata != "" {
+			options.Metadata = iOptions.Metadata
+		}
+		options.BigData = append(options.BigData, copyImageBigDataOptionSlice(iOptions.BigData)...)
+		options.NamesHistory = append(options.NamesHistory, copyStringSlice(iOptions.NamesHistory)...)
+		if options.Flags == nil {
+			options.Flags = make(map[string]interface{})
+		}
+		for k, v := range iOptions.Flags {
+			options.Flags[k] = v
+		}
+	}
+
+	if options.CreationDate.IsZero() {
+		options.CreationDate = time.Now().UTC()
+	}
+	if metadata != "" {
 		options.Metadata = metadata
+	}
 
-		var err error
-		res, err = s.imageStore.create(id, names, layer, options)
-		return err
-	})
+	res, err := s.imageStore.create(id, names, layer, options)
+	if err == nil && len(namesToAddAfterCreating) > 0 {
+		// set any names we pulled up from an additional image store, now that we won't be
+		// triggering a duplicate names error
+		err = s.imageStore.updateNames(res.ID, namesToAddAfterCreating, addNames)
+	}
 	return res, err
 }
 
@@ -2130,18 +2194,30 @@ func (s *store) Exists(id string) bool {
 	return s.containerStore.Exists(id)
 }
 
-func dedupeNames(names []string) []string {
-	seen := make(map[string]bool)
+func dedupeStrings(names []string) []string {
+	seen := make(map[string]struct{})
 	deduped := make([]string, 0, len(names))
 	for _, name := range names {
 		if _, wasSeen := seen[name]; !wasSeen {
-			seen[name] = true
+			seen[name] = struct{}{}
 			deduped = append(deduped, name)
 		}
 	}
 	return deduped
 }
 
+func dedupeDigests(digests []digest.Digest) []digest.Digest {
+	seen := make(map[digest.Digest]struct{})
+	deduped := make([]digest.Digest, 0, len(digests))
+	for _, d := range digests {
+		if _, wasSeen := seen[d]; !wasSeen {
+			seen[d] = struct{}{}
+			deduped = append(deduped, d)
+		}
+	}
+	return deduped
+}
+
 // Deprecated: Prone to race conditions, suggested alternatives are `AddNames` and `RemoveNames`.
 func (s *store) SetNames(id string, names []string) error {
 	return s.updateNames(id, names, setNames)
@@ -2156,7 +2232,7 @@ func (s *store) RemoveNames(id string, names []string) error {
 }
 
 func (s *store) updateNames(id string, names []string, op updateNameOperation) error {
-	deduped := dedupeNames(names)
+	deduped := dedupeStrings(names)
 
 	layerFound := false
 	if err := s.writeToLayerStore(func(rlstore rwLayerStore) error {
@@ -2188,11 +2264,12 @@ func (s *store) updateNames(id string, names []string, op updateNameOperation) e
 		if i, err := store.Get(id); err == nil {
 			// "pull up" the image so that we can change its names list
 			options := ImageOptions{
-				Metadata:     i.Metadata,
 				CreationDate: i.Created,
 				Digest:       i.Digest,
 				Digests:      copyDigestSlice(i.Digests),
+				Metadata:     i.Metadata,
 				NamesHistory: copyStringSlice(i.NamesHistory),
+				Flags:        copyStringInterfaceMap(i.Flags),
 			}
 			for _, key := range i.BigDataNames {
 				data, err := store.BigData(id, key)

utils.go

@@ -70,5 +70,5 @@ func applyNameOperation(oldNames []string, opParameters []string, op updateNameO
 	default:
 		return result, errInvalidUpdateNameOperation
 	}
-	return dedupeNames(result), nil
+	return dedupeStrings(result), nil
 }