Skip to content

chore: Mark 1.x as End-of-Support #994

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 12 commits into from
Aug 29, 2022

Conversation

texastony
Copy link
Contributor

@texastony texastony commented Aug 25, 2022

Issue #, if available: Mark 1.x as End-of-Support

Description of changes:

  • Client emits a log warning on creation detailing 1.x is in End-of-Support
  • Added the support policy from master

Update Support Policy, marking:

  • 1.x as EOS
  • 2.x as Maintenance
  • 3.x as GA

Back-ported CodeBuild NPM Publish automation.
However, the back port is not as useful as the Master version.
It does not pull the artifacts from NPM and test them after publishing.

To do that, I would need to back-port much of this PR, which deprecates Node10.

Squash Commit Message: chore: warn 1.x is End-of-Support

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

Check any applicable:

  • Were any files moved? Moving files changes their URL, which breaks all hyperlinks to the files.

@texastony
Copy link
Contributor Author

Apparently CodeBuild does not support NodeJS 12?

[Container] 2022/08/25 22:50:03 Phase complete: DOWNLOAD_SOURCE State: FAILED
[Container] 2022/08/25 22:50:03 Phase context status code: YAML_FILE_ERROR Message: Unknown runtime version named '12' of nodejs. This build image has the following versions: 10, 8

How can that be?
We have had CI pass with 12, right?

@texastony texastony marked this pull request as ready for review August 25, 2022 22:55
@texastony texastony requested a review from a team as a code owner August 25, 2022 22:55
@josecorella
Copy link
Contributor

ahh i see why it fails, on master we override the codebuild the default codebuild spec to use image image: aws/codebuild/standard:5.0 which has node 12 on it, however; mainline-1.x uses the default image: aws/codebuild/standard:2.0 which does not which is why the build is failing

@texastony
Copy link
Contributor Author

texastony commented Aug 25, 2022

ahh i see why it fails, on master we override the codebuild the default codebuild spec to use image image: aws/codebuild/standard:5.0 which has node 12 on it, however; mainline-1.x uses the default image: aws/codebuild/standard:2.0 which does not which is why the build is failing

Is that on the CodeBuild project or somewhere else?
I see it. Will fix.

@texastony
Copy link
Contributor Author

eslint is failing for variety of reasons...
I may need to override that check to get this done by Tuesday...

@josecorella
Copy link
Contributor

eslint is failing for variety of reasons... I may need to override that check to get this done by Tuesday...

@texastony, let's talk about this tomorrow to get consensus from everyone if this is the right approach

@texastony
Copy link
Contributor Author

texastony commented Aug 26, 2022

CB NodeJS10 failed starting Karma, though its possible Karma did run but not log anything:

npm ERR! aws-encryption-sdk-javascript@0.0.1 karma: `karma start karma.conf.js`

CB NodeJS12 failed while running Karma tests:

