Merge pull request #1731 from justaugustus/krel-ci-build

krel: Properly handle jobs with GCS Suffix set and don't attempt to set ACLs on K8s Infra buckets

cmd/krel/cmd/anago/push.go

@@ -190,7 +190,7 @@ func runPushRelease(
 	}
 
 	if err := release.NewPublisher().PublishVersion(
-		"release", opts.Version, opts.BuildDir, opts.Bucket, nil, false, false,
+		"release", opts.Version, opts.BuildDir, opts.Bucket, "", nil, false, false,
 	); err != nil {
 		return errors.Wrap(err, "publish release")
 	}

pkg/anago/release.go

@@ -103,7 +103,7 @@ type releaseImpl interface {
 	) error
 	ValidateImages(registry, version, buildPath string) error
 	PublishVersion(
-		buildType, version, buildDir, bucket string,
+		buildType, version, buildDir, bucket, gcsSuffix string,
 		versionMarkers []string,
 		privateBucket, fast bool,
 	) error
@@ -148,13 +148,13 @@ func (d *defaultReleaseImpl) ValidateImages(
 }
 
 func (d *defaultReleaseImpl) PublishVersion(
-	buildType, version, buildDir, bucket string,
+	buildType, version, buildDir, bucket, gcsSuffix string,
 	versionMarkers []string,
 	privateBucket, fast bool,
 ) error {
 	return release.
 		NewPublisher().
-		PublishVersion("release", version, buildDir, bucket, nil, false, false)
+		PublishVersion("release", version, buildDir, bucket, gcsSuffix, nil, false, false)
 }
 
 func (d *DefaultRelease) ValidateOptions() error {
@@ -228,7 +228,7 @@ func (d *DefaultRelease) PushArtifacts(versions []string) error {
 		}
 
 		if err := d.impl.PublishVersion(
-			"release", version, buildDir, bucket, nil, false, false,
+			"release", version, buildDir, bucket, "", nil, false, false,
 		); err != nil {
 			return errors.Wrap(err, "publish release")
 		}

pkg/build/build.go

@@ -17,9 +17,9 @@ limitations under the License.
 package build
 
 import (
-	"path/filepath"
-
 	"github.com/sirupsen/logrus"
+
+	"k8s.io/release/pkg/gcp/gcs"
 )
 
 var DefaultExtraVersionMarkers = []string{}
@@ -92,21 +92,21 @@ type Options struct {
 	ValidateRemoteImageDigests bool
 }
 
-// TODO: Support "release" buildType
+// TODO: Refactor so that version is not required as a parameter
 func (bi *Instance) getGCSBuildPath(version string) string {
-	gcsDest := bi.opts.BuildType
-
-	if bi.opts.GCSSuffix != "" {
-		gcsDest += "-" + bi.opts.GCSSuffix
-	}
-
-	if bi.opts.Fast {
-		gcsDest = filepath.Join(gcsDest, "fast")
+	// TODO: Parameterize this? Maybe a setter for defaults?
+	bucket := bi.opts.Bucket
+	if bi.opts.Bucket == "" {
+		bucket = "kubernetes-release-dev"
 	}
-	gcsDest = filepath.Join(gcsDest, version)
-	logrus.Infof("GCS destination is %s", gcsDest)
 
-	return gcsDest
+	return gcs.GetReleasePath(
+		bucket,
+		bi.opts.BuildType,
+		bi.opts.GCSSuffix,
+		version,
+		bi.opts.Fast,
+	)
 }
 
 func (bi *Instance) setBuildType() {

pkg/build/ci.go

@@ -123,14 +123,8 @@ func (bi *Instance) checkBuildExists() (bool, error) {
 		return false, nil
 	}
 
-	bucket := bi.opts.Bucket
-	if bi.opts.Bucket == "" {
-		bucket = "kubernetes-release-dev"
-	}
-
-	gcsDest := bi.getGCSBuildPath(version)
+	gcsBuildRoot := bi.getGCSBuildPath(bi.opts.Version)
 
-	gcsBuildRoot := filepath.Join(bucket, gcsDest)
 	kubernetesTar := filepath.Join(gcsBuildRoot, release.KubernetesTar)
 	binPath := filepath.Join(gcsBuildRoot, "bin")
 

pkg/build/push.go

@@ -105,6 +105,11 @@ func (bi *Instance) Push() error {
 	if err != nil {
 		return errors.Wrap(err, "find latest version")
 	}
+
+	if version == "" {
+		return errors.New("cannot push an empty version")
+	}
+
 	logrus.Infof("Latest version is %s", version)
 
 	if err := bi.CheckReleaseBucket(); err != nil {
@@ -141,8 +146,14 @@ func (bi *Instance) Push() error {
 	// Publish release to GCS
 	extraVersionMarkers := bi.opts.ExtraVersionMarkers
 	if err := release.NewPublisher().PublishVersion(
-		bi.opts.BuildType, version, bi.opts.BuildDir, bi.opts.Bucket, extraVersionMarkers,
-		bi.opts.PrivateBucket, bi.opts.Fast,
+		bi.opts.BuildType,
+		version,
+		bi.opts.BuildDir,
+		bi.opts.Bucket,
+		bi.opts.GCSSuffix,
+		extraVersionMarkers,
+		bi.opts.PrivateBucket,
+		bi.opts.Fast,
 	); err != nil {
 		return errors.Wrap(err, "publish release")
 	}

pkg/gcp/gcs/gcs.go

@@ -18,6 +18,7 @@ package gcs
 
 import (
 	"os"
+	"path/filepath"
 	"strings"
 
 	"github.com/pkg/errors"
@@ -118,6 +119,69 @@ func bucketCopy(src, dst string, opts *Options) error {
 	return nil
 }
 
+// GetReleasePath returns a GCS path to retrieve builds from or push builds to
+//
+// Expected destination format:
+//   gs://<bucket>/<buildType>[-<gcsSuffix>][/fast][/<version>]
+func GetReleasePath(
+	bucket, buildType, gcsSuffix, version string,
+	fast bool) string {
+	return getPath(
+		bucket,
+		buildType,
+		gcsSuffix,
+		version,
+		"release",
+		fast,
+	)
+}
+
+// GetMarkerPath returns a GCS path where version markers should be stored
+//
+// Expected destination format:
+//   gs://<bucket>/<buildType>[-<gcsSuffix>]
+func GetMarkerPath(
+	bucket, buildType, gcsSuffix string) string {
+	return getPath(
+		bucket,
+		buildType,
+		gcsSuffix,
+		"",
+		"marker",
+		false,
+	)
+}
+
+// GetReleasePath returns a GCS path to retrieve builds from or push builds to
+//
+// Expected destination format:
+//   gs://<bucket>/<buildType>[-<gcsSuffix>][/fast][/<version>]
+// TODO: Support "release" buildType
+func getPath(
+	bucket, buildType, gcsSuffix, version, pathType string,
+	fast bool) string {
+	gcsPath := bucket
+	gcsPath = filepath.Join(gcsPath, buildType)
+
+	if gcsSuffix != "" {
+		gcsPath += "-" + gcsSuffix
+	}
+
+	if pathType == "release" {
+		if fast {
+			gcsPath = filepath.Join(gcsPath, "fast")
+		}
+
+		if version != "" {
+			gcsPath = filepath.Join(gcsPath, version)
+		}
+	}
+
+	logrus.Infof("GCS path is %s", gcsPath)
+
+	return gcsPath
+}
+
 // NormalizeGCSPath takes a gcs path and ensures that the `GcsPrefix` is
 // prepended to it.
 func NormalizeGCSPath(gcsPath string) string {