Fix layer-related issues for oci_rebase_image#50
Draft
abayer wants to merge 1 commit intoDataDog:mainfrom
Draft
Conversation
Specifically: * Make sure we're using `platforms.OnlyStrict` instead of `platforms.Only` when constructing the platform matcher, because with `Only`, `arm/v8` will also match the other `arm/*` platforms, and, more immediately relevant, `amd64` will match the first item in the index with _either_ `amd64` or `386`. Which is...not what we want, obviously. * When generating the build files in `GenerateBuildFilesCmd`, don't add `oci_blob`s and traverse children for blobs which we've already added an `oci_blob` and traversed for. If there's an identical layer in more than one platform's image, and we've got `shallow` set to false (so that we're pulling layers as well as configs/indexes/manifests), the generated `BUILD.bazel` will end up invalid if we don't do this, because there'll be duplicate `oci_blob` rules (and duplicate rules for children, but in practice I didn't run into that because the only duplicates I hit in testing were layers). * Add the OCI base image name/digest annotations to the layers we're copying from the original image to the rebased image. `CopyContent` is leveraging those annotations to determine when we can avoid actually pushing a layer to the registry in favor of using `Mount`, and we're already doing that for the base image layers in `AppendLayers`. Note that there are still some interesting/annoying issues or quirks, and not just with `oci_rebase_image`: * With either `oci_image` or `oci_rebase_image` followed by an `oci_push`, if you're pushing to a different registry than the image(s) containing the pre-existing layers in the new image, you'll get a confusing error (`failed to push child content to registry: failed to create reader from ingestor: not found`). I think this is because the `Mount` fails, because you can't cross-repo mount a blob from a different registry host, and `oci_push` doesn't know where to find the layers from the pre-existing image(s) locally. I'm still trying to unravel that one, but am guessing it's happening because the layout passed to `ocitool push` doesn't contain the actual layers from the pre-existing image(s) and I'm not yet sure how to solve that. * With `oci_rebase_image`, the `oci_pull` for the original image _must_ have `shallow = False`, or `RebaseImage`'s call to `AppendLayers` can't locate the diff ID for the layers from the original image, for the fairly obvious in retrospect reason that...we're not pulling the layers unless we pass `shallow = False` to `oci_pull`. Oops. Signed-off-by: Andrew Bayer <andrew.bayer@gmail.com>
Contributor
Author
|
cc @jprobinson - given the messiness so far, I'm not sure if it doesn't just make sense to remove |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Specifically:
platforms.OnlyStrictinstead ofplatforms.Onlywhen constructing the platform matcher, because withOnly,arm/v8will also match the otherarm/*platforms, and, more immediately relevant,amd64will match the first item in the index with eitheramd64or386. Which is...not what we want, obviously.GenerateBuildFilesCmd, don't addoci_blobs and traverse children for blobs which we've already added anoci_bloband traversed for. If there's an identical layer in more than one platform's image, and we've gotshallowset to false (so that we're pulling layers as well as configs/indexes/manifests), the generatedBUILD.bazelwill end up invalid if we don't do this, because there'll be duplicateoci_blobrules (and duplicate rules for children, but in practice I didn't run into that because the only duplicates I hit in testing were layers).CopyContentis leveraging those annotations to determine when we can avoid actually pushing a layer to the registry in favor of usingMount, and we're already doing that for the base image layers inAppendLayers.Note that there are still some interesting/annoying issues or quirks, and not just with
oci_rebase_image:oci_imageoroci_rebase_imagefollowed by anoci_push, if you're pushing to a different registry than the image(s) containing the pre-existing layers in the new image, you'll get a confusing error (failed to push child content to registry: failed to create reader from ingestor: not found). I think this is because theMountfails, because you can't cross-repo mount a blob from a different registry host, andoci_pushdoesn't know where to find the layers from the pre-existing image(s) locally. I'm still trying to unravel that one, but am guessing it's happening because the layout passed toocitool pushdoesn't contain the actual layers from the pre-existing image(s) and I'm not yet sure how to solve that.oci_rebase_image, theoci_pullfor the original image must haveshallow = False, orRebaseImage's call toAppendLayerscan't locate the diff ID for the layers from the original image, for the fairly obvious in retrospect reason that...we're not pulling the layers unless we passshallow = Falsetooci_pull. Oops.