simplify dockerfile, parity with cedarish #15
Conversation
…sh. also makes the image slightly smaller.
|
+1 |
|
Can you provide some insight into why this change is required before you can use our Docker image? |
+1 |
|
Did you see my question, @progrium? |
|
Because we match the environment as closely as possible. This is literally the Dockerfile cedarish uses. Although the previous one was roughly comparable, we don't want to risk them getting out of sync. On top of that, it's best practice (despite what you may have seen, and what "makes sense" about caching) to have minimal layers and joining Can you provide some insight into your hesitation? |
|
+1 |
|
+1 this would be lovely for us :) |
|
+1! |
|
@progrium, the problem is that there shouldn't be a need to match the environment in the first place. Why don't you just use our published image? If the way we build that image poses a practical concern, we can definitely have a discussion about changing it, but that doesn't seem to be the issue. |
|
Shouldn't be a need to match the environment.... can you elaborate on that? |
|
@progrium, instead of having two scripts be identical, it seems retiring the script wanting to be compatible with our version is the easier choice. I probably need additional context, but couldn't you just replace these instances with |
|
Having two scripts be identical? Are you talking about what cedarish is doing now? Yes, I agree! That is the point here. I wouldn't use the current |
|
That said maybe it's a style of communication problem. But I'm perceiving this as "being difficult without explanation". We propose improvements that happen to make it closer to what we want to use, you keep suggesting "why not just do it our way" without much explanation. Maybe .... people are using Maybe ... you don't want to care what the community thinks because this isn't a project you want to spend maintaining support for a wider community? You just want to do it the way you want and not care? Maybe ... I don't know. See? I can't give you additional context without you sharing your additional context. |
|
@progrium, I probably just misunderstood what you're trying to do. If all you're trying to do is to deprecate the other image that's great. So, you want this pull request merged before making the switch because you're worried that we might not keep it up to date otherwise? That's a fair concern, although I can assure you that we will. I guess the answer is ultimately going to be trust. Even with the proposed change we could stop pushing changes to this repository. What I thought you wanted was to increase the similarity between how your image is built and how we build ours. That's my concern. I don't want to make it any easier to produce more or less similar images; I want to have a single image so we will in fact be able to guarantee compatibility. If this change is what you need for moving towards a single image instead of two images based on a single script, I'm fine merging it. |
|
First off, this is great news. Second, I'll admit that the basic Herokuish smoke tests do pass when using the existing However, besides trust and maintainability concerns, we do have concerns over how the Docker image is built. First, as you know, we want it to be as close as possible to the production Heroku cedar stack. If the Docker image is built with a set of commands that are similar but not the same to However, by actually using the same script, it makes it much more clear to everybody that it is as close as it can be. It also means that we know if you change
Next issue in how the Docker image is built. We have two related but separate concerns with Docker images. Size. And first time to download. Cedar14 is very big and seems it will only get bigger. To me this only increases the need to be as efficient as possible wherever we can. Right now, The bigger difference is first time to download. This is important because this is time needed with every new install of any platform based on cedarish. To us, every second counts. Although time to download seems directly tied to size, there are plenty of other factors. Dokku originally put an exported image on S3 (actually still does, but that will change) because Docker Hub used to be too slow and didn't compress layers. But the design of layers also makes a difference. There is slight overhead with each layer and the significant ones are not downloaded in parallel. Let me show you the difference between Obviously network speed introduces a huge variance, but cedarish consistently downloads faster than current heroku/cedar. Even though they are "more or less similar". But this is important to us. So I think this explains all the reasons the changes in this PR are important to us, and why we think they should be important to you as well. :) |
|
That's a solid case. Thank you for taking the time to submit this pull request and walk me through it. I can't wait to see you use our image. |
|
Very exciting. Thank you! On Tue, Apr 7, 2015 at 11:53 AM, Troels Thomsen [email protected]
Jeff Lindsay |
Since: * The default reported by `locale` is `POSIX`, which is an alias of `C`: https://www.gnu.org/software/libc/manual/html_node/Standard-Locales.html * There is no difference in the image contents with/without `LC_ALL=C` set, as confirmed by `container-diff diff --type file ...` (beyond the usual logs/fontcache/... differences). * The env var was only set whilst `setup.sh` was being run, so has no effect on runtime usage of the stack-images. * It was added to the stack-images repo in #15 without explanation, but seems to have been copied from here: https://github.com/progrium/cedarish/blob/967934e45d026fb2349422d50306ec462fc1789f/cedar14/Dockerfile#L3 ...which itself was copied from the Ubuntu 12.10 usage here: progrium/cedarish@06b175d I've not modified the Cedar-14 image, since it's EOL and has noisier diffs in general when using container-diff, so not worth the effort to vet. Refs W-7496420.
Since: * The default locale reported by `locale` is `POSIX`, which is an alias of `C`: https://www.gnu.org/software/libc/manual/html_node/Standard-Locales.html * There is no difference in the image contents with/without `LC_ALL=C` set, as confirmed by `container-diff diff --type file ...` (beyond the usual logs/fontcache/... differences). * The env var was only set whilst `setup.sh` was being run, so has no effect on runtime usage of the stack-images. * It was added to the stack-images repo in #15 without explanation, but seems to have been copied from here: https://github.com/progrium/cedarish/blob/967934e45d026fb2349422d50306ec462fc1789f/cedar14/Dockerfile#L3 ...which itself was copied from the Ubuntu 12.10 usage here: progrium/cedarish@06b175d See also: https://help.ubuntu.com/community/Locale I've not modified the Cedar-14 image, since it's EOL and has noisier diffs in general when using container-diff, so not worth the effort to vet. Refs W-7496420.
This PR makes the Dockerfile pretty much exactly what progrium/cedarish is. It also makes it slightly smaller and eliminates the need to update the Dockerfile separately from the cedar-14.sh script.
My hope is that we can deprecate progrium/cedarish so that Dokku, Deis, Flynn, etc can start using this repo as the official base image. Cedarish has become a repository that just follows the cedar-14.sh script here and makes its own releases. If we can simplify the ecosystem and get all our base images closer to the authoritative source, all the better. :)