-
Notifications
You must be signed in to change notification settings - Fork 169
Proposal: ONBUILD image #19
Comments
This proposal looks great. I see the With this proposal you currently end up with the following onbuild images:
The downside I'm currently facing is when the application is part of a solution. It is possible to run Having the |
@ahmetalpbalkan This looks good to me. I recommend Great work! Looking forward to retiring my image :-) |
@vlesierse great point about solutions. I have no idea how kpm restore works for multi-project solutions. So is it enough to run I'll leave it up to you folks. Are the multi-project solutions the common case in ASP.NET ecosystem? If so, we must shape the onbuild image accordingly. |
Multi-project solutions are very common; however, generally just one project is deployed because you use |
I believe Docker is mainly used in a production environment and pre-compiling makes sense in that case. If I'm correct |
@vlesierse startup time is mostly what's affected by precompilation. Not having to load Roslyn, read 100's of *.cs files and *.cshtml files from disk, parsing and compiling them all, etc. can be a huge saving. (In a trivial app or other very specific app patterns it might not be terribly significant.) BTW regarding |
@Eilon ok, having a |
Couldn't the convention be for people to specify the command that they want to run in the dockerfile they place with their source code? Also note that the Docker people are introducing additional ways to specify what commands should be run in containers (see "Docker Compose"), so I wouldn't get too hung up on that... |
@vlesierse thanks for bringing up the multi-project use case. I did not realize this before. This changes pretty much everything in onbuild image. It appears that since Dockerfile can't run commands on the client side (i.e. kpm pack), perhaps it should accept only the kpm pack output as the input? I'll try this multi-project setup out next week and see what we can do about it. @friism can you please elaborate (maybe an example)? Mostly onbuild images just work because the commands executed to download dependencies etc. on every Python app or Golang app are >90% the same. That's why onbuild images handle the common case. Also, I'm not sure if Docker Compose has anything to with commands executed inside a container (maybe I'm missing something). I favor waiting a bit for creating the onbuild image and come up with something right and handles the common case. |
@ahmetalpbalkan have you considered the topic of |
@friism hmm no idea what that is, I couldn't figure out by looking at the rails image. What does that do? |
hmm found this https://github.com/aspnet/Home/wiki/Lock-file but I'm trying to find an example. |
@ahmetalpbalkan There's one in musicstore: https://github.com/aspnet/MusicStore/tree/master/src/MusicStore My current thinking is that onbuild should copy it in with project.json, eg:
|
@friism sounds about right. |
@davidfowl re: lock files I do think that the lock file should be copied - the runtime uses that file to determine the exact versions of packages to load. I think it'll work without it, but you might get unexpected results. It also serves as an optimization because the entire package graph is already resolved. But Fowler will have ton confirm. |
@ahmetalpbalkan what about multiple |
@friism you're making an assumption like I'm using ASP.NET, on the contrary I don't know much about ASP.NET at this point, so I don't know what multiple projects mean. 😄 If it means multiple applications, then they probably should go into separate images. If it means an executable entrypoint and bunch of other projects providing DLLs which the entrypoint project depends on, then it should be a single image, however I'm not sure how a single onbuild image can handle building all these... My plan was to run PartsUnlimited sample application with this onbuild image but last time I tried it was not working on Mono for some reason (I think the feed was messed up somehow and I was getting version-related errors in restore). If you can get it working (I believe it has multiple projects, too) we can iterate on that. Just a reminder, with this ONBUILD image, we're aiming to handle the 90% of the use cases (i.e. common case) and if it's possible at all, I would love to move further on this proposal. |
Putting this in RC2 for now and assigning to @glennc make a decision/talk to @ahmetalpbalkan etc. |
For those who are not familiar, Dockerfiles can have an instruction called ONBUILD which invokes the preceding command when the image is used as a base image for building another image (typically app image).
Right now simplest ASP.NET application image would look like this:
With the help of ONBUILD directives, we can simplify this Dockerfile as short as this:
and that would be the only Dockerfile to build an ASP.NET app image and ship it to the Docker host. Relevant work is suggested by @friism (#11) and @vlesierse (#7) in the pull requests. Therefore I am starting a discussion about possible risks and implementation details.
Risks of breaking users
The dockerfile written above is almost the same for all web applications.
/app
(or/usr/src/app
open to debase, but ok)/app
and runskpm restore
(always needed?)k
executable withkestrel
parameter.Since onbuild image will bring quite some easiness, if we happen to change the path (i.e.
/app
) and people take dependency on that, we will be breaking the apps built with the -onbuild image.Also, if
kpm restore
takes any parameter (e.g. feed), unfortunately we won't be able to pass that to theONBUILD RUN kpm restore
. So it will stay as barekpm restore
. Does this handle the 90% use case?It's ok to break users now since this image is still in preview, however in the long term this will not be feasible, therefore we should be coming up with a solution that can stabilize and get adopted by the community over time.
Maintenance
Along with any version (e.g.
1.0.0-beta1
) we need to maintain an -onbuild Dockerfile equivalent as well (e.g.1.0.0-beta1-onbuild
). That's not a big cost, I have no concerns in this area. (example)Entrypoint
Right now it appears like
k kestrel
is the de facto entrypoint for the web server containers. However I do not want to leave thek run
(command line app) use case out of the onbuild images. The both entrypoint common in all use cases isk
itself and the rest can be changed.So in the onbuild image, if we specify the entrypoint as:
In the simplest Dockerfile (just contains
FROM ....
above the entrypoint will bek kestrel
.If users need to override the
k
parameter in their app image, they can use:FROM microsoft/aspnet
CMD ["run"]
and the entrypoint will be overriden as
k run
.If users need to override the entrypoint entirely, they can use
ENTRYPOINT
directive in their app's Dockerfile.Passing parameters: by overriding the CMD I can pass extra parameters to the k command (e.g.
k run --param foo
) in Docker cli from an onbuild image like:docker run -i -t myapp run --param foo
This will result in a cmd like
["k", "run", "--param", "foo"]
which is valid.Tagging onbuild images
An onbuild image for a normal image will just have
-onbuild
suffix in the Docker image tag. (e.g.microsoft/aspnet:1.0.0
will have correspondingmicrosoft/aspnet:1.0.0-onbuild
image).An ONBUILD dockerfile for a version should be stored at
version/onbuild/Dockerfile
(e.g. onbuild Dockerfile for1.0.0/Dockerfile
should be at1.0.0/onbuild/Dockerfile
). This will keep things organized at the root level view on the repo.This also appears to be the convention.
The ONBUILD image for
latest
image will just be tagged asonbuild
and will be referenced asmicrosoft/aspnet:onbuild
.Release steps
Once the PR is merged, we'll push the onbuild image for latest(
=1.0.0-beta1
) immediately and for the next version releases, ONBUILD image has to be included as a part of the version update PR. The README.md should also be updated to reflect the changes in the tags.Once changes are merged administrators must update the automated build configuration to point
latest
/onbuild
tags to the correct version.Going forward
@vlesierse's PR #7 uses
ENTRYPOINT
andCMD
separately, which makes sense in the description above.I really appreciate input from you guys on going forward. Do you see any problems with the proposal above or do you see additional issues/concerns we should be aware of? At this point iterating on @vlesierse's PR #7 looks like a better choice for me.
/cc: @davidfowl @johngossman
The text was updated successfully, but these errors were encountered: