Skip to content
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

refactor: [M3-8513] - Update Node.js from 18.14 to 20.17 #10866

Merged

Conversation

bnussman-akamai
Copy link
Member

@bnussman-akamai bnussman-akamai commented Aug 30, 2024

Description πŸ“

  • Updates us to the latest Node.js LTS version (20.17) ⬆️
  • This PR will have an associated PR in our internal CI/CD repo πŸ“

Why? ❓

  • As we continue to modernize and keeping our tooling up to date, some tools are getting mad 😠 about our current node version, which is fair. 18 is getting old.
  • Updating Node will allow us to stay unblocked as we modernize 🚧
  • There might be some nice new features or performance improvements that we can benefit from ✨
Screen.Recording.2024-08-30.at.12.07.43.PM.mov

How to test πŸ§ͺ

Note

When testing this, make sure your local environment uses Node 20.17. Check that with (node --version)
If you use Volta or NVM to manage your node version, it should update automatically for you, but double check this.

  • Verify local development works as expected (yarn up / yarn dev)
  • Verify production builds work (you can check the internal preview link)
  • Verify Storybook still works
  • Verify Cypress still works
  • Verify all CI passes
    • The failure on Code Coverage / base_branch can safely be disregarded

As an Author I have considered πŸ€”

  • πŸ‘€ Doing a self review
  • ❔ Our contribution guidelines
  • 🀏 Splitting feature into small PRs
  • βž• Adding a changeset
  • πŸ§ͺ Providing/Improving test coverage
  • πŸ” Removing all sensitive information from the code and PR description
  • 🚩 Using a feature flag to protect the release
  • πŸ‘£ Providing comprehensive reproduction steps
  • πŸ“‘ Providing or updating our documentation
  • πŸ•› Scheduling a pair reviewing session
  • πŸ“± Providing mobile support
  • β™Ώ Providing accessibility support

@bnussman-akamai bnussman-akamai self-assigned this Aug 30, 2024
@bnussman-akamai bnussman-akamai requested review from a team as code owners August 30, 2024 16:43
@bnussman-akamai bnussman-akamai requested review from AzureLatte, mjac0bs and abailly-akamai and removed request for a team August 30, 2024 16:43
Copy link
Contributor

@AzureLatte AzureLatte left a comment

Choose a reason for hiding this comment

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

Seeing AxiosError: Request failed with status code 502 in CI tests

@abailly-akamai
Copy link
Contributor

@AzureLatte those are unrelated - it's sad state of affairs @jdamore-linode described to me as such (sharig cause it'll keep happening and will get a bit better at the restart of the month):

There’s nothing immediate we can do about this πŸ˜• this is an API issue that seems to really impact our test accounts because of the way they rapidly create and delete resources β€” after a while, hitting the /account endpoint times out (which used to respond with 504s but since some API configuration change now leads to a 502) because the billing calculations that get made for that response become slower and slower as more resources get created and deleted over time.
We have a couple ways of mitigating this:
We have 4 extra test accounts that basically sit around doing nothing; when a test account starts getting 502s regularly, I swap them out with the test accounts that are sitting around
The billing calculations that get made by the API somehow reset at the beginning of each month, so this will (temporarily) fix itself come Sunday
This has been an issue as long as I’ve been here, but unfortunately it seems to be getting worse and worse over time which makes mitigating it difficult. For example, I already swapped out our test accounts last week, so I don’t have any spare test accounts to use to swap out prod-test-034 .
But I’m also more hopeful about this than I’ve ever been because for the first time ever, there seems to be some serious effort going into figuring this out.
In the meantime, I’ve also opened M3-8474 to explore having a cache of test account data that the tests can fall back on when /account responds with a 502. Getting that working should basically eliminate the issue for us forever, regardless of whether it’s ever solved at the API level, even though it’d be kind of unfortunate. I haven’t had very much time to work on ticket work lately so there’s no telling when I’ll be able to actually work on that, but this 502 thing has become an extreme pain for everybody so I’m very motivated to put it behind us.

@mjac0bs
Copy link
Contributor

mjac0bs commented Aug 30, 2024

The billing calculations that get made by the API somehow reset at the beginning of each month, so this will (temporarily) fix itself come Sunday
This has been an issue as long as I’ve been here, but unfortunately it seems to be getting worse and worse over time which makes mitigating it difficult.

@abailly-akamai I never realized this issue was cyclical on a monthly basis; this was a really helpful read. Thanks to you and Joe for sharing. πŸ™πŸΌ

@bnussman-akamai
Copy link
Member Author

Screenshot 2024-08-30 at 1 34 22β€―PM
Once we get this merged, I'm thinking we can re-generate our yarn.lock, which will bump things like micromatch, which will resolve the dependabots

Copy link
Contributor

@mjac0bs mjac0bs left a comment

Choose a reason for hiding this comment

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

Thank you! Confirmed:

  • my node version updates to 20.17
  • local development works as expected (both yarn up / yarn dev)
  • production builds work via the preview link
  • Storybook builds and runs locally
  • Cypress runs locally
  • the rest of CI passes (save the e2e tests that are still running and we know will at least partially fail because of the 502s)

Copy link
Contributor

@abailly-akamai abailly-akamai left a comment

Choose a reason for hiding this comment

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

This should help indeed, thx for the update πŸ‘

Everything seems to behave well on my end with this update

Approving pending resolving test failures

@mjac0bs mjac0bs added Approved Multiple approvals and ready to merge! and removed Ready for Review labels Sep 3, 2024
@bnussman-akamai bnussman-akamai merged commit ab154a4 into linode:develop Sep 4, 2024
18 of 19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Approved Multiple approvals and ready to merge!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants