Add options to generate and push the JSON version of the release notes #1185

puerco · 2020-03-17T23:46:26Z

What type of PR is this?

/kind feature

What this PR does / why we need it:

This PR adds to krel a new feature to generate the kubernetes release notes in the json format required by relnotes.k8s.io. When enabling this feature via --create-website-pr it will create a branch in the user's fork of kubernetes/sig-release (determined by --website-org / --website-repo ) where it will generate the json file and patch the assets.ts file. After that, it will commit the changes and push them to the user's fork.

Which issue(s) this PR fixes:

Fixes #1061
Related to #834

Special notes for your reviewer:

Some notes for this PR:

This release implements pushing the files to the user's fork while we finish work on Upload release notes JSON (relnotes.k8s.io) to a Google Cloud Bucket #1087
Tests are not yet included in this PR because I expect a number of changes after review.
Just like with the markdown draft, generating the branch in the user's fork has to be done in a clean/new repository. Reusing an existing repo for k/sig-release will not yet work until Improve krel release-notes local k/sig-release repository handling #1126 is resolved.
After this PR, the only thing left to do is create the PR.

Does this PR introduce a user-facing change?

This change adds four new flags to the krel release-notes subcommand
After this change it is not longer possible to run --create-website-pr and --create-draft-pr during the same invocation.

- This change adds four new flags to the krel release-notes subcommand:
`--create-website-pr`  - Flag to enable the release notes json generator
`--website-org` - GitHub Organization of the users fork of kubernetes/sig-release
`--website-repo` - Repository containing the fork of the user's repo
`--kubernetes-sigs-fork-path` - Filesystem path to the user's fork of k/sig-release
After this change it is not longer possible to run `--create-website-pr` and `--create-draft-pr` during the same invocation.
Implementation of --create-website-pr in the krel release-notes subcommand to generate the release notes in json format for relnotes.k8s.io

puerco · 2020-03-18T00:19:19Z

/retest

puerco · 2020-03-18T00:44:47Z

/retest

puerco · 2020-03-18T01:57:52Z

/test pull-release-test

puerco · 2020-03-18T07:06:12Z

Mmh I think something's up with this test ...

  changelog_test.go:202: 
        	Error Trace:	changelog_test.go:202
        	Error:      	Expected nil, but got: no latest tag available for branch release-1.16
        	Test:       	TestNewRCRelease

saschagrunert · 2020-03-18T07:48:31Z

Hey @puerco, can you please change the release notes in this PR to contain everything within a single release-notes block? Like

- This change adds four new flags to the krel release-notes subcommand:
  `--create-website-pr` - Flag to enable the release notes json generator
  `--website-org` - GitHub Organization of the users fork of kubernetes/sig-release
  `--website-repo` - Repository containing the fork of the user's repo
  `--kubernetes-sigs-fork-path` - Filesystem path to the user's fork of k/sig-release
  After this change it is not longer possible to run --create-website-pr and --create-draft-pr during the same invocation.
  Implementation of --create-website-pr in the krel release-notes subcommand to generate the release notes in json format for relnotes.k8s.io

puerco · 2020-03-18T07:58:11Z

Is this a little better?

saschagrunert · 2020-03-18T07:59:12Z

Is this a little better?

Yes this is better, thank you! :) I will take care of fixing the CI bug and reviewing your PR in the meanwhile.

saschagrunert

Hey, its generally LGTM. See my comments for more input 👇

cmd/krel/cmd/release_notes.go

saschagrunert · 2020-03-18T08:17:43Z

cmd/krel/cmd/release_notes.go

+	// amend the latest commit
+	userName, err := command.New("git", "config", "--get", "user.name").RunSuccessOutput()
+	if err != nil {
+		return errors.Wrap(err, "while trying to get the user's name")
+	}
+
+	userEmail, err := command.New("git", "config", "--get", "user.email").RunSuccessOutput()
+	if err != nil {
+		return errors.Wrap(err, "while trying to get the user's name")
+	}
+
+	logrus.Infof("amending the last commit to the user's config: %v <%v>", userName.OutputTrimNL(), userEmail.OutputTrimNL())
+
+	if err := command.NewWithWorkDir(
+		repository.Dir(), "git", "commit", "--amend", "--signoff", "--no-edit",
+		"--author", fmt.Sprintf("%v <%v>", userName.OutputTrimNL(), userEmail.OutputTrimNL()),
+	).RunSilentSuccess(); err != nil {
+		return errors.Wrap(err, "amending commit with user's data")
+	}


I feel that we should change the repo.Commit method to streamline this. Can we add a TODO here to indicate that this might be simplified?

I've added a new Commit function CommitWithOptions to the git package which can get custom options for the commit. I modified the Commit function to use it and to send the release notes PRs on behalf of the user, I added a new UserCommit function which reads the user data from the local git.

cmd/krel/cmd/release_notes.go

saschagrunert · 2020-03-18T08:29:07Z

cmd/krel/cmd/release_notes.go

