-
Notifications
You must be signed in to change notification settings - Fork 149
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
Fix overriding of alpine builder #142
Conversation
Test that extensions are present in alpine builder Signed-off-by: Natalie Arellano <[email protected]>
874ba80
to
2f1fcd2
Compare
Signed-off-by: Natalie Arellano <[email protected]>
Signed-off-by: Natalie Arellano <[email protected]>
Signed-off-by: Natalie Arellano <[email protected]>
Signed-off-by: Natalie Arellano <[email protected]>
Signed-off-by: Natalie Arellano <[email protected]>
Signed-off-by: Natalie Arellano <[email protected]>
Otherwise the hello extensions buildpack will detect Signed-off-by: Natalie Arellano <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These changes work, so I'm happy to land them. I have two questions to improve my understanding.
Makefile
Outdated
@@ -29,8 +29,6 @@ build-linux-builders: build-builder-alpine build-builder-bionic | |||
build-builder-alpine: build-linux-packages build-sample-root | |||
@echo "> Building 'alpine' builder..." | |||
$(PACK_CMD) builder create cnbs/sample-builder:alpine --config $(SAMPLES_ROOT)/builders/alpine/builder.toml $(PULL_POLICY_NEVER) | |||
docker run cnbs/sample-builder:alpine ls /cnb/extensions/samples_curl |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why are we removing these post-build checks from build-builder-alpine
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah well, I guess we could keep the checks if we want - they were added previously because I was pulling the sample builder and wondering, what could possibly be going on?
Thinking about this more, the alpine builder in its current state (without --buildpack
overriding the order) is actually broken 😞 for demo purposes which is not ideal. I think I will simplify the tutorial (and make the builder working by default) by introducing a BP_
environment variable to control the Hello Extensions buildpack behavior WRT extensions.
TLDR; I'll update this PR and create a corresponding docs PR to make everything better
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added the aforementioned improvements in cc5328a and buildpacks/docs#571
…nd failed the build by default (as it requires tree, which is not installed on the builder) Now, the buildpack will opt-in to the build only if a build-time environment variable is set Additionally, it can be made to require dependencies through the environment, instead of having to edit the /bin/build script Signed-off-by: Natalie Arellano <[email protected]>
Signed-off-by: Natalie Arellano <[email protected]>
Fix alpine builder and improve extensions demo
…after every failed build The hello-extensions buildpack can be configured through the environment after buildpacks/samples#142 lands Signed-off-by: Natalie Arellano <[email protected]>
CI failing with |
…e deploying We are currently overriding the builder that was just built by using --pull-policy=always when using the builder in a build This will be fixed in #142 but we can't merge that one until the pipeline is green Signed-off-by: Natalie Arellano <[email protected]>
I will take a look at the circleci issue |
Signed-off-by: Natalie Arellano <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
In CI a `pack build` is performed using one of libcnb's example CNBs and the upstream `cnb/sample-builder:alpine` builder, in order to test that our static MUSL compile works with non-libc base images. However, CI has just started failing with: ``` ERROR: failed to build: experimental features must be enabled when builder contains image extensions ``` (https://github.com/heroku/libcnb.rs/actions/runs/5530837180/jobs/10090742723?pr=592#step:10:107) This is due to the upstream images having been rebuilt with extension support, which is an experimental feature until the next stable Pack CLI release): buildpacks/samples#149 buildpacks/samples#142 As such until Pack CLI v0.30 is released as stable, we'll have to pin to an older `alpine` image by SHA256 ref. GUS-W-13746154.
In CI a `pack build` is performed using one of libcnb's example CNBs and the upstream `cnb/sample-builder:alpine` builder, in order to test that our static MUSL compile works with non-libc base images. However, CI has just started failing with: ``` ERROR: failed to build: experimental features must be enabled when builder contains image extensions ``` (https://github.com/heroku/libcnb.rs/actions/runs/5530837180/jobs/10090742723?pr=592#step:10:107) This is due to the upstream images having been rebuilt with extension support, which is an experimental feature until the next stable Pack CLI release): buildpacks/samples#149 buildpacks/samples#142 As such until Pack CLI v0.30 is released as stable, we'll have to pin to an older `alpine` image by SHA256 ref. GUS-W-13746154.
We need --pull-policy=always when the builder contains extensions (the builder must exist in a registry so that we can pull the manifest)
However pulling the remote builder overrides the locally built one, making our test useless and causing us to re-publish the builder that was already in Docker Hub
Spinning up a local registry should solve the problem