Update relocation map during porter publish #2186
Conversation
VinozzZ
force-pushed
the
update-relocation-map-publish
branch
2 times, most recently
from
e9baaa6
to
47d0d04
Compare
|
@VinozzZ Sorry it took me a bit to review this and in the meantime there are merge conflicts. Can you resolve them and then I'll do a review? |
Signed-off-by: Yingrong Zhao <yingrong.zhao@gmail.com>
Signed-off-by: Yingrong Zhao <yingrong.zhao@gmail.com>
Signed-off-by: Yingrong Zhao <yingrong.zhao@gmail.com>
Signed-off-by: Yingrong Zhao <yingrong.zhao@gmail.com>
Signed-off-by: Yingrong Zhao <yingrong.zhao@gmail.com>
VinozzZ
force-pushed
the
update-relocation-map-publish
branch
from
43a72c1
to
12d87c5
Compare
pkg/porter/publish.go
Outdated
| @@ -416,6 +357,31 @@ func (p *Porter) rewriteBundleWithInvocationImageDigest(ctx context.Context, m * | |||
| return bun, nil | |||
| } | |||
|
|
|||
| func (p *Porter) updateRelocationMapping(relocationMap relocation.ImageRelocationMap, layout registry.Layout, originImg string, newReference string) (relocation.ImageRelocationMap, error) { | |||
There was a problem hiding this comment.
The function name made me initially assume that this function only updated the relocation mapping file, and it never ocurred to me that it would also push the image. What do you think about changing the function name so that it's more obvious what it does?
| func (p *Porter) updateRelocationMapping(relocationMap relocation.ImageRelocationMap, layout registry.Layout, originImg string, newReference string) (relocation.ImageRelocationMap, error) { | |
| func (p *Porter) relocateImage(relocationMap relocation.ImageRelocationMap, layout registry.Layout, originImg string, newReference string) (relocation.ImageRelocationMap, error) { |
There was a problem hiding this comment.
That's good point and a great name. I will change it
pkg/porter/publish_test.go
Outdated
| description string | ||
| relocationMap relocation.ImageRelocationMap | ||
| layout registry.Layout | ||
| hasError bool |
There was a problem hiding this comment.
Let's change this to use wantError string so that we can check that it failed for the expected reason.
tests/integration/archive_test.go
Outdated
| @@ -77,6 +77,8 @@ func TestArchive_StableDigest(t *testing.T) { | |||
|
|
|||
| // Use a fixed bundle to work with so that we can rely on the registry and layer digests | |||
| const reference = "ghcr.io/getporter/examples/whalegap:v0.2.0" | |||
| const referencedImg = "carolynvs/whalesayd@sha256:8b92b7269f59e3ed824e811a1ff1ee64f0d44c0218efefada57a4bebc2d7ef6f" | |||
There was a problem hiding this comment.
I was expecting that these extra validations would be added to the Airgapped test above, since this feature supports airgap and testing it requires that we are making it impossible to use the original images by taking the original registry offline.
The test you are editing is for checking that when we archive twice, we get the same digest.
Can you move these checks into the other test?
Signed-off-by: Yingrong Zhao <yingrong.zhao@gmail.com>
VinozzZ
changed the title
What does this change
Currently, when publishing from a bundle, porter generates a new bundle that don't preserve content digests.
This PR updates the logic to use relocation maps in the original bundle to associate the old images with the newly copied images.
What issue does it fix
Closes #1819
Notes for the reviewer
Checklist
Reviewer Checklist