add paketo builder option for buildpack #190
Conversation
|
Hi @xiujuan95. Thanks for your PR. I'm waiting for a redhat-developer member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
openshift-ci-robot
added
the
needs-ok-to-test
There was a problem hiding this comment.
We just discussed in our scrum meeting. We think that we would not want to replace heroku with paketo but provide it as an alternative. Correct @sbose78 @qu1queee ?
This would mean that we relabel the old one's to be something like buildpacks-heroku, add new build strategies and builds and build runs for paketo, and add new specs in the e2e tests.
|
@SaschaSchwarze0 Thanks for your comments! If you all @qu1queee @sbose78 @zhangtbj agree with provide an paketo option for buildpack is better than replace it with And if provide an option, I plan to add a suffix to differ heroku and paketo. For example:
And for paketo, it should be:
Do you all agree? |
|
@xiujuan95 I agree, but let's also wait for @qu1queee and @sbose78 to confirm. EDIT: just saw that @qu1queee did a |
|
@xiujuan95 yes, lets keep paketo as an option not as a replacement. Also, I would be very interesting to see with which "runtimes" you tested this, e.g. could you say that this paketo works for golang, java, nodejs, etc? |
|
Yes, for the first phase, I think two buildpacks strategies is ok for now. But finally, we should JUST have one buildstrategy for buildpacks. The different between heroku and paketo is just the builder image. It is total different with the case of Kaniko and buildah (different tools) Two same buildpacks build strategies also make end-user confusion and I think most of the users don't know which is better or use, so we should provide a prefer one (then they may ask why google fans cannot choose google's buildpacks). If we keep two, do we want to also keep the duplicate e2e tests for them? It will take e2e tests longer. This is similar as the current case for heroku and cloudfoundry, we don't have two buildpacks buildstrategies for them, we just replaced from cloudfoundry to heroku and document in our buildstrategy document that we support two of them: So I suggest, if we all agree and verify the |
Thanks @qu1queee ! Now, I can't say all runtimes work fine. Because before I just tested use nodejs runtime. But in theory, currently, |
xiujuan95
changed the title
|
@qu1queee I verified
But golang and dotNetCore both have some issues. For golang, at detect step, it failed: And for dotNetCore, if failed at build step: I debug and try to understand lifecycle detect source code, but I still don't locate the root cause. I suspect go source code is not suitable for paketo. I am not sure, because I saw a message for go buildpack: About above two issues, I have asked for paketo community guys. Waiting for answer from them, now. |
|
@zhangtbj I do not think it is "just the replacement of the image". It is more and @xiujuan95 is outlining those in the description at the beginning:
From a build project perspective, the different buildpack strategies are just samples. Whether a consuming team is using all off them or decides for one is their decision. |
|
Yep, the change which Zoe introduced is not the key difference for both builders, and I think if heroku is upgraded to the new version, we also need to change the parameters. All the steps are same just 1 or 2 configurations. And if they are just samples, do we need to maintain and verify the tests both our them in our side? The reason why we decide to hardcode the builder image in the buildstartegy (#157) is we don't want the end-user to choose from multiple builder images and end user may also don't know the difference of them. And we also talked about in the meeting, some of the source code can be built by paketo correctly but fail by heruko. If we received some tickets about this case, should we ask end user to switch another buildpacks builder to retry? Or let us fix both of the build failures for heroku and paketo? |
|
/ok-to-test |
openshift-ci-robot
added
ok-to-test
|
I got an answer from communty guys:https://paketobuildpacks.slack.com/archives/CULAS8ACD/p1589352322139600?thread_ts=1589349077.139200&cid=CULAS8ACD They also think this go sample is not correct for They suggest me to use this one. I verified and passed. So now, I can say paketo also supports golang runtime. |
|
@zhangtbj I agree that from an end user perspective, at the end, just "buildpack" should be exposed. Whatever builder is below should not matter. But, at this point in time, I think we do not yet know which one this will be: we also want to investigate the Google Buildpacks, we might even end up to use the one or other builder depending on the programming language. BTW: I think kaniko vs buildah is the same. These are different tools yes, but similarly to the different builders in buildpack, they are solving the same problem which is doing a Docker build. The end user at the end should not care = he must not know the details about these tools. Still it makes sense that this project here contains samples for both. |
|
Yes, that is the point. That is why I agree we can have two co-existing buildpacks buildstrategies just now if all agree. And then we also continue investigating the Paketo and even Google buildpacks. But if we ALL agree any one is best. I think we should choose one as the buildpacks strategy, just one. We cannot provide many buildpacks buildstraties in the repo when there is a new buildpacks providers, maybe there is heroku, paketo, google, ibm, aws...... |
|
Even though Kaniko and Buildah builds Dockerfiles, the internals are fairly different and users are opinionated on which ones they would want to use. That's why I have kept them distinct :) we could revisit that. However , in case of Buildpacks, as a project, we should make something our "default" buildpacks strategy. It's more of a 'opinion' the project should stand by :) If we feel that Paketo makes more sense, we should have Paketo as the "buildpacks-v3" strategy and the other one named as "heroku-buildpacks-v3" strategy. This is (somewhat) similar to the way "source-to-image" strategy and the "redhat-source-to-image" strategy has been added. |
|
Good idea and agree And all of our test and document should just use the "default" buildpacks strategy. We have verified that Paketo makes more sense. If all agree, we can modify this PR like this style |
|
@zhangtbj I agree, but this does not mean heroku needs to be removed, it just means heroku is not longer the standard for the project, therefore when we speak about "buildpacks-v3" we mean paketo, and that would be all. @xiujuan95 I think your PR is almost good, I will review now to add some comments. I already fixed the contextDir in another PR, just be aware of that. |
|
Correct @qu1queee ! |
…to paketo-buildpack-zoe
|
/lgtm |
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: qu1queee The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci-robot
added
the
approved
Fix issue:#147
gcr.io/paketo-buildpacks/builder:latestbuilder image option for buildpack;In order to differ
paketoandheroku, I add a suffix to the name, likebuild-herokuandbuild-paketo.paketoandherokuFirst, for paketo, at
preparestep, we should makeuser 1000to own/tekton/homefile first. Because the docker credential is stored here. If we don't do this, then when access this secret will producepermission deniedissue.And also we should run with
user 1000atexportstep. Because onlyuser 1000has the authority to access docker credential file instead ofuser 0.Second, at
exportstep,--helperparameter has beed removed forpaketoBecause the version of
lifecycleinpaketoimage is0.7.3andherokuis0.6.1. For this0.7.3version, the docker credential helper has been removed, so this parameter also doesn't be needed.