[CNSL-1934] Add pre-release pending deploy branch management#105
[CNSL-1934] Add pre-release pending deploy branch management#105linhcrl wants to merge 1 commit intocockroachdb:mainfrom
Conversation
linhcrl
changed the title
linhcrl
force-pushed
the
pre-release-branch-management
branch
from
f81608f to
427284b
Compare
linhcrl
force-pushed
the
pre-release-branch-management
branch
5 times, most recently
from
f9a2f35 to
cb4189b
Compare
linhcrl
force-pushed
the
pre-release-branch-management
branch
from
cb4189b to
3f5a6bc
Compare
fantapop
left a comment
fantapop
left a comment
There was a problem hiding this comment.
I left some feedback. In general, this feels like a lot of logic to be inlining into these workflows and going untested. Did you consider the approach we took in the actions repo to separate out some of the logic to actual code files?
| GH_TOKEN: ${{ secrets.MANAGED_SERVICE_TOKEN }} | ||
| run: | | ||
| # Get all release tags matching release-YYYY-MM-DD-N pattern | ||
| all_tags=$(gh api repos/cockroachlabs/managed-service/tags --paginate --jq '.[].name' | grep -E '^release-[0-9]{4}-[0-9]{2}-[0-9]{2}-[0-9]+$') |
There was a problem hiding this comment.
what does --paginate do here? Does that restrict us to the first page?
There was a problem hiding this comment.
added a comment to explain why it's necessary
| > not_deployed.txt | ||
| > missing_trailer.txt | ||
| > unexpected_status.txt |
There was a problem hiding this comment.
This is kind of a weird syntax... I guess if it works. Normally I would use touch
| while IFS='|' read -r sha subject; do | ||
| echo "Checking commit $sha: $subject" | ||
|
|
||
| # Extract Managed-service-commit-SHA trailer from commit message |
There was a problem hiding this comment.
I'm not really clear whats going on here. Is trailer the right terminology to be used?
There was a problem hiding this comment.
"Trailer" should be the correct Git terminology here. Git commit trailers are standardized key-value pairs at the end of commit messages (like Signed-off-by: or Co-Authored-By:).
See the git-interpret-trailers for more info.
Context:
SDK commits include a Managed-service-commit-SHA: trailer that links back to the managed-service commit that generated the SDK changes.
What this part of the code does:
For each commit in the pending deploy branch, this section:
- Extracts the managed service commit SHA trailer
- If the trailer is missing, the commit gets categorized in
missing_trailer.txt(these are commits we can't verify the deployment status of) - If the trailer exists, it checks whether that managed-service SHA has been deployed by verifying it's in the latest release tag. If the SHA has not been deployed yet, we write it to the
not_deployed.txtfile. If we can't determine deployment status, we write it tounexpected_status.txt. - At the end, we look at the contents of
missing_trailer.txt,not_deployed.txt, andunexpected_status.txt. If any of them contain data, we fail the PR and comment on the PR with information about why the check failed. I included a screenshot of what this looks like
There was a problem hiding this comment.
Comment left by pending-deploy-check.yml on failed check
| - name: Fetch latest remote refs | ||
| run: | | ||
| # Ensure we have the latest branches and tags from origin | ||
| git fetch origin --tags --force | ||
| git fetch origin --prune |
There was a problem hiding this comment.
I was surprised this step would be necessary in either workflow. Can you document why that is in a comment?
| git fetch origin --tags --force | ||
| git fetch origin --prune | ||
|
|
||
| - name: Find latest pending deploy branch |
There was a problem hiding this comment.
It looks like a lot of the stuff from this workflow are copied from the other workflow. Maybe it would make sense to bundle this stuff into an action instead.
There was a problem hiding this comment.
It doesn't seem like there's enough overlap to warrant shared actions but I moved things into shell scripts for better readability and left comments to explain what's happening
linhcrl
force-pushed
the
pre-release-branch-management
branch
20 times, most recently
from
1b798ac to
932dd50
Compare
Introduces two workflows to manage the release process:
1. pending-deploy-pr.yml (workflow_dispatch trigger):
- Finds the latest pending-deploy-YYYYMMDD-hhmmss branch
- Creates a PR to merge it into main
2. pending-deploy-check.yml (pull_request trigger):
- Validates pending deploy PRs before merge
- Checks that Managed-service-commit-SHA trailers reference deployed
commits
- Blocks merge until all changes are confirmed deployed in
managed-service
- Posts PR comments detailing any undeployed commits
This ensures SDK releases only reference deployed CC API changes.
Co-Authored-By: roachdev-claude <[email protected]>
linhcrl
force-pushed
the
pre-release-branch-management
branch
from
932dd50 to
5a243b1
Compare
2 participants
Introduces two workflows to manage the release process:
pending-deploy-pr.yml (repository_dispatch trigger):
pending-deploy-check.yml (pull_request trigger):
This ensures SDK releases only include changes that are live in production.
Relevant screenshots
PR opened by pending-deploy-pr.yml
Comment left by pending-deploy-check.yml on failed check