From e37d731a44f9b58751e7ef19d8d4ca2b9eef5e64 Mon Sep 17 00:00:00 2001 From: Seth Vargo Date: Mon, 19 Aug 2024 16:52:51 -0500 Subject: [PATCH] Always update traffic if tag_traffic or revision_traffic is given (#535) Closes #533 This could potentially be a breaking change for anyone using a step that had an `image` or `source` with a `tag_traffic` or `revision_traffic`. Previously this would not re-deploy, but now it does. --- .github/workflows/integration.yml | 2 + README.md | 24 +++--- action.yml | 24 +++--- src/main.ts | 126 ++++++++++++++++-------------- tests/unit/main.test.ts | 56 ++++++++----- 5 files changed, 136 insertions(+), 96 deletions(-) diff --git a/.github/workflows/integration.yml b/.github/workflows/integration.yml index 751c42e..e33797b 100644 --- a/.github/workflows/integration.yml +++ b/.github/workflows/integration.yml @@ -113,6 +113,7 @@ jobs: env_vars_update_strategy: 'overwrite' secrets: /api/secrets/my-secret=${{ vars.SECRET_NAME }}:latest secrets_update_strategy: 'overwrite' + to_revision: 'LATEST=100' - name: 'Run re-deploy tests' run: 'npm run e2e-tests' @@ -201,6 +202,7 @@ jobs: with: image: 'gcr.io/cloudrun/hello' service: '${{ env.SERVICE_NAME }}' + to_revision: 'LATEST=100' - name: 'Run re-deploy tests' run: 'npm run e2e-tests' # Check that config isn't overwritten diff --git a/README.md b/README.md index f1ec6ce..2e62536 100644 --- a/README.md +++ b/README.md @@ -81,9 +81,9 @@ jobs: code](https://cloud.google.com/run/docs/deploying-source-code). - suffix: _(Optional)_ String suffix to append to the revision name. Revision names always start - with the service name automatically. For example, specifying 'v1' for a - service named 'helloworld', would lead to a revision named - 'helloworld-v1'. + with the service name automatically. For example, specifying `v1` for a + service named `helloworld`, would lead to a revision named + `helloworld-v1`. This option is only applies to services. - env_vars: _(Optional)_ List of environment variables that should be set in the environment. These are comma-separated or newline-separated `KEY=VALUE`. Keys or values @@ -179,7 +179,8 @@ jobs: Setting this to `true` will skip adding these special labels. -- tag: _(Optional)_ Traffic tag to assign to the newly-created revision. +- tag: _(Optional)_ Traffic tag to assign to the newly-created revision. This option is only + applies to services. - timeout: _(Optional)_ Maximum request execution time, specified as a duration like "10m5s" for ten minutes and 5 seconds. @@ -202,11 +203,12 @@ jobs: Please note, this GitHub Action does not parse or validate the flags. You are responsible for making sure the flags are available on the gcloud - version and subcommand. When using `tag_traffic` or `revision_traffic`, - the subcommand is `gcloud run services update-traffic`. For all other - values, the subcommand is `gcloud run deploy`. + version and subcommand. The provided flags will be appended to the + `deploy` command. When `revision_traffic` or `tag_traffic` are set, the + flags will also be appended to the subsequent `update-traffic` command. -- no_traffic: _(Optional, default: `false`)_ If true, the newly deployed revision will not receive traffic. +- no_traffic: _(Optional, default: `false`)_ If true, the newly deployed revision will not receive traffic. This option + is only applies to services. - revision_traffic: _(Optional)_ Comma-separated list of revision traffic assignments. @@ -218,14 +220,16 @@ jobs: with: revision_traffic: 'LATEST=100' - This is mutually-exclusive with `tag_traffic`. + This is mutually-exclusive with `tag_traffic`. This option is only applies + to services. - tag_traffic: _(Optional)_ Comma-separated list of tag traffic assignments. with: tag_traffic: 'my-tag=10' # percentage - This is mutually-exclusive with `revision_traffic`. + This is mutually-exclusive with `revision_traffic`. This option is only + applies to services. - project_id: _(Optional)_ ID of the Google Cloud project in which to deploy the service. diff --git a/action.yml b/action.yml index b6cb319..5e4a4d2 100644 --- a/action.yml +++ b/action.yml @@ -61,9 +61,9 @@ inputs: suffix: description: |- String suffix to append to the revision name. Revision names always start - with the service name automatically. For example, specifying 'v1' for a - service named 'helloworld', would lead to a revision named - 'helloworld-v1'. + with the service name automatically. For example, specifying `v1` for a + service named `helloworld`, would lead to a revision named + `helloworld-v1`. This option is only applies to services. required: false env_vars: @@ -186,7 +186,8 @@ inputs: tag: description: |- - Traffic tag to assign to the newly-created revision. + Traffic tag to assign to the newly-created revision. This option is only + applies to services. required: false timeout: @@ -215,14 +216,15 @@ inputs: Please note, this GitHub Action does not parse or validate the flags. You are responsible for making sure the flags are available on the gcloud - version and subcommand. When using `tag_traffic` or `revision_traffic`, - the subcommand is `gcloud run services update-traffic`. For all other - values, the subcommand is `gcloud run deploy`. + version and subcommand. The provided flags will be appended to the + `deploy` command. When `revision_traffic` or `tag_traffic` are set, the + flags will also be appended to the subsequent `update-traffic` command. required: false no_traffic: description: |- - If true, the newly deployed revision will not receive traffic. + If true, the newly deployed revision will not receive traffic. This option + is only applies to services. default: 'false' required: false @@ -238,7 +240,8 @@ inputs: with: revision_traffic: 'LATEST=100' - This is mutually-exclusive with `tag_traffic`. + This is mutually-exclusive with `tag_traffic`. This option is only applies + to services. required: false tag_traffic: @@ -248,7 +251,8 @@ inputs: with: tag_traffic: 'my-tag=10' # percentage - This is mutually-exclusive with `revision_traffic`. + This is mutually-exclusive with `revision_traffic`. This option is only + applies to services. required: false project_id: diff --git a/src/main.ts b/src/main.ts index 2c54aa0..0b6ae6d 100644 --- a/src/main.ts +++ b/src/main.ts @@ -119,8 +119,7 @@ export async function run(): Promise { const skipDefaultLabels = parseBoolean(getInput('skip_default_labels')); const flags = getInput('flags'); - let responseType = ResponseTypes.DEPLOY; // Default response type for output parsing - let cmd; + let deployCmd: string[] = []; // Throw errors if inputs aren't valid if (revTraffic && tagTraffic) { @@ -153,23 +152,15 @@ export async function run(): Promise { } // Find base command - if (revTraffic || tagTraffic) { - // Set response type for output parsing - responseType = ResponseTypes.UPDATE_TRAFFIC; - - // Update traffic - cmd = ['run', 'services', 'update-traffic', service]; - if (revTraffic) cmd.push('--to-revisions', revTraffic); - if (tagTraffic) cmd.push('--to-tags', tagTraffic); - } else if (metadata) { + if (metadata) { const contents = await readFile(metadata, 'utf8'); const parsed = parseYAML(contents); const kind = parsed?.kind; if (kind === 'Service') { - cmd = ['run', 'services', 'replace', metadata]; + deployCmd = ['run', 'services', 'replace', metadata]; } else if (kind === 'Job') { - cmd = ['run', 'jobs', 'replace', metadata]; + deployCmd = ['run', 'jobs', 'replace', metadata]; } else { throw new Error(`Unkown metadata type "${kind}", expected "Job" or "Service"`); } @@ -179,80 +170,92 @@ export async function run(): Promise { `not covered by the semver backwards compatibility guarantee.`, ); - cmd = ['run', 'jobs', 'deploy', job]; + deployCmd = ['run', 'jobs', 'deploy', job]; if (image) { - cmd.push('--image', image); + deployCmd.push('--image', image); } else if (source) { - cmd.push('--source', source); + deployCmd.push('--source', source); } // Set optional flags from inputs - setEnvVarsFlags(cmd, envVars, envVarsFile, envVarsUpdateStrategy); - setSecretsFlags(cmd, secrets, secretsUpdateStrategy); + setEnvVarsFlags(deployCmd, envVars, envVarsFile, envVarsUpdateStrategy); + setSecretsFlags(deployCmd, secrets, secretsUpdateStrategy); // There is no --update-secrets flag on jobs, but there will be in the // future. At that point, we can remove this. - const idx = cmd.indexOf('--update-secrets'); + const idx = deployCmd.indexOf('--update-secrets'); if (idx >= 0) { logWarning( `Cloud Run does not allow updating secrets on jobs, ignoring ` + `"secrets_update_strategy" value of "merge"`, ); - cmd[idx] = '--set-secrets'; + deployCmd[idx] = '--set-secrets'; } // Compile the labels const defLabels = skipDefaultLabels ? {} : defaultLabels(); const compiledLabels = Object.assign({}, defLabels, labels); if (compiledLabels && Object.keys(compiledLabels).length > 0) { - cmd.push('--labels', joinKVStringForGCloud(compiledLabels)); + deployCmd.push('--labels', joinKVStringForGCloud(compiledLabels)); } } else { - cmd = ['run', 'deploy', service]; + deployCmd = ['run', 'deploy', service]; if (image) { - cmd.push('--image', image); + deployCmd.push('--image', image); } else if (source) { - cmd.push('--source', source); + deployCmd.push('--source', source); } // Set optional flags from inputs - setEnvVarsFlags(cmd, envVars, envVarsFile, envVarsUpdateStrategy); - setSecretsFlags(cmd, secrets, secretsUpdateStrategy); + setEnvVarsFlags(deployCmd, envVars, envVarsFile, envVarsUpdateStrategy); + setSecretsFlags(deployCmd, secrets, secretsUpdateStrategy); if (tag) { - cmd.push('--tag', tag); + deployCmd.push('--tag', tag); } - if (suffix) cmd.push('--revision-suffix', suffix); - if (noTraffic) cmd.push('--no-traffic'); - if (timeout) cmd.push('--timeout', timeout); + if (suffix) deployCmd.push('--revision-suffix', suffix); + if (noTraffic) deployCmd.push('--no-traffic'); + if (timeout) deployCmd.push('--timeout', timeout); // Compile the labels const defLabels = skipDefaultLabels ? {} : defaultLabels(); const compiledLabels = Object.assign({}, defLabels, labels); if (compiledLabels && Object.keys(compiledLabels).length > 0) { - cmd.push('--update-labels', joinKVStringForGCloud(compiledLabels)); + deployCmd.push('--update-labels', joinKVStringForGCloud(compiledLabels)); } } + // Traffic flags + let updateTrafficCmd = ['run', 'services', 'update-traffic', service]; + if (revTraffic) updateTrafficCmd.push('--to-revisions', revTraffic); + if (tagTraffic) updateTrafficCmd.push('--to-tags', tagTraffic); + // Push common flags - cmd.push('--format', 'json'); + deployCmd.push('--format', 'json'); + updateTrafficCmd.push('--format', 'json'); + if (region?.length > 0) { - cmd.push( - '--region', - region - .flat() - .filter((e) => e !== undefined && e !== null && e !== '') - .join(','), - ); + const regions = region + .flat() + .filter((e) => e !== undefined && e !== null && e !== '') + .join(','); + deployCmd.push('--region', regions); + updateTrafficCmd.push('--region', regions); + } + if (projectId) { + deployCmd.push('--project', projectId); + updateTrafficCmd.push('--project', projectId); } - if (projectId) cmd.push('--project', projectId); // Add optional flags if (flags) { const flagList = parseFlags(flags); - if (flagList) cmd = cmd.concat(flagList); + if (flagList) { + deployCmd = deployCmd.concat(flagList); + updateTrafficCmd = updateTrafficCmd.concat(flagList); + } } // Install gcloud if not already installed. @@ -266,7 +269,8 @@ export async function run(): Promise { // Install gcloud component if needed and prepend the command if (gcloudComponent) { await installGcloudComponent(gcloudComponent); - cmd.unshift(gcloudComponent); + deployCmd.unshift(gcloudComponent); + updateTrafficCmd.unshift(gcloudComponent); } // Authenticate - this comes from google-github-actions/auth. @@ -280,25 +284,33 @@ export async function run(): Promise { const toolCommand = getToolCommand(); const options = { silent: !isDebug, ignoreReturnCode: true }; - const commandString = `${toolCommand} ${cmd.join(' ')}`; + const commandString = `${toolCommand} ${deployCmd.join(' ')}`; logInfo(`Running: ${commandString}`); - logDebug(JSON.stringify({ toolCommand: toolCommand, args: cmd, options: options }, null, ' ')); - - // Run gcloud cmd. - const output = await getExecOutput(toolCommand, cmd, options); - if (output.exitCode !== 0) { - const errMsg = output.stderr || `command exited ${output.exitCode}, but stderr had no output`; + logDebug( + JSON.stringify({ toolCommand: toolCommand, args: deployCmd, options: options }, null, ' '), + ); + + // Run deploy command + const deployCmdExec = await getExecOutput(toolCommand, deployCmd, options); + if (deployCmdExec.exitCode !== 0) { + const errMsg = + deployCmdExec.stderr || + `command exited ${deployCmdExec.exitCode}, but stderr had no output`; throw new Error(`failed to execute gcloud command \`${commandString}\`: ${errMsg}`); } + setActionOutputs(parseDeployResponse(deployCmdExec.stdout, { tag: tag })); - // Map outputs by response type - const outputs: DeployCloudRunOutputs = - responseType === ResponseTypes.UPDATE_TRAFFIC - ? parseUpdateTrafficResponse(output.stdout) - : parseDeployResponse(output.stdout, { tag: tag }); - - // Map outputs to GitHub actions output - setActionOutputs(outputs); + // Run revision/tag command + if (revTraffic || tagTraffic) { + const updateTrafficExec = await getExecOutput(toolCommand, updateTrafficCmd, options); + if (updateTrafficExec.exitCode !== 0) { + const errMsg = + updateTrafficExec.stderr || + `command exited ${updateTrafficExec.exitCode}, but stderr had no output`; + throw new Error(`failed to execute gcloud command \`${commandString}\`: ${errMsg}`); + } + setActionOutputs(parseUpdateTrafficResponse(updateTrafficExec.stdout)); + } } catch (err) { const msg = errorMessage(err); setFailed(`google-github-actions/deploy-cloudrun failed with: ${msg}`); diff --git a/tests/unit/main.test.ts b/tests/unit/main.test.ts index c0b56ce..4b2f2a0 100644 --- a/tests/unit/main.test.ts +++ b/tests/unit/main.test.ts @@ -101,7 +101,7 @@ test('#run', { concurrency: true }, async (suite) => { }); await run(); - const args = mocks.getExecOutput.mock.calls?.at(0).arguments?.at(1); + const args = mocks.getExecOutput.mock.calls?.at(0)?.arguments?.at(1); assertMembers(args, ['--project', 'my-test-project']); }); @@ -112,7 +112,7 @@ test('#run', { concurrency: true }, async (suite) => { }); await run(); - const args = mocks.getExecOutput.mock.calls?.at(0).arguments?.at(1); + const args = mocks.getExecOutput.mock.calls?.at(0)?.arguments?.at(1); assertMembers(args, ['--region', 'us-central1']); }); @@ -123,7 +123,7 @@ test('#run', { concurrency: true }, async (suite) => { }); await run(); - const args = mocks.getExecOutput.mock.calls?.at(0).arguments?.at(1); + const args = mocks.getExecOutput.mock.calls?.at(0)?.arguments?.at(1); assertMembers(args, ['--region', 'us-central1,us-east1']); }); @@ -209,7 +209,7 @@ test('#run', { concurrency: true }, async (suite) => { await run(); - const args = mocks.getExecOutput.mock.calls?.at(0).arguments?.at(1); + const args = mocks.getExecOutput.mock.calls?.at(0)?.arguments?.at(1); const envVars = splitKV(args.at(args.indexOf('--update-env-vars') + 1)); assert.deepStrictEqual(envVars, { FOO: 'BAR' }); }); @@ -223,7 +223,7 @@ test('#run', { concurrency: true }, async (suite) => { await run(); - const args = mocks.getExecOutput.mock.calls?.at(0).arguments?.at(1); + const args = mocks.getExecOutput.mock.calls?.at(0)?.arguments?.at(1); const envVars = splitKV(args.at(args.indexOf('--set-env-vars') + 1)); assert.deepStrictEqual(envVars, { FOO: 'BAR' }); }); @@ -236,7 +236,7 @@ test('#run', { concurrency: true }, async (suite) => { await run(); - const args = mocks.getExecOutput.mock.calls?.at(0).arguments?.at(1); + const args = mocks.getExecOutput.mock.calls?.at(0)?.arguments?.at(1); const envVars = splitKV(args.at(args.indexOf('--update-secrets') + 1)); assert.deepStrictEqual(envVars, { FOO: 'bar:latest' }); }); @@ -250,7 +250,7 @@ test('#run', { concurrency: true }, async (suite) => { await run(); - const args = mocks.getExecOutput.mock.calls?.at(0).arguments?.at(1); + const args = mocks.getExecOutput.mock.calls?.at(0)?.arguments?.at(1); const envVars = splitKV(args.at(args.indexOf('--set-secrets') + 1)); assert.deepStrictEqual(envVars, { FOO: 'bar:latest' }); }); @@ -271,7 +271,7 @@ test('#run', { concurrency: true }, async (suite) => { 'foo': 'bar', 'zip': 'zap', }; - const args = mocks.getExecOutput.mock.calls?.at(0).arguments?.at(1); + const args = mocks.getExecOutput.mock.calls?.at(0)?.arguments?.at(1); const labels = splitKV(args.at(args.indexOf('--update-labels') + 1)); assert.deepStrictEqual(labels, expectedLabels); }); @@ -289,7 +289,7 @@ test('#run', { concurrency: true }, async (suite) => { foo: 'bar', zip: 'zap', }; - const args = mocks.getExecOutput.mock.calls?.at(0).arguments?.at(1); + const args = mocks.getExecOutput.mock.calls?.at(0)?.arguments?.at(1); const labels = splitKV(args.at(args.indexOf('--update-labels') + 1)); assert.deepStrictEqual(labels, expectedLabels); }); @@ -307,7 +307,7 @@ test('#run', { concurrency: true }, async (suite) => { 'managed-by': 'github-actions', 'commit-sha': 'custom-value', }; - const args = mocks.getExecOutput.mock.calls?.at(0).arguments?.at(1); + const args = mocks.getExecOutput.mock.calls?.at(0)?.arguments?.at(1); const labels = splitKV(args.at(args.indexOf('--update-labels') + 1)); assert.deepStrictEqual(labels, expectedLabels); }); @@ -321,7 +321,7 @@ test('#run', { concurrency: true }, async (suite) => { await run(); - const args = mocks.getExecOutput.mock.calls?.at(0).arguments?.at(1); + const args = mocks.getExecOutput.mock.calls?.at(0)?.arguments?.at(1); assertMembers(args, ['--source', 'example-app']); }); @@ -333,7 +333,7 @@ test('#run', { concurrency: true }, async (suite) => { await run(); - const args = mocks.getExecOutput.mock.calls?.at(0).arguments?.at(1); + const args = mocks.getExecOutput.mock.calls?.at(0)?.arguments?.at(1); assertMembers(args, ['services', 'replace']); }); @@ -345,7 +345,7 @@ test('#run', { concurrency: true }, async (suite) => { await run(); - const args = mocks.getExecOutput.mock.calls?.at(0).arguments?.at(1); + const args = mocks.getExecOutput.mock.calls?.at(0)?.arguments?.at(1); assertMembers(args, ['jobs', 'replace']); }); @@ -357,7 +357,7 @@ test('#run', { concurrency: true }, async (suite) => { await run(); - const args = mocks.getExecOutput.mock.calls?.at(0).arguments?.at(1); + const args = mocks.getExecOutput.mock.calls?.at(0)?.arguments?.at(1); assertMembers(args, ['--timeout', '55m12s']); }); @@ -369,20 +369,23 @@ test('#run', { concurrency: true }, async (suite) => { await run(); - const args = mocks.getExecOutput.mock.calls?.at(0).arguments?.at(1); + const args = mocks.getExecOutput.mock.calls?.at(0)?.arguments?.at(1); assertMembers(args, ['--tag', 'test']); }); await suite.test('sets tag traffic if given', async (t) => { const mocks = defaultMocks(t.mock, { service: 'my-test-service', - tag: 'test', + tag_traffic: 'TEST=100', }); await run(); - const args = mocks.getExecOutput.mock.calls?.at(0).arguments?.at(1); - assertMembers(args, ['--tag', 'test']); + const deployArgs = mocks.getExecOutput.mock.calls?.at(0)?.arguments?.at(1); + assertMembers(deployArgs, ['run', 'deploy', 'my-test-service']); + + const updateTrafficArgs = mocks.getExecOutput.mock.calls?.at(1)?.arguments?.at(1); + assertMembers(updateTrafficArgs, ['--to-tags', 'TEST=100']); }); await suite.test('fails if tag traffic and revision traffic are provided', async (t) => { @@ -414,6 +417,21 @@ test('#run', { concurrency: true }, async (suite) => { ); }); + await suite.test('sets revision traffic if given', async (t) => { + const mocks = defaultMocks(t.mock, { + service: 'my-test-service', + revision_traffic: 'TEST=100', + }); + + await run(); + + const deployArgs = mocks.getExecOutput.mock.calls?.at(0)?.arguments?.at(1); + assertMembers(deployArgs, ['run', 'deploy', 'my-test-service']); + + const updateTrafficArgs = mocks.getExecOutput.mock.calls?.at(1)?.arguments?.at(1); + assertMembers(updateTrafficArgs, ['--to-revisions', 'TEST=100']); + }); + await suite.test('fails if service is not provided with revision traffic', async (t) => { defaultMocks(t.mock, { service: '', @@ -449,7 +467,7 @@ test('#run', { concurrency: true }, async (suite) => { await run(); - const args = mocks.getExecOutput.mock.calls?.at(0).arguments?.at(1); + const args = mocks.getExecOutput.mock.calls?.at(0)?.arguments?.at(1); assertMembers(args, ['run', 'jobs', 'deploy', 'my-test-job']); }); });