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

Add an option to retag rather than replacing the target image while rebasing #2023

Merged
merged 11 commits into from
Mar 8, 2024
1 change: 1 addition & 0 deletions internal/commands/rebase.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ func Rebase(logger logging.Logger, cfg config.Config, pack PackClient) *cobra.Co
cmd.Flags().BoolVar(&opts.Publish, "publish", false, "Publish the rebased application image directly to the container registry specified in <image-name>, instead of the daemon. The previous application image must also reside in the registry.")
cmd.Flags().StringVar(&opts.RunImage, "run-image", "", "Run image to use for rebasing")
cmd.Flags().StringVar(&policy, "pull-policy", "", "Pull policy to use. Accepted values are always, never, and if-not-present. The default is always")
cmd.Flags().StringVar(&opts.PreviousImage, "previous-image", "", "Image to rebase. Set to a particular tag reference, digest reference, or (when performing a daemon build) image ID. Use this flag in combination with <image-name> to avoid replacing the original image.")
cmd.Flags().StringVar(&opts.ReportDestinationDir, "report-output-dir", "", "Path to export build report.toml.\nOmitting the flag yield no report file.")
cmd.Flags().BoolVar(&opts.Force, "force", false, "Perform rebase operation without target validation (only available for API >= 0.12)")

Expand Down
38 changes: 38 additions & 0 deletions internal/commands/rebase_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,44 @@ func testRebaseCommand(t *testing.T, when spec.G, it spec.S) {
})
})
})
when("image name and previous image are provided", func() {
var expectedOpts client.RebaseOptions

it.Before(func() {
runImage := "test/image"
testMirror1 := "example.com/some/run1"
testMirror2 := "example.com/some/run2"

cfg.RunImages = []config.RunImage{{
Image: runImage,
Mirrors: []string{testMirror1, testMirror2},
}}
command = commands.Rebase(logger, cfg, mockClient)

repoName = "test/repo-image"
previousImage := "example.com/previous-image:tag" // Example of previous image with tag
opts := client.RebaseOptions{
RepoName: repoName,
Publish: false,
PullPolicy: image.PullAlways,
RunImage: "",
AdditionalMirrors: map[string][]string{
runImage: {testMirror1, testMirror2},
},
PreviousImage: previousImage,
}
expectedOpts = opts
})

it("works", func() {
mockClient.EXPECT().
Rebase(gomock.Any(), gomock.Eq(expectedOpts)).
Return(nil)

command.SetArgs([]string{repoName, "--previous-image", "example.com/previous-image:tag"})
h.AssertNil(t, command.Execute())
})
})
})
})
}
13 changes: 11 additions & 2 deletions pkg/client/rebase.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,9 @@ type RebaseOptions struct {
// Pass-through force flag to lifecycle rebase command to skip target data
// validated (will not have any effect if API < 0.12).
Force bool

// Image reference to use as the previous image for rebase.
PreviousImage string
}

// Rebase updates the run image layers in an app image.
Expand All @@ -56,7 +59,13 @@ func (c *Client) Rebase(ctx context.Context, opts RebaseOptions) error {
return errors.Wrapf(err, "invalid image name '%s'", opts.RepoName)
}

appImage, err := c.imageFetcher.Fetch(ctx, opts.RepoName, image.FetchOptions{Daemon: !opts.Publish, PullPolicy: opts.PullPolicy})
repoName := opts.RepoName

if opts.PreviousImage != "" {
repoName = opts.PreviousImage
}

appImage, err := c.imageFetcher.Fetch(ctx, repoName, image.FetchOptions{Daemon: !opts.Publish, PullPolicy: opts.PullPolicy})
Copy link
Member

Choose a reason for hiding this comment

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

@Parthiba-Hazra

Could you add some test coverage for this method?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes sure, let me check.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jjbustamante can you check if I am missing some other scenarios.

if err != nil {
return err
}
Expand Down Expand Up @@ -114,7 +123,7 @@ func (c *Client) Rebase(ctx context.Context, opts RebaseOptions) error {

c.logger.Infof("Rebasing %s on run image %s", style.Symbol(appImage.Name()), style.Symbol(baseImage.Name()))
rebaser := &phase.Rebaser{Logger: c.logger, PlatformAPI: build.SupportedPlatformAPIVersions.Latest(), Force: opts.Force}
report, err := rebaser.Rebase(appImage, baseImage, appImage.Name(), nil)
report, err := rebaser.Rebase(appImage, baseImage, opts.RepoName, nil)
if err != nil {
return err
}
Expand Down
32 changes: 32 additions & 0 deletions pkg/client/rebase_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -267,6 +267,38 @@ func testRebase(t *testing.T, when spec.G, it spec.S) {
})
})
})
when("previous image is provided", func() {
it("fetches the image using the previous image name", func() {
h.AssertNil(t, subject.Rebase(context.TODO(), RebaseOptions{
RepoName: "new/app",
PreviousImage: "some/app",
}))
args := fakeImageFetcher.FetchCalls["some/app"]
h.AssertNotNil(t, args)
h.AssertEq(t, args.Daemon, true)
})
})

when("previous image is set to new image name", func() {
it("returns error if Fetch function fails", func() {
err := subject.Rebase(context.TODO(), RebaseOptions{
RepoName: "some/app",
PreviousImage: "new/app",
})
h.AssertError(t, err, "image 'new/app' does not exist on the daemon: not found")
})
})

when("previous image is not provided", func() {
it("fetches the image using the repo name", func() {
h.AssertNil(t, subject.Rebase(context.TODO(), RebaseOptions{
RepoName: "some/app",
}))
args := fakeImageFetcher.FetchCalls["some/app"]
h.AssertNotNil(t, args)
h.AssertEq(t, args.Daemon, true)
})
})
})
})
}
Expand Down
Loading