Merge branch 'fix-registry-push-tags' of https://github.com/codeaholics/docker into codeaholics-fix-registry-push-tags

Docker-DCO-1.1-Signed-off-by: Michael Crosby <michael@crosbymichael.com> (github: crosbymichael)

server.go

@@ -1209,122 +1209,98 @@ func (srv *Server) ImagePull(localName string, tag string, out io.Writer, sf *ut
 }
 
 // Retrieve the all the images to be uploaded in the correct order
-// Note: we can't use a map as it is not ordered
+func (srv *Server) getImageList(localRepo map[string]string) ([]string, map[string][]string, error) {
-func (srv *Server) getImageList(localRepo map[string]string) ([][]*registry.ImgData, error) {
+	var (
-	imgList := map[string]*registry.ImgData{}
+		imageList   []string
-	depGraph := utils.NewDependencyGraph()
+		imagesSeen  map[string]bool     = make(map[string]bool)
+		tagsByImage map[string][]string = make(map[string][]string)
+	)
 
 	for tag, id := range localRepo {
-		img, err := srv.runtime.graph.Get(id)
+		var imageListForThisTag []string
+
+		tagsByImage[id] = append(tagsByImage[id], tag)
+
+		for img, err := srv.runtime.graph.Get(id); img != nil; img, err = img.GetParent() {
 			if err != nil {
-			return nil, err
+				return nil, nil, err
-		}
-		depGraph.NewNode(img.ID)
-		img.WalkHistory(func(current *Image) error {
-			imgList[current.ID] = &registry.ImgData{
-				ID:  current.ID,
-				Tag: tag,
-			}
-			parent, err := current.GetParent()
-			if err != nil {
-				return err
-			}
-			if parent == nil {
-				return nil
-			}
-			depGraph.NewNode(parent.ID)
-			depGraph.AddDependency(current.ID, parent.ID)
-			return nil
-		})
 			}
 
-	traversalMap, err := depGraph.GenerateTraversalMap()
+			if imagesSeen[img.ID] {
-	if err != nil {
+				// This image is already on the list, we can ignore it and all its parents
-		return nil, err
+				break
 			}
 
-	utils.Debugf("Traversal map: %v", traversalMap)
+			imagesSeen[img.ID] = true
-	result := [][]*registry.ImgData{}
+			imageListForThisTag = append(imageListForThisTag, img.ID)
-	for _, round := range traversalMap {
-		dataRound := []*registry.ImgData{}
-		for _, imgID := range round {
-			dataRound = append(dataRound, imgList[imgID])
-		}
-		result = append(result, dataRound)
-	}
-	return result, nil
 		}
 
-func flatten(slc [][]*registry.ImgData) []*registry.ImgData {
+		// reverse the image list for this tag (so the "most"-parent image is first)
-	result := []*registry.ImgData{}
+		for i, j := 0, len(imageListForThisTag)-1; i < j; i, j = i+1, j-1 {
-	for _, x := range slc {
+			imageListForThisTag[i], imageListForThisTag[j] = imageListForThisTag[j], imageListForThisTag[i]
-		result = append(result, x...)
 		}
-	return result
+
+		// append to main image list
+		imageList = append(imageList, imageListForThisTag...)
+	}
+
+	utils.Debugf("Image list: %v", imageList)
+	utils.Debugf("Tags by image: %v", tagsByImage)
+
+	return imageList, tagsByImage, nil
 }
 
 func (srv *Server) pushRepository(r *registry.Registry, out io.Writer, localName, remoteName string, localRepo map[string]string, sf *utils.StreamFormatter) error {
 	out = utils.NewWriteFlusher(out)
-	imgList, err := srv.getImageList(localRepo)
+	utils.Debugf("Local repo: %s", localRepo)
+	imgList, tagsByImage, err := srv.getImageList(localRepo)
 	if err != nil {
 		return err
 	}
-	flattenedImgList := flatten(imgList)
+
 	out.Write(sf.FormatStatus("", "Sending image list"))
 
 	var repoData *registry.RepositoryData
-	repoData, err = r.PushImageJSONIndex(remoteName, flattenedImgList, false, nil)
+	var imageIndex []*registry.ImgData
+
+	for imgId, tags := range tagsByImage {
+		for _, tag := range tags {
+			imageIndex = append(imageIndex, &registry.ImgData{
+				ID:  imgId,
+				Tag: tag,
+			})
+		}
+	}
+
+	repoData, err = r.PushImageJSONIndex(remoteName, imageIndex, false, nil)
 	if err != nil {
 		return err
 	}
 
 	for _, ep := range repoData.Endpoints {
 		out.Write(sf.FormatStatus("", "Pushing repository %s (%d tags)", localName, len(localRepo)))
-		// This section can not be parallelized (each round depends on the previous one)
-		for i, round := range imgList {
-			// FIXME: This section can be parallelized
-			for _, elem := range round {
-				var pushTags func() error
-				pushTags = func() error {
-					if i < (len(imgList) - 1) {
-						// Only tag the top layer in the repository
-						return nil
-					}
 
-					out.Write(sf.FormatStatus("", "Pushing tags for rev [%s] on {%s}", utils.TruncateID(elem.ID), ep+"repositories/"+remoteName+"/tags/"+elem.Tag))
+		for _, imgId := range imgList {
-					if err := r.PushRegistryTag(remoteName, elem.ID, elem.Tag, ep, repoData.Tokens); err != nil {
+			if r.LookupRemoteImage(imgId, ep, repoData.Tokens) {
-						return err
+				out.Write(sf.FormatStatus("", "Image %s already pushed, skipping", utils.TruncateID(imgId)))
-					}
+			} else {
-					return nil
+				if _, err := srv.pushImage(r, out, remoteName, imgId, ep, repoData.Tokens, sf); err != nil {
-				}
-				if _, exists := repoData.ImgList[elem.ID]; exists {
-					if err := pushTags(); err != nil {
-						return err
-					}
-					out.Write(sf.FormatProgress(utils.TruncateID(elem.ID), "Image already pushed, skipping", nil))
-					continue
-				} else if r.LookupRemoteImage(elem.ID, ep, repoData.Tokens) {
-					if err := pushTags(); err != nil {
-						return err
-					}
-					out.Write(sf.FormatProgress(utils.TruncateID(elem.ID), "Image already pushed, skipping", nil))
-					continue
-				}
-				checksum, err := srv.pushImage(r, out, remoteName, elem.ID, ep, repoData.Tokens, sf)
-				if err != nil {
 					// FIXME: Continue on error?
 					return err
 				}
-				elem.Checksum = checksum
+			}
 
-				if err := pushTags(); err != nil {
+			for _, tag := range tagsByImage[imgId] {
+				out.Write(sf.FormatStatus("", "Pushing tag for rev [%s] on {%s}", utils.TruncateID(imgId), ep+"repositories/"+remoteName+"/tags/"+tag))
+
+				if err := r.PushRegistryTag(remoteName, imgId, tag, ep, repoData.Tokens); err != nil {
 					return err
 				}
 			}
 		}
 	}
 
-	if _, err := r.PushImageJSONIndex(remoteName, flattenedImgList, true, repoData.Endpoints); err != nil {
+	if _, err := r.PushImageJSONIndex(remoteName, imageIndex, true, repoData.Endpoints); err != nil {
 		return err
 	}
 

utils/utils.go

@@ -894,122 +894,6 @@ func UserLookup(uid string) (*User, error) {
 	return nil, fmt.Errorf("User not found in /etc/passwd")
 }
 
-type DependencyGraph struct {
-	nodes map[string]*DependencyNode
-}
-
-type DependencyNode struct {
-	id   string
-	deps map[*DependencyNode]bool
-}
-
-func NewDependencyGraph() DependencyGraph {
-	return DependencyGraph{
-		nodes: map[string]*DependencyNode{},
-	}
-}
-
-func (graph *DependencyGraph) addNode(node *DependencyNode) string {
-	if graph.nodes[node.id] == nil {
-		graph.nodes[node.id] = node
-	}
-	return node.id
-}
-
-func (graph *DependencyGraph) NewNode(id string) string {
-	if graph.nodes[id] != nil {
-		return id
-	}
-	nd := &DependencyNode{
-		id:   id,
-		deps: map[*DependencyNode]bool{},
-	}
-	graph.addNode(nd)
-	return id
-}
-
-func (graph *DependencyGraph) AddDependency(node, to string) error {
-	if graph.nodes[node] == nil {
-		return fmt.Errorf("Node %s does not belong to this graph", node)
-	}
-
-	if graph.nodes[to] == nil {
-		return fmt.Errorf("Node %s does not belong to this graph", to)
-	}
-
-	if node == to {
-		return fmt.Errorf("Dependency loops are forbidden!")
-	}
-
-	graph.nodes[node].addDependency(graph.nodes[to])
-	return nil
-}
-
-func (node *DependencyNode) addDependency(to *DependencyNode) bool {
-	node.deps[to] = true
-	return node.deps[to]
-}
-
-func (node *DependencyNode) Degree() int {
-	return len(node.deps)
-}
-
-// The magic happens here ::
-func (graph *DependencyGraph) GenerateTraversalMap() ([][]string, error) {
-	Debugf("Generating traversal map. Nodes: %d", len(graph.nodes))
-	result := [][]string{}
-	processed := map[*DependencyNode]bool{}
-	// As long as we haven't processed all nodes...
-	for len(processed) < len(graph.nodes) {
-		// Use a temporary buffer for processed nodes, otherwise
-		// nodes that depend on each other could end up in the same round.
-		tmpProcessed := []*DependencyNode{}
-		for _, node := range graph.nodes {
-			// If the node has more dependencies than what we have cleared,
-			// it won't be valid for this round.
-			if node.Degree() > len(processed) {
-				continue
-			}
-			// If it's already processed, get to the next one
-			if processed[node] {
-				continue
-			}
-			// It's not been processed yet and has 0 deps. Add it!
-			// (this is a shortcut for what we're doing below)
-			if node.Degree() == 0 {
-				tmpProcessed = append(tmpProcessed, node)
-				continue
-			}
-			// If at least one dep hasn't been processed yet, we can't
-			// add it.
-			ok := true
-			for dep := range node.deps {
-				if !processed[dep] {
-					ok = false
-					break
-				}
-			}
-			// All deps have already been processed. Add it!
-			if ok {
-				tmpProcessed = append(tmpProcessed, node)
-			}
-		}
-		Debugf("Round %d: found %d available nodes", len(result), len(tmpProcessed))
-		// If no progress has been made this round,
-		// that means we have circular dependencies.
-		if len(tmpProcessed) == 0 {
-			return nil, fmt.Errorf("Could not find a solution to this dependency graph")
-		}
-		round := []string{}
-		for _, nd := range tmpProcessed {
-			round = append(round, nd.id)
-			processed[nd] = true
-		}
-		result = append(result, round)
-	}
-	return result, nil
-}
-
 // An StatusError reports an unsuccessful exit by a command.
 type StatusError struct {
 	Status     string

utils/utils_test.go

@@ -416,62 +416,6 @@ func TestParseRelease(t *testing.T) {
 	assertParseRelease(t, "3.8.0-19-generic", &KernelVersionInfo{Kernel: 3, Major: 8, Minor: 0, Flavor: "19-generic"}, 0)
 }
 
-func TestDependencyGraphCircular(t *testing.T) {
-	g1 := NewDependencyGraph()
-	a := g1.NewNode("a")
-	b := g1.NewNode("b")
-	g1.AddDependency(a, b)
-	g1.AddDependency(b, a)
-	res, err := g1.GenerateTraversalMap()
-	if res != nil {
-		t.Fatalf("Expected nil result")
-	}
-	if err == nil {
-		t.Fatalf("Expected error (circular graph can not be resolved)")
-	}
-}
-
-func TestDependencyGraph(t *testing.T) {
-	g1 := NewDependencyGraph()
-	a := g1.NewNode("a")
-	b := g1.NewNode("b")
-	c := g1.NewNode("c")
-	d := g1.NewNode("d")
-	g1.AddDependency(b, a)
-	g1.AddDependency(c, a)
-	g1.AddDependency(d, c)
-	g1.AddDependency(d, b)
-	res, err := g1.GenerateTraversalMap()
-
-	if err != nil {
-		t.Fatalf("%s", err)
-	}
-
-	if res == nil {
-		t.Fatalf("Unexpected nil result")
-	}
-
-	if len(res) != 3 {
-		t.Fatalf("Expected map of length 3, found %d instead", len(res))
-	}
-
-	if len(res[0]) != 1 || res[0][0] != "a" {
-		t.Fatalf("Expected [a], found %v instead", res[0])
-	}
-
-	if len(res[1]) != 2 {
-		t.Fatalf("Expected 2 nodes for step 2, found %d", len(res[1]))
-	}
-
-	if (res[1][0] != "b" && res[1][1] != "b") || (res[1][0] != "c" && res[1][1] != "c") {
-		t.Fatalf("Expected [b, c], found %v instead", res[1])
-	}
-
-	if len(res[2]) != 1 || res[2][0] != "d" {
-		t.Fatalf("Expected [d], found %v instead", res[2])
-	}
-}
-
 func TestParsePortMapping(t *testing.T) {
 	data, err := PartParser("ip:public:private", "192.168.1.1:80:8080")
 	if err != nil {