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

Rebaser changes for run image extension #1032

Merged
merged 6 commits into from
Mar 28, 2023
Merged

Conversation

jabrown85
Copy link
Contributor

Open Questions

  1. Should the runImage not be present, should we be failing? The spec doesn't say it allows for the fallback to the old stack key..but it feels aggressive to me.
  2. The spec says specifically to fail is the value of rebaseable is false. I coded it as such, but anything other than false is treated as true. This may be unexpected.

Ref: #1000

Signed-off-by: Jesse Brown <jabrown85@gmail.com>
For platforms >= 0.12 - the rebaser will now fail if the io.buildpacks.rebasable label is set to false when the force flag is not set.

Signed-off-by: Jesse Brown <jabrown85@gmail.com>
For Platforms >= 0.12, the default rebase target has moved to the `runImage` key in the `io.buildpacks.lifecycle.metadata` label. This change allows the lifecycle to read the `runImage` key and use it as the default rebase target.

Signed-off-by: Jesse Brown <jabrown85@gmail.com>
@jabrown85 jabrown85 requested a review from a team as a code owner March 9, 2023 16:01
@jabrown85 jabrown85 self-assigned this Mar 9, 2023
Signed-off-by: Jesse Brown <jabrown85@gmail.com>
Signed-off-by: Jesse Brown <jabrown85@gmail.com>
@natalieparellano
Copy link
Member

Should the runImage not be present, should we be failing? The spec doesn't say it allows for the fallback to the old stack key..but it feels aggressive to me.

I think we should fall through to the stack key. Otherwise it would be hard to rebase old apps that were created before the platform was upgraded.

@jabrown85
Copy link
Contributor Author

Should the runImage not be present, should we be failing? The spec doesn't say it allows for the fallback to the old stack key..but it feels aggressive to me.

I think we should fall through to the stack key. Otherwise it would be hard to rebase old apps that were created before the platform was upgraded.

I agree. Do we need a spec change then? Rebase spec doesn't say this will happen.

Signed-off-by: Jesse Brown <jabrown85@gmail.com>
@natalieparellano
Copy link
Member

Do we need a spec change then? Rebase spec doesn't say this will happen.

Sounds good. I made a code suggestion here: https://github.com/buildpacks/spec/pull/335/files#r1142221664 on buildpacks/spec#335

@natalieparellano
Copy link
Member

@jabrown85 this one looks fully updated - should we merge it?

@jabrown85
Copy link
Contributor Author

I'm happy if you are @natalieparellano! No one has reviewed it officially so go right ahead 😄

Copy link
Member

@natalieparellano natalieparellano left a comment

Choose a reason for hiding this comment

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

Looks great!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[RFC #0105] Rebaser changes for run image extension (Dockerfiles phase 3)
2 participants