这是indexloc提供的服务,不要输入任何密码
Skip to content

Conversation

@josegonzalez
Copy link
Member

Rather than hardcode two builders, allow builders to specify a builder-detect trigger. This trigger can be used to specify if the builder should or should not be used for an application. Each builder takes stdin and can decide if it wants to emit it or emit it's own image source type.

If the final value is empty, then Dokku will default to herokuish (and cnb once that is stable). In addition, a future change may allow users to manually specify a builder in the case they wish to override the choice selected by Dokku.

This change enables users to build custom builder plugins and have those plugins used for building an image asset. By way of example, an enterprising user could create a builder-lambda based on lambci, and then pair this with a scheduler plugin that updates a lambda function on AWS. Alternatively, a user might decide they wish to place their Dockerfile in a specific directory for their applications - such as an _infrastructure directory - and create a plugin to override how that is detected within Dokku.

Note: If this PR is just doc changes, please put [ci skip] in the body that way tests do not run.

@jameysharp
Copy link

Awesome, thank you!

This is kind of a weird hook because order matters but there's no good way to control the order. If I'm reading the plugn trigger implementation correctly, you could rename plugins so lexicographic order is the desired order; I guess that would mean 10-dockerfile, 20-herokuish, 30-cnb or something like that. That doesn't seem great though, since almost nothing else in Dokku works that way, and in principle one might want different orders for different triggers. Also it's not clear to me that an admin can safely rename arbitrary plugins.

Here's an alternative; it might be silly, but I'll let you judge that. Create a file, let's say BUILDERS, which lists all the builders you want to use, one per line, sorted from from most-preferred to least. Then you can pick the right one with grep:

plugn trigger builder-detect "$APP" "$TMP_WORK_DIR" |
  grep -m1 -Fxf - BUILDERS

Give the admin a way to edit that order (just /home/dokku/BUILDERS maybe?) and this detection priority becomes fully configurable.

Also, with the PR as it stands, emitting multiple lines from a builder-detect hook is harmless but useless, as the last line is always the selected one. Making the order configurable would mean a plugin can report multiple styles of build that it recognizes and Dokku can actually do something useful with that information.

But also this might all be over-engineering. 🤷

Meanwhile, in case some future reader wants more rationale for this PR: the reason I want it is to use Nix to build Docker images, instead of either Dockerfile or Herokuish/CNB.

@jameysharp
Copy link

Alright, here's a Nix builder plugin that makes use of the new builder-detect hook:

https://github.com/jameysharp/dokku-builder-nix

I tested it against this branch, in the Dokku Vagrant image, and it works for the one thing I tried pushing to it. So this branch seems good to me!

@josegonzalez
Copy link
Member Author

I do also wonder if the built-in builders should be renamed. My concern is if I ever need to add commands, those will not work. We could probably remove the existing order of operations issues by renaming builder-cnb to builder-pack (which makes more sense anyways).

We could also change the detection a bit to match the style of heroku's buildpack detect phase. Each builder can echo whether it is in use or not, with the last one winning by default. As I alluded to in my OP, and to build on your idea, the user can also do something like dokku builder:set [app|--global] builder pack to override the detectable builder for the app. I think thats would suffice for all use cases.

I'll redo update the PR with the rename and introduce the builder:set command, which can also stand in for any builder-specific properties that are currently managed by environment variables.

@jameysharp
Copy link

It occurs to me that another option is to refuse to pick a builder if multiple choices apply, and require builder:set to disambiguate in that case. You'd still need special cases in the Dockerfile builder then, to ignore Herokuish/CNB projects, for compatibility with the current behavior. But I think this is the least surprising approach, instead of getting an effectively random builder chosen when you push.

Rather than hardcode two builders, allow builders to specify a `builder-detect` trigger. This trigger can be used to specify if the builder should or should not be used for an application. Each builder takes stdin and can decide if it wants to emit it or emit it's own image source type.

If the final value is empty, then Dokku will default to herokuish (and cnb once that is stable). In addition, a future change may allow users to manually specify a builder in the case they wish to override the choice selected by Dokku.

This change enables users to build custom builder plugins and have those plugins used for building an image asset. By way of example, an enterprising user could create a `builder-lambda` based on lambci, and then pair this with a scheduler plugin that updates a lambda function on AWS. Alternatively, a user might decide they wish to place their Dockerfile in a specific directory for their applications - such as an `_infrastructure` directory - and create a plugin to override how that is detected within Dokku.
The builder-pack rename makes this unnecessary.
Also rename internal cnb references to pack (where possible).
This plugin will allow users to override the builder used for their application, enabling users to use custom builders if desired.
@josegonzalez josegonzalez merged commit d697004 into master Mar 1, 2021
@josegonzalez josegonzalez deleted the builder-detect branch March 1, 2021 03:26
github-actions bot pushed a commit that referenced this pull request Mar 1, 2021
# History

## 0.24.0

Install/update via the bootstrap script:

```shell
wget https://raw.githubusercontent.com/dokku/dokku/v0.24.0/bootstrap.sh
sudo DOKKU_TAG=v0.24.0 bash bootstrap.sh
```

See the [0.24.0 migration guide](/docs/appendices/0.24.0-migration-guide.md) for more information on migrating to 0.24.0.

### Bug Fixes

- #4449: @josegonzalez Gitignore trigger symlink
- #4447: @josegonzalez Checkout code to ensure the bump-azure script is available

### New Features

- #4453: @josegonzalez Simplify tar and zip deploys via git:from-archive
- #4450: @josegonzalez Simplify docker image deploys via git:from-image
- #4379: @josegonzalez Allow builders to be detected based on repository contents
- #4425: @josegonzalez Implement heroku's postdeploy deployment task
- #4424: @josegonzalez Implement git:auth command
- #4419: @josegonzalez Add parallelism to certain proxy commands

### Refactors

- #4374: @josegonzalez Change exit code when app does not exist

### Documentation

- #4451: @josegonzalez Update links to builder documentation to avoid extra rewrite
- #4448: @josegonzalez Add documentation for git push to dokku-in-docker
@jameysharp
Copy link

Thank you for merging this! I don't think GitHub notified me at the time, so I only just noticed. Seems to work great!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants