Merge pull request #1126 from JamesLaverack/krel-release-notes-changes

Improve krel release-notes local k/sig-release repository handling
kubernetes · May 7, 2020 · a8fd0e2 · a8fd0e2
2 parents 7fa1431 + 0601453
commit a8fd0e2
Show file tree

Hide file tree

Showing 3 changed files with 39 additions and 47 deletions.
diff --git a/cmd/krel/cmd/release_notes.go b/cmd/krel/cmd/release_notes.go
@@ -87,17 +87,15 @@ permissions to your fork of k/sig-release and k-sigs/release-notes.`,
 }
 
 type releaseNotesOptions struct {
-	tag                    string
-	draftOrg               string
-	draftRepo              string
-	createDraftPR          bool
-	createWebsitePR        bool
-	outputDir              string
-	sigreleaseForkPath     string
-	kubernetessigsForkPath string
-	Format                 string
-	websiteOrg             string
-	websiteRepo            string
+	tag             string
+	draftOrg        string
+	draftRepo       string
+	createDraftPR   bool
+	createWebsitePR bool
+	outputDir       string
+	Format          string
+	websiteOrg      string
+	websiteRepo     string
 }
 
 type releaseNotesResult struct {
@@ -173,20 +171,6 @@ func init() {
 		"The format for notes output (options: markdown, json)",
 	)
 
-	releaseNotesCmd.PersistentFlags().StringVar(
-		&releaseNotesOpts.kubernetessigsForkPath,
-		"kubernetes-sigs-fork-path",
-		filepath.Join(os.TempDir(), "k8s-sigs"),
-		"fork kubernetes-sigs/release-notes and output a copy of the json release notes to this directory",
-	)
-
-	releaseNotesCmd.PersistentFlags().StringVar(
-		&releaseNotesOpts.sigreleaseForkPath,
-		"sigrelease-fork-path",
-		filepath.Join(os.TempDir(), "k8s-sigrelease"),
-		"fork k/sig-release and output a copy of the release notes draft to this directory",
-	)
-
 	rootCmd.AddCommand(releaseNotesCmd)
 }
 
@@ -328,7 +312,7 @@ func validateWebsitePROptions() error {
 }
 
 // createDraftPR pushes the release notes draft to the users fork
-func createDraftPR(tag string, result *releaseNotesResult) error {
+func createDraftPR(tag string, result *releaseNotesResult) (err error) {
 	s, err := util.TagStringToSemver(tag)
 	if err != nil {
 		return errors.Wrapf(err, "no valid tag: %v", tag)
@@ -338,14 +322,18 @@ func createDraftPR(tag string, result *releaseNotesResult) error {
 
 	// Prepare the fork of k/sig-release
 	sigReleaseRepo, err := prepareFork(
-		branchname, releaseNotesOpts.sigreleaseForkPath,
+		branchname,
 		git.DefaultGithubOrg, git.DefaultGithubReleaseRepo,
 		releaseNotesOpts.draftOrg, releaseNotesOpts.draftRepo,
 	)
 	if err != nil {
 		return errors.Wrap(err, "preparing local fork of kubernetes/sig-release")
 	}
 
+	defer func() {
+		err = sigReleaseRepo.Cleanup()
+	}()
+
 	// generate the notes
 	targetdir := filepath.Join(sigReleaseRepo.Dir(), "releases", fmt.Sprintf("release-%d.%d", s.Major, s.Minor))
 	logrus.Debugf("release notes markdown will be written to %v", targetdir)
@@ -375,19 +363,19 @@ func createDraftPR(tag string, result *releaseNotesResult) error {
 }
 
 // prepareFork Prepare a branch a repo
-func prepareFork(branchName, repoPath, upstreamOrg, upstreamRepo, myOrg, myRepo string) (repo *git.Repo, err error) {
+func prepareFork(branchName, upstreamOrg, upstreamRepo, myOrg, myRepo string) (repo *git.Repo, err error) {
 	// checkout the upstream repository
 	logrus.Infof("cloning/updating repository %s/%s", upstreamOrg, upstreamRepo)
 
-	repo, err = git.CloneOrOpenGitHubRepo(
-		repoPath, upstreamOrg, upstreamRepo, true,
+	repo, err = git.CleanCloneGitHubRepo(
+		upstreamOrg, upstreamRepo, false,
 	)
 	if err != nil {
 		return nil, errors.Wrapf(err, "cloning %s/%s", upstreamOrg, upstreamRepo)
 	}
 
 	// test if the fork remote is already existing
-	url := git.GetRepoURL(myOrg, myRepo, true)
+	url := git.GetRepoURL(myOrg, myRepo, false)
 	if repo.HasRemote(userForkName, url) {
 		logrus.Infof(
 			"Using already existing remote %v (%v) in repository",
@@ -401,14 +389,8 @@ func prepareFork(branchName, repoPath, upstreamOrg, upstreamRepo, myOrg, myRepo
 		}
 	}
 
-	// verify the branch doesn't already exist on the user's fork
-	err = repo.HasRemoteBranch(branchName)
-	if err == nil {
-		return nil, errors.Errorf("remote repo already has a branch named %s", branchName)
-	}
-
 	// checkout the new branch
-	err = repo.Checkout("-b", branchName)
+	err = repo.Checkout("-B", branchName)
 	if err != nil {
 		return nil, errors.Wrapf(err, "creating new branch %s", branchName)
 	}
@@ -483,8 +465,8 @@ func processJSONOutput(repoPath string) error {
 }
 
 // createWebsitePR creates the JSON version of the release notes and pushes them to a user fork
-func createWebsitePR(tag string, result *releaseNotesResult) error {
-	_, err := util.TagStringToSemver(tag)
+func createWebsitePR(tag string, result *releaseNotesResult) (err error) {
+	_, err = util.TagStringToSemver(tag)
 	if err != nil {
 		return errors.Wrapf(err, "no valid tag: %v", tag)
 	}
@@ -494,12 +476,15 @@ func createWebsitePR(tag string, result *releaseNotesResult) error {
 
 	// checkout kubernetes-sigs/release-notes
 	k8sSigsRepo, err := prepareFork(
-		branchname, releaseNotesOpts.kubernetessigsForkPath, defaultKubernetesSigsOrg,
+		branchname, defaultKubernetesSigsOrg,
 		defaultKubernetesSigsRepo, releaseNotesOpts.websiteOrg, releaseNotesOpts.websiteRepo,
 	)
 	if err != nil {
 		return errors.Wrap(err, "preparing local fork branch")
 	}
+	defer func() {
+		err = k8sSigsRepo.Cleanup()
+	}()
 
 	// add a reference to the new json file in assets.ts
 	if err := addReferenceToAssetsFile(k8sSigsRepo.Dir(), jsonNotesFilename); err != nil {

diff --git a/docs/krel/release-notes.md b/docs/krel/release-notes.md
@@ -40,9 +40,7 @@ Before running `krel release-notes` export your GitHub token to \$GITHUB_TOKEN:
       --draft-repo string                  the name of the fork of k/sig-release, the Release Notes Draft PR will be created from this repository (default "sig-release")
       --format string                      The format for notes output (options: markdown, json) (default "markdown")
   -h, --help                               help for release-notes
-      --kubernetes-sigs-fork-path string   fork kubernetes-sigs/release-notes and output a copy of the json release notes to this directory (default "/tmp/k8s-sigs")
   -o, --output-dir string                  output a copy of the release notes to this directory (default ".")
-      --sigrelease-fork-path string        fork k/sig-release and output a copy of the release notes draft to this directory (default "/tmp/k8s-sigrelease")
   -t, --tag string                         version tag for the notes
       --website-org string                 a Github organization owner of the fork of kuberntets-sigs/release-notes where the Website PR will be created
       --website-repo string                the name of the fork of kuberntets-sigs/release-notes, the Release Notes Draft PR will be created from this repository (default "release-notes")

diff --git a/pkg/git/git.go b/pkg/git/git.go
@@ -198,9 +198,18 @@ func CloneOrOpenDefaultGitHubRepoSSH(repoPath string) (*Repo, error) {
 	)
 }
 
-// CloneOrOpenGitHubRepo creates a temp directory containing the provided
-// GitHub repository via the owner and repo. If useSSH is true, then it will
-// clone the repository using the defaultGithubAuthRoot.
+// CleanCloneGitHubRepo creates a guaranteed fresh checkout of a given repository. The returned *Repo has a Cleanup()
+// method that should be used to delete the repository on-disk afterwards.
+func CleanCloneGitHubRepo(owner, repo string, useSSH bool) (*Repo, error) {
+	repoURL := GetRepoURL(owner, repo, useSSH)
+	// The use of a blank string for the repo path triggers special behaviour in CloneOrOpenRepo that causes a true
+	// temporary directory with a random name to be created.
+	return CloneOrOpenRepo("", repoURL, useSSH)
+}
+
+// CloneOrOpenGitHubRepo works with a repository in the given directory, or creates one if the directory is empty. The
+// repo uses the provided GitHub repository via the owner and repo. If useSSH is true, then it will clone the
+// repository using the defaultGithubAuthRoot.
 func CloneOrOpenGitHubRepo(repoPath, owner, repo string, useSSH bool) (*Repo, error) {
 	repoURL := GetRepoURL(owner, repo, useSSH)
 	return CloneOrOpenRepo(repoPath, repoURL, useSSH)
@@ -879,7 +888,7 @@ func (r *Repo) AddRemote(name, owner, repo string) error {
 // PushToRemote push the current branch to a spcified remote, but only if the
 // repository is not in dry run mode
 func (r *Repo) PushToRemote(remote, remoteBranch string) error {
-	args := []string{"push"}
+	args := []string{"push", "--set-upstream"}
 	if r.dryRun {
 		logrus.Infof("Won't push due to dry run repository")
 		args = append(args, "--dry-run")