API modified to use source; now supports EmptyDirectory

API is now modified to use a Source struct to handle multiple volumes.

Two volume types are supported now, HostDirectory and EmptyDirectory.

pkg/api/types.go

@@ -62,10 +62,22 @@ type Volume struct {
 	// Required: This must be a DNS_LABEL.  Each volume in a pod must have
 	// a unique name.
 	Name string `yaml:"name" json:"name"`
-	// When multiple volume types are supported, only one of them may be specified.
+	// Source represents the location and type of a volume to mount.
+	// This is optional for now. If not specified, the Volume is implied to be an EmptyDir.
+	// This implied behavior is deprecated and will be removed in a future version.
+        Source *VolumeSource `yaml:"source" json:"source"`
+}
+
+type VolumeSource struct {
+	// Only one of the following sources may be specified
+	// HostDirectory represents a pre-existing directory on the host machine that is directly
+	// exposed to the container. This is generally used for system agents or other privileged
+	// things that are allowed to see the host machine. Most containers will NOT need this.
+	// TODO(jonesdl) We need to restrict who can use host directory mounts and
+	// who can/can not mount host directories as read/write.
 	HostDirectory *HostDirectory `yaml:"hostDir" json:"hostDir"`
-	// DEPRECATED: If no volume type is specified, HostDirectory will be assumed.
+	// EmptyDirectory represents a temporary directory that shares a pod's lifetime.
-	// The path of the directory will be specified in the VolumeMount struct in this case.
+	EmptyDirectory *EmptyDirectory `yaml:"emptyDir" json:"emptyDir"`
 }
 
 // Bare host directory volume.
@@ -73,6 +85,8 @@ type HostDirectory struct {
 	Path string `yaml:"path" json:"path"`
 }
 
+type EmptyDirectory struct {}
+
 // Port represents a network port in a single container
 type Port struct {
 	// Optional: If specified, this must be a DNS_LABEL.  Each named port

pkg/api/validation.go

@@ -76,25 +76,46 @@ func validateVolumes(volumes []Volume) (util.StringSet, errorList) {
 	allNames := util.StringSet{}
 	for i := range volumes {
 		vol := &volumes[i] // so we can set default values
-		if vol.HostDirectory != nil {
+		errs := errorList{}
-			errs := validateHostDir(vol.HostDirectory)
+		// TODO(thockin) enforce that a source is set once we deprecate the implied form.
-			allErrs.Append(errs...)
+		if vol.Source != nil {
+			errs = validateSource(vol.Source)
 		}
 		if !util.IsDNSLabel(vol.Name) {
-			allErrs.Append(makeInvalidError("Volume.Name", vol.Name))
+			errs.Append(makeInvalidError("Volume.Name", vol.Name))
 		} else if allNames.Has(vol.Name) {
-			allErrs.Append(makeDuplicateError("Volume.Name", vol.Name))
+			errs.Append(makeDuplicateError("Volume.Name", vol.Name))
-		} else {
+		}
+		if len(errs) == 0 {
 			allNames.Insert(vol.Name)
+		} else {
+			allErrs.Append(errs...)
 		}
 	}
 	return allNames, allErrs
 }
 
+func validateSource(source *VolumeSource) errorList {
+	numVolumes := 0
+	allErrs := errorList{}
+	if source.HostDirectory != nil {
+		numVolumes++
+		allErrs.Append(validateHostDir(source.HostDirectory)...)
+	}
+	if source.EmptyDirectory != nil {
+		numVolumes++
+		//EmptyDirs have nothing to validate
+	}
+	if numVolumes != 1 {
+		allErrs.Append(makeInvalidError("Volume.Source", source))
+	}
+	return allErrs
+}
+
 func validateHostDir(hostDir *HostDirectory) errorList {
 	allErrs := errorList{}
 	if hostDir.Path == "" {
-		allErrs.Append(makeNotFoundError("Volume.HostDir.Path", hostDir.Path))
+		allErrs.Append(makeNotFoundError("HostDir.Path", hostDir.Path))
 	}
 	return allErrs
 }
@@ -178,8 +199,6 @@ func validateVolumeMounts(mounts []VolumeMount, volumes util.StringSet) errorLis
 		if len(mnt.MountType) != 0 {
 			glog.Warning("DEPRECATED: VolumeMount.MountType will be removed. The Volume struct will handle types")
 		}
-
-
 	}
 	return allErrs
 }

pkg/api/validation_test.go

