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 support for 'image' volume driver #15841

Merged
merged 1 commit into from
Sep 23, 2022
Merged

Add support for 'image' volume driver #15841

merged 1 commit into from
Sep 23, 2022

Conversation

mheon
Copy link
Member

@mheon mheon commented Sep 16, 2022

We added the concept of image volumes in 2.2.0, to support inspecting an image from within a container. However, this is a strictly read-only mount, with no modification allowed.

By contrast, the new image volume driver creates a c/storage container as its underlying storage, so we have a read/write layer. This, in and of itself, is not especially interesting, but what it will enable in the future is. If we add a new command to allow these image volumes to be committed, we can now distribute volumes - and changes to them - via a standard OCI image registry (which is rather new and quite exciting).

Future work in this area:

  • Add support for podman volume push (commit volume changes and push resulting image to OCI registry).
  • Add support for podman volume pull (currently, we require that the image a volume is created from be already pulled; it would be simpler if we had a dedicated command that did the pull and made a volume from it)
  • Add support for scratch images (make an empty image on demand to use as the base of the volume)
  • Add UOR support to podman volume push and podman volume pull to enable both with non-image volume drivers
Added support for a new volume driver, `image`, which allows volumes to be created that are backed by images.

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Sep 16, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: mheon

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

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 16, 2022
@mheon
Copy link
Member Author

mheon commented Sep 16, 2022

Still needs manpage updates and tests.

@vrothberg Thanks for the inspiration here, and PTAL when you get a chance

@rhatdan
Copy link
Member

rhatdan commented Sep 17, 2022

So this is going to create a layered volume, just not necessarily an executable image? Interesting idea.
Would you have a podman volume commit with a tag?

@mheon
Copy link
Member Author

mheon commented Sep 19, 2022

Probably? Still need to work out the UI bits of how volume push and volume pull ought to work. Fortunately we have meetings to discuss it at.

@mheon mheon force-pushed the image_driver branch 13 times, most recently from 8bc4bc1 to 66a6b74 Compare September 21, 2022 15:10
@mheon
Copy link
Member Author

mheon commented Sep 21, 2022

Manpages updated, test added. This should be ready. @containers/podman-maintainers PTAL

Luap99
Luap99 reviewed Sep 21, 2022
View reviewed changes
Copy link
Member

@Luap99 Luap99 left a comment

Choose a reason for hiding this comment

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

I see one big problem here, this is not backwards compatible. What if I called my volume driver image in containers.conf. I think this needs to be rewritten so this can still work if this name is used in containers.conf

@mheon
Copy link
Member Author

mheon commented Sep 21, 2022

So prefer a volume plugin named image over the volume driver named image?

@Luap99
Copy link
Member

Luap99 commented Sep 21, 2022

So prefer a volume plugin named image over the volume driver named image?

Yes just so we are fully backwards compatible, I mean we store the driver name in the DB so once you update your stuff will break.

@mheon
Copy link
Member Author

mheon commented Sep 21, 2022

@Luap99 Done

@Luap99
Copy link
Member

Luap99 commented Sep 21, 2022

You have a build issue but could you also a test test for this corner case, create volume, run container, remove volume.

@mheon mheon force-pushed the image_driver branch 2 times, most recently from b0b1aa2 to 5860030 Compare September 21, 2022 18:23
@mheon mheon force-pushed the image_driver branch 7 times, most recently from ffe6433 to f541eee Compare September 22, 2022 14:38
Luap99
Luap99 reviewed Sep 22, 2022
View reviewed changes
Copy link
Member

@Luap99 Luap99 left a comment

Choose a reason for hiding this comment

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

LGTM

@mheon
Copy link
Member Author

mheon commented Sep 22, 2022

Cirrus appears to be very angry.

@mheon
Copy link
Member Author

mheon commented Sep 22, 2022

Rebased and force-pushed in the hope that CI will be less angry

@mheon
Add support for 'image' volume driver
fc6dcd1 
We added the concept of image volumes in 2.2.0, to support
inspecting an image from within a container. However, this is a
strictly read-only mount, with no modification allowed.

By contrast, the new `image` volume driver creates a c/storage
container as its underlying storage, so we have a read/write
layer. This, in and of itself, is not especially interesting, but
what it will enable in the future is. If we add a new command to
allow these image volumes to be committed, we can now distribute
volumes - and changes to them - via a standard OCI image registry
(which is rather new and quite exciting).

Future work in this area:
- Add support for `podman volume push` (commit volume changes and
  push resulting image to OCI registry).
- Add support for `podman volume pull` (currently, we require
  that the image a volume is created from be already pulled; it
  would be simpler if we had a dedicated command that did the
  pull and made a volume from it)
- Add support for scratch images (make an empty image on demand
  to use as the base of the volume)
- Add UOR support to `podman volume push` and
  `podman volume pull` to enable both with non-image volume
  drivers

Signed-off-by: Matthew Heon <[email protected]>
@mheon
Copy link
Member Author

mheon commented Sep 22, 2022

CI seems to be passing now

@mheon
Copy link
Member Author

mheon commented Sep 22, 2022

This is ready.

rhatdan
rhatdan reviewed Sep 23, 2022
View reviewed changes
docs/source/markdown/podman-volume-create.1.md
@@ -17,7 +17,13 @@ driver options can be set using the **--opt** flag.

#### **--driver**=*driver*

Specify the volume driver name (default **local**). Setting this to a value other than **local** Podman attempts to create the volume using a volume plugin with the given name. Such plugins must be defined in the **volume_plugins** section of the **[containers.conf(5)](https://github.com/containers/common/blob/main/docs/containers.conf.5.md)** configuration file.
Specify the volume driver name (default **local**).
There are two drivers supported by Podman itself: **local** and **image**.
Copy link
Member

Choose a reason for hiding this comment

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

Do we support volume plugins?

@rhatdan
Copy link
Member

rhatdan commented Sep 23, 2022

/lgtm
Do I see these images with podman images command?

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Sep 23, 2022
@openshift-merge-robot openshift-merge-robot merged commit a80c406 into containers:main Sep 23, 2022
@edsantiago edsantiago mentioned this pull request Sep 26, 2022
Closed
@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 20, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 20, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@Luap99 Luap99 Luap99 left review comments

@rhatdan rhatdan rhatdan left review comments

Assignees

@rhatdan rhatdan

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. release-note
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@mheon @rhatdan @Luap99 @openshift-merge-robot