From 37ef3413e6b6b441e7562be508be7defe45a9f56 Mon Sep 17 00:00:00 2001 From: Robert Lin Date: Thu, 9 Sep 2021 15:52:57 -0400 Subject: [PATCH] dev/release: improve automation for ensuring followup (#24614) Aims to improve some processes and automation around release follow-ups. - Update patch request template to be clearer about actions for release captains, including documenting follow-ups. - `yarn release tracking:issues`: Automatically close out old release tracking issues if subsequent ones are created after leaving a comment linking to the new release. We often have leftover tracking issues, especially for managed instances. - `yarn release release:finalize`: Add outstanding patch request issues to comment for potential follow-up - `yarn release release:finalize`: Add link to generated comment for visibility (post-release steps often get skipped) - Backlink to tracking issue in release campaign --- .../ISSUE_TEMPLATE/request_patch_release.md | 23 +-- dev/release/release-config.jsonc | 3 +- dev/release/src/batchChanges.ts | 10 +- dev/release/src/config.ts | 1 + dev/release/src/github.ts | 164 ++++++++++++------ dev/release/src/release.ts | 84 ++++++--- 6 files changed, 192 insertions(+), 93 deletions(-) diff --git a/.github/ISSUE_TEMPLATE/request_patch_release.md b/.github/ISSUE_TEMPLATE/request_patch_release.md index e944a0e91f7..ff7d0d6ec67 100644 --- a/.github/ISSUE_TEMPLATE/request_patch_release.md +++ b/.github/ISSUE_TEMPLATE/request_patch_release.md @@ -9,10 +9,12 @@ assignees: '' @sourcegraph/distribution I am requesting the following commits be included in a patch release. They are already merged into `main`: -The intent of the questions below is to ensure we keep Sourcegraph high quality and [only create patch releases based on a strict criteria.](https://about.sourcegraph.com/handbook/engineering/releases#when-are-patch-releases-performed) If you can answer yes to many or most of these questions, we will be happy to create the patch release. - - +--- + +The intent of the questions below is to ensure we keep Sourcegraph high quality and [only create patch releases based on a strict criteria.](https://about.sourcegraph.com/handbook/engineering/releases#when-are-patch-releases-performed) If you can answer yes to many or most of these questions, we will be happy to create the patch release. + I have read [when and why we perform patch releases](https://about.sourcegraph.com/handbook/engineering/releases#when-are-patch-releases-performed) and answer the questions as follows: > Are users/customers actively asking us for these changes and cannot wait until the next full release? @@ -39,12 +41,13 @@ I have read [when and why we perform patch releases](https://about.sourcegraph.c --- -**For the [release captain](https://about.sourcegraph.com/handbook/engineering/releases#release-captain)** - after reviewing and approving this request: +**For the [release captain](https://about.sourcegraph.com/handbook/engineering/releases#release-captain)** - after reviewing this request: -- If there is [already an upcoming patch release](https://github.com/sourcegraph/sourcegraph/issues?q=is%3Aissue+label%3Arelease-tracking+), add the listed commits alongside a link to this issue -- If there is no upcoming patch release, create a new one: - - Update [`dev/release/release-config.jsonc`](https://sourcegraph.com/github.com/sourcegraph/sourcegraph/-/blob/dev/release/release-config.jsonc) with the patch release in `upcomingRelease` and `releaseDate` (and open a PR to `main` to update it) - - `yarn release tracking:issues` - - Add the listed commits alongside a link to this issue to the generated [release tracking issue](https://github.com/sourcegraph/sourcegraph/issues?q=is%3Aissue+label%3Arelease-tracking+) - -Comment and close this issue once the relevant commit(s) have been cherry-picked into the release branch. +- [ ] **Comment on this issue** with a decision regarding the request. +- [ ] If approved, **add it to a patch release**: + - If there is [already an upcoming patch release](https://github.com/sourcegraph/sourcegraph/issues?q=is%3Aissue+label%3Arelease-tracking+), add the listed commits alongside a link to this issue + - If there is no upcoming patch release, create a new one: + - Update [`dev/release/release-config.jsonc`](https://sourcegraph.com/github.com/sourcegraph/sourcegraph/-/blob/dev/release/release-config.jsonc) with the patch release in `upcomingRelease` and `releaseDate` (and open a PR to `main` to update it) + - `yarn release tracking:issues` + - Add the listed commits alongside a link to this issue to the generated [release tracking issue](https://github.com/sourcegraph/sourcegraph/issues?q=is%3Aissue+label%3Arelease-tracking+) +- [ ] **Comment and close this issue once the relevant commit(s) have been cherry-picked into the release branch**. diff --git a/dev/release/release-config.jsonc b/dev/release/release-config.jsonc index c87d2bfaaee..d1c9851bd3d 100644 --- a/dev/release/release-config.jsonc +++ b/dev/release/release-config.jsonc @@ -31,6 +31,7 @@ "tags": false, "changesets": false, "trackingIssues": false, - "calendar": false + "calendar": false, + "slack": false } } diff --git a/dev/release/src/batchChanges.ts b/dev/release/src/batchChanges.ts index 230d075e52b..f0167f69b78 100644 --- a/dev/release/src/batchChanges.ts +++ b/dev/release/src/batchChanges.ts @@ -33,7 +33,6 @@ export async function sourcegraphCLIConfig(): Promise { */ export interface BatchChangeOptions { name: string - description: string namespace: string cliConfig: SourcegraphCLIConfig } @@ -44,7 +43,6 @@ export interface BatchChangeOptions { export function releaseTrackingBatchChange(version: string, cliConfig: SourcegraphCLIConfig): BatchChangeOptions { return { name: `release-sourcegraph-${version}`, - description: `Track publishing of sourcegraph@${version}`, namespace: 'sourcegraph', cliConfig, } @@ -62,7 +60,11 @@ export function batchChangeURL(options: BatchChangeOptions): string { /** * Create a new batch change from a set of changes. */ -export async function createBatchChange(changes: CreatedChangeset[], options: BatchChangeOptions): Promise { +export async function createBatchChange( + changes: CreatedChangeset[], + options: BatchChangeOptions, + description: string +): Promise { // create a batch change spec const importChangesets = changes.map(change => ({ repository: `github.com/${change.repository}`, @@ -72,7 +74,7 @@ export async function createBatchChange(changes: CreatedChangeset[], options: Ba return await applyBatchChange( { name: options.name, - description: options.description, + description, importChangesets, }, options diff --git a/dev/release/src/config.ts b/dev/release/src/config.ts index 4697c314cd9..4286be0be66 100644 --- a/dev/release/src/config.ts +++ b/dev/release/src/config.ts @@ -27,6 +27,7 @@ export interface Config { tags?: boolean changesets?: boolean trackingIssues?: boolean + slack?: boolean calendar?: boolean } } diff --git a/dev/release/src/github.ts b/dev/release/src/github.ts index 1faa486b42a..75253866d6e 100644 --- a/dev/release/src/github.ts +++ b/dev/release/src/github.ts @@ -27,8 +27,23 @@ export function releaseName(release: semver.SemVer): string { return `${release.major}.${release.minor}${release.patch !== 0 ? `.${release.patch}` : ''}` } -// https://github.com/sourcegraph/sourcegraph/labels/release-tracking -const labelReleaseTracking = 'release-tracking' +export enum IssueLabel { + // https://github.com/sourcegraph/sourcegraph/labels/release-tracking + RELEASE_TRACKING = 'release-tracking', + // https://github.com/sourcegraph/sourcegraph/labels/patch-release-request + PATCH_REQUEST = 'patch-release-request', + + // New labels to better distinguish release-tracking issues + RELEASE = 'release', + PATCH = 'patch', + MANAGED = 'managed-instances', +} + +enum IssueTitleSuffix { + RELEASE_TRACKING = 'release tracking issue', + PATCH_TRACKING = 'patch release tracking issue', + MANAGED_TRACKING = 'upgrade managed instances tracking issue', +} /** * Template used to generate tracking issue @@ -45,7 +60,7 @@ interface IssueTemplate { /** * Title for issue. */ - title: (v: semver.SemVer) => string + titleSuffix: IssueTitleSuffix /** * Labels to apply on issues. */ @@ -74,6 +89,37 @@ interface IssueTemplateArguments { oneWorkingDayAfterRelease: Date } +/** + * Configure templates for the release tool to generate issues with. + * + * Ensure these templates are up to date with the state of the tooling and release processes. + */ +// eslint-disable-next-line @typescript-eslint/explicit-function-return-type +const getTemplates = () => { + const releaseIssue: IssueTemplate = { + owner: 'sourcegraph', + repo: 'about', + path: 'handbook/engineering/releases/release_issue_template.md', + titleSuffix: IssueTitleSuffix.RELEASE_TRACKING, + labels: [IssueLabel.RELEASE_TRACKING, IssueLabel.RELEASE], + } + const patchReleaseIssue: IssueTemplate = { + owner: 'sourcegraph', + repo: 'about', + path: 'handbook/engineering/releases/patch_release_issue_template.md', + titleSuffix: IssueTitleSuffix.PATCH_TRACKING, + labels: [IssueLabel.RELEASE_TRACKING, IssueLabel.PATCH], + } + const upgradeManagedInstanceIssue: IssueTemplate = { + owner: 'sourcegraph', + repo: 'about', + path: 'handbook/engineering/releases/upgrade_managed_issue_template.md', + titleSuffix: IssueTitleSuffix.MANAGED_TRACKING, + labels: [IssueLabel.RELEASE_TRACKING, IssueLabel.MANAGED], + } + return { releaseIssue, patchReleaseIssue, upgradeManagedInstanceIssue } +} + async function execTemplate( octokit: Octokit, template: IssueTemplate, @@ -97,40 +143,10 @@ async function execTemplate( ) } -/** - * Configure templates for the release tool to generate issues with. - * - * Ensure these templates are up to date with the state of the tooling and release processes. - */ -// eslint-disable-next-line @typescript-eslint/explicit-function-return-type -const getTemplates = () => { - const releaseIssue: IssueTemplate = { - owner: 'sourcegraph', - repo: 'about', - path: 'handbook/engineering/releases/release_issue_template.md', - title: trackingIssueTitle, - labels: [labelReleaseTracking], - } - const patchReleaseIssue: IssueTemplate = { - owner: 'sourcegraph', - repo: 'about', - path: 'handbook/engineering/releases/patch_release_issue_template.md', - title: trackingIssueTitle, - labels: [labelReleaseTracking], - } - const upgradeManagedInstanceIssue: IssueTemplate = { - owner: 'sourcegraph', - repo: 'about', - path: 'handbook/engineering/releases/upgrade_managed_issue_template.md', - title: (version: semver.SemVer) => `${version.version} upgrade managed instances tracking issue`, - labels: [labelReleaseTracking, 'managed-instances'], - } - return { releaseIssue, patchReleaseIssue, upgradeManagedInstanceIssue } -} - interface MaybeIssue { title: string url: string + number: number created: boolean } @@ -192,7 +208,7 @@ export async function ensureTrackingIssues({ const issue = await ensureIssue( octokit, { - title: template.title(version), + title: trackingIssueTitle(version, template), labels: template.labels, body: parentIssue ? `${body}\n\n---\n\nAlso see [${parentIssue.title}](${parentIssue.url})` : body, assignees, @@ -207,6 +223,20 @@ export async function ensureTrackingIssues({ parentIssue = { ...issue } } created.push({ ...issue }) + + // close previous iterations of this issue + const previous = await queryIssues(octokit, template.titleSuffix, template.labels) + for (const previousIssue of previous) { + if (dryRun) { + console.log(`dryRun enabled, skipping closure of #${previousIssue.number} '${previousIssue.title}'`) + continue + } + const comment = await commentOnIssue(octokit, previousIssue, `Superseded by #${issue.number}`) + console.log( + `Closing #${previousIssue.number} '${previousIssue.title}' - commented with an update: ${comment}` + ) + await closeIssue(octokit, previousIssue) + } } return created } @@ -243,7 +273,7 @@ async function ensureIssue( assignees: string[] body: string milestone?: number - labels?: string[] + labels: string[] }, dryRun: boolean ): Promise { @@ -255,18 +285,18 @@ async function ensureIssue( milestone, labels, } - const issue = await getIssueByTitle(octokit, title) + const issue = await getIssueByTitle(octokit, title, labels) if (issue) { - return { title, url: issue.url, created: false } + return { title, url: issue.url, number: issue.number, created: false } } if (dryRun) { console.log('Dry run enabled, skipping issue creation') console.log(`Issue that would have been created:\n${JSON.stringify(issueData, null, 1)}`) console.log(`With body: ${body}`) - return { title, url: '', created: false } + return { title, url: '', number: 0, created: false } } const createdIssue = await octokit.issues.create({ body, ...issueData }) - return { title, url: createdIssue.data.html_url, created: true } + return { title, url: createdIssue.data.html_url, number: createdIssue.data.number, created: true } } export async function listIssues( @@ -277,6 +307,7 @@ export async function listIssues( } export interface Issue { + title: string number: number url: string @@ -286,7 +317,32 @@ export interface Issue { } export async function getTrackingIssue(client: Octokit, release: semver.SemVer): Promise { - return getIssueByTitle(client, trackingIssueTitle(release)) + const templates = getTemplates() + const template = release.patch ? templates.patchReleaseIssue : templates.releaseIssue + return getIssueByTitle(client, trackingIssueTitle(release, template), template.labels) +} + +function trackingIssueTitle(release: semver.SemVer, template: IssueTemplate): string { + return `${release.version} ${template.titleSuffix}` +} + +export async function commentOnIssue(client: Octokit, issue: Issue, body: string): Promise { + const comment = await client.issues.createComment({ + body, + issue_number: issue.number, + owner: issue.owner, + repo: issue.repo, + }) + return comment.data.url +} + +async function closeIssue(client: Octokit, issue: Issue): Promise { + await client.issues.update({ + state: 'closed', + issue_number: issue.number, + owner: issue.owner, + repo: issue.repo, + }) } interface Milestone { @@ -319,29 +375,33 @@ async function getReleaseMilestone(client: Octokit, release: semver.SemVer): Pro : null } -function trackingIssueTitle(version: semver.SemVer): string { - if (!version.patch) { - return `${version.major}.${version.minor} release tracking issue` - } - return `${version.version} patch release tracking issue` -} - -async function getIssueByTitle(octokit: Octokit, title: string): Promise { +export async function queryIssues(octokit: Octokit, titleQuery: string, labels: string[]): Promise { const owner = 'sourcegraph' const repo = 'sourcegraph' const response = await octokit.search.issuesAndPullRequests({ per_page: 100, - q: `type:issue repo:${owner}/${repo} is:open ${JSON.stringify(title)}`, + q: `type:issue repo:${owner}/${repo} is:open ${labels + .map(label => `label:${label}`) + .join(' ')} ${JSON.stringify(titleQuery)}`, }) + return response.data.items.map(item => ({ + title: item.title, + number: item.number, + url: item.html_url, + owner, + repo, + })) +} - const matchingIssues = response.data.items.filter(issue => issue.title === title) +async function getIssueByTitle(octokit: Octokit, title: string, labels: string[]): Promise { + const matchingIssues = (await queryIssues(octokit, title, labels)).filter(issue => issue.title === title) if (matchingIssues.length === 0) { return null } if (matchingIssues.length > 1) { throw new Error(`Multiple issues matched issue title ${JSON.stringify(title)}`) } - return { number: matchingIssues[0].number, url: matchingIssues[0].html_url, owner, repo } + return matchingIssues[0] } export type EditFunc = (d: string) => void diff --git a/dev/release/src/release.ts b/dev/release/src/release.ts index 28fcba46650..c414b558ecb 100644 --- a/dev/release/src/release.ts +++ b/dev/release/src/release.ts @@ -16,6 +16,9 @@ import { createTag, ensureTrackingIssues, releaseName, + commentOnIssue, + queryIssues, + IssueLabel, } from './github' import { ensureEvent, getClient, EventOptions, calendarTime } from './google-calendar' import { postMessage, slackURL } from './slack' @@ -186,8 +189,10 @@ ${trackingIssues.map(index => `- ${slackURL(index.title, index.url)}`).join('\n' patchRequestTemplate )}, or it will not be included.` } - await postMessage(annoncement, slackAnnounceChannel) - console.log(`Posted to Slack channel ${slackAnnounceChannel}`) + if (!dryRun.slack) { + await postMessage(annoncement, slackAnnounceChannel) + console.log(`Posted to Slack channel ${slackAnnounceChannel}`) + } } else { console.log('No tracking issues were created, skipping Slack announcement') } @@ -265,7 +270,9 @@ ${trackingIssues.map(index => `- ${slackURL(index.title, index.url)}`).join('\n' * Tracking issue: ${trackingIssue.url} * ${blockingMessage}: ${blockingIssuesURL}` - await postMessage(message, config.slackAnnounceChannel) + if (!config.dryRun.slack) { + await postMessage(message, config.slackAnnounceChannel) + } }, }, { @@ -317,8 +324,7 @@ ${trackingIssues.map(index => `- ${slackURL(index.title, index.url)}`).join('\n' const batchChangeURL = batchChanges.batchChangeURL(batchChange) const trackingIssue = await getTrackingIssue(await getAuthenticatedGitHubClient(), release) if (!trackingIssue) { - // Do not block release staging on lack of tracking issue - console.error(`Tracking issue for version ${release.version} not found - has it been created yet?`) + throw new Error(`Tracking issue for version ${release.version} not found - has it been created yet?`) } // default PR content @@ -492,19 +498,25 @@ cc @${config.captainGitHubUsername} // Create batch change to track changes try { console.log(`Creating batch change in ${batchChange.cliConfig.SRC_ENDPOINT}`) - await batchChanges.createBatchChange(createdChanges, batchChange) + await batchChanges.createBatchChange( + createdChanges, + batchChange, + `Track publishing of sourcegraph v${release.version}: ${trackingIssue?.url}` + ) } catch (error) { console.error(error) console.error('Failed to create batch change for this release, continuing with announcement') } // Announce release update in Slack - await postMessage( - `:captain: *Sourcegraph ${release.version} has been staged.* + if (!dryRun.slack) { + await postMessage( + `:captain: *Sourcegraph ${release.version} has been staged.* Batch change: ${batchChangeURL}`, - slackAnnounceChannel - ) + slackAnnounceChannel + ) + } } }, }, @@ -537,7 +549,7 @@ Batch change: ${batchChangeURL}`, }, { id: 'release:finalize', - description: 'Run final tasks for the sourcegraph/sourcegraph release pull request', + description: 'Run final tasks for sourcegraph/sourcegraph release pull requests', run: async config => { const { upcoming: release } = await releaseVersions(config) let failed = false @@ -573,7 +585,7 @@ Batch change: ${batchChangeURL}`, id: 'release:close', description: 'Mark a release as closed', run: async config => { - const { slackAnnounceChannel } = config + const { slackAnnounceChannel, dryRun } = config const { upcoming: release } = await releaseVersions(config) const githubClient = await getAuthenticatedGitHubClient() @@ -588,22 +600,36 @@ Batch change: ${batchChangeURL}`, * Release batch change: ${batchChangeURL}` // Slack - await postMessage(`:captain: ${releaseMessage}`, slackAnnounceChannel) - console.log(`Posted to Slack channel ${slackAnnounceChannel}`) + const slackMessage = `:captain: ${releaseMessage}` + if (!dryRun.slack) { + await postMessage(slackMessage, slackAnnounceChannel) + console.log(`Posted to Slack channel ${slackAnnounceChannel}`) + } else { + console.log(`dryRun enabled, skipping Slack post to ${slackAnnounceChannel}: ${slackMessage}`) + } - // GitHub + // GitHub tracking issues const trackingIssue = await getTrackingIssue(githubClient, release) if (!trackingIssue) { console.warn(`Could not find tracking issue for release ${release.version} - skipping`) } else { - await githubClient.issues.createComment({ - owner: trackingIssue.owner, - repo: trackingIssue.repo, - issue_number: trackingIssue.number, - body: `${releaseMessage} + // Note patch release requests if there are any outstanding + let comment = `${releaseMessage} -@${config.captainGitHubUsername}: Please complete the post-release steps before closing this issue.`, - }) +@${config.captainGitHubUsername}: Please complete the post-release steps before closing this issue.` + const patchRequestIssues = await queryIssues(githubClient, '', [IssueLabel.PATCH_REQUEST]) + if (patchRequestIssues && patchRequestIssues.length > 0) { + comment += ` +Please also update outstanding patch requests, if relevant: + +${patchRequestIssues.map(issue => `* #${issue.number}`).join('\n')}` + } + if (!dryRun.trackingIssues) { + const commentURL = await commentOnIssue(githubClient, trackingIssue, comment) + console.log(`Please make sure to follow up on the release issue: ${commentURL}`) + } else { + console.log(`dryRun enabled, skipping GitHub comment to ${trackingIssue.url}: ${comment}`) + } } }, }, @@ -634,8 +660,10 @@ Batch change: ${batchChangeURL}`, id: '_test:slack', description: 'Test Slack integration', argNames: ['channel', 'message'], - run: async (_config, channel, message) => { - await postMessage(message, channel) + run: async ({ dryRun }, channel, message) => { + if (!dryRun.slack) { + await postMessage(message, channel) + } }, }, { @@ -659,7 +687,11 @@ Batch change: ${batchChangeURL}`, cliConfig: await batchChanges.sourcegraphCLIConfig(), } - await batchChanges.createBatchChange(batchChangeConfig.changes, batchChange) + await batchChanges.createBatchChange( + batchChangeConfig.changes, + batchChange, + 'release tool testing batch change' + ) console.log(`Created batch change ${batchChanges.batchChangeURL(batchChange)}`) }, }, @@ -673,7 +705,7 @@ Batch change: ${batchChangeURL}`, { id: '_test:dockerensure', description: 'test docker ensure function', - run: async config => { + run: async () => { try { await ensureDocker() } catch (error) {