Bug 1339754 - Fix tag sorting according to semantic versioning rules

Author: soltysh

pkg/image/api/helper.go

@@ -858,17 +858,17 @@ func LatestObservedTagGeneration(stream *ImageStream, tag string) int64 {
 }
 
 var (
-	reMajorSemantic = regexp.MustCompile(`^[\d]+$`)
-	reMinorSemantic = regexp.MustCompile(`^[\d]+\.[\d]+$`)
+	reMinorSemantic    = regexp.MustCompile(`^[\d]+\.[\d]+$`)
+	reMinorReplacement = regexp.MustCompile(`[\d]+\.[\d]+`)
+	reMinorWithPatch   = regexp.MustCompile(`^[\d]+\.[\d]+-\w+$`)
 )
 
 // PrioritizeTags orders a set of image tags with a few conventions:
 //
 // 1. the "latest" tag, if present, should be first
-// 2. any tags that represent a semantic major version ("5", "v5") should be next, in descending order
-// 3. any tags that represent a semantic minor version ("5.1", "v5.1") should be next, in descending order
-// 4. any tags that represent a full semantic version ("5.1.3-other", "v5.1.3-other") should be next, in descending order
-// 5. any remaining tags should be sorted in lexicographic order
+// 2. any tags that represent a semantic minor version ("5.1", "v5.1", "v5.1-rc1") should be next, in descending order
+// 3. any tags that represent a full semantic version ("5.1.3-other", "v5.1.3-other") should be next, in descending order
+// 4. any remaining tags should be sorted in lexicographic order
 //
 // The method updates the tags in place.
 func PrioritizeTags(tags []string) {
@@ -884,7 +884,7 @@ func PrioritizeTags(tags []string) {
 	}
 
 	exact := make(map[string]string)
-	var major, minor, micro semver.Versions
+	var minor, micro semver.Versions
 	other := make([]string, 0, len(remaining))
 	for _, tag := range remaining {
 		short := strings.TrimLeft(tag, "v")
@@ -894,28 +894,25 @@ func PrioritizeTags(tags []string) {
 			exact[v.String()] = tag
 			micro = append(micro, v)
 			continue
-		case reMajorSemantic.MatchString(short):
-			if v, err = semver.Parse(short + ".0.0"); err == nil {
+		case reMinorSemantic.MatchString(short):
+			if v, err = semver.Parse(short + ".0"); err == nil {
 				exact[v.String()] = tag
-				major = append(major, v)
+				minor = append(minor, v)
 				continue
 			}
-		case reMinorSemantic.MatchString(short):
-			if v, err = semver.Parse(short + ".0"); err == nil {
+		case reMinorWithPatch.MatchString(short):
+			repl := reMinorReplacement.FindString(short)
+			if v, err = semver.Parse(strings.Replace(short, repl, repl+".0", 1)); err == nil {
 				exact[v.String()] = tag
 				minor = append(minor, v)
 				continue
 			}
 		}
 		other = append(other, tag)
 	}
-	sort.Sort(sort.Reverse(major))
 	sort.Sort(sort.Reverse(minor))
 	sort.Sort(sort.Reverse(micro))
 	sort.Sort(sort.StringSlice(other))
-	for _, v := range major {
-		finalTags = append(finalTags, exact[v.String()])
-	}
 	for _, v := range minor {
 		finalTags = append(finalTags, exact[v.String()])
 	}

pkg/image/api/helper_test.go

@@ -1412,10 +1412,25 @@ func TestDockerImageReferenceEquality(t *testing.T) {
 }
 
 func TestPrioritizeTags(t *testing.T) {
-	tags := []string{"5", "other", "latest", "v5.5", "v6", "5.2.3", "v5.3.6-bother", "5.3.6-abba", "5.6"}
-	PrioritizeTags(tags)
-	if !reflect.DeepEqual(tags, []string{"latest", "v6", "5", "5.6", "v5.5", "v5.3.6-bother", "5.3.6-abba", "5.2.3", "other"}) {
-		t.Errorf("unexpected order: %v", tags)
+	tests := []struct {
+		tags     []string
+		expected []string
+	}{
+		{
+			tags:     []string{"other", "latest", "v5.5", "5.2.3", "v5.3.6-bother", "5.3.6-abba", "5.6"},
+			expected: []string{"latest", "5.6", "v5.5", "v5.3.6-bother", "5.3.6-abba", "5.2.3", "other"},
+		},
+		{
+			tags:     []string{"1.1-beta1", "1.2-rc1", "1.1-rc1", "1.1-beta2", "1.2-beta1", "1.2-alpha1", "1.2-beta4", "latest"},
+			expected: []string{"latest", "1.2-rc1", "1.2-beta4", "1.2-beta1", "1.2-alpha1", "1.1-rc1", "1.1-beta2", "1.1-beta1"},
+		},
+	}
+
+	for i, tc := range tests {
+		PrioritizeTags(tc.tags)
+		if !reflect.DeepEqual(tc.tags, tc.expected) {
+			t.Errorf("%d: unexpected order: %v", i, tc.tags)
+		}
 	}
 }
 

pkg/image/importer/importer_test.go

@@ -239,13 +239,13 @@ func TestImport(t *testing.T) {
 				},
 			},
 			expect: func(isi *api.ImageStreamImport, t *testing.T) {
-				if !reflect.DeepEqual(isi.Status.Repository.AdditionalTags, []string{"other"}) {
+				if !reflect.DeepEqual(isi.Status.Repository.AdditionalTags, []string{"v2"}) {
 					t.Errorf("unexpected additional tags: %#v", isi.Status.Repository)
 				}
 				if len(isi.Status.Repository.Images) != 5 {
 					t.Errorf("unexpected number of images: %#v", isi.Status.Repository.Images)
 				}
-				expectedTags := []string{"3", "v2", "v1", "3.1", "abc"}
+				expectedTags := []string{"3.1", "3", "abc", "other", "v1"}
 				for i, image := range isi.Status.Repository.Images {
 					if image.Status.Status != unversioned.StatusFailure || image.Status.Message != "Internal error occurred: no such manifest tag" {
 						t.Errorf("unexpected status %d: %#v", i, isi.Status.Repository.Images)