From aeabc45ccef10fc858677765675bd86944a93db6 Mon Sep 17 00:00:00 2001 From: Peter Hunt Date: Tue, 2 Jul 2019 11:40:14 -0400 Subject: Improve parsing of mounts Specifically, we were needlessly doing a double lookup to find which config mounts were user volumes. Improve this by refactoring a bit of code from inspect Signed-off-by: Peter Hunt --- libpod/container_inspect.go | 83 +++++++++++++++----------------------- libpod/container_internal.go | 31 +++++++++++++++ libpod/kube.go | 95 ++++++++++++++++++++++---------------------- 3 files changed, 112 insertions(+), 97 deletions(-) (limited to 'libpod') diff --git a/libpod/container_inspect.go b/libpod/container_inspect.go index 6085f1210..0a4d7016f 100644 --- a/libpod/container_inspect.go +++ b/libpod/container_inspect.go @@ -368,58 +368,41 @@ func (c *Container) getInspectMounts(ctrSpec *spec.Spec) ([]InspectMount, error) return inspectMounts, nil } - // We need to parse all named volumes and mounts into maps, so we don't - // end up with repeated lookups for each user volume. - // Map destination to struct, as destination is what is stored in - // UserVolumes. - namedVolumes := make(map[string]*ContainerNamedVolume) - mounts := make(map[string]spec.Mount) - for _, namedVol := range c.config.NamedVolumes { - namedVolumes[namedVol.Dest] = namedVol - } - for _, mount := range ctrSpec.Mounts { - mounts[mount.Destination] = mount - } + namedVolumes, mounts := c.sortUserVolumes(ctrSpec) + for _, volume := range namedVolumes { + mountStruct := InspectMount{} + mountStruct.Type = "volume" + mountStruct.Destination = volume.Dest + mountStruct.Name = volume.Name + + // For src and driver, we need to look up the named + // volume. + volFromDB, err := c.runtime.state.Volume(volume.Name) + if err != nil { + return nil, errors.Wrapf(err, "error looking up volume %s in container %s config", volume.Name, c.ID()) + } + mountStruct.Driver = volFromDB.Driver() + mountStruct.Source = volFromDB.MountPoint() + + parseMountOptionsForInspect(volume.Options, &mountStruct) - for _, vol := range c.config.UserVolumes { - // We need to look up the volumes. - // First: is it a named volume? - if volume, ok := namedVolumes[vol]; ok { - mountStruct := InspectMount{} - mountStruct.Type = "volume" - mountStruct.Destination = volume.Dest - mountStruct.Name = volume.Name - - // For src and driver, we need to look up the named - // volume. - volFromDB, err := c.runtime.state.Volume(volume.Name) - if err != nil { - return nil, errors.Wrapf(err, "error looking up volume %s in container %s config", volume.Name, c.ID()) - } - mountStruct.Driver = volFromDB.Driver() - mountStruct.Source = volFromDB.MountPoint() - - parseMountOptionsForInspect(volume.Options, &mountStruct) - - inspectMounts = append(inspectMounts, mountStruct) - } else if mount, ok := mounts[vol]; ok { - // It's a mount. - // Is it a tmpfs? If so, discard. - if mount.Type == "tmpfs" { - continue - } - - mountStruct := InspectMount{} - mountStruct.Type = "bind" - mountStruct.Source = mount.Source - mountStruct.Destination = mount.Destination - - parseMountOptionsForInspect(mount.Options, &mountStruct) - - inspectMounts = append(inspectMounts, mountStruct) + inspectMounts = append(inspectMounts, mountStruct) + } + for _, mount := range mounts { + // It's a mount. + // Is it a tmpfs? If so, discard. + if mount.Type == "tmpfs" { + continue } - // We couldn't find a mount. Log a warning. - logrus.Warnf("Could not find mount at destination %q when building inspect output for container %s", vol, c.ID()) + + mountStruct := InspectMount{} + mountStruct.Type = "bind" + mountStruct.Source = mount.Source + mountStruct.Destination = mount.Destination + + parseMountOptionsForInspect(mount.Options, &mountStruct) + + inspectMounts = append(inspectMounts, mountStruct) } return inspectMounts, nil diff --git a/libpod/container_internal.go b/libpod/container_internal.go index 43d2b6e61..884f5a0a9 100644 --- a/libpod/container_internal.go +++ b/libpod/container_internal.go @@ -1534,3 +1534,34 @@ func (c *Container) prepareCheckpointExport() (err error) { return nil } + +// sortUserVolumes sorts the volumes specified for a container +// between named and normal volumes +func (c *Container) sortUserVolumes(ctrSpec *spec.Spec) ([]*ContainerNamedVolume, []spec.Mount) { + namedUserVolumes := []*ContainerNamedVolume{} + userMounts := []spec.Mount{} + + // We need to parse all named volumes and mounts into maps, so we don't + // end up with repeated lookups for each user volume. + // Map destination to struct, as destination is what is stored in + // UserVolumes. + namedVolumes := make(map[string]*ContainerNamedVolume) + mounts := make(map[string]spec.Mount) + for _, namedVol := range c.config.NamedVolumes { + namedVolumes[namedVol.Dest] = namedVol + } + for _, mount := range ctrSpec.Mounts { + mounts[mount.Destination] = mount + } + + for _, vol := range c.config.UserVolumes { + if volume, ok := namedVolumes[vol]; ok { + namedUserVolumes = append(namedUserVolumes, volume) + } else if mount, ok := mounts[vol]; ok { + userMounts = append(userMounts, mount) + } else { + logrus.Warnf("Could not find mount at destination %q when parsing user volumes for container %s", vol, c.ID()) + } + } + return namedUserVolumes, userMounts +} diff --git a/libpod/kube.go b/libpod/kube.go index 6a52ff80b..08ab7a4a4 100644 --- a/libpod/kube.go +++ b/libpod/kube.go @@ -340,40 +340,58 @@ func libpodMaxAndMinToResourceList(c *Container) (v1.ResourceList, v1.ResourceLi return maxResources, minResources } -func generateKubeVolumeMount(hostSourcePath string, mounts []specs.Mount) (v1.VolumeMount, v1.Volume, error) { +// libpodMountsToKubeVolumeMounts converts the containers mounts to a struct kube understands +func libpodMountsToKubeVolumeMounts(c *Container) ([]v1.VolumeMount, []v1.Volume, error) { + var vms []v1.VolumeMount + var vos []v1.Volume + + // TjDO when named volumes are supported in play kube, also parse named volumes here + _, mounts := c.sortUserVolumes(c.config.Spec) + for _, m := range mounts { + vm, vo, err := generateKubeVolumeMount(m) + if err != nil { + return vms, vos, err + } + vms = append(vms, vm) + vos = append(vos, vo) + } + return vms, vos, nil +} + +// generateKubeVolumeMount takes a user specfied mount and returns +// a kubernetes VolumeMount (to be added to the container) and a kubernetes Volume +// (to be added to the pod) +func generateKubeVolumeMount(m specs.Mount) (v1.VolumeMount, v1.Volume, error) { vm := v1.VolumeMount{} vo := v1.Volume{} - for _, m := range mounts { - if m.Destination == hostSourcePath { - name, err := convertVolumePathToName(m.Source) - if err != nil { - return vm, vo, err - } - vm.Name = name - vm.MountPath = m.Destination - if util.StringInSlice("ro", m.Options) { - vm.ReadOnly = true - } - vo.Name = name - vo.HostPath = &v1.HostPathVolumeSource{} - vo.HostPath.Path = m.Source - isDir, err := isHostPathDirectory(m.Source) - // neither a directory or a file lives here, default to creating a directory - // TODO should this be an error instead? - var hostPathType v1.HostPathType - if err != nil { - hostPathType = v1.HostPathDirectoryOrCreate - } else if isDir { - hostPathType = v1.HostPathDirectory - } else { - hostPathType = v1.HostPathFile - } - vo.HostPath.Type = &hostPathType - return vm, vo, nil - } + name, err := convertVolumePathToName(m.Source) + if err != nil { + return vm, vo, err + } + vm.Name = name + vm.MountPath = m.Destination + if util.StringInSlice("ro", m.Options) { + vm.ReadOnly = true + } + + vo.Name = name + vo.HostPath = &v1.HostPathVolumeSource{} + vo.HostPath.Path = m.Source + isDir, err := isHostPathDirectory(m.Source) + // neither a directory or a file lives here, default to creating a directory + // TODO should this be an error instead? + var hostPathType v1.HostPathType + if err != nil { + hostPathType = v1.HostPathDirectoryOrCreate + } else if isDir { + hostPathType = v1.HostPathDirectory + } else { + hostPathType = v1.HostPathFile } - return vm, vo, errors.New("unable to find mount source") + vo.HostPath.Type = &hostPathType + + return vm, vo, nil } func isHostPathDirectory(hostPathSource string) (bool, error) { @@ -400,23 +418,6 @@ func convertVolumePathToName(hostSourcePath string) (string, error) { return strings.Replace(strings.Trim(hostSourcePath, "/"), "/", "-", -1), nil } -// libpodMountsToKubeVolumeMounts converts the containers mounts to a struct kube understands -func libpodMountsToKubeVolumeMounts(c *Container) ([]v1.VolumeMount, []v1.Volume, error) { - // At this point, I dont think we can distinguish between the default - // volume mounts and user added ones. For now, we pass them all. - var vms []v1.VolumeMount - var vos []v1.Volume - for _, hostSourcePath := range c.config.UserVolumes { - vm, vo, err := generateKubeVolumeMount(hostSourcePath, c.config.Spec.Mounts) - if err != nil { - return vms, vos, err - } - vms = append(vms, vm) - vos = append(vos, vo) - } - return vms, vos, nil -} - func determineCapAddDropFromCapabilities(defaultCaps, containerCaps []string) *v1.Capabilities { var ( drop []v1.Capability -- cgit v1.2.3-54-g00ecf