Skip to content
This repository has been archived by the owner on May 22, 2024. It is now read-only.

feat: support go provided.al2 runtime #1559

Merged
merged 3 commits into from
Sep 6, 2023
Merged

feat: support go provided.al2 runtime #1559

merged 3 commits into from
Sep 6, 2023

Conversation

Skn0tt
Copy link
Contributor

@Skn0tt Skn0tt commented Sep 5, 2023

🎉 Thanks for submitting a pull request! 🎉

Summary

AWS will deprecate the go1.x runtime by end of year, asking customers to deploy to provided.al2 going forward. It requires ZIPs to contain a single file called bootstrap, similar to Rust. This PR adds a featureflag to ZISI that makes that switch, and writes the result of it into manifest.json so we can detect this during deployment.

Part of https://github.com/netlify/pod-dev-foundations/issues/581.


For us to review and ship your PR efficiently, please perform the following steps:

  • Open a bug/issue before writing your code 🧑‍💻.
    This ensures we can discuss the changes and get feedback from everyone that should be involved. If you`re fixing
    a typo or something that`s on fire 🔥 (e.g. incident related), you can skip this step.
  • Read the contribution guidelines 📖. This ensures your code follows our style guide and
    passes our tests.
  • Update or add tests (if any source code was changed or added) 🧪
  • Update or add documentation (if features were changed or added) 📝
  • Make sure the status checks below are successful ✅

A picture of a cute animal (not mandatory, but encouraged)

@Skn0tt Skn0tt requested a review from eduardoboucas September 5, 2023 15:16
@Skn0tt Skn0tt self-assigned this Sep 5, 2023
@Skn0tt Skn0tt requested a review from a team as a code owner September 5, 2023 15:16
@github-actions
Copy link
Contributor

github-actions bot commented Sep 5, 2023

⏱ Benchmark results

Comparing with 8788624

largeDepsEsbuild: 3.6s

⬆️ 1.32% increase vs. 8788624

^                           3.5s    3.6s  
│                           ┌──┐    ┌──┐  
│                           |  |    |▒▒|  
│                           |  |    |▒▒|  
│ ──────────────────────────┼──┼────|▒▒|──
│   2.6s    2.7s    2.7s    |  |    |▒▒|  
│   ┌──┐    ┌──┐    ┌──┐    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |▒▒|  
└───┴──┴────┴──┴────┴──┴────┴──┴────┴──┴──>
    T-4     T-3     T-2     T-1      T    
Legend

largeDepsNft: 11.5s

⬆️ 3.13% increase vs. 8788624

^                                  11.5s  
│                          11.2s    ┌──┐  
│                           ┌──┐    |▒▒|  
│                           |  |    |▒▒|  
│ ──────────────────────────┼──┼────|▒▒|──
│           8.4s    8.4s    |  |    |▒▒|  
│   8.2s    ┌──┐    ┌──┐    |  |    |▒▒|  
│   ┌──┐    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |▒▒|  
└───┴──┴────┴──┴────┴──┴────┴──┴────┴──┴──>
    T-4     T-3     T-2     T-1      T    
Legend

largeDepsZisi: 21.9s

⬆️ 3.88% increase vs. 8788624

^                                  21.9s  
│                           21s     ┌──┐  
│                           ┌──┐    |▒▒|  
│                           |  |    |▒▒|  
│ ──────────────────────────┼──┼────|▒▒|──
│                  16.4s    |  |    |▒▒|  
│  15.8s   15.6s    ┌──┐    |  |    |▒▒|  
│   ┌──┐    ┌──┐    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |▒▒|  
└───┴──┴────┴──┴────┴──┴────┴──┴────┴──┴──>
    T-4     T-3     T-2     T-1      T    
Legend

config,
path: zipPath,
entryFilename: zipOptions.filename,
goUseAL2Runtime: featureFlags.zisi_golang_use_al2,
Copy link
Member

Choose a reason for hiding this comment

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

What will we do with this property?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We'll read it in open-api, so we can propagate it through nf-server into BitBalloon and functions-origin. They can then read it and use provided.al2 as the AWS runtime, if it's present. If you have an idea for a solution where we don't need to propagate this so far, i'd be happy to hear it!

Copy link
Member

@eduardoboucas eduardoboucas Sep 6, 2023

Choose a reason for hiding this comment

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

Could we use the existing runtimeVersion property for that?

runtimeVersion?: string

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I guess so, but that'd still require changes to all three services. But that field is probably better-suited. I'll make that update!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done in 80a3c66

config,
path: zipPath,
entryFilename: zipOptions.filename,
runtimeVersion: featureFlags.zisi_golang_use_al2 ? 'provided.al2' : undefined,
Copy link
Member

Choose a reason for hiding this comment

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

Just for consistency with what we do in Node.js, it might be nice to always return the runtime version, even if it's the old one.

Not a big deal, though.

Suggested change
runtimeVersion: featureFlags.zisi_golang_use_al2 ? 'provided.al2' : undefined,
runtimeVersion: featureFlags.zisi_golang_use_al2 ? 'provided.al2' : 'go1.x',

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thought about that as well, but I didn't want to introduce changes when the flag is off. undefined should do that.

@Skn0tt Skn0tt merged commit 2715587 into main Sep 6, 2023
@Skn0tt Skn0tt deleted the go-al2 branch September 6, 2023 12:17
Skn0tt added a commit to netlify/build that referenced this pull request May 21, 2024
* feat: support go provided.al2 runtime

* refactor: write into runtimeVersion
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants