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

Investigate new base builder for buildpacks #5

Closed
ipmb opened this issue Sep 27, 2023 · 16 comments
Closed

Investigate new base builder for buildpacks #5

ipmb opened this issue Sep 27, 2023 · 16 comments

Comments

@ipmb
Copy link
Member

ipmb commented Sep 27, 2023

Heroku 22 builders have some incompatibilities we need to consider around CNB/non-CNB buildpacks.

heroku/cnb-builder-images#298 (comment)
heroku/buildpacks-python#14

There is also a new version of the 20 builder now heroku/buildpacks-python#117

@edmorley
Copy link

Hi Peter - hope you are well.

I just wanted to let you know that the heroku/buildpacks:20 and heroku/builder-classic:22 CNB builder images are now deprecated and output a warning to the build log as of heroku/cnb-builder-images#429.

I see the following references to these under the apppackio org:
https://github.com/search?q=org%3Aapppackio%20(heroku%2Fbuilder-classic%20OR%20heroku%2Fbuildpacks)&type=code

@edmorley
Copy link

It also looks like the buildpacks list here is out of date:

// $ pack builder inspect heroku/buildpacks:20 -o json | jq '.remote_info.buildpacks[].id'
"heroku/go",
"heroku/gradle",
"heroku/java",
"heroku/java-function",
"heroku/jvm",
"heroku/jvm-function-invoker",
"heroku/maven",
"heroku/nodejs",
"heroku/nodejs-engine",
"heroku/nodejs-function",
"heroku/nodejs-function-invoker",
"heroku/nodejs-npm",
"heroku/nodejs-yarn",
"heroku/php",
"heroku/procfile",
"heroku/python",
"heroku/ruby",
"heroku/scala",
},

@ipmb
Copy link
Member Author

ipmb commented Oct 30, 2023

Thanks @edmorley!

How are you handling migration between the CNB Python buildpacks and the classic shimmed one? The main thing that has held back a blanket upgrade to the 22/CNB stack is that (afaik), it is not backwards compatible. post-compile comes to mind, but I think there may be others.

@edmorley
Copy link

I agree there are a handful of things that aren't backwards compatible at the moment wrt the Python buildpack - I'm currently tracking these via the parity label here:
https://github.com/heroku/buildpacks-python/issues?q=is%3Aissue+is%3Aopen+label%3A%22classic+buildpack+parity%22

Regarding bin/pre_compile / bin/post_compile I'm leaning towards not supporting it for the reasons in heroku/buildpacks-python#14. However, if I don't support it, I will add an error/warning to the CNB to help with migration UX (of migrating to an inline buildpack or script reference in project.toml).

ipmb added a commit that referenced this issue Oct 30, 2023
@ipmb
Copy link
Member Author

ipmb commented Oct 30, 2023

@edmorley thanks for your work here. Opening issues downstream is above-and-beyond 😄

I do have concern that deprecating the legacy buildpack before the new one is at parity or an official migration doc exists will confuse a lot of users.

I expect many users will see the deprecation warning, upgrade as directed, then find out the new builder is incompatible, possibly only after deploying their application.

ipmb added a commit that referenced this issue Oct 30, 2023
@edmorley
Copy link

I do have concern that deprecating the legacy buildpack before the new one is at parity or an official migration doc exists will confuse a lot of users.

I agree there are still a couple of rough edges for some of the Heroku CNBs - they are high up our list to address. One of the issues is that from our side Heroku's CNB images are effectively pre-beta (they are not used for anything within Heroku itself at the moment), but some non-Heroku platforms are using them in production already. We're doing our best to be respectful of that, but we need feedback on the CNBs, and one of the best ways to get more feedback is to nudge people over from the shimmed images (many of whom don't even know they are using a "held together with tape and glue" shimmed buildpack). If the Heroku stack EOLs are anything to go by, people take ages to migrate to anything new, so I wanted to start the deprecation process sooner rather than later.

@edmorley
Copy link

edmorley commented Oct 30, 2023

many of whom don't even know they are using a "held together with tape and glue" shimmed buildpack

For example: heroku/heroku-buildpack-python#1484

(Though in that case, they were using a custom builder image too; or at least a custom buildpack order injected by DigitalOcean)

@edmorley
Copy link

Oh meant to say - another forcing function here is:
heroku/cnb-shim#69 (comment)

@edmorley
Copy link

The legacy shimmed builder image deprecation warnings were upgraded to errors this morning:
heroku/cnb-builder-images#478

The errors can be skipped by setting ALLOW_EOL_SHIMMED_BUILDER=1.

@ipmb
Copy link
Member Author

ipmb commented Mar 18, 2024

Thanks @edmorley. I'm not seeing these errors in a Python build with heroku/buildpacks:20 just now. Any idea on why that is? https://github.com/apppackio/apppack-codebuild-image/actions/runs/8328576129/job/22788886566#step:6:346

@edmorley
Copy link

The way the deprecation warnings (and now errors) were implemented was via a buildpack added to every group in the builder's default buildpack detection ordering. If a manual buildpack ordering is passed (eg via project.toml) to override the builder's default, then the deprecation/error buildpack won't run.

Longer term there is talk of the upstream CNB project supporting metadata in the builder that the platform (eg Pack CLI, or any other equivalent) could use to display deprecation warnings - however, for now I couldn't think of a better way of implementing the warnings.

@edmorley
Copy link

(Although in practice, it's the "blog post written 2 years ago that tells people to pack build --builder heroku/buildpacks:20 case that we most want to prevent people from using accidentally; and that case will error)

@edmorley
Copy link

edmorley commented Mar 25, 2024

however, for now I couldn't think of a better way of implementing the warnings.

I came up with something - I've added the deprecation notice to the cnb-shim directly (and used the stack ID to differentiate between Heroku and non-Heroku builder images to work around the fact that cnb-shim is generic), in:
heroku/cnb-shim#90

@ipmb
Copy link
Member Author

ipmb commented Mar 25, 2024

Based on the flurry of support requests that just came through, I'm guessing this new approach means that this is no longer true? #5 (comment)

If a manual buildpack ordering is passed (eg via project.toml) to override the builder's default, then the deprecation/error buildpack won't run.

@edmorley
Copy link

Yeah:
#5 (comment)

@ipmb
Copy link
Member Author

ipmb commented Mar 27, 2024

We will switch our default buildpack to heroku/builder:22 on May 1. Announcement here: https://apppack.io/blog/upgrading-our-default-heroku-stack-for-buildpacks/

Pending PR: #6

I had hoped that there would be full feature parity between the shimmed and CNB buildpacks before we made the switch, but the EOL warnings/errors put us between a rock and a hard place.

ipmb added a commit that referenced this issue May 6, 2024
* Use CNB builder for heroku-22 stack

refs #5

* Switch default builder to heroku/builder:22

* Add new buildpack

* Test app.json too

* Procfile buildpack now uses command and args

* Test against legacy Heroku builder

* Adjust tests for bash wrapper

New procfile buildpacks add it
@ipmb ipmb closed this as completed May 16, 2024
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

No branches or pull requests

2 participants