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.

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

New rebase option to "retag" rather than replacing the target image #1748

Closed
1 task done
daniv-msft opened this issue May 5, 2023 · 10 comments · Fixed by #2023
Closed
1 task done

New rebase option to "retag" rather than replacing the target image #1748

daniv-msft opened this issue May 5, 2023 · 10 comments · Fixed by #2023
Assignees
Labels
help wanted Need some extra hands to get this done. size/md Medium level of effort status/ready Issue ready to be worked on. type/enhancement Issue that requests a new feature or improvement.
Milestone

Comments

@daniv-msft
Copy link

Description

When you use pack rebase to patch an image, you don't always want to replace it directly in your registry.
It would be great to have an optional parameter on the rebase command to retag to something else when rebasing. As of today we need to use Docker directly to clone the target image before rebasing, which isn't great as if anything bad happens during the rebase we'll have a duplicated image in the target registry.

Proposed solution

Have a new optional --tag parameter on the rebase command that allows you to define the new tag to use when rebasing. Alternatively, maybe we can reuse the --publish command so that if a full image name with tag is used this value is used?

Describe alternatives you've considered

Using Docker to perform these operations (not great, cf. above).

Additional context

  • This feature should be documented somewhere
@daniv-msft daniv-msft added status/triage Issue or PR that requires contributor attention. type/enhancement Issue that requests a new feature or improvement. labels May 5, 2023
@dfreilich
Copy link
Member

Good point! I think we had previously discussed it, and decided against doing it at the moment, but it makes sense to want to tag it as a different tag, in order to test it out first. I wouldn't want to override the publish flag to use it for adding a new tag, because then it would be a change in UX from the rest of the contexts where we have that flag.

@dfreilich dfreilich added help wanted Need some extra hands to get this done. size/md Medium level of effort and removed status/triage Issue or PR that requires contributor attention. labels May 8, 2023
@dfreilich
Copy link
Member

One open question - In pack build, that -t/--tag flag is used to specify additional tags, in addition to the output image name. Here, would we want -t/--tag to specify additional tags, or the only tags released by the command?

Looking into the technical details, the core of the rebase operation is done by the lifecycle here, and having a different output tag for the rebase operation is sanctioned according to the spec.

Given that, we'd welcome a PR, once we have a resolution about the UX of the tag flag! It should be as simple as:

  1. Adding a Tags []string array to the pkg/client/RebaseOptions
  2. Add a -t/--tag flag in the internal/commands/rebase.go definition (changing it to StringVarP, so that it can have a -t shortform)
  3. Pass that tag/tags, if present, to the lifecycle in pkg/client/rebase.go
  4. Ensure previous tests pass, and add new unit/integration tests (no need for acceptance tests)

@natalieparellano
Copy link
Member

@joeybrown-sf
Copy link

I think this is a reasonable request. In lifecycle, the -previous-image flag can be used to accomplish this.

For instance, in the following example we want to rebase the image foobar:22 but do not want to overwrite the image. Instead, we want to write it to a new tag, foobar:23.

The command to accomplish this is as follows:

CNB_PLATFORM_API=0.11 lifecycle rebase -previous-image localhost:5003/test/tester/foobar:22 localhost:5003/test/tester/foobar:23

I would probably recommend the same flag name for consistency. Pack build also has the previous-image flag.

@natalieparellano natalieparellano added status/ready Issue ready to be worked on. help wanted Need some extra hands to get this done. and removed help wanted Need some extra hands to get this done. labels Aug 18, 2023
@Parthiba-Hazra
Copy link
Contributor

Hi @natalieparellano @dfreilich
if this issue is not been worked yet, can I go for this?

@natalieparellano
Copy link
Member

@Parthiba-Hazra that would be great! I will assign you

@Parthiba-Hazra
Copy link
Contributor

@natalieparellano is there any thing you want share with me before diving on this enhancement, maybe something that can help to get with started? anyway I am going for this.

@natalieparellano
Copy link
Member

@Parthiba-Hazra apologies for being slow to respond here - I'll discuss with @jjbustamante and one of us should get back early next week

@Parthiba-Hazra
Copy link
Contributor

@natalieparellano yes sure, I opened a pr on this one, it does the result. I will need some review on that.

@jjbustamante
Copy link
Member

Thank you @Parthiba-Hazra

I will try to take a look on this and review it!

@jjbustamante jjbustamante added this to the 0.34.0 milestone Feb 5, 2024
@jjbustamante jjbustamante modified the milestones: 0.34.0, 0.35.0 Mar 8, 2024
@jjbustamante jjbustamante modified the milestones: 0.35.0, 0.34.0 Mar 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Need some extra hands to get this done. size/md Medium level of effort status/ready Issue ready to be worked on. type/enhancement Issue that requests a new feature or improvement.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants