Add new --remote-builder flag for remote builds via the Function Builder API #961
Conversation
NikhilSharmaWe
force-pushed
the
remote-builder
branch
2 times, most recently
from
92d679c
to
7ad71bc
Compare
|
Thanks for the contribution. Generally, we need to see "How Has This Been Tested?" fully populated before we can consider a review. |
NikhilSharmaWe
force-pushed
the
remote-builder
branch
2 times, most recently
from
441cdcf
to
895a8be
Compare
|
How Has This Been Tested? Is still missing. |
NikhilSharmaWe
force-pushed
the
remote-builder
branch
from
895a8be
to
2869d28
Compare
Please share the output and verify that faas-cli deploy can use the image. I'd expect you to use a stack.yml file without giving any extra arguments - this all gets loaded by the YAML file |
|
The output looks a lot better. But I can't see the response from the server? There should be build logs. We'd also want to do a build for a Go function with a syntax error so that we can prove the error flow works and gives the user the problem on the console whilst exiting non zero. |
NikhilSharmaWe
force-pushed
the
remote-builder
branch
from
2869d28
to
406ae30
Compare
|
@alexellis When we try to use go function with a syntax error in the handler code, this error is logged: Is this what you expect? or we expect detailed logs something like we get in the case of build, which also gives user info about the statement responsible for the issue and solution: But since we are not doing Please give your thoughts. |
|
The API call that you're making should have both a status code to determine errors and a set of logs. Look at the docs for the format. I also thought that authentication was required for the builder API - the payload secret? |
|
@alexellis The payload secret is fetched through: I currently do it manually. |
NikhilSharmaWe
force-pushed
the
remote-builder
branch
3 times, most recently
from
ed9ed28
to
1f3eb4f
Compare
|
I'm just wondering how you are able to only pass in a single argument here? Shouldn't there be an additional argument for the payload secret? |
NikhilSharmaWe
force-pushed
the
remote-builder
branch
3 times, most recently
from
419204b
to
a1c8f99
Compare
|
We discussed the approach on the weekly call, I think you may have dropped off by then. faas-cli shouldn't be calling any exec commands to kubectl. In this case, have the user pass in the password via a file. I'm not sure that having it go in via a flag is a good idea as it will get leaked in bash history.
Could you make the change and demo it to us on the community call in ~ 1.5 hours from now? |
NikhilSharmaWe
force-pushed
the
remote-builder
branch
from
1e0ea45
to
9076a8f
Compare
|
Could you try this also with |
NikhilSharmaWe
force-pushed
the
remote-builder
branch
2 times, most recently
from
03d17f2
to
6c6de6b
Compare
NikhilSharmaWe
force-pushed
the
remote-builder
branch
4 times, most recently
from
baa6a2e
to
9bf96f6
Compare
|
Hi, please could you rebase this PR? Also run In the commit body, you can write anything else you want to summarise - including wrapping the text to 80 chars. This is what I strive to follow myself - https://cbea.ms/git-commit/ |
|
@welteki I'd like you to try this out on your machine for a regular build/publish (unaffected) and for a remote build - with one single build and multiple - also trying out build-args. Thanks |
NikhilSharmaWe
force-pushed
the
remote-builder
branch
2 times, most recently
from
052f112
to
fac7588
Compare
Tested this:
Build args are not working for remote builds. I used the python-http template and tried setting a custom python version through build args. faas-cli template store pull python3-http
faas-cli new python-fn --lang python3-http --prefix ttl.shfaas-cli up -f python-fn.yml \
--build-arg PYTHON_VERSION=3.10 \
--remote-builder http://127.0.0.1:8081/build \
--payload-secret ./payload.txtThe remote builder builds the function but in the logs I see lines like: It uses the default python version |
|
@welteki I have found the fix. Thank you for testing and informing the issue. |
Signed-off-by: Nikhil Sharma <nikhilsharma230303@gmail.com>
NikhilSharmaWe
force-pushed
the
remote-builder
branch
from
fac7588
to
cebed84
Compare
| BuildOptPackages: buildOptPackages, | ||
| BuildLabelMap: buildLabelMap, | ||
| } | ||
| if err := makeTar(builderConfig{Image: imageName, BuildArgs: buildArgMap}, path.Join("build", functionName), tarPath); err != nil { |
There was a problem hiding this comment.
@welteki
Now I also add buildArgMap in the builderConfig for remote-builder, which I was not doing before.
Can you try it out now?
|
@NikhilSharmaWe can you try the example provided by Han please? |
|
@alexellis I have tested and it works fine now with Build Args. |
alexellis
changed the title
|
@NikhilSharmaWe please could you add example commands and usage under the curl instructions in the docs? Clone the repo and run it locally so you can test the layout etc. https://github.com/openfaas/docs/blob/master/docs/openfaas-pro/builder.md |
alexellis
changed the title
Description
This PR works on adding the new
remote-builderflag for faas-cli to be able to perform build operations from Function Builder API in Openfaas Pro. A newpayload-secretflag is also added, in which the path of the payload.txt file containing the payload secret is specified.Demo's :
Motivation and Context
Fixes: Remote build command using the Function Builder API #931
How Has This Been Tested?
--remote-builderflag is successfully runTypes of changes
Checklist:
git commit -s