chore: start using package-lock.json for repeatable Lambda Layer builds #2627
Conversation
We should have repeatable (strict dep versions) builds for our Lambda Layer released packages.
github-actions
bot
added
the
agent-nodejs
💚 Build Succeeded
Expand to view the summary
Build stats
Test stats 🧪
🤖 GitHub commentsTo re-run your PR in the CI, just comment with:
|
…irect 'traceparent' dep
…esn't cause node v14 user issues, we'll probably prefer this)
This also moves the authoritative logic for creating the lambda layer zip file to dev-utils/make-lambda-layer-zip.sh.
This reverts commit 2d359b9.
| @@ -17,7 +17,7 @@ pipeline { | |||
| NOTIFY_TO = 'build-apm+apm-agent-nodejs@elastic.co' | |||
| NPMRC_SECRET = 'secret/jenkins-ci/npmjs/elasticmachine' | |||
| TOTP_SECRET = 'totp/code/npmjs-elasticmachine' | |||
| NODE_VERSION = 'v14.17.5' | |||
| BUILD_NODE_VERSION = 'v16.15.0' | |||
There was a problem hiding this comment.
REVIEW NOTE:
- Switch to v16 for npm v8 which has
lockfileVersion: 2. - Rename to
BUILD_NODE_VERSIONto differentiate from other unrelated usages ofNODE_VERSIONin this and other files in the repo.
| @@ -383,7 +383,7 @@ pipeline { | |||
| environment { | |||
| HOME = "${env.WORKSPACE}" | |||
| RESULT_FILE = 'apm-agent-benchmark-results.json' | |||
| NODE_VERSION = '14' | |||
| BENCH_NODE_VERSION = '14' | |||
There was a problem hiding this comment.
REVIEW NOTE: Rename to BENCH_NODE_VERSION to differentiate from other unrelated usages of NODE_VERSION in this and other files in the repo.
| mv node_modules nodejs | ||
| .PHONY: dist | ||
| dist: validate-branch-name | ||
| ../dev-utils/make-lambda-layer-zip.sh |
There was a problem hiding this comment.
REVIEW NOTE: Move the authoritative logic for building the lambda layer zip to a separate script that can be used separate from CI (useful for local dev as well), and to not have the same logic in two places.
…ning that with npm v6 which silently doesn't support --omit=dev; .nvmrc doesn't support comments
|
I tested the created lambda layer with my play/dev Lambda briefly. |
Also rename the release pipeline step and Makefile target to "github-release" because that's what it is doing.
There was a problem hiding this comment.
No objection to this in principle. Using Node 16 makes sense to me and having the logic for doing build extracted from the CI Makefile so it can be used in different contexts seems like a reasonable step.
Approving, but my understanding of the .ci folder is very black box so I can't really comment on the more ground level changes made there -- so file this review under caveat emptor.
I think this change from #2627 may have broken running the benchmarks.
The change away from `NODE_VERSION` in #2627 surprisingly broke running the benchmarks. `NODE_VERSION` is used by nvm and exporting it (?) makes a difference?
The change away from `NODE_VERSION` in #2627 surprisingly broke running the benchmarks. `NODE_VERSION` is used by nvm and exporting it (?) makes a difference?
The `BRANCH_NAME=... make -C .ci dist` failed on
cp: ../build/aws/elastic-apm-node-lambda-layer-...zip: No such file or directory
because the "../build/aws" dir had not been created.
This was broken in #2627.
The `BRANCH_NAME=... make -C .ci dist` failed on
cp: ../build/aws/elastic-apm-node-lambda-layer-...zip: No such file or directory
because the "../build/aws" dir had not been created.
This was broken in #2627.
We should have repeatable (strict dep versions) builds for our Lambda
Layer released packages.
Closes: #2626
Checklist