Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update Branch Manager handbook instructions for branch cut activities #757

Merged
merged 2 commits into from
Sep 4, 2019

Conversation

justaugustus
Copy link
Member

Updated documentation based on experiences from the 1.16.0-beta.0 release.

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

/assign @tpepper @hoegaarden @nikhita @imkin @Katharine @spiffxp
cc: @kubernetes/release-engineering
/milestone v1.16
/priority important-soon
/kind documentation

@k8s-ci-robot k8s-ci-robot added this to the v1.16 milestone Aug 14, 2019
@k8s-ci-robot k8s-ci-robot added the priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. label Aug 14, 2019
@k8s-ci-robot k8s-ci-robot added kind/documentation Categorizes issue or PR as related to documentation. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. area/release-eng Issues or PRs related to the Release Engineering subproject sig/release Categorizes an issue or PR as relevant to SIG Release. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Aug 14, 2019
@justaugustus
Copy link
Member Author

/assign @idealhack

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 14, 2019
@justaugustus justaugustus mentioned this pull request Aug 14, 2019
5 tasks
@hoegaarden
Copy link
Contributor

/lgtm
/hold
... in case someone else want's to review, esp. regarding the test-infra stuff I have near-zero context on

@k8s-ci-robot k8s-ci-robot added do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. lgtm "Looks good to me", indicates that a PR is ready to be merged. labels Aug 14, 2019
@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 14, 2019
@justaugustus
Copy link
Member Author

justaugustus commented Aug 14, 2019

Added a note about configuring the milestoneapplier plugin.
Example: kubernetes/test-infra#13888

Copy link
Member

@nikhita nikhita left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As someone new to the process, I feel like it would be easier if the alpha/beta releases sections would be converted to use checklists and/or bullets but that can be done in a follow up. Thanks for the overhaul, @justaugustus! :)

- Remove the oldest release variant e.g., `'1.12'`
- Add an entry for the newest release variant e.g., `'1.16'`
- Ensure the following:
- The `K8S_RELEASE` marker for `experimental` matches `master`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we also mention that this needs to be stable or is that still under discussion? If the latter, should we create an issue to track this?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

