box icon indicating copy to clipboard operation
box copied to clipboard

feature: flatten with scope

Open erikh opened this issue 9 years ago • 7 comments
trafficstars

WIP, this'll probably be edited a few times.

Right now flatten does a few things wrong:

  • it merges the from image with the rest of the images; that's probably not what people want most of the time (although the option should be added in at some point)
  • it cannot scope what is flattened and what is not, e.g., flatten two run statements into one, but leave the next copy statement alone.
  • it cannot influence the cache, and any post-flattening commands will have their cache invalidated.

Proposal:

Surface-wise, we need to extend the flatten statement itself.

flatten "cacheKey" do
  run "foo"
  run "bar"
end

The cache key can be literally supplied as an argument. Corresponding with a cache key of some sort (this could be derived from environment, local files, other container's files, etc to ensure cache validation occurs safely) could influence the cache in a way that was safe to handle for multiple invocations of each run without corrupting or otherwise poorly influencing the cache. This, however, is completely driven by the end user and could lead to some very bad abuses, consistency problems in their internal build systems, etc. Maybe this should be a separate block statement so it can be filtered without losing flatten functionality.

Block mode is shown here which would allow all the statements inside the flatten to be flattened. Blocks should not be required and it is assumed that flatten without a block will flatten everything to the container before the run statement.

Internals-wise, we need to do two things:

  • solve #33
  • implement a layer recording device, since 99% of this is here already I don't think it'll take too much effort.
  • flatten hooks into new executor calls: FlattenAll() and FlattenLast(n int) which flattens the entire run or the last n elements in the run respectively.

All of the above should satisfy our flatten needs. The full flatten can have the from omitted for free if we track only the layers we created.

erikh avatar Nov 01 '16 00:11 erikh

Also, simply flattening all layers above the base image is probably a good and simple optimisation for many use-cases. Being able to that without having to indent all of the line inside a block would be very handy, I think, e.g. something like flatten :above_base_image at the end of a file.

errordeveloper avatar Dec 05 '16 13:12 errordeveloper

that should already work (and is the only way flatten works now); I'll test in a bit.

erikh avatar Dec 05 '16 13:12 erikh

nope. I'll make this the default tomorrow, now that we have the layer editing framework this should not be hard.

erikh avatar Dec 05 '16 13:12 erikh

this is going to take more work than anticipated. Will be a week or so.

erikh avatar Dec 08 '16 11:12 erikh

Ok, I looked into this tonight and realized that a few problems exist:

whiteout files (which are preserved in docker images) will not be handled right in a cross-platform way. This means we cannot just unpack to the host (regardless of all the safety issues that brings). Copying into a fresh container is a good solution.

However, because of the copy bug I've been trying to get patched into docker, that would reset all the files in the layers back into root.

There aren't really any good alternatives. At some point these files have to be extracted on top of each other, and to do it in a way that doesn't lock people into linux is not possible as far as I can tell.

Happily would accept any solution. For context, the existing flatten copies all files OUT of docker (which does not have the permissions issue) into a tarfile, builds an image based on that, and then blows it back into docker.

erikh avatar Dec 18 '16 11:12 erikh

Makes sense. I don't think there is an urgent need to fix it, unless we want to warn the user to avoid surprises... Fixing upstream sounds like the best approach!

On Sun, 18 Dec 2016 at 11:26 Erik Hollensbe [email protected] wrote:

Ok, I looked into this tonight and realized that a few problems exist:

whiteout files (which are preserved in docker images) will not be handled right in a cross-platform way. This means we cannot just unpack to the host (regardless of all the safety issues that brings). Copying into a fresh container is a good solution.

However, because of the copy bug I've been trying to get patched into docker, that would reset all the files in the layers back into root.

There aren't really any good alternatives. At some point these files have to be extracted on top of each other, and to do it in a way that doesn't lock people into linux is not possible as far as I can tell.

Happily would accept any solution. For context, the existing flatten copies all files OUT of docker (which does not have the permissions issue) into a tarfile, builds an image based on that, and then blows it back into docker.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/erikh/box/issues/34#issuecomment-267816031, or mute the thread https://github.com/notifications/unsubscribe-auth/AAPWSxBHugrbbUnutubQ1DaR2_MXDM3xks5rJRhxgaJpZM4KlqSg .

errordeveloper avatar Dec 18 '16 12:12 errordeveloper

there's more urgent reasons to fix the copy issue, yeah.

On Sun, Dec 18, 2016 at 4:43 AM, Ilya Dmitrichenko <[email protected]

wrote:

Makes sense. I don't think there is an urgent need to fix it, unless we want to warn the user to avoid surprises... Fixing upstream sounds like the best approach!

On Sun, 18 Dec 2016 at 11:26 Erik Hollensbe [email protected] wrote:

Ok, I looked into this tonight and realized that a few problems exist:

whiteout files (which are preserved in docker images) will not be handled right in a cross-platform way. This means we cannot just unpack to the host (regardless of all the safety issues that brings). Copying into a fresh container is a good solution.

However, because of the copy bug I've been trying to get patched into docker, that would reset all the files in the layers back into root.

There aren't really any good alternatives. At some point these files have to be extracted on top of each other, and to do it in a way that doesn't lock people into linux is not possible as far as I can tell.

Happily would accept any solution. For context, the existing flatten copies all files OUT of docker (which does not have the permissions issue) into a tarfile, builds an image based on that, and then blows it back into docker.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/erikh/box/issues/34#issuecomment-267816031, or mute the thread <https://github.com/notifications/unsubscribe-auth/ AAPWSxBHugrbbUnutubQ1DaR2_MXDM3xks5rJRhxgaJpZM4KlqSg>

.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/erikh/box/issues/34#issuecomment-267819122, or mute the thread https://github.com/notifications/unsubscribe-auth/AABJ6xwYIhWWhS1b2OKBvj7fpmc-382Jks5rJSpVgaJpZM4KlqSg .

erikh avatar Dec 18 '16 12:12 erikh