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.

Sign up for GitHub

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

Jump to bottom

allowing on cluster build for go runtime #1445

Merged
merged 5 commits into from
Jul 28, 2023
Merged

allowing on cluster build for go runtime #1445

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
ff1e364
allowing on cluster build for go runtime
Shashankft9 Nov 23, 2022
6776ab0
rebase
Shashankft9 Jul 20, 2023
9939828
warning message added for go and rust builder
Shashankft9 Jul 20, 2023
2469823
gofmt
Shashankft9 Jul 21, 2023
7a4b2ff
fixups
matejvasek Jul 25, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
18 changes: 11 additions & 7 deletions docs/building-functions/on_cluster_build.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,12 +49,16 @@ git remote add origin git@github.com:my-repo/my-function.git
```
4. Update the Function configuration in `func.yaml` to enable on cluster builds for the Git repository:
```yaml
build: git # required, specify `git` build type
git:
url: https://github.com/my-repo/my-function.git # required, git repository with the function source code
revision: main # optional, git revision to be used (branch, tag, commit)
# contextDir: myfunction # optional, needed only if the function is not located
# in the repository root folder
build:
git:
url: https://github.com/my-repo/my-function.git # required, git repository with the function source code
revision: main # optional, git revision to be used (branch, tag, commit)
# contextDir: myfunction # optional, needed only if the function is not located in the repository root folder
# builderImages: # optional, needed only if the runtime is golang
# pack: ghcr.io/boson-project/go-function-builder:tip
buildpacks: []
builder: ""
Copy link
Contributor

Choose a reason for hiding this comment

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

Builder should be "pack".

Copy link
Contributor

Choose a reason for hiding this comment

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

but empty string probably would work too

Copy link
Member

Choose a reason for hiding this comment

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

If these two items are intended as hints, commenting them would probably be best.

The idea here is that the code has static defaults. Anything placed in the config file is specifically requesting that exact value, so while the first value, []buildpacks is innocuous, the second, builder="" is, if literally translated, requesting the system to build without a builder. In this case we do set the default if the value comes in empty like this, because building without a builder is nonsensical, but I presume this exists to give a hint to the user how to choose a builder and buildpacks, so a commented entry is the right tool for the job.

buildEnvs: []
```
5. Implement the business logic of your Function, then commit and push changes
```bash
Expand All @@ -64,7 +68,7 @@ git push origin main
```
6. Deploy your Function
```bash
kn func deploy
kn func deploy --remote --registry docker.io/my-repo --git-url https://github.com/my-repo/my-function.git
```
If you are not logged in the container registry referenced in your function configuration,
you will prompted to provide credentials for the remote container registry that hosts the Function image. You should see output similar to the following:
Expand Down
2 changes: 1 addition & 1 deletion pipelines/tekton/validate.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ func validatePipeline(f fn.Function) error {
return ErrRuntimeRequired
}

if f.Runtime == "go" || f.Runtime == "rust" {
if f.Runtime == "rust" {
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we could remove this if completely since following if check if there are additional buildpacks.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think it might be better to keep it here, to explicitly tell user about not supported runtime.

Copy link
Contributor

Choose a reason for hiding this comment

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

But the issue with Go and Rust is that they use custom buildpacks.
That problem is detected in following if.

What this if does is it prevents users from building Go and Rust apps even if user provides custom builder that would work all right.

Copy link
Contributor

Choose a reason for hiding this comment

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

@zroubalik @Shashankft9 what about just printing warning here "Go/Rust default builder do not support in cluster build" instead of returning an error? Could we accept that?

Copy link
Member

Choose a reason for hiding this comment

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

@zroubalik @Shashankft9 what about just printing warning here "Go/Rust default builder do not support in cluster build" instead of returning an error? Could we accept that?

@Shashankft9 Can you add a simple warning here that Go and Rust default builders do not support in-cluster builds?

return ErrRuntimeNotSupported{f.Runtime}
}

Expand Down