- Remove [Branch Managers](/release-managers.md#branch-managers) from any previous release branches
- Add all current [Branch Managers](/release-managers.md#branch-managers)
2. [Create CI/Presubmit jobs for the new release](#create-cipresubmit-jobs)
3. [Run `./branchff` approximately a day after the branch has been created]((#branch-fast-forward))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IIUC anyone who'll need to run branchff needs to be in the release-managers GitHub team? Can we also mention that in the Onboarding section?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We mention this in the beginning of the Branch Content Management section.
I'm planning to refactor this doc a bunch and have more guidance on Release Engineering onboarding in the repo, but I'm going to punt on that for this PR.


`./gcbmgr stage release-1.12 --build-at-head --nomock`
`prow`’s [`branchprotector`](https://git.k8s.io/test-infra/prow/cmd/branchprotector/README.md) runs once per day and automatically adds [branch protection](https://help.github.com/articles/about-protected-branches/) to any new branch in the `k/k` repo.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: would be better to explicitly spell out k/k to kubernetes/kubernetes since newcomers also read the handbook :)


`grep` for [`manual-release-bump-required`](https://github.com/kubernetes/test-infra/search?q=manual-release-bump-required&unscoped_q=manual-release-bump-required) in [test-infra](https://github.com/kubernetes/test-infra) and duplicate block entries appropriately

7. Update release dashboards in [`testgrid/config.yaml`](https://github.com/kubernetes/test-infra/blob/master/testgrid/config.yaml) ([example commit](https://github.com/kubernetes/test-infra/pull/13882/commits/8aad58b09ae718139b44c70512d2d133d094ed82))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

config.yaml link does not exist

@@ -292,77 +310,143 @@ The following are some ways to determine if the release process was successful:

1. The build tag and release artifacts become [visible on GitHub at https://github.com/kubernetes/kubernetes/releases](https://github.com/kubernetes/kubernetes/releases)

1. The release is logged automatically by [k8s-release-robot](https://github.com/k8s-release-robot) in [sig-release](https://github.com/k8s-release-robot/sig-release)
2. The release is logged automatically by [k8s-release-robot](https://github.com/k8s-release-robot) in [sig-release](https://github.com/k8s-release-robot/sig-release)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bad link for sig-release .. 😬

Copy link
Member

@tpepper tpepper left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

More or less lgtm, though I had to spend a loooot of time finding your meaningful changes versus the mechanical version number increments and markdown styling changes because this is one giant commit.

One question though: this is a lot of changing of branch creation and actions a human needs to run to manage the test-infra changes for a new branch. It was previously asserted this was all automated, and we removed the test-infra role from the release team for that reason. For all of this manual activity, what portions of it can be automated into a push button update for a new branch?

- [Staging Repositories](#staging-repositories)
- [Debugging](#debugging)
- [References](#references)
- [Background information](#background-information)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would make review a lot easier if the ToC style update was separate from content modifications. It looks like this is just Cap's to Lower Case? But I need to manually compare across 60 lines looking if a section was added or removed.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comes up in our reviews without fail and I keep forgetting to address it.
I use Visual Studio Code with two neat extensions:

The combination of the two enable linting for Markdown files and table of contents generation.
ToC generation happens on save, so I have limited control there.

I'm willing to give up some control there for the sake of the table of contents always being up to date.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does VSCode support "git add -i"?


The first time a specific branch is mentioned for the release process instead of the `master` branch is when beta.0 is being cut, for example:
Behind the scenes `anago` is doing a branch create and push.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"...doing a git branch create and git push..."


This being the first build from the newly created release branch, the publication of this build does not send out the typical changelog detail notification, but rather will only send a shorter message with subject line "[k8s release-1.15 branch has been created](https://groups.google.com/forum/#!topic/kubernetes-announce/Gu2JYa4BzAg)".

To publish (release) the build artifacts from staging beta for example, run:

`./gcbmgr release release-1.15 --buildversion=v1.15.0-alpha.3.N+commit-hash --nomock`
`./gcbmgr release release-1.16 --buildversion=v1.16.0-alpha.3.N+commit-hash --nomock`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All of the s/1.15/1.16/ and s/1.16/1.17/ changes would be nice to have as a separate commit. Again that portion is trivially reviewed and merits little discussion, but results in a lot of diffstat which distracts from discussion worthy changes.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I pretty much only touch versions when I've passed over a section... as if to say, "Someone has looked over this recently-ish.". I don't necessarily want to do wholesale search/replaces and have them in a separate commit if I haven't personally reviewed the section.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm fine saying solving this is out of scope for this PR, but I agree re: increased review burden. There are also a lot of x.y references in this doc, and it'd be cool to just pick one up top and stick with it. eg: "for the purposes of this doc, the last release is 1.1, the in-development release is 1.2, and the not-yet-started release is 1.3" (eg: https://github.com/kubernetes/community/blob/b7f39f6935264984e24fe8c27c0fd8e4fffac364/contributors/devel/sig-architecture/conformance-tests.md#conformance-test-version-skew-policy)

## Post-release Activities

- Set the `K8S_RELEASE` marker for the current release variant to `stable-x.y` in the [`variants.yaml` file for `kubekins-e2e`](https://github.com/kubernetes/test-infra/blob/fa43d4a7a6c88c0dedd0db83b250cec485b60736/images/kubekins-e2e/variants.yaml). ([reference PR review comment](https://github.com/kubernetes/test-infra/pull/13870#discussion_r313628808))

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should that PR discussion get reflected in https://github.com/kubernetes/test-infra/blob/master/docs/kubernetes-versions.md plus a description of K8s_RELEASE?

And then this reference to the K8S_RELEASE marker can reference that documentation?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(ie: there seemed to be a fair amount of confusion or uncertainty on what the marker's intent is...that decided should be in official docs for reference as in 3/6/9/12 months we'll be wondering again and again and we'll have fresh issue and PR discussions to reference)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 Explicitly documenting post-release steps here.

I would recommend adding something about making sure the "beta" 'channel' goes back to pointing to master / being unused post-release (ref: #696 (comment))

re: K8S_RELEASE, I'm not thrilled when comments in PRs are linked as reference material, but I think it's out of scope to refine here. I would prefer to see reference docs on that variable and others in a README.md for kubekins, so we can understand what is in that image and why. (ref: kubernetes/test-infra#13129 (comment))


3. Update [`images/kubekins-e2e/variants.yaml`](https://github.com/kubernetes/test-infra/blob/master/images/kubekins-e2e/variants.yaml). ([example PR](https://github.com/kubernetes/test-infra/pull/13876))

(The `variants.yaml` allows us to target various branches or branch combinations during CI tests. The `K8S_VERSION` variable maps to the version marker files viewable in the [`kubernetes-release` GCS bucket](https://gcsweb.k8s.io/gcs/kubernetes-release/release/) e.g., [`latest-1.16.txt`](https://storage.googleapis.com/kubernetes-release/release/latest-1.16.txt))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is good, but it's now a great summary buried where only a subset of relevant folks will see it. IMO should be in https://github.com/kubernetes/test-infra/blob/master/docs/kubernetes-versions.md

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok so for example I would dump most of this content around kubekins into a README.md here https://github.com/kubernetes/test-infra/tree/master/images/kubekins-e2e, e.g. the constraints to adhere to, why things are what they are, etc.

the parts that I would leave here are the instructions directly related to "add new variant", "drop old variant"

- Remove the oldest release variant e.g., `'1.12'`
- Add an entry for the newest release variant e.g., `'1.16'`
- Ensure the following:
- The `K8S_RELEASE` marker for `experimental` matches `master`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@justaugustus
Copy link
Member Author

Thanks for the reviews, @hoegaarden, @nikhita, and @tpepper!

As someone new to the process, I feel like it would be easier if the alpha/beta releases sections would be converted to use checklists and/or bullets but that can be done in a follow up.

That's the plan! This is just a little update to capture some of the branch creation/CI tasks that needed updating.

One question though: this is a lot of changing of branch creation and actions a human needs to run to manage the test-infra changes for a new branch. It was previously asserted this was all automated, and we removed the test-infra role from the release team for that reason. For all of this manual activity, what portions of it can be automated into a push button update for a new branch?

The test-infra changes are great improvements, but we are not fully automated yet, just enough so to have removed the role (I think @Katharine has mentioned this on a few calls).
Some things that come to mind are here and are already captured in issues/PRs linked out in that comment.


This is ready for review again.
I'm going to punt on the version marker files for this PR and handle in the near term as it's going to take a bit of investigation.
I've opened an issue to tie the threads together: #759

Copy link
Member

@spiffxp spiffxp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some suggestions

- [Configure Merge Automation](#configure-merge-automation)
- [Tide](#tide)
- [Code Freeze](#code-freeze)
- [Code Thaw](#code-thaw)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

these three bullets lost their indent under "Configure Merge Automation"

1. Update this handbook where appropriate for the next release cycle.
1. Have willingness to accommodate with different timezone esp. for the release team and your Associates.
1. Participate in conversations that happen on [#sig-release] and [#release-management]
2. You must allocate time to setup your system to run the release tools; eventually you'll have a routine going and become more familiar with the process.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

general comment on renumbering ordered lists: this obscures what's actually changing, and makes it more toilsome to update (even if you shed that toil to a plugin)

we don't require one way or the other in our markdown style guide (https://github.com/kubernetes/community/blob/master/contributors/guide/style-guide.md#lists), but it would ease review burden if you left them at 1.


`grep` for [`manual-release-bump-required`](https://github.com/kubernetes/test-infra/search?q=manual-release-bump-required&unscoped_q=manual-release-bump-required) in [test-infra](https://github.com/kubernetes/test-infra) and duplicate block entries appropriately

7. Update release dashboards in the [Testgrid config](https://git.k8s.io/test-infra/config/testgrids/config.yaml) ([example commit](https://github.com/kubernetes/test-infra/pull/13882/commits/8aad58b09ae718139b44c70512d2d133d094ed82))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

an idea for followup: shard out to config/testgrids/sig-release/config.yaml instead (ref: kubernetes/test-infra#13541)


This being the first build from the newly created release branch, the publication of this build does not send out the typical changelog detail notification, but rather will only send a shorter message with subject line "[k8s release-1.15 branch has been created](https://groups.google.com/forum/#!topic/kubernetes-announce/Gu2JYa4BzAg)".

To publish (release) the build artifacts from staging beta for example, run:

`./gcbmgr release release-1.15 --buildversion=v1.15.0-alpha.3.N+commit-hash --nomock`
`./gcbmgr release release-1.16 --buildversion=v1.16.0-alpha.3.N+commit-hash --nomock`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm fine saying solving this is out of scope for this PR, but I agree re: increased review burden. There are also a lot of x.y references in this doc, and it'd be cool to just pick one up top and stick with it. eg: "for the purposes of this doc, the last release is 1.1, the in-development release is 1.2, and the not-yet-started release is 1.3" (eg: https://github.com/kubernetes/community/blob/b7f39f6935264984e24fe8c27c0fd8e4fffac364/contributors/devel/sig-architecture/conformance-tests.md#conformance-test-version-skew-policy)

@spiffxp
Copy link
Member

spiffxp commented Aug 14, 2019

You are free to do in a followup, but while you are here it would be nice to have a tombstone dropped in release-team to point to this (ref: #761)

justaugustus and others added 2 commits August 19, 2019 08:04
- Clarify k/k as kubernetes/kubernetes for newer contributors
- Fix link to Testgrid config
- Fix link to k/sig-release
- Clarify anago is doing git branch and git push during beta release
- Update milestoneapplier config to include the `v` in version
- Add a note that milestoneapplier should be bumped after code freeze

Signed-off-by: Stephen Augustus <[email protected]>
Co-Authored-By: Aaron Crickenberger <[email protected]>
@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Sep 3, 2019
Copy link
Member

@idealhack idealhack left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mostly LGTM, I'd go for merge and iterate.

/lgtm

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: hoegaarden, idealhack, justaugustus

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:

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

@Bubblemelon Bubblemelon mentioned this pull request Sep 4, 2019
18 tasks
@justaugustus
Copy link
Member Author

@idealhack -- good plan. I've added an action item on #696 to incorporate the remaining feedback.

/hold cancel

@k8s-ci-robot k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Sep 4, 2019
@k8s-ci-robot k8s-ci-robot merged commit 7187fa0 into kubernetes:master Sep 4, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. area/release-eng Issues or PRs related to the Release Engineering subproject cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/documentation Categorizes issue or PR as related to documentation. lgtm "Looks good to me", indicates that a PR is ready to be merged. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. sig/release Categorizes an issue or PR as relevant to SIG Release. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants