promote: add check_r2_assets tool #3977
Merged
+618
−1
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
richardlau
force-pushed
the
assets
branch
2 times, most recently
from
8727a29 to
cc8c87e
Compare
richardlau
commented
Dec 9, 2024
Comment on lines
+27
to
+30
| relative_srcdir=${srcdir/$staging_rootdir/"$site/"} | ||
| relative_dstdir=${dstdir/$dist_rootdir/"$site/"} | ||
|
|
||
| node --no-warnings /home/staging/tools/promote/check_r2_assets.mjs $staging_bucket/$relative_srcdir/$2 $prod_bucket/$relative_dstdir/$2 |
There was a problem hiding this comment.
@targos FYI I have deployed this PR onto www and am fairly confident it should work (without doing an actual release). If this does interfere with nodejs/node#56119 then commenting out these additional lines should restore the old behaviour (i.e. check the DO droplet only).
To illustrate the difference: $ node --no-warnings /home/staging/tools/promote/check_assets.js /home/staging/nodejs/release/v22.12.0 /home/dist/nodejs/release/v22.12.0
... Checking assets
... Expecting a total of 47 assets for v22.x
... 0 assets waiting in staging
... 47 assets already promoted
✓ Complete set of expected assets in place for v22.x
$Equivalent check on R2: $ node --no-warnings /home/staging/tools/promote/check_r2_assets.mjs r2:dist-staging/nodejs/release/v22.12.0 r2:dist-prod/nodejs/release/v22.12.0
... Checking R2 assets
... Expecting a total of 47 assets for v22.x
... 47 assets waiting in R2 staging
⚠ 47 assets already promoted in R2 will be overwritten, is this OK?
✓ Complete set of expected assets in place for v22.x
Promote if you are certain this is the the correct course of action
$ |
richardlau
force-pushed
the
assets
branch
2 times, most recently
from
bda6f57 to
1f52a06
Compare
Add tool and tests for checking if assets on R2 look ready for promotion. Relies on `rclone` being available -- there's a test for the case where it is not. The new tool is based on existing `check_assets.js` but: - Written in ESM. - Uses built-in Set objects instead of emulating sets with Arrays. - Always assumes an asset file exists. `check_assets.js` has some fallback logic, but it's not straightforward and nowadays we can consider a missing asset file to be a process failure. - Reflects that `.done` files are not used for R2 (basically they don't exist so no handling logic). Also note that for R2, promotion is a copy and not a move so after a partial promotion subsequent checks will result in overwrite warnings for files previously promoted. - `SHASUMS256.txt` can end up in the staging directory. Ignore for the staging directory as `check_assets.js` does for dist. (The shasums are generated by later parts of the release process.)
targos
approved these changes
Dec 10, 2024
| console.log('... Checking R2 assets'); | ||
| // No expected asset list available for this line | ||
| if (expectedAssets.size === 0) { | ||
| console.log(` \u001b[31m\u001b[1m✖\u001b[22m\u001b[39m No expected asset list is available for ${line}, does one need to be created?`); |
There was a problem hiding this comment.
Feel free to ignore, but these could be simplified with util.styleText
There was a problem hiding this comment.
SGTM, but let's do that in a follow up. The same applies to check_assets.js.
Neither am I 🙂. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Add tool and tests for checking if assets on R2 look ready for promotion. Relies on
rclonebeing available -- there's a test for the case where it is not.The new tool is based on existing
check_assets.jsbut:check_assets.jshas some fallback logic, but it's not straightforward and nowadays we can consider a missing asset file to be a process failure..donefiles are not used for R2 (basically they don't exist so no handling logic). Also note that for R2, promotion is a copy and not a move so after a partial promotion subsequent checks will result in overwrite warnings for files previously promoted.SHASUMS256.txtcan end up in the staging directory. Ignore for the staging directory ascheck_assets.jsdoes for dist. (The shasums are generated by later parts of the release process.)Fixes: #3958
FYI @nodejs/releasers