·[1AChrome Headless 103.0.5060.53 (Linux x86_64) decrypt verify incomplete chipertext will fail for a signed algorithm suite FAILED
    Error: Timeout of 2000ms exceeded. For async tests and hooks, ensure "done()" is called; if returning a Promise, ensure it resolves.
...
Chrome Headless 103.0.5060.53 (Linux x86_64) test kmsEncryptWithMaxEncryptedDataKeysTest, less than max FAILED
   Error: Invalid Signature

So NodeJS12 had one timeout and one real failure...

@texastony
Copy link
Contributor Author

On my local:

nvm install 10; npm run clean; rm -rf node_modules; npm ci;
npm run build;
npm run karma

...<everything looks good, but then>

[73796:0x10288c000]    72247 ms: Mark-sweep 1377.6 (1409.1) -> 1377.6 (1409.1) MB, 286.1 / 0.0 ms  (average mu = 0.185, current mu = 0.000) last resort GC in old space requested
[73796:0x10288c000]    72527 ms: Mark-sweep 1377.6 (1409.1) -> 1377.6 (1409.1) MB, 279.9 / 0.0 ms  (average mu = 0.101, current mu = 0.000) last resort GC in old space requested
...
FATAL ERROR: CALL_AND_RETRY_LAST Allocation failed - JavaScript heap out of memory
...
[1]    73795 abort      npm run karma

Ok... Karma is running out of memory on my local,
and possibly on CB as well.

@texastony
Copy link
Contributor Author

On my local:

nvm use 12; npm run clean; rm -rf node_modules; npm ci; npm run build;
npm run karma

TOTAL: 229 SUCCESS

NodeJs12 Karma failure is probably transitive. So what to do with NodeJS10...

@texastony
Copy link
Contributor Author

texastony commented Aug 26, 2022

Our CB tells node to use 4GB, but our CB compute only has 3GB.

Let's see if I can get it pass on my local with 4GB,
and then I can bump CB's compute...

nvm use 10; npm run clean; rm -rf node_modules; npm ci; npm run build;  node --max-old-space-size=4096 `which npm` run karma

@texastony
Copy link
Contributor Author

On my local, with nvm use 10:

node --max-old-space-size=4096 `which npm` run integration

Everything passed.
Which is wild, given that coverage-browser fails at initialization.
After discussing with my colleague (@josecorella ),
we have elected to disable NodeJS10 testing in CodeBuild.

Determining the exact cause of coverage-browser failing for NodeJS10
takes more hours than we are willing to spend on a End-of-Support release.

We know that all tests pass for NodeJS12.
We know that Node (mocha) and Integration (both node and browser) pass for NodeJS10.

We are happy enough with that.

@texastony
Copy link
Contributor Author

With b8e1c6a, I left NodeJS10 testing, but skipped coverage-browser,
as @josecorella and I could not figure that out in a reasonable amount of time.

@texastony
Copy link
Contributor Author

Fudge, on b8e1c6a, NodeJS12 failed coverage-browser, but in a novel way.
No, it was the same way it failed on 503e45c,
I just did not pay attention to it as I was focused on NodeJS10...

Interesting logs for b8e1c6a NodeJS12:

 「wdm」: Compiled successfully.
26 08 2022 20:32:46.328:INFO [karma-server]: Karma v5.2.3 server started at http://localhost:9876/
26 08 2022 20:32:46.329:INFO [launcher]: Launching browsers ChromeHeadlessDisableCors with concurrency unlimited
6 08 2022 20:32:46.390:INFO [launcher]: Starting browser ChromeHeadless

So everything is grand, and we launched Karam and Chrome, but then:

26 08 2022 20:32:57.459:INFO [Chrome Headless 103.0.5060.53 (Linux x86_64)]: Connected on socket 7Dq8HQnVkbGTYTxCAAAA with id 74037777
26 08 2022 20:33:00.853:ERROR [launcher]: ChromeHeadless crashed.
    [0826/203254.318600:ERROR:bus.cc(398)] Failed to connect to the bus: Failed to connect to socket /var/run/dbus/system_bus_socket: No such file or directory
DevTools listening on ws://127.0.0.1:9222/devtools/browser/248f69a1-4d63-409c-a5bd-23608ecff99f
[0826/203254.974716:WARNING:bluez_dbus_manager.cc(247)] Floss manager not present, cannot set Floss enable/disable.
[0826/203255.452102:WARNING:sandbox_linux.cc(376)] InitializeSandbox() called with multiple threads in process gpu-process.

The above repeats two more times until

26 08 2022 20:33:35.210:WARN [Chrome Headless 103.0.5060.53 (Linux x86_64)]: Disconnected (0 times), because no message in 30000 ms.
Chrome Headless 103.0.5060.53 (Linux x86_64) ERROR
  Disconnected, because no message in 30000 ms.

Which get us

npm ERR! aws-encryption-sdk-javascript@0.0.1 karma: `karma start karma.conf.js`

And our failed build.

So the Karma test runner cannot connect to the Chrome browser it created...
FUDGE.

@texastony texastony marked this pull request as draft August 26, 2022 22:44
@texastony
Copy link
Contributor Author

texastony commented Aug 26, 2022

My local is on 12.22.10, while CB is on 12.22.12. On my local:

nvm use 12; npm run clean; rm -rf node_modules; npm ci; npm run build; node --max-old-space-size=4096 `which npm` run coverage-browser

All tests pass... could it be that CB is actually running out of RAM?

For the record, NodeJS10 passed; which means integration tests for Browser (and Node) are passing, even if the Browser's unit tests are failing to start.

@texastony
Copy link
Contributor Author

On d4dd3b1, NodeJS12 failed a coverage-browser test:

�[1A�[2K�[1A�[2K�[1A�[2KChrome Headless 103.0.5060.53 (Linux x86_64) 'browser encrypt tests': 1 9db31afc-4be1-4ffe-9aeb-727192d78897 FAILED
	Error: Timeout - Async function did not complete within 5000ms (set by jasmine.DEFAULT_TIMEOUT_INTERVAL)

@texastony
Copy link
Contributor Author

texastony commented Aug 26, 2022

NodeJS10 failed for f293d5a b/c 10 is not available on 5.0.

But NodeJS12 passed coverage-browser!
So we are getting close.

@texastony texastony marked this pull request as ready for review August 26, 2022 23:25
@texastony texastony marked this pull request as draft August 26, 2022 23:28
@texastony texastony marked this pull request as ready for review August 26, 2022 23:40
Comment on lines +24 to +27
# Generate new version and CHANGELOG entry and push it
- npx lerna version --conventional-commits --git-remote origin --yes ${VERSION_BUMP:+$VERSION_BUMP --force-publish}
# Log the commit for posterity
- git log -n 1
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

were you able to test this on your local to see if it worked?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not yet

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

testing, on my local:

nvm use 12; npm run clean; rm -rf node_modules; npm ci --unsafe-perm;
git checkout -b 1.x-test-learna-version
git push --set-upstream public_tony 1.x-test-learna-version
npx lerna version --conventional-commits --git-remote origin --yes ${VERSION_BUMP:+$VERSION_BUMP --force-publish}
<generate GitHub auth token>
<export GitHub auth token as GITHUB_PAT>
gh auth login
npx lerna version --conventional-commits --git-remote public_tony --yes ${"":+"" --force-publish}

Results are here texastony@03424c5

Copy link
Contributor

@josecorella josecorella left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@texastony texastony merged commit 266ee9f into aws:mainline-1.x Aug 29, 2022
@texastony texastony deleted the 1.x-warning-client-creation branch August 29, 2022 20:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants