From f84ff740f06af0bf1312498b95861ad41bde7a71 Mon Sep 17 00:00:00 2001 From: Danny Jones Date: Mon, 14 Jul 2014 18:39:30 -0700 Subject: [PATCH 1/3] Adds initial volumes package; Supports host-dirs Adds the framework for external volume mounts. Currently supports bare host directory mounts. Modifies the API to support host directory mounts from Volumes instead of VolumeMounts. --- pkg/api/types.go | 10 ++++++ pkg/api/validation.go | 17 ++++++++++ pkg/api/validation_test.go | 9 ++--- pkg/kubelet/kubelet.go | 37 +++++++++++++++++---- pkg/kubelet/kubelet_test.go | 40 +++++++++++++++++++++-- pkg/volume/doc.go | 19 +++++++++++ pkg/volume/volume.go | 65 +++++++++++++++++++++++++++++++++++++ pkg/volume/volume_test.go | 40 +++++++++++++++++++++++ 8 files changed, 224 insertions(+), 13 deletions(-) create mode 100644 pkg/volume/doc.go create mode 100644 pkg/volume/volume.go create mode 100644 pkg/volume/volume_test.go diff --git a/pkg/api/types.go b/pkg/api/types.go index bdaec3756ae..3ad179e3b0d 100644 --- a/pkg/api/types.go +++ b/pkg/api/types.go @@ -62,6 +62,15 @@ 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. + HostDirectory *HostDirectory `yaml:"hostDir" json:"hostDir"` + // DEPRECATED: If no volume type is specified, HostDirectory will be assumed. + // The path of the directory will be specified in the VolumeMount struct in this case. +} + +// Bare host directory volume. +type HostDirectory struct { + Path string `yaml:"path" json:"path"` } // Port represents a network port in a single container @@ -92,6 +101,7 @@ type VolumeMount struct { MountPath string `yaml:"mountPath,omitempty" json:"mountPath,omitempty"` Path string `yaml:"path,omitempty" json:"path,omitempty"` // One of: "LOCAL" (local volume) or "HOST" (external mount from the host). Default: LOCAL. + // DEPRECATED: MountType will be removed in a future version of the API. MountType string `yaml:"mountType,omitempty" json:"mountType,omitempty"` } diff --git a/pkg/api/validation.go b/pkg/api/validation.go index a29dec05c0d..92dd44e40b4 100644 --- a/pkg/api/validation.go +++ b/pkg/api/validation.go @@ -76,6 +76,10 @@ 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 := validateHostDir(vol.HostDirectory) + allErrs.Append(errs...) + } if !util.IsDNSLabel(vol.Name) { allErrs.Append(makeInvalidError("Volume.Name", vol.Name)) } else if allNames.Has(vol.Name) { @@ -87,6 +91,14 @@ func validateVolumes(volumes []Volume) (util.StringSet, errorList) { return allNames, allErrs } +func validateHostDir(hostDir *HostDirectory) errorList { + allErrs := errorList{} + if hostDir.Path == "" { + allErrs.Append(makeNotFoundError("Volume.HostDir.Path", hostDir.Path)) + } + return allErrs +} + var supportedPortProtocols = util.NewStringSet("TCP", "UDP") func validatePorts(ports []Port) errorList { @@ -163,6 +175,11 @@ func validateVolumeMounts(mounts []VolumeMount, volumes util.StringSet) errorLis mnt.Path = "" } } + if len(mnt.MountType) != 0 { + glog.Warning("DEPRECATED: VolumeMount.MountType will be removed. The Volume struct will handle types") + } + + } return allErrs } diff --git a/pkg/api/validation_test.go b/pkg/api/validation_test.go index 84ae5ff20c5..eabd0cba37a 100644 --- a/pkg/api/validation_test.go +++ b/pkg/api/validation_test.go @@ -25,9 +25,9 @@ import ( func TestValidateVolumes(t *testing.T) { successCase := []Volume{ - {Name: "abc"}, - {Name: "123"}, - {Name: "abc-123"}, + {Name: "abc", HostDirectory: &HostDirectory{"/mnt/path1"}}, + {Name: "123", HostDirectory: &HostDirectory{"/mnt/path2"}}, + {Name: "abc-123", HostDirectory: &HostDirectory{"/mnt/path3"}}, } names, errs := validateVolumes(successCase) if len(errs) != 0 { @@ -206,7 +206,8 @@ func TestValidateManifest(t *testing.T) { { Version: "v1beta1", ID: "abc", - Volumes: []Volume{{Name: "vol1"}, {Name: "vol2"}}, + Volumes: []Volume{{Name: "vol1", HostDirectory: &HostDirectory{"/mnt/vol1"}}, + {Name: "vol2", HostDirectory: &HostDirectory{"/mnt/vol2"}}}, Containers: []Container{ { Name: "abc", diff --git a/pkg/kubelet/kubelet.go b/pkg/kubelet/kubelet.go index 6909411121e..65e3644d2bf 100644 --- a/pkg/kubelet/kubelet.go +++ b/pkg/kubelet/kubelet.go @@ -36,6 +36,7 @@ import ( "github.com/GoogleCloudPlatform/kubernetes/pkg/health" "github.com/GoogleCloudPlatform/kubernetes/pkg/tools" "github.com/GoogleCloudPlatform/kubernetes/pkg/util" + "github.com/GoogleCloudPlatform/kubernetes/pkg/volume" "github.com/coreos/go-etcd/etcd" "github.com/fsouza/go-dockerclient" "github.com/golang/glog" @@ -62,6 +63,8 @@ func New() *Kubelet { return &Kubelet{} } +type volumeMap map[string]volume.Interface + // Kubelet is the main kubelet implementation. type Kubelet struct { Hostname string @@ -74,6 +77,7 @@ type Kubelet struct { HTTPCheckFrequency time.Duration pullLock sync.Mutex HealthChecker health.HealthChecker + extVolumes map[string]volumes.Interface } type manifestUpdate struct { @@ -178,13 +182,16 @@ func makeEnvironmentVariables(container *api.Container) []string { return result } -func makeVolumesAndBinds(manifestID string, container *api.Container) (map[string]struct{}, []string) { +func makeVolumesAndBinds(manifestID string, container *api.Container, podVolumes volumeMap) (map[string]struct{}, []string) { volumes := map[string]struct{}{} binds := []string{} for _, volume := range container.VolumeMounts { var basePath string - if volume.MountType == "HOST" { + if hostVol, 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) + } else if volume.MountType == "HOST" { + // DEPRECATED: VolumeMount.MountType will be moved to the Volume struct. basePath = fmt.Sprintf("%s:%s", volume.MountPath, volume.MountPath) } else { volumes[volume.MountPath] = struct{}{} @@ -237,10 +244,26 @@ func milliCPUToShares(milliCPU int) int { return shares } +func (kl *Kubelet) mountExternalVolumes(manifest *api.ContainerManifest) (volumeMap, error) { + podVolumes := make(volumeMap) + for _, vol := range manifest.Volumes { + extVolume, err := volume.CreateVolume(&vol, manifest.ID) + if err != nil { + return nil, err + } + podVolumes[vol.Name] = extVolume + // Only prepare the volume if it is not already mounted. + if _, err := os.Stat(extVolume.GetPath()); os.IsNotExist(err) { + extVolume.SetUp() + } + } + return podVolumes, nil +} + // Run a single container from a manifest. Returns the docker container ID -func (kl *Kubelet) runContainer(manifest *api.ContainerManifest, container *api.Container, netMode string) (id DockerID, err error) { +func (kl *Kubelet) runContainer(manifest *api.ContainerManifest, container *api.Container, podVolumes volumeMap, netMode string) (id DockerID, err error) { envVariables := makeEnvironmentVariables(container) - volumes, binds := makeVolumesAndBinds(manifest.ID, container) + volumes, binds := makeVolumesAndBinds(manifest.ID, container, podVolumes) exposedPorts, portBindings := makePortsAndBindings(container) opts := docker.CreateContainerOptions{ @@ -540,7 +563,7 @@ func (kl *Kubelet) createNetworkContainer(manifest *api.ContainerManifest) (Dock Ports: ports, } kl.DockerPuller.Pull("busybox") - return kl.runContainer(manifest, container, "") + return kl.runContainer(manifest, container, nil, "") } func (kl *Kubelet) syncManifest(manifest *api.ContainerManifest, dockerContainers DockerContainers, keepChannel chan<- DockerID) error { @@ -557,7 +580,7 @@ func (kl *Kubelet) syncManifest(manifest *api.ContainerManifest, dockerContainer netID = dockerNetworkID } keepChannel <- netID - + podVolumes, err := kl.mountExternalVolumes(manifest) for _, container := range manifest.Containers { if dockerContainer, found := dockerContainers.FindPodContainer(manifest.ID, container.Name); found { containerID := DockerID(dockerContainer.ID) @@ -587,7 +610,7 @@ func (kl *Kubelet) syncManifest(manifest *api.ContainerManifest, dockerContainer glog.Errorf("Failed to create container: %v skipping manifest %s container %s.", err, manifest.ID, container.Name) continue } - containerID, err := kl.runContainer(manifest, &container, "container:"+string(netID)) + containerID, err := kl.runContainer(manifest, &container, podVolumes, "container:"+string(netID)) if err != nil { // TODO(bburns) : Perhaps blacklist a container after N failures? glog.Errorf("Error running manifest %s container %s: %v", manifest.ID, container.Name, err) diff --git a/pkg/kubelet/kubelet_test.go b/pkg/kubelet/kubelet_test.go index 1777e6fec4c..26dae7911db 100644 --- a/pkg/kubelet/kubelet_test.go +++ b/pkg/kubelet/kubelet_test.go @@ -29,6 +29,7 @@ import ( "github.com/GoogleCloudPlatform/kubernetes/pkg/health" "github.com/GoogleCloudPlatform/kubernetes/pkg/tools" "github.com/GoogleCloudPlatform/kubernetes/pkg/util" + "github.com/GoogleCloudPlatform/kubernetes/pkg/volume" "github.com/coreos/go-etcd/etcd" "github.com/fsouza/go-dockerclient" "github.com/google/cadvisor/info" @@ -528,6 +529,31 @@ func TestMakeEnvVariables(t *testing.T) { } } +func TestMountExternalVolumes(t *testing.T) { + kubelet, _, _ := makeTestKubelet(t) + manifest := api.ContainerManifest{ + Volumes: []api.Volume{ + { + Name: "host-dir", + HostDirectory: &api.HostDirectory{"/dir/path"}, + }, + }, + } + podVolumes, _ := kubelet.mountExternalVolumes(&manifest) + expectedPodVolumes := make(map[string]volumes.Interface) + expectedPodVolumes["host-dir"] = &volumes.HostDirectoryVolume{"/dir/path"} + if len(expectedPodVolumes) != len(podVolumes) { + t.Errorf("Unexpected volumes. Expected %#v got %#v. Manifest was: %#v", expectedPodVolumes, podVolumes, manifest) + } + for name, expectedVolume := range expectedPodVolumes { + if _, ok := podVolumes[name]; !ok { + t.Errorf("Pod volumes map is missing key: %s. %#v", expectedVolume, podVolumes) + } + } +} + + + func TestMakeVolumesAndBinds(t *testing.T) { container := api.Container{ VolumeMounts: []api.VolumeMount{ @@ -548,12 +574,22 @@ func TestMakeVolumesAndBinds(t *testing.T) { ReadOnly: false, MountType: "HOST", }, + { + MountPath: "/mnt/path4", + Name: "disk4", + ReadOnly: false, + }, }, } - volumes, binds := makeVolumesAndBinds("pod", &container) + + podVolumes := make(volumeMap) + podVolumes["disk4"] = &volume.HostDirectory{"/mnt/host"} + + volumes, binds := makeVolumesAndBinds("pod", &container, podVolumes) expectedVolumes := []string{"/mnt/path", "/mnt/path2"} - expectedBinds := []string{"/exports/pod/disk:/mnt/path", "/exports/pod/disk2:/mnt/path2:ro", "/mnt/path3:/mnt/path3"} + expectedBinds := []string{"/exports/pod/disk:/mnt/path", "/exports/pod/disk2:/mnt/path2:ro", "/mnt/path3:/mnt/path3", + "/mnt/host:/mnt/path4"} if len(volumes) != len(expectedVolumes) { t.Errorf("Unexpected volumes. Expected %#v got %#v. Container was: %#v", expectedVolumes, volumes, container) } diff --git a/pkg/volume/doc.go b/pkg/volume/doc.go new file mode 100644 index 00000000000..e0f8d95904c --- /dev/null +++ b/pkg/volume/doc.go @@ -0,0 +1,19 @@ +/* +Copyright 2014 Google Inc. All rights reserved. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +// Package volumes includes internal representations of external volume types +// as well as utility methods required to mount/unmount volumes to kubelets. +package volume diff --git a/pkg/volume/volume.go b/pkg/volume/volume.go new file mode 100644 index 00000000000..878ce37ed7a --- /dev/null +++ b/pkg/volume/volume.go @@ -0,0 +1,65 @@ +/* +Copyright 2014 Google Inc. All rights reserved. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package volumes + +import ( + "errors" + "github.com/GoogleCloudPlatform/kubernetes/pkg/api" +) + + +// All volume types are expected to implement this interface +type Interface interface { + // Prepares and mounts/unpacks the volume to a directory path. + SetUp() + // Returns the directory path the volume is mounted to. + GetPath() string + // Unmounts the volume and removes traces of the SetUp procedure. + TearDown() +} + +// Host Directory Volumes represent a bare host directory mount. +type HostDirectoryVolume struct { + Path string +} + +// Simple host directory mounts require no setup or cleanup, but still +// need to fulfill the interface definitions. +func (hostVol *HostDirectoryVolume) SetUp() {} + +func (hostVol *HostDirectoryVolume) TearDown() {} + +func (hostVol *HostDirectoryVolume) GetPath() string { + return hostVol.Path +} + +// Interprets API volume as a HostDirectory +func createHostDirectoryVolume(volume *api.Volume) *HostDirectoryVolume { + return &HostDirectoryVolume{volume.HostDirectory.Path} +} + +// Interprets parameters passed in the API as an internal structure +// with utility procedures for mounting. +func CreateVolume(volume *api.Volume) (Interface, error) { + // TODO(jonesdl) We should probably not check every + // pointer and directly resolve these types instead. + if volume.HostDirectory != nil { + return createHostDirectoryVolume(volume), nil + } else { + return nil, errors.New("Unsupported volume type.") + } +} diff --git a/pkg/volume/volume_test.go b/pkg/volume/volume_test.go new file mode 100644 index 00000000000..280beba160e --- /dev/null +++ b/pkg/volume/volume_test.go @@ -0,0 +1,40 @@ +opyright 2014 Google Inc. All rights reserved. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package volumes + +import ( + "testing" + + "github.com/GoogleCloudPlatform/kubernetes/pkg/api" +) + +func TestCreateVolumes(t *testing.T) { + volumes := []api.Volume{ + { + Name: "host-dir", + HostDirectory: &api.HostDirectory{"/dir/path"}, + }, + } + expectedPaths := []string{"/dir/path"} + for i, volume := range volumes { + extVolume, _ := CreateVolume(&volume) + expectedPath := expectedPaths[i] + path := extVolume.GetPath() + if expectedPath != path { + t.Errorf("Unexpected bind path. Expected %v, got %v", expectedPath, path) + } + } +} From f1a7850454ffb4363f80b2b3ed6fe9ec5af1b50a Mon Sep 17 00:00:00 2001 From: Danny Jones Date: Wed, 16 Jul 2014 12:30:51 -0700 Subject: [PATCH 2/3] Adds EmptyDirectory volume struct Adds EmptyDirectory to volumes. This represents a directory on the host, given to a pod that should not persist beyond. The current draft does not cleanup after itself. --- pkg/volume/volume.go | 67 +++++++++++++++++++++++++++++---------- pkg/volume/volume_test.go | 5 +-- 2 files changed, 53 insertions(+), 19 deletions(-) diff --git a/pkg/volume/volume.go b/pkg/volume/volume.go index 878ce37ed7a..359e1487c97 100644 --- a/pkg/volume/volume.go +++ b/pkg/volume/volume.go @@ -14,10 +14,12 @@ See the License for the specific language governing permissions and limitations under the License. */ -package volumes +package volume import ( "errors" + "fmt" + "os" "github.com/GoogleCloudPlatform/kubernetes/pkg/api" ) @@ -32,33 +34,64 @@ type Interface interface { TearDown() } -// Host Directory Volumes represent a bare host directory mount. -type HostDirectoryVolume struct { +// Host Directory volumes represent a bare host directory mount. +// The directory in Path will be directly exposed to the container. +type HostDirectory struct { Path string } -// Simple host directory mounts require no setup or cleanup, but still +// Host directory mounts require no setup or cleanup, but still // need to fulfill the interface definitions. -func (hostVol *HostDirectoryVolume) SetUp() {} +func (hostVol *HostDirectory) SetUp() {} -func (hostVol *HostDirectoryVolume) TearDown() {} +func (hostVol *HostDirectory) TearDown() {} -func (hostVol *HostDirectoryVolume) GetPath() string { +func (hostVol *HostDirectory) GetPath() string { return hostVol.Path } -// Interprets API volume as a HostDirectory -func createHostDirectoryVolume(volume *api.Volume) *HostDirectoryVolume { - return &HostDirectoryVolume{volume.HostDirectory.Path} +// 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 } -// Interprets parameters passed in the API as an internal structure -// with utility procedures for mounting. -func CreateVolume(volume *api.Volume) (Interface, error) { - // TODO(jonesdl) We should probably not check every - // pointer and directly resolve these types instead. - if volume.HostDirectory != nil { - return createHostDirectoryVolume(volume), nil +// SetUp creates the new directory. +func (emptyDir *EmptyDirectory) SetUp() { + os.MkdirAll(emptyDir.GetPath(), 0750) +} + +// TODO(jonesdl) when we can properly invoke TearDown(), we should delete +// the directory created by SetUp. +func (emptyDir *EmptyDirectory) TearDown() {} + +func (emptyDir *EmptyDirectory) GetPath() string { + // TODO(jonesdl) We will want to add a flag to designate a root + // 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} +} + +// Interprets API volume as an EmptyDirectory +func createEmptyDirectory(volume *api.Volume, podID string) *EmptyDirectory { + return &EmptyDirectory{volume.Name, podID} +} + +// CreateVolume returns an Interface capable of mounting a volume described by an +// *api.Volume, or an error. +func CreateVolume(volume *api.Volume, podID string) (Interface, error) { + // 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 + } else if source.EmptyDirectory != nil { + return createEmptyDirectory(volume, podID), nil } else { return nil, errors.New("Unsupported volume type.") } diff --git a/pkg/volume/volume_test.go b/pkg/volume/volume_test.go index 280beba160e..625a488438d 100644 --- a/pkg/volume/volume_test.go +++ b/pkg/volume/volume_test.go @@ -1,4 +1,5 @@ -opyright 2014 Google Inc. All rights reserved. +/* +Copyright 2014 Google Inc. All rights reserved. Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -13,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package volumes +package volume import ( "testing" From bb2843498df6d534741fa7554bc36502dc9c603a Mon Sep 17 00:00:00 2001 From: Danny Jones Date: Wed, 16 Jul 2014 12:32:59 -0700 Subject: [PATCH 3/3] 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 | 20 +++++++++++++++++--- pkg/api/validation.go | 37 ++++++++++++++++++++++++++++--------- pkg/api/validation_test.go | 13 +++++++------ pkg/kubelet/kubelet.go | 22 ++++++++++++++-------- pkg/kubelet/kubelet_test.go | 10 +++++----- pkg/volume/doc.go | 2 +- pkg/volume/volume.go | 26 ++++++++++++++++++++------ pkg/volume/volume_test.go | 15 ++++++++++++--- 8 files changed, 104 insertions(+), 41 deletions(-) diff --git a/pkg/api/types.go b/pkg/api/types.go index 3ad179e3b0d..29dc5fc5a9c 100644 --- a/pkg/api/types.go +++ b/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. - // The path of the directory will be specified in the VolumeMount struct in this case. + // EmptyDirectory represents a temporary directory that shares a pod's lifetime. + 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 diff --git a/pkg/api/validation.go b/pkg/api/validation.go index 92dd44e40b4..8b7c76609c3 100644 --- a/pkg/api/validation.go +++ b/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 := validateHostDir(vol.HostDirectory) - allErrs.Append(errs...) + errs := errorList{} + // TODO(thockin) enforce that a source is set once we deprecate the implied form. + 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)) - } else { + errs.Append(makeDuplicateError("Volume.Name", vol.Name)) + } + 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 } diff --git a/pkg/api/validation_test.go b/pkg/api/validation_test.go index eabd0cba37a..e7a3d271e6c 100644 --- a/pkg/api/validation_test.go +++ b/pkg/api/validation_test.go @@ -25,15 +25,16 @@ import ( func TestValidateVolumes(t *testing.T) { successCase := []Volume{ - {Name: "abc", HostDirectory: &HostDirectory{"/mnt/path1"}}, - {Name: "123", HostDirectory: &HostDirectory{"/mnt/path2"}}, - {Name: "abc-123", HostDirectory: &HostDirectory{"/mnt/path3"}}, + {Name: "abc"}, + {Name: "123", Source: &VolumeSource{HostDirectory: &HostDirectory{"/mnt/path2"}}}, + {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"}}, - {Name: "vol2", HostDirectory: &HostDirectory{"/mnt/vol2"}}}, + Volumes: []Volume{{Name: "vol1", Source: &VolumeSource{HostDirectory: &HostDirectory{"/mnt/vol1"}}}, + {Name: "vol2", Source: &VolumeSource{HostDirectory: &HostDirectory{"/mnt/vol2"}}}}, Containers: []Container{ { Name: "abc", diff --git a/pkg/kubelet/kubelet.go b/pkg/kubelet/kubelet.go index 65e3644d2bf..4befbbc5f41 100644 --- a/pkg/kubelet/kubelet.go +++ b/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 - // Only prepare the volume if it is not already mounted. - if _, err := os.Stat(extVolume.GetPath()); os.IsNotExist(err) { - extVolume.SetUp() + // TODO(jonesdl) When the default volume behavior is no longer supported, this case + // should never occur and an error should be thrown instead. + if extVolume == nil { + 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) diff --git a/pkg/kubelet/kubelet_test.go b/pkg/kubelet/kubelet_test.go index 26dae7911db..71bdf0060b6 100644 --- a/pkg/kubelet/kubelet_test.go +++ b/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["host-dir"] = &volumes.HostDirectoryVolume{"/dir/path"} + expectedPodVolumes := make(volumeMap) + 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{ diff --git a/pkg/volume/doc.go b/pkg/volume/doc.go index e0f8d95904c..dc6c5a49bb0 100644 --- a/pkg/volume/doc.go +++ b/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 diff --git a/pkg/volume/volume.go b/pkg/volume/volume.go index 359e1487c97..05a590cbcb6 100644 --- a/pkg/volume/volume.go +++ b/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 } diff --git a/pkg/volume/volume_test.go b/pkg/volume/volume_test.go index 625a488438d..b28400f5a36 100644 --- a/pkg/volume/volume_test.go +++ b/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 {