chore: Remove Docker as a build dependency#5092
Conversation
|
What I did not do yet: Add caching for |
a10ca09 to
90c358f
Compare
siavashs
left a comment
There was a problem hiding this comment.
We need to clean up the Makefile (see comment below) and ui/app/ CONTRIBUTING.md too
90c358f to
6a66618
Compare
There are a few advantages to this setup: * Dependabot can now help with keeping versions recent. * The setup is more idiomatic and simpler. In particular, our build steps are more deterministic due to the inclusion of `package-lock.json`. * We have to get rid of the Docker based approach for the mantime ui, so this change makes things more uniform across the repo. * The command `promu crossbuild` does not support running Docker. With this change, we could delete `email.tmpl` from our repo, and it would not affect our build process in any way. The disadvantage is that this setup requires a local toolchain (stored in `template/node_modules`) to update `email.tmpl`. Why does this PR not delete `email.tmpl`? This file is small, and `go` developers do not need to install any `node` tools to run. ```sh go get github.com/prometheus/alertmanager/template ``` Signed-off-by: Solomon Jacobs <solomonjacobs@protonmail.com>
There are a few advantages to this setup: * Dependabot can now help with keeping versions recent. * The setup is more idiomatic and simpler. In particular, our build steps are more deterministic due to the inclusion of `package-lock.json`. * We have to get rid of the Docker based approach for the mantime ui, so this change makes things more uniform across the repo. * The command `promu crossbuild` does not support running Docker. With this change, we could delete `ui/app/script.js` from our repo, and it would not affect our build process in any way. * This change should help users, which are building `podman` with `Alertmanager`. The disadvantage is that this setup requires a local toolchain (stored in `ui/app/node_modules`) to update `ui/app/script.js`. The file `ui/app/script.js` will be removed from the repo in a future commit. This will break the following invocation: ```sh go get github.com/prometheus/alertmanager/ui ``` The dependency `openapitools/openapi-generator-cli` could not be removed, since it depends on `Java`. Signed-off-by: Solomon Jacobs <solomonjacobs@protonmail.com>
6a66618 to
e0bfa79
Compare
|
Have we tried podman? |
|
@ultrotter It depends on what you mean. Before You always had to pull the This PR specifically removes promu crossbuild --podman -p linux/amd64What you can't do is use the created artifacts locally (except for
There used to be bind-mounts in |
|
@SoloJacobs What I meant is having an easy target that can build the tools "in a container" with a Dockerfile that pulls the build deps... It could be easy for the target to do This is pretty much what you described and might not need (write) bind mounts at all... |
|
To move things forward a bit, I would like to add this extra target after the ui changes have landed. I did add the build target you requested here: #5102 |
The advantages and disadvantages of this PR are described in the commit message. The main motivation is the we use
prom crossbuild, which does not allow usingDockerduring the build step. Because we can't execute the build steps during the cross build, we are forced to check our build artefacts into the source code.Pull Request Checklist
(for developers nothing changed Edit: the CONTRIBUTING.md needed an update afterall)
Which user-facing changes does this PR introduce?