Merge pull request #14122 from xlson/publisher-rhel-final-aware

Corrects an issue where nightly rpms would be misstagged.

scripts/build/release_publisher/externalrelease.go

@@ -16,23 +16,31 @@ type releaseFromExternalContent struct {
 
 func (re releaseFromExternalContent) prepareRelease(baseArchiveUrl, whatsNewUrl string, releaseNotesUrl string, nightly bool) (*release, error) {
 	version := re.rawVersion[1:]
-	isBeta := strings.Contains(version, "beta")
+	beta := strings.Contains(version, "beta")
+	var rt ReleaseType
+	if beta {
+		rt = BETA
+	} else if nightly {
+		rt = NIGHTLY
+	} else {
+		rt = STABLE
+	}
 
 	builds := []build{}
 	for _, ba := range re.artifactConfigurations {
-		sha256, err := re.getter.getContents(fmt.Sprintf("%s.sha256", ba.getUrl(baseArchiveUrl, version, isBeta)))
+		sha256, err := re.getter.getContents(fmt.Sprintf("%s.sha256", ba.getUrl(baseArchiveUrl, version, rt)))
 		if err != nil {
 			return nil, err
 		}
-		builds = append(builds, newBuild(baseArchiveUrl, ba, version, isBeta, sha256))
+		builds = append(builds, newBuild(baseArchiveUrl, ba, version, rt, sha256))
 	}
 
 	r := release{
 		Version:         version,
 		ReleaseDate:     time.Now().UTC(),
-		Stable:          !isBeta && !nightly,
-		Beta:            isBeta,
-		Nightly:         nightly,
+		Stable:          rt.stable(),
+		Beta:            rt.beta(),
+		Nightly:         rt.nightly(),
 		WhatsNewUrl:     whatsNewUrl,
 		ReleaseNotesUrl: releaseNotesUrl,
 		Builds:          builds,

scripts/build/release_publisher/localrelease.go

@@ -18,6 +18,9 @@ type releaseLocalSources struct {
 }
 
 func (r releaseLocalSources) prepareRelease(baseArchiveUrl, whatsNewUrl string, releaseNotesUrl string, nightly bool) (*release, error) {
+	if !nightly {
+		return nil, errors.New("Local releases only supported for nightly builds.")
+	}
 	buildData := r.findBuilds(baseArchiveUrl)
 
 	rel := release{
@@ -70,7 +73,7 @@ func createBuildWalker(path string, data *buildData, archiveTypes []buildArtifac
 				data.version = version
 				data.builds = append(data.builds, build{
 					Os:     archive.os,
-					Url:    archive.getUrl(baseArchiveUrl, version, false),
+					Url:    archive.getUrl(baseArchiveUrl, version, NIGHTLY),
 					Sha256: string(shaBytes),
 					Arch:   archive.arch,
 				})

scripts/build/release_publisher/publisher.go

@@ -61,13 +61,33 @@ func (p *publisher) postRelease(r *release) error {
 	return nil
 }
 
+type ReleaseType int
+
+const (
+	STABLE ReleaseType = iota + 1
+	BETA
+	NIGHTLY
+)
+
+func (rt ReleaseType) beta() bool {
+	return rt == BETA
+}
+
+func (rt ReleaseType) stable() bool {
+	return rt == STABLE
+}
+
+func (rt ReleaseType) nightly() bool {
+	return rt == NIGHTLY
+}
+
 type buildArtifact struct {
 	os         string
 	arch       string
 	urlPostfix string
 }
 
-func (t buildArtifact) getUrl(baseArchiveUrl, version string, isBeta bool) string {
+func (t buildArtifact) getUrl(baseArchiveUrl, version string, releaseType ReleaseType) string {
 	prefix := "-"
 	rhelReleaseExtra := ""
 
@@ -75,7 +95,7 @@ func (t buildArtifact) getUrl(baseArchiveUrl, version string, isBeta bool) strin
 		prefix = "_"
 	}
 
-	if !isBeta && t.os == "rhel" {
+	if releaseType.stable() && t.os == "rhel" {
 		rhelReleaseExtra = "-1"
 	}
 
@@ -141,10 +161,10 @@ var buildArtifactConfigurations = []buildArtifact{
 	},
 }
 
-func newBuild(baseArchiveUrl string, ba buildArtifact, version string, isBeta bool, sha256 string) build {
+func newBuild(baseArchiveUrl string, ba buildArtifact, version string, rt ReleaseType, sha256 string) build {
 	return build{
 		Os:     ba.os,
-		Url:    ba.getUrl(baseArchiveUrl, version, isBeta),
+		Url:    ba.getUrl(baseArchiveUrl, version, rt),
 		Sha256: sha256,
 		Arch:   ba.arch,
 	}

scripts/build/release_publisher/publisher_test.go

@@ -4,44 +4,98 @@ import "testing"
 
 func TestPreparingReleaseFromRemote(t *testing.T) {
 
-	var builder releaseBuilder
-
-	versionIn := "v5.2.0-beta1"
-	expectedVersion := "5.2.0-beta1"
-	whatsNewUrl := "https://whatsnews.foo/"
-	relNotesUrl := "https://relnotes.foo/"
-	expectedArch := "amd64"
-	expectedOs := "linux"
-	buildArtifacts := []buildArtifact{{expectedOs, expectedArch, ".linux-amd64.tar.gz"}}
-
-	builder = releaseFromExternalContent{
-		getter:                 mockHttpGetter{},
-		rawVersion:             versionIn,
-		artifactConfigurations: buildArtifactConfigurations,
-	}
-
-	rel, _ := builder.prepareRelease("https://s3-us-west-2.amazonaws.com/grafana-releases/release/grafana", whatsNewUrl, relNotesUrl, false)
-
-	if !rel.Beta || rel.Stable {
-		t.Errorf("%s should have been tagged as beta (not stable), but wasn't	.", versionIn)
-	}
-
-	if rel.Version != expectedVersion {
-		t.Errorf("Expected version to be %s, but it was %s.", expectedVersion, rel.Version)
-	}
-
-	expectedBuilds := len(buildArtifacts)
-	if len(rel.Builds) != expectedBuilds {
-		t.Errorf("Expected %v builds, but got %v.", expectedBuilds, len(rel.Builds))
-	}
-
-	build := rel.Builds[0]
-	if build.Arch != expectedArch {
-		t.Errorf("Expected arch to be %v, but it was %v", expectedArch, build.Arch)
-	}
-
-	if build.Os != expectedOs {
-		t.Errorf("Expected arch to be %v, but it was %v", expectedOs, build.Os)
+	cases := []struct {
+		version         string
+		expectedVersion string
+		whatsNewUrl     string
+		relNotesUrl     string
+		nightly         bool
+		expectedBeta    bool
+		expectedStable  bool
+		expectedArch    string
+		expectedOs      string
+		expectedUrl     string
+		baseArchiveUrl  string
+		buildArtifacts  []buildArtifact
+	}{
+		{
+			version:         "v5.2.0-beta1",
+			expectedVersion: "5.2.0-beta1",
+			whatsNewUrl:     "https://whatsnews.foo/",
+			relNotesUrl:     "https://relnotes.foo/",
+			nightly:         false,
+			expectedBeta:    true,
+			expectedStable:  false,
+			expectedArch:    "amd64",
+			expectedOs:      "linux",
+			expectedUrl:     "https://s3-us-west-2.amazonaws.com/grafana-releases/release/grafana-5.2.0-beta1.linux-amd64.tar.gz",
+			baseArchiveUrl:  "https://s3-us-west-2.amazonaws.com/grafana-releases/release/grafana",
+			buildArtifacts:  []buildArtifact{{"linux", "amd64", ".linux-amd64.tar.gz"}},
+		},
+		{
+			version:         "v5.2.3",
+			expectedVersion: "5.2.3",
+			whatsNewUrl:     "https://whatsnews.foo/",
+			relNotesUrl:     "https://relnotes.foo/",
+			nightly:         false,
+			expectedBeta:    false,
+			expectedStable:  true,
+			expectedArch:    "amd64",
+			expectedOs:      "rhel",
+			expectedUrl:     "https://s3-us-west-2.amazonaws.com/grafana-releases/release/grafana-5.2.3-1.x86_64.rpm",
+			baseArchiveUrl:  "https://s3-us-west-2.amazonaws.com/grafana-releases/release/grafana",
+			buildArtifacts:  []buildArtifact{{"rhel", "amd64", ".x86_64.rpm"}},
+		},
+		{
+			version:         "v5.4.0-pre1asdf",
+			expectedVersion: "5.4.0-pre1asdf",
+			whatsNewUrl:     "https://whatsnews.foo/",
+			relNotesUrl:     "https://relnotes.foo/",
+			nightly:         true,
+			expectedBeta:    false,
+			expectedStable:  false,
+			expectedArch:    "amd64",
+			expectedOs:      "rhel",
+			expectedUrl:     "https://s3-us-west-2.amazonaws.com/grafana-releases/release/grafana-5.4.0-pre1asdf.x86_64.rpm",
+			baseArchiveUrl:  "https://s3-us-west-2.amazonaws.com/grafana-releases/release/grafana",
+			buildArtifacts:  []buildArtifact{{"rhel", "amd64", ".x86_64.rpm"}},
+		},
+	}
+
+	for _, test := range cases {
+		builder := releaseFromExternalContent{
+			getter:                 mockHttpGetter{},
+			rawVersion:             test.version,
+			artifactConfigurations: test.buildArtifacts,
+		}
+
+		rel, _ := builder.prepareRelease(test.baseArchiveUrl, test.whatsNewUrl, test.relNotesUrl, test.nightly)
+
+		if rel.Beta != test.expectedBeta || rel.Stable != test.expectedStable {
+			t.Errorf("%s should have been tagged as beta=%v, stable=%v.", test.version, test.expectedBeta, test.expectedStable)
+		}
+
+		if rel.Version != test.expectedVersion {
+			t.Errorf("Expected version to be %s, but it was %s.", test.expectedVersion, rel.Version)
+		}
+
+		expectedBuilds := len(test.buildArtifacts)
+		if len(rel.Builds) != expectedBuilds {
+			t.Errorf("Expected %v builds, but got %v.", expectedBuilds, len(rel.Builds))
+		}
+
+		build := rel.Builds[0]
+		if build.Arch != test.expectedArch {
+			t.Errorf("Expected arch to be %v, but it was %v", test.expectedArch, build.Arch)
+		}
+
+		if build.Os != test.expectedOs {
+			t.Errorf("Expected os to be %v, but it was %v", test.expectedOs, build.Os)
+		}
+
+		if build.Url != test.expectedUrl {
+			t.Errorf("Expected url to be %v, but it was %v", test.expectedUrl, build.Url)
+		}
 	}
 }
 
@@ -116,4 +170,9 @@ func TestPreparingReleaseFromLocal(t *testing.T) {
 	if build.Os != expectedOs {
 		t.Fatalf("Expected os to be %s, but was %s", expectedOs, build.Os)
 	}
+
+	_, err := builder.prepareRelease("", "", "", false)
+	if err == nil {
+		t.Error("Error was nil, but expected an error as the local releaser only supports nightly builds.")
+	}
 }