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

imagebuildah,multi-stage: do not remove base-image when built without --layers #5081

Merged
merged 1 commit into from
Oct 12, 2023
Merged

imagebuildah,multi-stage: do not remove base-image when built without --layers #5081

merged 1 commit into from
Oct 12, 2023

Conversation

flouthoc
Copy link
Collaborator

@flouthoc flouthoc commented Oct 9, 2023

When building a multi-stage image ( without --layers ) and a stage contains only a base-image buildah removes the base-image itself as part of cleanup process. This is a bug and following commit fixes that.

Reproducer

FROM parent
FROM another-base
COPY --from=0 somefile .

buildah build -t multi-stage .

Closes: containers/podman#20291

What type of PR is this?

/kind api-change
/kind bug
/kind cleanup
/kind deprecation
/kind design
/kind documentation
/kind failing-test
/kind feature
/kind flake
/kind other

What this PR does / why we need it:

How to verify it

Which issue(s) this PR fixes:

Special notes for your reviewer:

Does this PR introduce a user-facing change?

imagebuildah,multi-stage: do not remove base-image when build without `--layers`

@openshift-ci openshift-ci bot added the approved label Oct 9, 2023
@flouthoc flouthoc mentioned this pull request Oct 9, 2023
Closed
@flouthoc flouthoc changed the title imagebuildah,multi-stage: do not remove base-image when build without --layers imagebuildah,multi-stage: do not remove base-image when built without --layers Oct 9, 2023
@packit-as-a-service
Copy link

Ephemeral COPR build failed. @containers/packit-build please check.

@flouthoc
Copy link
Collaborator Author

flouthoc commented Oct 9, 2023

@containers/buildah-maintainers PTAL,

@lsm5 it seems rpm-build also blocks merge bot.

@flouthoc
imagebuildah,multi-stage: do not remove base images
07d955f 
When building a multi-stage image ( without `--layers` ) and a
stage contains only a base-image buildah removes the base-image
itself as part of cleanup process. This is a bug and following commit
fixes that.

Reproducer

```Dockerfile
FROM parent
FROM another-base
COPY --from=0 somefile .
```

`buildah build -t multi-stage .`

Closes: containers/podman#20291

Signed-off-by: Aditya R <[email protected]>
@lsm5
Copy link
Member

lsm5 commented Oct 9, 2023

@containers/buildah-maintainers PTAL,

@lsm5 it seems rpm-build also blocks merge bot.

I'm checking on the ELN issue, but right now this PR doesn't yet have the lgtm label, so I'm not sure if it's actually blocked on that. Do you see mergebot blocked on ELN failures in other PRs with lgtm label already?

@rhatdan
Copy link
Member

rhatdan commented Oct 9, 2023

LGTM

@rhatdan
Copy link
Member

rhatdan commented Oct 11, 2023

@lsm5 @nalind @giuseppe @umohnani8 @vrothberg PTAL

giuseppe
giuseppe approved these changes Oct 12, 2023
View reviewed changes
Copy link
Member

@giuseppe giuseppe left a comment

Choose a reason for hiding this comment

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

LGTM

@rhatdan
Copy link
Member

rhatdan commented Oct 12, 2023

/approve
/lgtm

@openshift-ci openshift-ci bot added the lgtm label Oct 12, 2023
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Oct 12, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: flouthoc, giuseppe, rhatdan

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:
  • OWNERS [flouthoc,giuseppe,rhatdan]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@flouthoc
Copy link
Collaborator Author

@rhatdan I don't think this will be merged, since rpm-build is failing so merge bot is blocked, I had merge some previous PR's manually. cc @lsm5

@lsm5
Copy link
Member

lsm5 commented Oct 12, 2023

ok, this time i'm seeing the Not mergeable. Job rpm-build:fedora-eln-aarch64 has not succeeded. Let me check again in repo settings.

@lsm5
Copy link
Member

lsm5 commented Oct 12, 2023

ok, this time i'm seeing the Not mergeable. Job rpm-build:fedora-eln-aarch64 has not succeeded. Let me check again in repo settings.

ok, in Settings -> Branches, for the main repo Success task, I changed the value from Any source to Cirrus CI. I think going forward, jobs shouldn't be blocked on rpm build. But this one might need a manual merge.

@rhatdan rhatdan merged commit a059244 into containers:main Oct 12, 2023
33 of 35 checks passed
@nalind
Copy link
Member

nalind commented Oct 12, 2023

Tide can be told to ignore specific status checks with its "override" command.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jan 11, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@giuseppe giuseppe giuseppe approved these changes

Assignees

@rhatdan rhatdan

Labels
approved lgtm locked - please file new issue/PR
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Podman build multi-stage with squash should not remove intermediate base image from local cache
5 participants
@flouthoc @lsm5 @rhatdan @nalind @giuseppe