gh2gcs: Improve log/error messages, usage text, and other cosmetic fixes

- Use new octal format (0oNNN) for os.FileMode()

Signed-off-by: Stephen Augustus <[email protected]>

cmd/gh2gcs/BUILD.bazel

@@ -8,12 +8,6 @@ go_library(
     deps = ["//cmd/gh2gcs/cmd:go_default_library"],
 )
 
-go_binary(
-    name = "kubepkg",
-    embed = [":go_default_library"],
-    visibility = ["//visibility:public"],
-)
-
 filegroup(
     name = "package-srcs",
     srcs = glob(["**"]),
@@ -30,3 +24,9 @@ filegroup(
     tags = ["automanaged"],
     visibility = ["//visibility:public"],
 )
+
+go_binary(
+    name = "gh2gcs",
+    embed = [":go_default_library"],
+    visibility = ["//visibility:public"],
+)

cmd/gh2gcs/cmd/root.go

@@ -31,9 +31,9 @@ import (
 
 // rootCmd represents the base command when called without any subcommands
 var rootCmd = &cobra.Command{
-	Use:               "gh2gcs --org kubernetes --repo release [--tag v0.0.0]",
+	Use:               "gh2gcs --org kubernetes --repo release --bucket <bucket> --release-dir <release-dir> [--tag v0.0.0] [--include-prereleases] [--output-dir <temp-dir>]",
 	Short:             "gh2gcs uploads GitHub releases to Google Cloud Storage",
-	Example:           "gh2gcs --org kubernetes --repo release --tag v0.0.0",
+	Example:           "gh2gcs --org kubernetes --repo release --bucket k8s-staging-release-test --release-dir release --tag v0.0.0",
 	SilenceUsage:      true,
 	SilenceErrors:     true,
 	PersistentPreRunE: initLogging,
@@ -69,60 +69,54 @@ func init() {
 		"org",
 		// TODO: Remove test value
 		"containernetworking",
-		// TODO: Improve usage text
-		"org",
+		"GitHub org/user",
 	)
 
 	rootCmd.PersistentFlags().StringVar(
 		&opts.repo,
 		"repo",
 		// TODO: Remove test value
 		"plugins",
-		// TODO: Improve usage text
-		"repo",
+		"GitHub repo",
 	)
 
 	// TODO: This should be a string array to accept multiple tags
 	rootCmd.PersistentFlags().StringVar(
 		&opts.tag,
 		"tag",
 		"",
-		// TODO: Improve usage text
-		"tag",
+		"release tag to upload to GCS",
 	)
 
 	rootCmd.PersistentFlags().BoolVar(
 		&opts.includePrereleases,
 		"include-prereleases",
+		// TODO: Need to wire this
 		false,
-		// TODO: Improve usage text
-		"include-prereleases",
+		"specifies whether prerelease assets should be uploaded to GCS",
 	)
 
 	rootCmd.PersistentFlags().StringVar(
 		&opts.bucket,
 		"bucket",
 		// TODO: Remove test value
 		"k8s-staging-release-test",
-		// TODO: Improve usage text
-		"bucket",
+		"GCS bucket to upload to",
 	)
 
 	rootCmd.PersistentFlags().StringVar(
 		&opts.releaseDir,
 		"release-dir",
 		// TODO: Remove test value
 		"augustus/release",
-		// TODO: Improve usage text
-		"release-dir",
+		"directory to upload to within the specified GCS bucket",
 	)
 
 	rootCmd.PersistentFlags().StringVar(
 		&opts.outputDir,
 		"output-dir",
 		"",
-		// TODO: Improve usage text
-		"output-dir",
+		"local directory for releases to be downloaded to",
 	)
 
 	rootCmd.PersistentFlags().StringVar(
@@ -138,17 +132,12 @@ func initLogging(*cobra.Command, []string) error {
 }
 
 func run(opts *options) error {
-	if err := gcp.PreCheck(); err != nil {
-		return errors.Wrap(err, "pre-checking for GCP package usage")
+	if err := opts.SetAndValidate(); err != nil {
+		return errors.Wrap(err, "validating gh2gcs options")
 	}
 
-	if opts.outputDir == "" {
-		tmpDir, err := ioutil.TempDir("", "gh2gcs")
-		if err != nil {
-			return err
-		}
-
-		opts.outputDir = tmpDir
+	if err := gcp.PreCheck(); err != nil {
+		return errors.Wrap(err, "pre-checking for GCP package usage")
 	}
 
 	// Create a real GitHub API client
@@ -169,7 +158,7 @@ func run(opts *options) error {
 	} else {
 		releaseTags, err := gh.GetReleaseTags(opts.org, opts.repo, opts.includePrereleases)
 		if err != nil {
-			return err
+			return errors.Wrap(err, "getting release tags")
 		}
 
 		releaseConfig.Tags = releaseTags
@@ -179,19 +168,30 @@ func run(opts *options) error {
 
 	for _, rc := range uploadConfig.ReleaseConfigs {
 		if err := gh2gcs.DownloadReleases(&rc, gh, opts.outputDir); err != nil {
-			return err
+			return errors.Wrap(err, "downloading release assets")
 		}
 
 		if err := gh2gcs.Upload(&rc, gh, opts.outputDir); err != nil {
-			return err
+			return errors.Wrap(err, "uploading release assets to GCS")
 		}
 	}
 
 	return nil
 }
 
-// Validate verifies if all set options are valid
-func (o *options) Validate() error {
-	// TODO: Add validation logic for options
+// SetAndValidate sets some default options and verifies if options are valid
+func (o *options) SetAndValidate() error {
+	logrus.Info("Validating gh2gcs options...")
+
+	// TODO: Temp dir should cleanup after itself
+	if o.outputDir == "" {
+		tmpDir, err := ioutil.TempDir("", "gh2gcs")
+		if err != nil {
+			return errors.Wrap(err, "creating temp directory")
+		}
+
+		o.outputDir = tmpDir
+	}
+
 	return nil
 }

pkg/gcp/gcs/gcs.go

@@ -32,13 +32,15 @@ var (
 	recursiveFlag  = "-r"
 )
 
+// CopyToGCS copies a local directory to the specified GCS path
 func CopyToGCS(src, gcsPath string, recursive, concurrent bool) error {
 	gcsPath = normalizeGCSPath(gcsPath)
 
 	logrus.Infof("Copying %s to GCS (%s)", src, gcsPath)
 	return bucketCopy(src, gcsPath, recursive, concurrent)
 }
 
+// CopyToLocal copies a GCS path to the specified local directory
 func CopyToLocal(gcsPath, dst string, recursive, concurrent bool) error {
 	gcsPath = normalizeGCSPath(gcsPath)
 

pkg/gh2gcs/gh2gcs.go

@@ -23,10 +23,14 @@ import (
 	"k8s.io/release/pkg/github"
 )
 
+// Config contains a slice of `ReleaseConfig` to be used when unmarshalling a
+// yaml config containing multiple repository configs.
 type Config struct {
 	ReleaseConfigs []ReleaseConfig
 }
 
+// ReleaseConfig contains source (GitHub) and destination (GCS) information
+// to perform a copy/upload operation using gh2gcs.
 type ReleaseConfig struct {
 	Org        string
 	Repo       string
@@ -35,6 +39,8 @@ type ReleaseConfig struct {
 	ReleaseDir string
 }
 
+// DownloadReleases downloads release assets to a local directory
+// Assets to download are derived from the tags specified in `ReleaseConfig`.
 func DownloadReleases(releaseCfg *ReleaseConfig, ghClient *github.GitHub, outputDir string) error {
 	tags := releaseCfg.Tags
 	if err := ghClient.DownloadReleaseAssets(releaseCfg.Org, releaseCfg.Repo, tags, outputDir); err != nil {
@@ -44,6 +50,8 @@ func DownloadReleases(releaseCfg *ReleaseConfig, ghClient *github.GitHub, output
 	return nil
 }
 
+// Upload copies a set of release assets from local directory to GCS
+// Assets to upload are derived from the tags specified in `ReleaseConfig`.
 func Upload(releaseCfg *ReleaseConfig, ghClient *github.GitHub, outputDir string) error {
 	uploadBase := filepath.Join(outputDir, releaseCfg.Org, releaseCfg.Repo)
 	gcsPath := filepath.Join(releaseCfg.GCSBucket, releaseCfg.ReleaseDir)

pkg/github/github.go

@@ -360,6 +360,7 @@ func (t TagsPerBranch) addIfNotExisting(branch, tag string) {
 // Releases returns a list of GitHub releases for the provided `owner` and
 // `repo`. If `includePrereleases` is `true`, then the resulting slice will
 // also contain pre/drafted releases.
+// TODO: Create a more descriptive method name and update references
 func (g *GitHub) Releases(owner, repo string, includePrereleases bool) ([]*github.RepositoryRelease, error) {
 	allReleases, _, err := g.client.ListReleases(
 		context.Background(), owner, repo, nil,
@@ -388,7 +389,7 @@ func (g *GitHub) Releases(owner, repo string, includePrereleases bool) ([]*githu
 func (g *GitHub) GetReleaseTags(owner, repo string, includePrereleases bool) ([]string, error) {
 	releases, err := g.Releases(owner, repo, includePrereleases)
 	if err != nil {
-		return nil, err
+		return nil, errors.Wrap(err, "getting releases")
 	}
 
 	releaseTags := []string{}
@@ -399,14 +400,16 @@ func (g *GitHub) GetReleaseTags(owner, repo string, includePrereleases bool) ([]
 	return releaseTags, nil
 }
 
+// DownloadReleaseAssets downloads a set of GitHub release assets to an
+// `outputDir`. Assets to download are derived from the `releaseTags`.
 func (g *GitHub) DownloadReleaseAssets(owner, repo string, releaseTags []string, outputDir string) error {
 	var releases []*github.RepositoryRelease
 
 	if len(releaseTags) > 0 {
 		for _, tag := range releaseTags {
 			release, _, err := g.client.GetReleaseByTag(context.Background(), owner, repo, tag)
 			if err != nil {
-				return err
+				return errors.Wrap(err, "getting release tags")
 			}
 
 			releases = append(releases, release)
@@ -420,31 +423,35 @@ func (g *GitHub) DownloadReleaseAssets(owner, repo string, releaseTags []string,
 		logrus.Infof("Download assets for %s/%s@%s", owner, repo, releaseTag)
 
 		releaseDir := filepath.Join(outputDir, owner, repo, releaseTag)
-		if err := os.MkdirAll(releaseDir, os.FileMode(0777)); err != nil {
-			return err
+		if err := os.MkdirAll(releaseDir, os.FileMode(0o777)); err != nil {
+			return errors.Wrap(err, "creating output directory for release assets")
 		}
 
 		logrus.Infof("Writing assets to %s", releaseDir)
 
 		assets := release.Assets
 		for _, asset := range assets {
+			if asset.GetID() == 0 {
+				return errors.New("asset ID should never be zero")
+			}
+
 			logrus.Infof("GitHub asset ID: %v, download URL: %s", *asset.ID, *asset.BrowserDownloadURL)
 			assetBody, _, err := g.client.DownloadReleaseAsset(context.Background(), owner, repo, *asset.ID)
 			if err != nil {
-				return err
+				return errors.Wrap(err, "downloading release assets")
 			}
 
 			filename := *asset.Name
 			absFile := filepath.Join(releaseDir, filename)
 			defer assetBody.Close()
 			assetFile, err := os.Create(absFile)
 			if err != nil {
-				return err
+				return errors.Wrap(err, "creating release asset file")
 			}
 
 			defer assetFile.Close()
 			if _, err := io.Copy(assetFile, assetBody); err != nil {
-				return err
+				return errors.Wrap(err, "copying release asset to file")
 			}
 		}
 	}