Handling Volume at early stage

It will resolve #544 as well as refactor volume handling part.

pkg/kobject/kobject.go

@@ -61,14 +61,15 @@ type ConvertOptions struct {
 // ServiceConfig holds the basic struct of a container
 type ServiceConfig struct {
 	// use tags to mark from what element this value comes
-	ContainerName   string
-	Image           string              `compose:"image" bundle:"Image"`
-	Environment     []EnvVar            `compose:"environment" bundle:"Env"`
-	Port            []Ports             `compose:"ports" bundle:"Ports"`
-	Command         []string            `compose:"command" bundle:"Command"`
-	WorkingDir      string              `compose:"" bundle:"WorkingDir"`
-	Args            []string            `compose:"args" bundle:"Args"`
-	Volumes         []string            `compose:"volumes" bundle:"Volumes"`
+	ContainerName string
+	Image         string   `compose:"image" bundle:"Image"`
+	Environment   []EnvVar `compose:"environment" bundle:"Env"`
+	Port          []Ports  `compose:"ports" bundle:"Ports"`
+	Command       []string `compose:"command" bundle:"Command"`
+	WorkingDir    string   `compose:"" bundle:"WorkingDir"`
+	Args          []string `compose:"args" bundle:"Args"`
+	// VolList is list of volumes extracted from docker-compose file
+	VolList         []string            `compose:"volumes" bundle:"Volumes"`
 	Network         []string            `compose:"network" bundle:"Networks"`
 	Labels          map[string]string   `compose:"labels" bundle:"Labels"`
 	Annotations     map[string]string   `compose:"" bundle:""`
@@ -94,6 +95,8 @@ type ServiceConfig struct {
 	TmpFs           []string            `compose:"tmpfs" bundle:""`
 	Dockerfile      string              `compose:"dockerfile" bundle:""`
 	Replicas        int                 `compose:"replicas" bundle:""`
+	// Volumes is a struct which contains all information about each volume
+	Volumes []Volumes `compose:"" bundle:""`
 }
 
 // EnvVar holds the environment variable struct of a container
@@ -109,3 +112,15 @@ type Ports struct {
 	HostIP        string
 	Protocol      api.Protocol
 }
+
+// Volumes holds the volume struct of container
+type Volumes struct {
+	SvcName    string // Service name to which volume is linked
+	MountPath  string // Mountpath extracted from docker-compose file
+	VFrom      string // denotes service name from which volume is coming
+	VolumeName string // name of volume if provided explicitly
+	Host       string // host machine address
+	Container  string // Mountpath
+	Mode       string // access mode for volume
+	PVCName    string // name of PVC
+}

pkg/loader/compose/v1v2.go

@@ -31,6 +31,7 @@ import (
 	"github.com/docker/libcompose/lookup"
 	"github.com/docker/libcompose/project"
 	"github.com/kubernetes/kompose/pkg/kobject"
+	"github.com/kubernetes/kompose/pkg/transformer"
 	"github.com/pkg/errors"
 )
 
@@ -205,7 +206,7 @@ func libComposeToKomposeMapping(composeObject *project.Project) (kobject.Kompose
 		envs := loadEnvVars(composeServiceConfig.Environment)
 		serviceConfig.Environment = envs
 
-		//Validate dockerfile path
+		// Validate dockerfile path
 		if filepath.IsAbs(serviceConfig.Dockerfile) {
 			log.Fatalf("%q defined in service %q is an absolute path, it must be a relative path.", serviceConfig.Dockerfile, name)
 		}
@@ -222,7 +223,7 @@ func libComposeToKomposeMapping(composeObject *project.Project) (kobject.Kompose
 		if composeServiceConfig.Volumes != nil {
 			for _, volume := range composeServiceConfig.Volumes.Volumes {
 				v := normalizeServiceNames(volume.String())
-				serviceConfig.Volumes = append(serviceConfig.Volumes, v)
+				serviceConfig.VolList = append(serviceConfig.VolList, v)
 			}
 		}
 
@@ -268,12 +269,124 @@ func libComposeToKomposeMapping(composeObject *project.Project) (kobject.Kompose
 			log.Infof("Service name in docker-compose has been changed from %q to %q", name, normalizeServiceNames(name))
 		}
 	}
+
+	// This will handle volume at earlier stage itself, it will resolves problems occurred due to `volumes_from` key
+	handleVolume(&komposeObject)
+
 	return komposeObject, nil
 }
 
+// This function will retrieve volumes for each service, as well as it will parse volume information and store it in Volumes struct
+func handleVolume(komposeObject *kobject.KomposeObject) {
+	for name, _ := range komposeObject.ServiceConfigs {
+		// retrieve volumes of service
+		vols, err := retrieveVolume(name, *komposeObject)
+		if err != nil {
+			errors.Wrap(err, "could not retrieve volume")
+		}
+		// We can't assign value to struct field in map while iterating over it, so temporary variable `temp` is used here
+		var temp = komposeObject.ServiceConfigs[name]
+		temp.Volumes = vols
+		komposeObject.ServiceConfigs[name] = temp
+	}
+}
+
 func checkLabelsPorts(noOfPort int, labels string, svcName string) error {
 	if noOfPort == 0 && (labels == "NodePort" || labels == "LoadBalancer") {
 		return errors.Errorf("%s defined in service %s with no ports present. Issues may occur when bringing up artifacts.", labels, svcName)
 	}
 	return nil
 }
+
+// returns all volumes associated with service, if `volumes_from` key is used, we have to retrieve volumes from the services which are mentioned there. Hence, recursive function is used here.
+func retrieveVolume(svcName string, komposeObject kobject.KomposeObject) (volume []kobject.Volumes, err error) {
+	// if volumes-from key is present
+	if komposeObject.ServiceConfigs[svcName].VolumesFrom != nil {
+		// iterating over services from `volumes-from`
+		for _, depSvc := range komposeObject.ServiceConfigs[svcName].VolumesFrom {
+			// recursive call for retrieving volumes of services from `volumes-from`
+			dVols, err := retrieveVolume(depSvc, komposeObject)
+			if err != nil {
+				return nil, errors.Wrapf(err, "could not retrieve the volume")
+			}
+			var cVols []kobject.Volumes
+			cVols, err = ParseVols(komposeObject.ServiceConfigs[svcName].VolList, svcName)
+			if err != nil {
+				return nil, errors.Wrapf(err, "error generting current volumes")
+			}
+
+			for _, cv := range cVols {
+				// check whether volumes of current service is same or not as that of dependent volumes coming from `volumes-from`
+				ok, dv := getVol(cv, dVols)
+				if ok {
+					// change current volumes service name to dependent service name
+					if dv.VFrom == "" {
+						cv.VFrom = dv.SvcName
+						cv.SvcName = dv.SvcName
+					} else {
+						cv.VFrom = dv.VFrom
+						cv.SvcName = dv.SvcName
+					}
+					cv.PVCName = dv.PVCName
+				}
+				volume = append(volume, cv)
+
+			}
+			// iterating over dependent volumes
+			for _, dv := range dVols {
+				// check whether dependent volume is already present or not
+				if checkVolDependent(dv, volume) {
+					// if found, add service name to `VFrom`
+					dv.VFrom = dv.SvcName
+					volume = append(volume, dv)
+				}
+			}
+		}
+	} else {
+		// if `volumes-from` is not present
+		volume, err = ParseVols(komposeObject.ServiceConfigs[svcName].VolList, svcName)
+		if err != nil {
+			return nil, errors.Wrapf(err, "error generting current volumes")
+		}
+	}
+	return
+}
+
+// checkVolDependent returns false if dependent volume is present
+func checkVolDependent(dv kobject.Volumes, volume []kobject.Volumes) bool {
+	for _, vol := range volume {
+		if vol.PVCName == dv.PVCName {
+			return false
+		}
+	}
+	return true
+
+}
+
+func ParseVols(volNames []string, svcName string) ([]kobject.Volumes, error) {
+	var volumes []kobject.Volumes
+	var err error
+
+	for i, vn := range volNames {
+		var v kobject.Volumes
+		v.VolumeName, v.Host, v.Container, v.Mode, err = transformer.ParseVolume(vn)
+		if err != nil {
+			return nil, errors.Wrapf(err, "could not parse volume %q: %v", vn, err)
+		}
+		v.SvcName = svcName
+		v.MountPath = fmt.Sprintf("%s:%s", v.Host, v.Container)
+		v.PVCName = fmt.Sprintf("%s-claim%d", v.SvcName, i)
+		volumes = append(volumes, v)
+	}
+	return volumes, nil
+}
+
+// for dependent volumes, returns true and the respective volume if mountpath are same
+func getVol(toFind kobject.Volumes, Vols []kobject.Volumes) (bool, kobject.Volumes) {
+	for _, dv := range Vols {
+		if toFind.MountPath == dv.MountPath {
+			return true, dv
+		}
+	}
+	return false, kobject.Volumes{}
+}

pkg/loader/compose/v3.go

@@ -233,7 +233,7 @@ func dockerComposeToKomposeMapping(composeObject *types.Config) (kobject.Kompose
 		// Parse the volumes
 		// Again, in v3, we use the "long syntax" for volumes in terms of parsing
 		// https://docs.docker.com/compose/compose-file/#long-syntax-2
-		serviceConfig.Volumes = loadV3Volumes(composeServiceConfig.Volumes)
+		serviceConfig.VolList = loadV3Volumes(composeServiceConfig.Volumes)
 
 		// Label handler
 		// Labels used to influence conversion of kompose will be handled
@@ -260,6 +260,7 @@ func dockerComposeToKomposeMapping(composeObject *types.Config) (kobject.Kompose
 		// Final step, add to the array!
 		komposeObject.ServiceConfigs[normalizeServiceNames(name)] = serviceConfig
 	}
+	handleVolume(&komposeObject)
 
 	return komposeObject, nil
 }

pkg/transformer/kubernetes/k8sutils.go

@@ -491,77 +491,6 @@ func (k *Kubernetes) SortServicesFirst(objs *[]runtime.Object) {
 	*objs = ret
 }
 
-func (k *Kubernetes) findDependentVolumes(svcname string, komposeObject kobject.KomposeObject) (volumes []api.Volume, volumeMounts []api.VolumeMount, err error) {
-	// Get all the volumes and volumemounts this particular service is dependent on
-	for _, dependentSvc := range komposeObject.ServiceConfigs[svcname].VolumesFrom {
-		vols, volMounts, err := k.findDependentVolumes(dependentSvc, komposeObject)
-		if err != nil {
-			err = errors.Wrap(err, "k.findDependentVolumes failed")
-			return nil, nil, err
-		}
-		volumes = append(volumes, vols...)
-		volumeMounts = append(volumeMounts, volMounts...)
-	}
-	// add the volumes info of this service
-	volMounts, vols, _, err := k.ConfigVolumes(svcname, komposeObject.ServiceConfigs[svcname])
-	if err != nil {
-		err = errors.Wrap(err, "k.ConfigVolumes failed")
-		return nil, nil, err
-	}
-	volumes = append(volumes, vols...)
-	volumeMounts = append(volumeMounts, volMounts...)
-	return volumes, volumeMounts, nil
-}
-
-// VolumesFrom creates volums and volumeMounts for volumes_from
-func (k *Kubernetes) VolumesFrom(objects *[]runtime.Object, komposeObject kobject.KomposeObject) error {
-	for _, obj := range *objects {
-		switch t := obj.(type) {
-		case *api.ReplicationController:
-			svcName := t.ObjectMeta.Name
-			for _, dependentSvc := range komposeObject.ServiceConfigs[svcName].VolumesFrom {
-				volumes, volumeMounts, err := k.findDependentVolumes(dependentSvc, komposeObject)
-				if err != nil {
-					return errors.Wrap(err, "k.findDependentVolumes")
-				}
-				t.Spec.Template.Spec.Volumes = append(t.Spec.Template.Spec.Volumes, volumes...)
-				t.Spec.Template.Spec.Containers[0].VolumeMounts = append(t.Spec.Template.Spec.Containers[0].VolumeMounts, volumeMounts...)
-			}
-		case *extensions.Deployment:
-			svcName := t.ObjectMeta.Name
-			for _, dependentSvc := range komposeObject.ServiceConfigs[svcName].VolumesFrom {
-				volumes, volumeMounts, err := k.findDependentVolumes(dependentSvc, komposeObject)
-				if err != nil {
-					return errors.Wrap(err, "k.findDependentVolumes")
-				}
-				t.Spec.Template.Spec.Volumes = append(t.Spec.Template.Spec.Volumes, volumes...)
-				t.Spec.Template.Spec.Containers[0].VolumeMounts = append(t.Spec.Template.Spec.Containers[0].VolumeMounts, volumeMounts...)
-			}
-		case *extensions.DaemonSet:
-			svcName := t.ObjectMeta.Name
-			for _, dependentSvc := range komposeObject.ServiceConfigs[svcName].VolumesFrom {
-				volumes, volumeMounts, err := k.findDependentVolumes(dependentSvc, komposeObject)
-				if err != nil {
-					return errors.Wrap(err, "k.findDependentVolumes")
-				}
-				t.Spec.Template.Spec.Volumes = append(t.Spec.Template.Spec.Volumes, volumes...)
-				t.Spec.Template.Spec.Containers[0].VolumeMounts = append(t.Spec.Template.Spec.Containers[0].VolumeMounts, volumeMounts...)
-			}
-		case *deployapi.DeploymentConfig:
-			svcName := t.ObjectMeta.Name
-			for _, dependentSvc := range komposeObject.ServiceConfigs[svcName].VolumesFrom {
-				volumes, volumeMounts, err := k.findDependentVolumes(dependentSvc, komposeObject)
-				if err != nil {
-					return errors.Wrap(err, "k.findDependentVolumes")
-				}
-				t.Spec.Template.Spec.Volumes = append(t.Spec.Template.Spec.Volumes, volumes...)
-				t.Spec.Template.Spec.Containers[0].VolumeMounts = append(t.Spec.Template.Spec.Containers[0].VolumeMounts, volumeMounts...)
-			}
-		}
-	}
-	return nil
-}
-
 // SortedKeys Ensure the kubernetes objects are in a consistent order
 func SortedKeys(komposeObject kobject.KomposeObject) []string {
 	var sortedKeys []string

pkg/transformer/kubernetes/k8sutils_test.go

@@ -47,7 +47,7 @@ func TestCreateService(t *testing.T) {
 		Command:       []string{"cmd"},
 		WorkingDir:    "dir",
 		Args:          []string{"arg1", "arg2"},
-		Volumes:       []string{"/tmp/volume"},
+		VolList:       []string{"/tmp/volume"},
 		Network:       []string{"network1", "network2"}, // not supported
 		Labels:        nil,
 		Annotations:   map[string]string{"abc": "def"},
@@ -91,7 +91,7 @@ func TestCreateServiceWithMemLimit(t *testing.T) {
 		Command:       []string{"cmd"},
 		WorkingDir:    "dir",
 		Args:          []string{"arg1", "arg2"},
-		Volumes:       []string{"/tmp/volume"},
+		VolList:       []string{"/tmp/volume"},
 		Network:       []string{"network1", "network2"}, // not supported
 		Labels:        nil,
 		Annotations:   map[string]string{"abc": "def"},
@@ -140,7 +140,7 @@ func TestCreateServiceWithServiceUser(t *testing.T) {
 		Command:       []string{"cmd"},
 		WorkingDir:    "dir",
 		Args:          []string{"arg1", "arg2"},
-		Volumes:       []string{"/tmp/volume"},
+		VolList:       []string{"/tmp/volume"},
 		Network:       []string{"network1", "network2"}, // not supported
 		Labels:        nil,
 		Annotations:   map[string]string{"kompose.service.type": "nodeport"},
@@ -184,7 +184,7 @@ func TestTransformWithPid(t *testing.T) {
 		Command:       []string{"cmd"},
 		WorkingDir:    "dir",
 		Args:          []string{"arg1", "arg2"},
-		Volumes:       []string{"/tmp/volume"},
+		VolList:       []string{"/tmp/volume"},
 		Network:       []string{"network1", "network2"},
 		Restart:       "always",
 		Pid:           "host",
@@ -220,7 +220,7 @@ func TestTransformWithInvaildPid(t *testing.T) {
 		Command:       []string{"cmd"},
 		WorkingDir:    "dir",
 		Args:          []string{"arg1", "arg2"},
-		Volumes:       []string{"/tmp/volume"},
+		VolList:       []string{"/tmp/volume"},
 		Network:       []string{"network1", "network2"},
 		Restart:       "always",
 		Pid:           "badvalue",
@@ -329,7 +329,8 @@ func TestRecreateStrategyWithVolumesPresent(t *testing.T) {
 	service := kobject.ServiceConfig{
 		ContainerName: "name",
 		Image:         "image",
-		Volumes:       []string{"/tmp/volume"},
+		VolList:       []string{"/tmp/volume"},
+		Volumes:       []kobject.Volumes{{SvcName: "app", MountPath: "/tmp/volume", PVCName: "app-claim0"}},
 	}
 
 	komposeObject := kobject.KomposeObject{

pkg/transformer/kubernetes/kubernetes.go

@@ -49,6 +49,7 @@ import (
 	"k8s.io/kubernetes/pkg/api/meta"
 	"k8s.io/kubernetes/pkg/labels"
 	"sort"
+	"strings"
 )
 
 // Kubernetes implements Transformer interface and represents Kubernetes transformer
@@ -376,56 +377,53 @@ func (k *Kubernetes) ConfigVolumes(name string, service kobject.ServiceConfig) (
 	volumeMounts := []api.VolumeMount{}
 	volumes := []api.Volume{}
 	var PVCs []*api.PersistentVolumeClaim
+	var volumeName string
 
 	// Set a var based on if the user wants to use empty volumes
 	// as opposed to persistent volumes and volume claims
 	useEmptyVolumes := k.Opt.EmptyVols
 
 	var count int
+	//interating over array of `Vols` struct as it contains all necessary information about volumes
 	for _, volume := range service.Volumes {
 
-		volumeName, host, container, mode, err := transformer.ParseVolume(volume)
-		if err != nil {
-			log.Warningf("Failed to configure container volume: %v", err)
-			continue
-		}
-
-		log.Debug("Volume name %s", volumeName)
-
 		// check if ro/rw mode is defined, default rw
-		readonly := len(mode) > 0 && mode == "ro"
+		readonly := len(volume.Mode) > 0 && volume.Mode == "ro"
 
-		if volumeName == "" {
+		if volume.VolumeName == "" {
 			if useEmptyVolumes {
-				volumeName = fmt.Sprintf("%s-empty%d", name, count)
+				volumeName = strings.Replace(volume.PVCName, "claim", "empty", 1)
 			} else {
-				volumeName = fmt.Sprintf("%s-claim%d", name, count)
+				volumeName = volume.PVCName
 			}
 			count++
+		} else {
+			volumeName = volume.VolumeName
 		}
-
-		// create a new volume mount object and append to list
 		volmount := api.VolumeMount{
 			Name:      volumeName,
 			ReadOnly:  readonly,
-			MountPath: container,
+			MountPath: volume.Container,
 		}
 		volumeMounts = append(volumeMounts, volmount)
-
 		// Get a volume source based on the type of volume we are using
 		// For PVC we will also create a PVC object and add to list
 		var volsource *api.VolumeSource
+
 		if useEmptyVolumes {
 			volsource = k.ConfigEmptyVolumeSource("volume")
 		} else {
+
 			volsource = k.ConfigPVCVolumeSource(volumeName, readonly)
+			if volume.VFrom == "" {
+				createdPVC, err := k.CreatePVC(volumeName, volume.Mode)
 
-			createdPVC, err := k.CreatePVC(volumeName, mode)
-			if err != nil {
-				return nil, nil, nil, errors.Wrap(err, "k.CreatePVC failed")
+				if err != nil {
+					return nil, nil, nil, errors.Wrap(err, "k.CreatePVC failed")
+				}
+
+				PVCs = append(PVCs, createdPVC)
 			}
-
-			PVCs = append(PVCs, createdPVC)
 		}
 
 		// create a new volume object using the volsource and add to list
@@ -435,10 +433,12 @@ func (k *Kubernetes) ConfigVolumes(name string, service kobject.ServiceConfig) (
 		}
 		volumes = append(volumes, vol)
 
-		if len(host) > 0 {
-			log.Warningf("Volume mount on the host %q isn't supported - ignoring path on the host", host)
+		if len(volume.Host) > 0 {
+			log.Warningf("Volume mount on the host %q isn't supported - ignoring path on the host", volume.Host)
 		}
+
 	}
+
 	return volumeMounts, volumes, PVCs, nil
 }
 
@@ -603,8 +603,7 @@ func (k *Kubernetes) Transform(komposeObject kobject.KomposeObject, opt kobject.
 
 		allobjects = append(allobjects, objects...)
 	}
-	// If docker-compose has a volumes_from directive it will be handled here
-	k.VolumesFrom(&allobjects, komposeObject)
+
 	// sort all object so Services are first
 	k.SortServicesFirst(&allobjects)
 	return allobjects, nil

pkg/transformer/kubernetes/kubernetes_test.go

@@ -40,7 +40,7 @@ func newServiceConfig() kobject.ServiceConfig {
 		Command:       []string{"cmd"},
 		WorkingDir:    "dir",
 		Args:          []string{"arg1", "arg2"},
-		Volumes:       []string{"/tmp/volume"},
+		VolList:       []string{"/tmp/volume"},
 		Network:       []string{"network1", "network2"}, // not supported
 		Labels:        nil,
 		Annotations:   map[string]string{"abc": "def"},
@@ -54,6 +54,7 @@ func newServiceConfig() kobject.ServiceConfig {
 		Tty:           true,
 		TmpFs:         []string{"/tmp"},
 		Replicas:      2,
+		Volumes:       []kobject.Volumes{{SvcName: "app", MountPath: "/tmp/volume", PVCName: "app-claim0"}},
 	}
 }
 

pkg/transformer/openshift/openshift.go

@@ -470,10 +470,7 @@ func (o *OpenShift) Transform(komposeObject kobject.KomposeObject, opt kobject.C
 		allobjects = append(allobjects, objects...)
 	}
 
-	// If docker-compose has a volumes_from directive it will be handled here
-	o.VolumesFrom(&allobjects, komposeObject)
-
-	// sort all object so all services are first
+	// sort all object so Services are first
 	o.SortServicesFirst(&allobjects)
 
 	return allobjects, nil

pkg/transformer/openshift/openshift_test.go

@@ -42,7 +42,7 @@ func newServiceConfig() kobject.ServiceConfig {
 		Command:       []string{"cmd"},
 		WorkingDir:    "dir",
 		Args:          []string{"arg1", "arg2"},
-		Volumes:       []string{"/tmp/volume"},
+		VolList:       []string{"/tmp/volume"},
 		Network:       []string{"network1", "network2"}, // not supported
 		Labels:        nil,
 		Annotations:   map[string]string{"abc": "def"},
@@ -406,7 +406,8 @@ func TestRecreateStrategyWithVolumesPresent(t *testing.T) {
 	service := kobject.ServiceConfig{
 		ContainerName: "name",
 		Image:         "image",
-		Volumes:       []string{"/tmp/volume"},
+		VolList:       []string{"/tmp/volume"},
+		Volumes:       []kobject.Volumes{{SvcName: "app", MountPath: "/tmp/volume", PVCName: "app-claim0"}},
 	}
 	komposeObject := kobject.KomposeObject{
 		ServiceConfigs: map[string]kobject.ServiceConfig{"app": service},