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

Add podman image mount #6851

Merged
merged 1 commit into from
Jul 29, 2020
Merged

Add podman image mount #6851

merged 1 commit into from
Jul 29, 2020

Conversation

rhatdan
Copy link
Member

@rhatdan rhatdan commented Jul 3, 2020 •
edited
Loading

There are many use cases where you want to just mount an image
without creating a container on it. For example you might want
to just examine the content in an image after you pull it for
security analysys. Or you might want to just use the executables
on the image without running it in a container.

The image is mounted readonly since we do not want people changing
images.

Signed-off-by: Daniel J Walsh [email protected]

@openshift-ci-robot openshift-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 3, 2020
@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 3, 2020
@rhatdan
Copy link
Member Author

rhatdan commented Jul 3, 2020

@mrunalp PTAL

@rhatdan rhatdan mentioned this pull request Jul 3, 2020
Merged
@openshift-ci-robot openshift-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 6, 2020
@openshift-ci-robot openshift-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 9, 2020
@rhatdan rhatdan force-pushed the mount branch 3 times, most recently from d34f6dd to f5ec590 Compare July 10, 2020 10:58
@rhatdan rhatdan mentioned this pull request Jul 10, 2020
Closed
@rhatdan
Copy link
Member Author

rhatdan commented Jul 10, 2020

Fixes: #1433

@rhatdan rhatdan force-pushed the mount branch 2 times, most recently from 1a0df3b to b9f9864 Compare July 13, 2020 19:45
kolyshkin
kolyshkin reviewed Jul 13, 2020
View reviewed changes
completions/bash/podman Outdated Show resolved Hide resolved
kolyshkin
kolyshkin reviewed Jul 13, 2020
View reviewed changes
completions/bash/podman Outdated Show resolved Hide resolved
kolyshkin
kolyshkin reviewed Jul 13, 2020
View reviewed changes
test/e2e/mount_test.go Outdated Show resolved Hide resolved
kolyshkin
kolyshkin reviewed Jul 13, 2020
View reviewed changes
test/e2e/mount_test.go Outdated Show resolved Hide resolved
kolyshkin
kolyshkin reviewed Jul 13, 2020
View reviewed changes
test/e2e/mount_test.go Outdated Show resolved Hide resolved
@rhatdan rhatdan changed the title [wip] add podman image mount Add podman image mount Jul 21, 2020
@openshift-ci-robot openshift-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 21, 2020
vrothberg
vrothberg reviewed Jul 22, 2020
View reviewed changes
Copy link
Member

@vrothberg vrothberg left a comment

Choose a reason for hiding this comment

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

Nice work! Just a few comments from the code review. Will do some testing on my machine now.

I wonder if the error logic should be similar to rmi where we continue removing images even if there was an error removing one. I imagine it could be useful.

Maybe an --ignore flag for unmount could be helpful as well to ignore errors if the image and/or the mount don't exist anymore.

cmd/podman/images/mount.go Outdated Show resolved Hide resolved
cmd/podman/images/mount.go Outdated Show resolved Hide resolved
cmd/podman/images/mount.go Show resolved Hide resolved
docs/source/markdown/podman-image-mount.1.md Outdated Show resolved Hide resolved
pkg/domain/entities/images.go Outdated Show resolved Hide resolved
pkg/domain/entities/images.go Show resolved Hide resolved
pkg/domain/infra/tunnel/images.go Outdated Show resolved Hide resolved
pkg/domain/infra/tunnel/images.go Outdated Show resolved Hide resolved
pkg/domain/infra/abi/images.go Show resolved Hide resolved
pkg/domain/infra/abi/images.go Outdated Show resolved Hide resolved
@vrothberg
Copy link
Member

Tests fail locally as well:

libpod (mount) $ sudo ./bin/podman container umount 910
Error: unrecognized command `podman container umount`

giuseppe
giuseppe reviewed Jul 23, 2020
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.

rootless changes LGTM

pkg/domain/infra/abi/images.go Show resolved Hide resolved
@rhatdan rhatdan force-pushed the mount branch 2 times, most recently from cf85df3 to 81773ef Compare July 23, 2020 16:30
@rhatdan
Copy link
Member Author

rhatdan commented Jul 23, 2020

@vrothberg @giuseppe @mheon @TomSweeneyRedHat @ashley-cui @QiWang19 @baude I think this is ready to go and has passed the tests, now waiting for remote, which does not use this code.

@TomSweeneyRedHat
Copy link
Member

@rhatdan looks like you're on the happy test hunt still unless this is happening everywhere.

@vrothberg
Copy link
Member

I restarted to see if those were flakes.

@rhatdan rhatdan force-pushed the mount branch 4 times, most recently from fb803a1 to 97165b6 Compare July 28, 2020 12:26
@rhatdan
Add podman image mount
6979d14 
There are many use cases where you want to just mount an image
without creating a container on it. For example you might want
to just examine the content in an image after you pull it for
security analysys.  Or you might want to just use the executables
on the image without running it in a container.

The image is mounted readonly since we do not want people changing
images.

Signed-off-by: Daniel J Walsh <[email protected]>
vrothberg
vrothberg reviewed Jul 29, 2020
View reviewed changes
Copy link
Member

@vrothberg vrothberg left a comment

Choose a reason for hiding this comment

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

LGTM

Let's get this in. @giuseppe PTAL

giuseppe
giuseppe approved these changes Jul 29, 2020
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

@openshift-ci-robot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: 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:

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

@giuseppe
Copy link
Member

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Jul 29, 2020
@openshift-merge-robot openshift-merge-robot merged commit 7f0c094 into containers:master Jul 29, 2020
@edsantiago edsantiago mentioned this pull request Aug 27, 2020
Closed
@tao12345666333 tao12345666333 mentioned this pull request Sep 29, 2020
Open
@github-actions github-actions bot added the locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. label Sep 24, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 24, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@vrothberg vrothberg vrothberg left review comments

@kolyshkin kolyshkin kolyshkin left review comments

@giuseppe giuseppe giuseppe approved these changes

@mrunalp mrunalp Awaiting requested review from mrunalp

@TomSweeneyRedHat TomSweeneyRedHat Awaiting requested review from TomSweeneyRedHat

Assignees

@giuseppe giuseppe

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@rhatdan @vrothberg @TomSweeneyRedHat @openshift-ci-robot @giuseppe @kolyshkin @openshift-merge-robot