feat: action to rollback cloud run if health check fails #2
Conversation
|
Warning Rate limit exceeded@rot1024 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 15 minutes and 26 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
WalkthroughThis pull request introduces a new GitHub Action named Deploy Cloud Run service. The action is designed to facilitate the deployment of a Cloud Run service by retrieving the current revision, deploying a new image, and updating traffic to point to the latest revision. It accepts inputs for the region, image, service, and optionally a health check URL. When a health check URL is provided, the action performs a check and, if it fails (HTTP status 300 or above), it rolls back the traffic to the previous revision. The action outputs the state of the deployment indicating either success or failure. Changes
Sequence Diagram(s)sequenceDiagram
participant A as GitHub Runner
participant G as gcloud CLI
participant H as Health Check
A->>G: Retrieve current revision\n(gcloud run services describe)
G-->>A: Return current revision
A->>G: Deploy new image\n(gcloud run deploy)
G-->>A: Confirm deployment
A->>G: Update traffic to latest revision
G-->>A: Traffic updated
Note over A: Optional health check if URL provided
A->>H: Send health check request
H-->>A: Return HTTP status code
alt Health check fails (status ≥ 300)
A->>G: Rollback\n(Update traffic to previous revision)
G-->>A: Rollback successful
A->>A: Mark deployment as failed
else Health check passes
A->>A: Mark deployment as successful
end
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
.github/actions/deploy-cloud-run/action.yml (4)
1-15: Nitpick: Fix Typo in Input Description
There's a minor spelling error in the "image" input description ("Image nmae"). Correcting it to "Image name" would improve clarity.- image: - description: Image nmae - required: true + image: + description: Image name + required: true
48-60: Suggestion: Use Modern Command Substitution and Safe Variable Use in Health Check
In the health check step, consider using the$()syntax instead of backticks for better readability and nestability. Also, quoting the variable in the numeric comparison ensures safety if$RESULTis empty.- RESULT=`curl -s -o /dev/null -w "%{http_code}" "${{ inputs.health_check_url }}"` - echo "Status code is $RESULT" - echo "status_code=$RESULT" >> "$GITHUB_OUTPUT" - if [ $RESULT -ge 300 ]; then - echo "Health check failed" - echo "error=true" >> "$GITHUB_OUTPUT" - fi + RESULT=$(curl -s -o /dev/null -w "%{http_code}" "${{ inputs.health_check_url }}") + echo "Status code is ${RESULT}" + echo "status_code=${RESULT}" >> "$GITHUB_OUTPUT" + if [ "${RESULT}" -ge 300 ]; then + echo "Health check failed" + echo "error=true" >> "$GITHUB_OUTPUT" + fi
61-68: Suggestion: Explicitly Check Health Check Error Output
The rollback step currently triggers if${{ steps.health_check.outputs.error }}is set. For improved clarity and robustness, consider explicitly checking that it equals'true'. This avoids accidental activation if the output were set to any non-empty string.Example adjustment for the
ifcondition in the rollback step:- - name: Rollback - if: ${{ steps.health_check.outputs.error }} + - name: Rollback + if: ${{ steps.health_check.outputs.error == 'true' }}
69-77: Suggestion: Consistency in Final Step Condition
The "Finish" step checks if theerroroutput is empty to determine success. For consistency with the rollback step (after the above suggestion), consider explicitly comparing against "true" (or "false") throughout. Additionally, setting a default value forerrorin the health check step (e.g.,echo "error=false" >> "$GITHUB_OUTPUT") when the health check is skipped could improve clarity.Would you like a diff suggestion to standardize the error handling for both rollback and finish steps?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/actions/deploy-cloud-run/action.yml(1 hunks)
🔇 Additional comments (2)
.github/actions/deploy-cloud-run/action.yml (2)
16-30: Good Use of Command and Output Capturing
The "Get current revision" step correctly retrieves the current Cloud Run revision and stores it in the GitHub output. Consider adding error handling in case thegcloudcommand returns an empty value, but overall the implementation is clear and functional.
40-47: LGTM: Update Traffic Step is Clear
The "Update traffic" step correctly forces the Cloud Run service traffic to update to the latest revision. All the input interpolations and commands appear correct.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
.github/actions/deploy-cloud-run/action.yml (2)
3-15: Well-Defined Inputs with Guidance
All required inputs (region,image,service) and the optionalhealth_check_urlare defined with meaningful descriptions. For thehealth_check_url, consider clarifying (either via documentation or an inline comment) that multiple URLs should be provided in a specific format (e.g., newline separated) to ensure they are parsed correctly in the loop.
48-64: Health Check Step: Consider Enhancing URL Handling
The health check loop iterates over${{ inputs.health_check_url }}without explicit quoting. If a user supplies multiple URLs separated by line breaks or spaces, the splitting might not behave as intended. Consider setting the Internal Field Separator (IFS) for newline splitting or providing clear guidelines on the input format to ensure robust handling of multiple URLs. For example:-run: | - for url in ${{ inputs.health_check_url }}; do +run: | + IFS=$'\n' + for url in ${{ inputs.health_check_url }}; do
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/actions/deploy-cloud-run/action.yml(1 hunks)
🔇 Additional comments (7)
.github/actions/deploy-cloud-run/action.yml (7)
1-2: Clear Action Name and Description
The action name and description are concise and clearly communicate the intent of rolling back on health check failure.
16-18: Appropriate Use of Composite Runner
Therunsblock uses the composite runner effectively to orchestrate multiple steps.
19-30: Robust Revision Retrieval
The "Get current revision" step cleanly retrieves the latest ready revision usinggcloud run services describeand captures it into the GitHub Actions output. The use of a multi-line command with backslashes improves readability.
31-39: Deploy Step Uses Correct Variable Interpolation
The deploy step correctly invokesgcloud run deployand uses the GitHub Actions expression syntax"${{ inputs.service }}"for the service name, in line with best practices. This addresses previous concerns regarding variable interpolation.
40-47: Traffic Update Step is Straightforward
The "Update traffic" step uses thegcloud run services update-trafficcommand correctly to switch traffic to the latest revision.
65-72: Rollback Step is Correctly Conditioned
The rollback step is appropriately conditioned on the presence of an error output from the Health check step and correctly reverts the service to the previously stored revision.
73-81: Finish Step Provides Clear Outcome
The "Finish" step correctly differentiates between successful and failed deployments based on whether an error was noted. Exiting with a status code of 1 in failure cases properly signals the action’s outcome.
| - name: Deploy | ||
| shell: bash | ||
| run: | | ||
| gcloud run deploy "${{ inputs.service }}" \ | ||
| --image "${{ inputs.image }}" \ | ||
| --region "${{ inputs.region }}" \ | ||
| --platform managed \ | ||
| --quiet |
There was a problem hiding this comment.
We could utilize this GH actions
| # ensure that the traffic is updated to the latest revision always | ||
| - name: Update traffic | ||
| shell: bash | ||
| run: | | ||
| gcloud run services update-traffic "${{ inputs.service }}" \ | ||
| --to-latest \ | ||
| --region "${{ inputs.region }}" |
There was a problem hiding this comment.
If we are using aforementioned GH actions, we could make sure that Cloud Run will route traffic to the latest revision via GH actions inputs.
| # if the URL is invalid, curl itself will return an error code | ||
| - name: Health check | ||
| id: health_check | ||
| if: ${{ inputs.health_check_url }} |
There was a problem hiding this comment.
We could utilize aforementioned GH actions output to get URL.
But the problem is to hit / or /api/ping as we have different healthcheck path for API service and Web service.
| service: | ||
| description: Name of the Cloud Run service | ||
| required: true | ||
| health_check_url: |
There was a problem hiding this comment.
Since the services are publicly accessible anyway and we don't implement special host-based routing within the service itself, We might use path instead and leverage cloud-run URL for testing instead
| health_check_url: | |
| health_check_path: |
There was a problem hiding this comment.
Using Cloud Run URLs may cause the application to not work properly. Even if you use this, the addition of the Host header to curl is still required. In other words, the need to specify a URL unfortunately cannot be eliminated.
There was a problem hiding this comment.
Noted on this. If this is the hard constraint from the service side then my suggestion is not relevant.
There was a problem hiding this comment.
Note: this might be useful if we want to explicitly shows the path or list of paths for healthcheck
| echo "Current revision: $REV" | ||
| echo "revision=$REV" >> "$GITHUB_OUTPUT" | ||
|
|
||
| - name: Deploy |
There was a problem hiding this comment.
OTOH and could be out of scope but to be on the safer approach, we might as well utilize revision tags to do test on service healthcheck path before it's serving live traffic. Somewhat like blue/green deployment.
Hence potentially reducing chance of having to do rollback
https://cloud.google.com/run/docs/rollouts-rollbacks-traffic-migration#tags
| - name: Deploy | ||
| shell: bash | ||
| run: | | ||
| gcloud run deploy "${{ inputs.service }}" \ | ||
| --image "${{ inputs.image }}" \ | ||
| --region "${{ inputs.region }}" \ | ||
| --platform managed \ | ||
| --quiet |
There was a problem hiding this comment.
If we want to utilize the deploy-cloudrun github-action we can do somethink like this
| - name: Deploy | |
| shell: bash | |
| run: | | |
| gcloud run deploy "${{ inputs.service }}" \ | |
| --image "${{ inputs.image }}" \ | |
| --region "${{ inputs.region }}" \ | |
| --platform managed \ | |
| --quiet | |
| - id: 'deploy_green' | |
| name: Deploy new tagged-revision in green version | |
| uses: 'google-github-actions/deploy-cloudrun@v2' | |
| with: | |
| service: ${{ inputs.service }} | |
| image: ${{ inputs.image }} | |
| region: ${{ inputs.region }} | |
| tag_traffic: 'green=0' # this mean the new release will be having 0% traffic directed to it | |
| - id: 'health_check_green' | |
| name: Check if healthcheck path in green version returns healthy responses | |
| if: ${{ inputs.health_check_url }} | |
| shell: bash | |
| run: | | |
| for url in "{{ inputs.health_check_url }}"; do | |
| echo "Checking health of $url" | |
| echo "Parsing host of $url" | |
| HOST=`echo $url | sed -E 's|https?://([^/]+).*|\1|'` | |
| RESULT=`curl -m 10 -H "Host: $HOST" -s -o /dev/null -w "%{http_code}" "$url"` | |
| echo "Status code is $RESULT" | |
| if [ $RESULT -ge 300 ]; then | |
| echo "Health check failed" | |
| echo "error=true" >> "$GITHUB_OUTPUT" | |
| break | |
| fi | |
| done |
There was a problem hiding this comment.
@rot1024 updated the suggestion to supply the Host Header
| # ensure that the traffic is updated to the latest revision always | ||
| - name: Update traffic | ||
| shell: bash | ||
| run: | | ||
| gcloud run services update-traffic "${{ inputs.service }}" \ | ||
| --to-latest \ | ||
| --region "${{ inputs.region }}" |
There was a problem hiding this comment.
We can later direct all traffic to green tag if healthcheck is okay
| # ensure that the traffic is updated to the latest revision always | |
| - name: Update traffic | |
| shell: bash | |
| run: | | |
| gcloud run services update-traffic "${{ inputs.service }}" \ | |
| --to-latest \ | |
| --region "${{ inputs.region }}" | |
| - id: 'direct_green' | |
| name: Direct all traffics to green/latest revision if healthcheck is passing | |
| if: ${{ steps.health_check.outputs.error != "true" }} # I believe `true` in health_check output is not parsed as bool but string | |
| uses: 'google-github-actions/deploy-cloudrun@v2' | |
| with: | |
| service: ${{ inputs.service }} | |
| image: ${{ inputs.image }} | |
| region: ${{ inputs.region }} | |
| revision_traffic: 'LATEST=100' # this mean the new release will be having 100% traffic directed to it | |
| - name: Remove green tag | |
| needs: direct_green | |
| shell: bash | |
| run: | | |
| gcloud run services update-traffic "${{ inputs.service }}" \ | |
| --remove-tags green \ | |
| --region "${{ inputs.region }}" |
Summary by CodeRabbit