@@ -25,15 +25,16 @@ import (
 
 func TestValidateVolumes(t *testing.T) {
 	successCase := []Volume{
-		{Name: "abc", HostDirectory: &HostDirectory{"/mnt/path1"}},
+		{Name: "abc"},
-		{Name: "123", HostDirectory: &HostDirectory{"/mnt/path2"}},
+		{Name: "123", Source: &VolumeSource{HostDirectory: &HostDirectory{"/mnt/path2"}}},
-		{Name: "abc-123", HostDirectory: &HostDirectory{"/mnt/path3"}},
+		{Name: "abc-123", Source: &VolumeSource{HostDirectory: &HostDirectory{"/mnt/path3"}}},
+		{Name: "empty", Source: &VolumeSource{EmptyDirectory: &EmptyDirectory{}}},
 	}
 	names, errs := validateVolumes(successCase)
 	if len(errs) != 0 {
 		t.Errorf("expected success: %v", errs)
 	}
-	if len(names) != 3 || !names.Has("abc") || !names.Has("123") || !names.Has("abc-123") {
+	if len(names) != 4 || !names.Has("abc") || !names.Has("123") || !names.Has("abc-123") || !names.Has("empty") {
 		t.Errorf("wrong names result: %v", names)
 	}
 
@@ -206,8 +207,8 @@ func TestValidateManifest(t *testing.T) {
 		{
 			Version: "v1beta1",
 			ID:      "abc",
-			Volumes: []Volume{{Name: "vol1", HostDirectory: &HostDirectory{"/mnt/vol1"}},
+			Volumes: []Volume{{Name: "vol1", Source: &VolumeSource{HostDirectory: &HostDirectory{"/mnt/vol1"}}},
-				          {Name: "vol2", HostDirectory: &HostDirectory{"/mnt/vol2"}}},
+			                  {Name: "vol2", Source: &VolumeSource{HostDirectory: &HostDirectory{"/mnt/vol2"}}}},
 			Containers: []Container{
 				{
 					Name:       "abc",

pkg/kubelet/kubelet.go

@@ -77,7 +77,6 @@ type Kubelet struct {
 	HTTPCheckFrequency time.Duration
 	pullLock           sync.Mutex
 	HealthChecker      health.HealthChecker
-	extVolumes	   map[string]volumes.Interface
 }
 
 type manifestUpdate struct {
@@ -187,13 +186,15 @@ func makeVolumesAndBinds(manifestID string, container *api.Container, podVolumes
 	binds := []string{}
 	for _, volume := range container.VolumeMounts {
 		var basePath string
-		if hostVol, ok := podVolumes[volume.Name]; ok {
+		if vol, ok := podVolumes[volume.Name]; ok {
 			// Host volumes are not Docker volumes and are directly mounted from the host.
-			basePath = fmt.Sprintf("%s:%s", hostVol.GetPath(), volume.MountPath)
+			basePath = fmt.Sprintf("%s:%s", vol.GetPath(), volume.MountPath)
 		} else if volume.MountType == "HOST" {
-			// DEPRECATED: VolumeMount.MountType will be moved to the Volume struct.
+			// DEPRECATED: VolumeMount.MountType will be handled by the Volume struct.
 			basePath = fmt.Sprintf("%s:%s", volume.MountPath, volume.MountPath)
 		} else {
+			// TODO(jonesdl) This clause should be deleted and an error should be thrown. The default
+			// behavior is now supported by the EmptyDirectory type.
 			volumes[volume.MountPath] = struct{}{}
 			basePath = fmt.Sprintf("/exports/%s/%s:%s", manifestID, volume.Name, volume.MountPath)
 		}
@@ -251,11 +252,13 @@ func (kl *Kubelet) mountExternalVolumes(manifest *api.ContainerManifest) (volume
 		if err != nil {
 			return nil, err
 		}
-		podVolumes[vol.Name] = extVolume
+		// TODO(jonesdl) When the default volume behavior is no longer supported, this case
-		// Only prepare the volume if it is not already mounted.
+		// should never occur and an error should be thrown instead.
-		if _, err := os.Stat(extVolume.GetPath()); os.IsNotExist(err) {
+		if extVolume == nil {
-			extVolume.SetUp()
+			continue
 		}
+		podVolumes[vol.Name] = extVolume
+		extVolume.SetUp()
 	}
 	return podVolumes, nil
 }
@@ -581,6 +584,9 @@ func (kl *Kubelet) syncManifest(manifest *api.ContainerManifest, dockerContainer
 	}
 	keepChannel <- netID
 	podVolumes, err := kl.mountExternalVolumes(manifest)
+	if err != nil {
+		glog.Errorf("Unable to mount volumes for manifest %s: (%v)", manifest.ID, err)
+	}
 	for _, container := range manifest.Containers {
 		if dockerContainer, found := dockerContainers.FindPodContainer(manifest.ID, container.Name); found {
 			containerID := DockerID(dockerContainer.ID)

pkg/kubelet/kubelet_test.go

@@ -535,13 +535,15 @@ func TestMountExternalVolumes(t *testing.T) {
 		Volumes: []api.Volume{
 			{
 				Name: "host-dir",
-				HostDirectory: &api.HostDirectory{"/dir/path"},
+				Source: &api.VolumeSource{
+					HostDirectory: &api.HostDirectory{"/dir/path"},
+				},
 			},
 		},
 	}
 	podVolumes, _ := kubelet.mountExternalVolumes(&manifest)
-	expectedPodVolumes := make(map[string]volumes.Interface)
+	expectedPodVolumes := make(volumeMap)
-	expectedPodVolumes["host-dir"] = &volumes.HostDirectoryVolume{"/dir/path"}
+	expectedPodVolumes["host-dir"] = &volume.HostDirectory{"/dir/path"}
 	if len(expectedPodVolumes) != len(podVolumes) {
 		t.Errorf("Unexpected volumes. Expected %#v got %#v.  Manifest was: %#v", expectedPodVolumes, podVolumes, manifest)
 	}
@@ -552,8 +554,6 @@ func TestMountExternalVolumes(t *testing.T) {
 	}
 }
 
-
-
 func TestMakeVolumesAndBinds(t *testing.T) {
 	container := api.Container{
 		VolumeMounts: []api.VolumeMount{

pkg/volume/doc.go

@@ -14,6 +14,6 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-// Package volumes includes internal representations of external volume types
+// Package volume includes internal representations of external volume types
 // as well as utility methods required to mount/unmount volumes to kubelets.
 package volume

pkg/volume/volume.go

@@ -21,16 +21,19 @@ import (
 	"fmt"
 	"os"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/api"
+	"github.com/golang/glog"
 )
 
 
 // All volume types are expected to implement this interface
 type Interface interface {
 	// Prepares and mounts/unpacks the volume to a directory path.
+	// This procedure must be idempotent.
 	SetUp()
 	// Returns the directory path the volume is mounted to.
 	GetPath() string
 	// Unmounts the volume and removes traces of the SetUp procedure.
+	// This procedure must be idempotent.
 	TearDown()
 }
 
@@ -52,7 +55,6 @@ func (hostVol *HostDirectory) GetPath() string {
 
 // EmptyDirectory volumes are temporary directories exposed to the pod.
 // These do not persist beyond the lifetime of a pod.
-
 type EmptyDirectory struct {
 	Name string
 	PodID string
@@ -60,7 +62,11 @@ type EmptyDirectory struct {
 
 // SetUp creates the new directory.
 func (emptyDir *EmptyDirectory) SetUp() {
-	os.MkdirAll(emptyDir.GetPath(), 0750)
+	if _, err := os.Stat(emptyDir.GetPath()); os.IsNotExist(err) {
+		os.MkdirAll(emptyDir.GetPath(), 0750)
+	} else {
+		glog.Warningf("Directory already exists: (%v)", emptyDir.GetPath())
+	}
 }
 
 // TODO(jonesdl) when we can properly invoke TearDown(), we should delete
@@ -72,6 +78,7 @@ func (emptyDir *EmptyDirectory) GetPath() string {
 	// directory for kubelet to write to. For now this will just be /exports
 	return fmt.Sprintf("/exports/%v/%v", emptyDir.PodID, emptyDir.Name)
 }
+
 // Interprets API volume as a HostDirectory
 func createHostDirectory(volume *api.Volume) *HostDirectory {
 	return &HostDirectory{volume.Source.HostDirectory.Path}
@@ -83,16 +90,23 @@ func createEmptyDirectory(volume *api.Volume, podID string) *EmptyDirectory {
 }
 
 // CreateVolume returns an Interface capable of mounting a volume described by an
-// *api.Volume, or an error.
+// *api.Volume and whether or not it is mounted, or an error.
 func CreateVolume(volume *api.Volume, podID string) (Interface, error) {
+	source := volume.Source
+	// TODO(jonesdl) We will want to throw an error here when we no longer
+	// support the default behavior.
+	if source == nil {
+		return nil, nil
+	}
+	var vol Interface
 	// TODO(jonesdl) We should probably not check every pointer and directly
 	// resolve these types instead.
-	source := volume.Source
 	if source.HostDirectory != nil {
-		return createHostDirectory(volume), nil
+		vol = createHostDirectory(volume)
 	} else if source.EmptyDirectory != nil {
-		return createEmptyDirectory(volume, podID), nil
+		vol = createEmptyDirectory(volume, podID)
 	} else {
 		return nil, errors.New("Unsupported volume type.")
 	}
+	return vol, nil
 }

pkg/volume/volume_test.go

@@ -26,12 +26,21 @@ func TestCreateVolumes(t *testing.T) {
 	volumes := []api.Volume{
 		{
 			Name: "host-dir",
-			HostDirectory: &api.HostDirectory{"/dir/path"},
+			Source: &api.VolumeSource{
+				HostDirectory: &api.HostDirectory{"/dir/path"},
+			},
+		},
+		{
+			Name: "empty-dir",
+			Source: &api.VolumeSource{
+				EmptyDirectory: &api.EmptyDirectory{},
+			},
 		},
 	}
-	expectedPaths := []string{"/dir/path"}
+	fakePodID := "my-id"
+	expectedPaths := []string{"/dir/path", "/exports/my-id/empty-dir"}
 	for i, volume := range volumes {
-		extVolume, _ := CreateVolume(&volume)
+		extVolume, _ := CreateVolume(&volume, fakePodID)
 		expectedPath := expectedPaths[i]
 		path := extVolume.GetPath()
 		if expectedPath != path {