Make Docker image Best Practices-Compliant

[sr229]

Unfortunately, this image does not adhere to Best practices as stated in #65. We should rewrite the Dockerfile to adhere to the following directives:

• using a user account
• Optimizing layers (minimizing layers)

I'm afraid such official image has to set an example like the rest of the Docker Library images.

[kylecarbs]

cc: @nhooyr

[nhooyr]

There is already an open issue for sudo. Not sure what you mean in terms of minimizing layers, its very minimal already.

[vincentclaes]

if the dockerfile follows the best practices it can run on open shift cluster. now i cannot run it on open shift because it does not allow to be run as root. that's why a 👍 for me

[vincentclaes]

@nhooyr what OS are you building your dockerfile? i get error messages when building on osx high sierra

[nhooyr]

I'm on Mojave. What error messages do you get?

[sr229]

This is a tracking issue since I'm already implementing it @nhooyr. The other issue was more on debating on the Docker image's compliancy.

[nhooyr]

@sr229 I appreciate and admire your initiative but please wait for consensus before implementing things. I don't want to have to reject things after you put your time into them.

Please respond to my question regarding minimizing layers.

Also #65 is the tracking issue for implementing sudo. So unless there is more to minimizing layers, this issue should be closed.

[sr229]

Each RUN and COPY layer is equivalent to 1 layer. Even if we have a small enough image, having many layers does not equal to optimization.

Without minding the build layer. We can reduce the two RUNs into one RUN to be one layer. Currently we have 4 layers, which can be reduced to 3 layers which could result in faster extraction. This is the same concept we're trying to do in Theia/Gitpod @nhooyr

[nhooyr]

Without minding the build layer. We can reduce the two RUNs into one RUN to be one layer. Currently we have 4 layers, which can be reduced to 3 layers which could result in faster extraction. This is the same concept we're trying to do in Theia/Gitpod @nhooyr

That will make things harder to read as the two RUN instructions are unrelated. Performance is not all that matters. I also doubt this optimization will be user visible as it's very minor. It's unlikely to get approved by me.

So I'm closing this as wontfix.

[sr229]

I don't see why it won't be readable. You're closing the issue because point 2 does not match your specific requirements, point 1 still needs to be done to compensate for clusters where compliancy is a must (OpenShift).

This is arguably biased.

[nhooyr]

I don't see why it won't be readable. You're closing the issue because point 2 does not match your specific requirements

Because the two instructions are doing two different things. The first one installs runtime deps for code-server and the second one generates locales. By separating the instructions, their distinctness becomes clear. A single instruction would not get this across without comments separating them, which is janky. Furthermore, the optimization is very minor and wouldn't affect users.

point 1 still needs to be done to compensate for clusters where compliancy is a must (OpenShift)

That change is still being discussed and no decision has been made and there is already a tracking issue. #65

Furthermore, you're not blocked on it. Its trivial to extend the Dockerfile and add your own user.

This is arguably biased.

Calling me biased does not help your case. Please stick to technical arguments. I am using my best judgement to help guide the development of code-server.

[kennylevinsen]

We can reduce the two RUNs into one RUN to be one layer.

Layers are meant to be a single logical change, allowing reuse on rebuild when that layer is not modified, as well as allowing reuse across different containers that do the same thing. This is not possible if unrelated actions are combined into a single layer. That is why a single Dockerfile doesn't translate to a single layer.

Minimizing layer count is good, but the sole point of layers in Docker is lost if they no longer contain only a single logical change each.

[sr229]

@kennylevinsen While that is true, abusing layers just for this sole reason is why we have too many unoptimized images. Layer counts does not translate to how fast your application can pull and extract, and keeping some changes in one layer versus two is more ideal.

Please do not count layer as one logical change, Layers are counted in the idea there is substantial changes in the FS that will be overlayed. That's not how it works.

[kennylevinsen]

Layer counts does not translate to how fast your application can pull and extract

Exactly. One 100MB layer does not pull or extract faster than ten 10MB layers. Make an update, and you'll see that the one layer approach needs to download 90MB more than the ten layer approach (which also wastes storage).

and keeping some changes in one layer versus two is more ideal.

Depends on what you mean by changes. One change, sure, don't split single tasks if there is no reason to do so.

Several changes, no. Fewer layers guarantee waste, where more layers give the possibility of avoiding it.

Please do not count layer as one logical change, Layers are counted in the idea there is substantial changes in the FS that will be overlayed. That's not how it works.

That's exactly how it works, and how docker is designed. There's a reason that docker stubbornly creates a layer for each and every statement—because docker wants you to create many, tiny layers.

Attempting to combine many things into a single layer is simply against the design of docker, and optimizes an irrelevant metric (layer count), at the cost of relevant, limited and sometimes even billed metrics (storage and bandwidth consumption).

(Oh, and just to clarify: I do not consider one logical change to be installing one out of hundreds of dependencies, but rather something like "install base dependencies".)