feat: Track history in per layer metadata file#568
Conversation
|
@malt3 I'm trying this again! could you trigger the integration tests to get a read on if docker is going to accept this? |
|
Hey! Integration tests are not fully working for forked PRs right now. They will exclusively run as part of the merge queue. Apologies for the inconvenience. I'll take a look myself and run tests on my end. |
| config_history = config.get("history") | ||
| if config_history and len(config_history) > layer_index: | ||
| layer_history = config_history[layer_index] | ||
| else: | ||
| layer_history = None |
There was a problem hiding this comment.
I think this is not good enough.
The real rule for this is that we may have:
- No history (you handle that)
- too little history (you handle that)
- Up to len(layers) layer with
empty_layer== false - In between those, any number of history entries with
"empty_layer": true
We need to align the non-empty history entries to the layers and use some heuristic to assign the empty_layer history entries. Either we assign the empty_layer entries to the previous layer or the next layer (doesn't really matter, pick one). This also means that the metadata for a single layer must hold a list of history entries, where only one member may be empty_layer false. Still, in most cases we will have exactly one entry. :)
There was a problem hiding this comment.
Ah okay I think I finally understand, took me a second to remember that things like ENV won't actually generate an on-disk layer. I'll update to collapse the empty layer histories into the layer before it.
There was a problem hiding this comment.
Do you have any pointers to running the specific integration test that gets tripped up locally? Don't want to waste your time doing testing round trips.
There was a problem hiding this comment.
bazel query e2e:all
bazel test (any name that comes back)
There was a problem hiding this comment.
Also:
cd e2e/go
bazel run --platforms=//platform:linux_amd64 image:load
2 participants
This supersedes #454 with a hopefully more robust strategy. Instead of generating an out-of-band history file, we track history using the pre-existing per-layer metadata. For bazel-native layers we automatically generate it. Layers we import, we best effort extract the per-layer history from the Image config.
Opening this a bit early for feedback. I still want to add more in-depth tests
Right now we copy the History struct into the api package. It looked like the api was pretty low-dep so I wanted to keep it that way.