+// tryToFindPreviuousTag gets a release tag and returns the one before it
+func tryToFindPreviuousTag(tag string) (string, error) {
+	url, err := git.GetDefaultKubernetesRepoURL()
+	if err != nil {
+		return "", err
+	}
+
+	status, err := command.New(
+		"git", "ls-remote", "--sort=v:refname",
+		"--tags", url,
+	).
+		Pipe("grep", "-Eo", "v[0-9].[0-9]+.[0-9]+[-a-z0-9\\.]*$").
+		Pipe("grep", "-B1", tag).
+		Pipe("head", "-1").
+		RunSilentSuccessOutput()
+
+	if err != nil {
+		return "", err
+	}
+
+	return strings.TrimSpace(status.Output()), nil
+}


This function should be part of the git package. I would also make it a bit more flexible by maybe introducing only a new LSRemote() function.

(I think this can be done in a follow-up PR as well.)

cmd/krel/cmd/release_notes.go

droslean · 2020-03-18T08:45:11Z

/retest

droslean · 2020-03-18T09:00:03Z

/test pull-release-test

saschagrunert · 2020-03-18T09:50:24Z

We probably have to increase the timeout in .bazelrc

//cmd/krel/cmd:go_default_test                                          TIMEOUT in 1815.0s

saschagrunert · 2020-03-19T07:15:09Z

@puerco can you please rebase on top of the latest master and drop that merge commit?

puerco · 2020-03-19T17:07:18Z

@saschagrunert yes I'm sorry, I switched computers to continue work on this and accidentally pushed the merge commit. I'll send the updates in a little bit.

saschagrunert

/lgtm

k8s-ci-robot · 2020-03-23T10:37:28Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: puerco, saschagrunert

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~cmd/krel/OWNERS~~ [saschagrunert]
~~pkg/OWNERS~~ [saschagrunert]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

k8s-ci-robot requested review from hoegaarden and justaugustus March 17, 2020 23:46

k8s-ci-robot added area/release-eng Issues or PRs related to the Release Engineering subproject sig/release Categorizes an issue or PR as relevant to SIG Release. labels Mar 17, 2020

puerco force-pushed the relnotes-json branch from 850964d to b3098bf Compare March 18, 2020 00:18

puerco force-pushed the relnotes-json branch from b3098bf to c228d1a Compare March 18, 2020 00:44

puerco force-pushed the relnotes-json branch from c228d1a to 61bd85a Compare March 18, 2020 01:17

puerco force-pushed the relnotes-json branch from 61bd85a to 51419c3 Compare March 18, 2020 02:13

saschagrunert mentioned this pull request Mar 18, 2020

Fix github.LatestGitHubTagsPerBranch() to list all branches #1187

Merged

saschagrunert reviewed Mar 18, 2020

View reviewed changes

droslean mentioned this pull request Mar 18, 2020

[Flaky Test] release-test (pull-release-test) kubernetes/kubernetes#89197

Closed

k8s-ci-robot added the do-not-merge/contains-merge-commits label Mar 19, 2020

puerco force-pushed the relnotes-json branch from e831106 to 71badf8 Compare March 19, 2020 20:15

k8s-ci-robot removed the do-not-merge/contains-merge-commits label Mar 19, 2020

puerco force-pushed the relnotes-json branch from 71badf8 to e1bf136 Compare March 19, 2020 20:32

Add options to generate and push the JSON version of the release notes

a5d955b

puerco force-pushed the relnotes-json branch from e1bf136 to a5d955b Compare March 20, 2020 17:19

saschagrunert approved these changes Mar 23, 2020

View reviewed changes

k8s-ci-robot assigned saschagrunert Mar 23, 2020

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Mar 23, 2020

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 23, 2020

k8s-ci-robot merged commit 63e6396 into kubernetes:master Mar 23, 2020

k8s-ci-robot added this to the v1.19 milestone Mar 23, 2020

puerco deleted the relnotes-json branch May 13, 2020 18:23

Add options to generate and push the JSON version of the release notes #1185

Add options to generate and push the JSON version of the release notes #1185

Uh oh!

Conversation

puerco commented Mar 17, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What type of PR is this?

What this PR does / why we need it:

Which issue(s) this PR fixes:

Special notes for your reviewer:

Does this PR introduce a user-facing change?

Uh oh!

puerco commented Mar 18, 2020

Uh oh!

puerco commented Mar 18, 2020

Uh oh!

puerco commented Mar 18, 2020

Uh oh!

puerco commented Mar 18, 2020

Uh oh!

saschagrunert commented Mar 18, 2020

Uh oh!

puerco commented Mar 18, 2020

Uh oh!

saschagrunert commented Mar 18, 2020

Uh oh!

saschagrunert left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

saschagrunert Mar 18, 2020

Choose a reason for hiding this comment

Uh oh!

puerco Mar 19, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

saschagrunert Mar 18, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

droslean commented Mar 18, 2020

Uh oh!

droslean commented Mar 18, 2020

Uh oh!

saschagrunert commented Mar 18, 2020

Uh oh!

saschagrunert commented Mar 19, 2020

Uh oh!

puerco commented Mar 19, 2020

Uh oh!

saschagrunert left a comment

Choose a reason for hiding this comment

Uh oh!

k8s-ci-robot commented Mar 23, 2020

Uh oh!

Uh oh!

puerco commented Mar 17, 2020 •

edited

Loading