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

WIP Support nfs local volume mounts #2415

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions cmd/podman/volume_create.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,8 @@ var (
return volumeCreateCmd(&volumeCreateCommand)
},
Example: `podman volume create myvol
podman volume create
podman volume create --label foo=bar myvol`,
podman volume create --label foo=bar
podman volume create --opt type=nfs --opt o=addr=192.168.0.2,rw --opt device=:/nfsshare mynfsvol`,
Copy link
Member

Choose a reason for hiding this comment

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

I've been trying to keep these to three examples. Could we kill line 31 and add '--label foo=bar' to line 29?

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed

}
)

Expand Down
10 changes: 9 additions & 1 deletion docs/podman-volume-create.1.md
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,13 @@ Set metadata for a volume (e.g., --label mykey=value).

**-o**, **--opt**=[]

Set driver specific options.
Set driver specific options. To setup NFS volume you need to specify:

type: `-o type=nfs` To indicate the nfs mount.

o: `-o o=addr=nfsserver.example.com,rw` Options including the address of the nfs server.

device: `-o device=/nfsshare`, the remote nfs share.

## EXAMPLES

Expand All @@ -39,6 +45,8 @@ $ podman volume create myvol
$ podman volume create

$ podman volume create --label foo=bar myvol

# podman volume create --opt type=nfs --opt o=addr=192.168.0.2,rw --opt device=/nfsshare mynfsvol
```

## SEE ALSO
Expand Down
7 changes: 7 additions & 0 deletions libpod/runtime_ctr.go
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,13 @@ func (r *Runtime) newContainer(ctx context.Context, rSpec *spec.Spec, options ..
return nil, errors.Wrapf(err, "error creating named volume %q", vol.Name)
}

options := newVol.Options()
if len(options) > 0 {
if err := newVol.Mount(); err != nil {
return nil, err
}
}

if err := ctr.copyWithTarFromImage(vol.Dest, newVol.MountPoint()); err != nil && !os.IsNotExist(err) {
return nil, errors.Wrapf(err, "Failed to copy content into new volume mount %q", vol.Name)
}
Expand Down
62 changes: 62 additions & 0 deletions libpod/volume.go
Original file line number Diff line number Diff line change
@@ -1,5 +1,13 @@
package libpod

import (
"net"
"strings"

"github.com/containers/storage/pkg/mount"
"github.com/pkg/errors"
)

// Volume is the type used to create named volumes
// TODO: all volumes should be created using this and the Volume API
type Volume struct {
Expand Down Expand Up @@ -70,3 +78,57 @@ func (v *Volume) Scope() string {
func (v *Volume) IsCtrSpecific() bool {
return v.config.IsCtrSpecific
}

// Mount the volume
func (v *Volume) Mount() error {
if v.MountPoint() == "" {
return errors.Errorf("missing device in volume options")
}
mounted, err := mount.Mounted(v.MountPoint())
if err != nil {
return errors.Wrapf(err, "failed to determine if %v is mounted", v.Name())
}
if mounted {
return nil
}
options := v.Options()
if len(options) == 0 {
return errors.Errorf("volume %v is not mountable, no options available", v.Name())
}
mountOpts := options["o"]
device := options["device"]
if options["type"] == "nfs" {
Copy link
Member

Choose a reason for hiding this comment

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

This should not be an option. Volume has a "type" field, and that is what we should be using.

Copy link
Member

Choose a reason for hiding this comment

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

Correction, I meant Driver - I think I'd count NFS mounts as a type of volume driver...

Or we should add a type field. Let's not stuff critical information into maps. Make this top-level.

Copy link
Member Author

Choose a reason for hiding this comment

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

We can add this under the covers, but if we want to follow the Docker CLI, we have to allow users to specify nfs in the manner.

Copy link
Member

Choose a reason for hiding this comment

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

Sure, matching their CLI is fine, but let's not let the CLI determine how we set up internal data structures

if addrValue := getAddress(mountOpts); addrValue != "" && net.ParseIP(addrValue).To4() == nil {
ipAddr, err := net.ResolveIPAddr("ip", addrValue)
if err != nil {
return errors.Wrapf(err, "error resolving passed in nfs address")
}
mountOpts = strings.Replace(mountOpts, "addr="+addrValue, "addr="+ipAddr.String(), 1)
}
if device[0] != ':' {
device = ":" + device
}
}
err = mount.Mount(device, v.MountPoint(), options["type"], mountOpts)
return errors.Wrap(err, "failed to mount local volume")
}

// Unmount the volume from the system
func (v *Volume) Unmount() error {
if v.MountPoint() == "" {
return errors.Errorf("missing device in volume options")
}
return mount.Unmount(v.MountPoint())
}

// getAddress finds out address/hostname from options
func getAddress(opts string) string {
optsList := strings.Split(opts, ",")
for i := 0; i < len(optsList); i++ {
if strings.HasPrefix(optsList[i], "addr=") {
addr := strings.SplitN(optsList[i], "=", 2)[1]
return addr
}
}
return ""
}
Copy link
Member

Choose a reason for hiding this comment

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

Should this function live in common instead?

Copy link
Member Author

Choose a reason for hiding this comment

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

It is only used in volume now.

1 change: 1 addition & 0 deletions libpod/volume_internal.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,5 +18,6 @@ func newVolume(runtime *Runtime) (*Volume, error) {

// teardownStorage deletes the volume from volumePath
func (v *Volume) teardownStorage() error {
v.Unmount()
return os.RemoveAll(filepath.Join(v.runtime.config.VolumePath, v.Name()))
}