Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Implement annotation-based rebase hints #960

Merged
merged 10 commits into from
Aug 16, 2021
Merged

Implement annotation-based rebase hints #960

merged 10 commits into from
Aug 16, 2021

Conversation

imjasonh
Copy link
Collaborator

@imjasonh imjasonh commented Mar 10, 2021 •
edited
Loading

Adds crane.Rebase helper function for higher-level functionality including annotation-based hints

With this change, crane rebase <image> without args will attempt to determine the old and new base from (WIP placeholder) annotations, perform the rebase, and push a new image back to <image> (--rebased flag will override). The rebased image will have base-by-digest and base-by-tag annotations added for future rebase operations.

  • --original if provided overrides the positional arg
  • --old_base and --new_base override any detected annotations
  • --rebased (deprecated, now --tag to match other surfaces) overrides pushing over the original image
  • base-digest and base-tag annotations are populated from --new_base by digest+tag (or previously detected base-tag)

This is WIP while we sort out whether the annotation will be a reserved annotation in the OCI spec. If the code looks good, I'll take a stab at updating rebase.md as well (it could probably use it anyway 😅 ).

@imjasonh imjasonh force-pushed the crane-rebase branch 3 times, most recently from 7b856c9 to ae7e1ca Compare March 11, 2021 02:13
@codecov-io
Copy link

codecov-io commented Mar 11, 2021 •
edited
Loading

Codecov Report

Merging #960 (a210291) into main (2f4c6f0) will decrease coverage by 0.74%.
The diff coverage is 18.18%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #960      +/-   ##
==========================================
- Coverage   73.05%   72.31%   -0.75%     
==========================================
  Files         107      108       +1     
  Lines        4792     4858      +66     
==========================================
+ Hits         3501     3513      +12     
- Misses        775      827      +52     
- Partials      516      518       +2
Impacted Files Coverage Δ
pkg/crane/rebase.go 0.00% <0.00%> (ø)
pkg/v1/mutate/rebase.go 53.84% <75.00%> (+5.45%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2f4c6f0...7f23862. Read the comment docs.

@imjasonh imjasonh force-pushed the crane-rebase branch 2 times, most recently from a210291 to 7f23862 Compare March 11, 2021 18:28
This was referenced Mar 17, 2021
Open
Closed
@imjasonh imjasonh mentioned this pull request Apr 16, 2021
Merged
@imjasonh imjasonh force-pushed the crane-rebase branch 2 times, most recently from ce92012 to e994afe Compare April 16, 2021 20:44
@imjasonh imjasonh changed the title WIP: Implement annotation-based rebase hints Implement annotation-based rebase hints May 3, 2021
@codecov-commenter
Copy link

codecov-commenter commented May 3, 2021 •
edited
Loading

Codecov Report

Merging #960 (2a77755) into main (bea59b9) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #960   +/-   ##
=======================================
  Coverage   75.44%   75.44%           
=======================================
  Files         108      108           
  Lines        7692     7692           
=======================================
  Hits         5803     5803           
  Misses       1334     1334           
  Partials      555      555

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update bea59b9...2a77755. Read the comment docs.

@imjasonh imjasonh force-pushed the crane-rebase branch 2 times, most recently from 6e64c12 to 7e0d5e7 Compare May 3, 2021 20:35
@imjasonh imjasonh mentioned this pull request May 5, 2021
Merged
This was referenced May 7, 2021
Open
Merged
jonjohnsonjr
jonjohnsonjr reviewed May 17, 2021
View reviewed changes
pkg/crane/rebase.go Outdated Show resolved Hide resolved
@imjasonh imjasonh force-pushed the crane-rebase branch 2 times, most recently from b7d5960 to a3db8db Compare June 22, 2021 20:25
@imjasonh imjasonh force-pushed the crane-rebase branch 2 times, most recently from 0b0fec4 to fd79eba Compare August 4, 2021 15:18
@imjasonh imjasonh mentioned this pull request Aug 12, 2021
Merged
@imjasonh
Implement annotation-based rebase hints
b27d11c 
`crane rebase` now inspects the original image for annotations to
identify its old base and new base images. If those are found, and
--old_base and --new_base flags aren't specified, those will be used.

If the --rebased flag is not passed, the rebased image will be the
tagged with the original image reference; if it was originally specified
by digest, the rebased image will be tagged with :rebased (instead of
stripping the digest and pushing to :latest)

`crane rebase` now prints the full pushed image reference, instead of
just the digest (aiding embedding in other bash pipelines), and adds
annotations to aid future rebasings.

This also adds a bash test that covers the rebase case for detected base
image information.
jonjohnsonjr
jonjohnsonjr reviewed Aug 12, 2021
View reviewed changes
cmd/crane/cmd/rebase.go Outdated Show resolved Hide resolved
cmd/crane/cmd/rebase.go Outdated Show resolved Hide resolved
cmd/crane/doc/crane_rebase.md Outdated Show resolved Hide resolved
cmd/crane/cmd/rebase.go Outdated Show resolved Hide resolved
cmd/crane/rebase_test.sh
rm ${tmp}/b.txt

# Rebase using annotations
rebased=$(./crane rebase ${orig})
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is amazing 👍

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. I still need to hook it up into CI tests...

pkg/v1/mutate/rebase.go Outdated Show resolved Hide resolved
@jonjohnsonjr
Copy link
Collaborator

jonjohnsonjr commented Aug 12, 2021 •
edited
Loading

This is WIP while we sort out whether the annotation will be a reserved annotation in the OCI spec. If the code looks good, I'll take a stab at updating rebase.md as well (it could probably use it anyway 😅 ).

Wanna do that? If not, ready for final review?

@imjasonh
Copy link
Collaborator Author

Wanna do that? If not, ready for final review?

Done, anything else you'd like to see?

@imjasonh imjasonh merged commit de8aff8 into google:main Aug 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jonjohnsonjr jonjohnsonjr jonjohnsonjr approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@imjasonh @codecov-io @codecov-commenter @jonjohnsonjr