From c884297990712c346daa4ff6938d5df7df54167b Mon Sep 17 00:00:00 2001 From: Paul Morie Date: Wed, 27 Jul 2016 17:53:40 -0400 Subject: [PATCH 1/2] Fix collisions issues / timeouts for mounts For non-attachable volumes, do not call GetVolumeName on the plugin and instead generate a unique name based on the identity of the pod and the name of the volume within the pod. --- .../cache/actual_state_of_world.go | 2 +- .../cache/actual_state_of_world.go | 44 ++++---- .../cache/actual_state_of_world_test.go | 104 +++++++++++++----- .../cache/desired_state_of_world.go | 31 ++++-- pkg/volume/git_repo/git_repo.go | 2 +- .../operationexecutor/operation_executor.go | 25 +++-- pkg/volume/util/volumehelper/volumehelper.go | 6 + 7 files changed, 144 insertions(+), 70 deletions(-) diff --git a/pkg/controller/volume/attachdetach/cache/actual_state_of_world.go b/pkg/controller/volume/attachdetach/cache/actual_state_of_world.go index 6a33e1d97d7..52df9cb36c6 100644 --- a/pkg/controller/volume/attachdetach/cache/actual_state_of_world.go +++ b/pkg/controller/volume/attachdetach/cache/actual_state_of_world.go @@ -224,7 +224,7 @@ type nodeToUpdateStatusFor struct { } func (asw *actualStateOfWorld) MarkVolumeAsAttached( - volumeSpec *volume.Spec, nodeName string, devicePath string) error { + _ api.UniqueVolumeName, volumeSpec *volume.Spec, nodeName string, devicePath string) error { _, err := asw.AddVolumeNode(volumeSpec, nodeName, devicePath) return err } diff --git a/pkg/kubelet/volumemanager/cache/actual_state_of_world.go b/pkg/kubelet/volumemanager/cache/actual_state_of_world.go index 15f2d1a47d2..d3906a52e3f 100644 --- a/pkg/kubelet/volumemanager/cache/actual_state_of_world.go +++ b/pkg/kubelet/volumemanager/cache/actual_state_of_world.go @@ -51,14 +51,6 @@ type ActualStateOfWorld interface { // operationexecutor to interact with it. operationexecutor.ActualStateOfWorldAttacherUpdater - // AddVolume adds the given volume to the cache indicating the specified - // volume is attached to this node. A unique volume name is generated from - // the volumeSpec and returned on success. - // If a volume with the same generated name already exists, this is a noop. - // If no volume plugin can support the given volumeSpec or more than one - // plugin can support it, an error is returned. - AddVolume(volumeSpec *volume.Spec, devicePath string) (api.UniqueVolumeName, error) - // AddPodToVolume adds the given pod to the given volume in the cache // indicating the specified volume has been successfully mounted to the // specified pod. @@ -274,9 +266,8 @@ type mountedPod struct { } func (asw *actualStateOfWorld) MarkVolumeAsAttached( - volumeSpec *volume.Spec, nodeName string, devicePath string) error { - _, err := asw.AddVolume(volumeSpec, devicePath) - return err + volumeName api.UniqueVolumeName, volumeSpec *volume.Spec, _, devicePath string) error { + return asw.addVolume(volumeName, volumeSpec, devicePath) } func (asw *actualStateOfWorld) MarkVolumeAsDetached( @@ -315,27 +306,34 @@ func (asw *actualStateOfWorld) MarkDeviceAsUnmounted( return asw.SetVolumeGloballyMounted(volumeName, false /* globallyMounted */) } -func (asw *actualStateOfWorld) AddVolume( - volumeSpec *volume.Spec, devicePath string) (api.UniqueVolumeName, error) { +// addVolume adds the given volume to the cache indicating the specified +// volume is attached to this node. If no volume name is supplied, a unique +// volume name is generated from the volumeSpec and returned on success. If a +// volume with the same generated name already exists, this is a noop. If no +// volume plugin can support the given volumeSpec or more than one plugin can +// support it, an error is returned. +func (asw *actualStateOfWorld) addVolume( + volumeName api.UniqueVolumeName, volumeSpec *volume.Spec, devicePath string) error { asw.Lock() defer asw.Unlock() volumePlugin, err := asw.volumePluginMgr.FindPluginBySpec(volumeSpec) if err != nil || volumePlugin == nil { - return "", fmt.Errorf( + return fmt.Errorf( "failed to get Plugin from volumeSpec for volume %q err=%v", volumeSpec.Name(), err) } - volumeName, err := - volumehelper.GetUniqueVolumeNameFromSpec(volumePlugin, volumeSpec) - if err != nil { - return "", fmt.Errorf( - "failed to GetUniqueVolumeNameFromSpec for volumeSpec %q using volume plugin %q err=%v", - volumeSpec.Name(), - volumePlugin.GetPluginName(), - err) + if len(volumeName) == 0 { + volumeName, err = volumehelper.GetUniqueVolumeNameFromSpec(volumePlugin, volumeSpec) + if err != nil { + return fmt.Errorf( + "failed to GetUniqueVolumeNameFromSpec for volumeSpec %q using volume plugin %q err=%v", + volumeSpec.Name(), + volumePlugin.GetPluginName(), + err) + } } pluginIsAttachable := false @@ -357,7 +355,7 @@ func (asw *actualStateOfWorld) AddVolume( asw.attachedVolumes[volumeName] = volumeObj } - return volumeObj.volumeName, nil + return nil } func (asw *actualStateOfWorld) AddPodToVolume( diff --git a/pkg/kubelet/volumemanager/cache/actual_state_of_world_test.go b/pkg/kubelet/volumemanager/cache/actual_state_of_world_test.go index 1a26860563d..d38fbe3e7fd 100644 --- a/pkg/kubelet/volumemanager/cache/actual_state_of_world_test.go +++ b/pkg/kubelet/volumemanager/cache/actual_state_of_world_test.go @@ -26,12 +26,14 @@ import ( "k8s.io/kubernetes/pkg/volume/util/volumehelper" ) -// Calls AddVolume() once to add volume +var emptyVolumeName = api.UniqueVolumeName("") + +// Calls MarkVolumeAsAttached() once to add volume // Verifies newly added volume exists in GetUnmountedVolumes() // Verifies newly added volume doesn't exist in GetGloballyMountedVolumes() -func Test_AddVolume_Positive_NewVolume(t *testing.T) { +func Test_MarkVolumeAsAttached_Positive_NewVolume(t *testing.T) { // Arrange - volumePluginMgr, _ := volumetesting.GetTestVolumePluginMgr(t) + volumePluginMgr, plugin := volumetesting.GetTestVolumePluginMgr(t) asw := NewActualStateOfWorld("mynode" /* nodeName */, volumePluginMgr) pod := &api.Pod{ ObjectMeta: api.ObjectMeta{ @@ -53,13 +55,14 @@ func Test_AddVolume_Positive_NewVolume(t *testing.T) { } volumeSpec := &volume.Spec{Volume: &pod.Spec.Volumes[0]} devicePath := "fake/device/path" + generatedVolumeName, _ := volumehelper.GetUniqueVolumeNameFromSpec(plugin, volumeSpec) // Act - generatedVolumeName, err := asw.AddVolume(volumeSpec, devicePath) + err := asw.MarkVolumeAsAttached(emptyVolumeName, volumeSpec, "" /* nodeName */, devicePath) // Assert if err != nil { - t.Fatalf("AddVolume failed. Expected: Actual: <%v>", err) + t.Fatalf("MarkVolumeAsAttached failed. Expected: Actual: <%v>", err) } verifyVolumeExistsAsw(t, generatedVolumeName, true /* shouldExist */, asw) @@ -67,13 +70,57 @@ func Test_AddVolume_Positive_NewVolume(t *testing.T) { verifyVolumeDoesntExistInGloballyMountedVolumes(t, generatedVolumeName, asw) } -// Calls AddVolume() twice to add the same volume +// Calls MarkVolumeAsAttached() once to add volume, specifying a name -- +// establishes that the supplied volume name is used to register the volume +// rather than the generated one. +// Verifies newly added volume exists in GetUnmountedVolumes() +// Verifies newly added volume doesn't exist in GetGloballyMountedVolumes() +func Test_MarkVolumeAsAttached_SuppliedVolumeName_Positive_NewVolume(t *testing.T) { + // Arrange + volumePluginMgr, _ := volumetesting.GetTestVolumePluginMgr(t) + asw := NewActualStateOfWorld("mynode" /* nodeName */, volumePluginMgr) + pod := &api.Pod{ + ObjectMeta: api.ObjectMeta{ + Name: "pod1", + UID: "pod1uid", + }, + Spec: api.PodSpec{ + Volumes: []api.Volume{ + { + Name: "volume-name", + VolumeSource: api.VolumeSource{ + GCEPersistentDisk: &api.GCEPersistentDiskVolumeSource{ + PDName: "fake-device1", + }, + }, + }, + }, + }, + } + volumeSpec := &volume.Spec{Volume: &pod.Spec.Volumes[0]} + devicePath := "fake/device/path" + volumeName := api.UniqueVolumeName("this-would-never-be-a-volume-name") + + // Act + err := asw.MarkVolumeAsAttached(volumeName, volumeSpec, "" /* nodeName */, devicePath) + + // Assert + if err != nil { + t.Fatalf("MarkVolumeAsAttached failed. Expected: Actual: <%v>", err) + } + + verifyVolumeExistsAsw(t, volumeName, true /* shouldExist */, asw) + verifyVolumeExistsInUnmountedVolumes(t, volumeName, asw) + verifyVolumeDoesntExistInGloballyMountedVolumes(t, volumeName, asw) +} + +// Calls MarkVolumeAsAttached() twice to add the same volume // Verifies second call doesn't fail // Verifies newly added volume exists in GetUnmountedVolumes() // Verifies newly added volume doesn't exist in GetGloballyMountedVolumes() -func Test_AddVolume_Positive_ExistingVolume(t *testing.T) { +func Test_MarkVolumeAsAttached_Positive_ExistingVolume(t *testing.T) { // Arrange - volumePluginMgr, _ := volumetesting.GetTestVolumePluginMgr(t) + volumePluginMgr, plugin := volumetesting.GetTestVolumePluginMgr(t) devicePath := "fake/device/path" asw := NewActualStateOfWorld("mynode" /* nodeName */, volumePluginMgr) pod := &api.Pod{ @@ -94,19 +141,20 @@ func Test_AddVolume_Positive_ExistingVolume(t *testing.T) { }, }, } - volumeSpec := &volume.Spec{Volume: &pod.Spec.Volumes[0]} - generatedVolumeName, err := asw.AddVolume(volumeSpec, devicePath) + generatedVolumeName, _ := volumehelper.GetUniqueVolumeNameFromSpec(plugin, volumeSpec) + + err := asw.MarkVolumeAsAttached(emptyVolumeName, volumeSpec, "" /* nodeName */, devicePath) if err != nil { - t.Fatalf("AddVolume failed. Expected: Actual: <%v>", err) + t.Fatalf("MarkVolumeAsAttached failed. Expected: Actual: <%v>", err) } // Act - generatedVolumeName, err = asw.AddVolume(volumeSpec, devicePath) + err = asw.MarkVolumeAsAttached(emptyVolumeName, volumeSpec, "" /* nodeName */, devicePath) // Assert if err != nil { - t.Fatalf("AddVolume failed. Expected: Actual: <%v>", err) + t.Fatalf("MarkVolumeAsAttached failed. Expected: Actual: <%v>", err) } verifyVolumeExistsAsw(t, generatedVolumeName, true /* shouldExist */, asw) @@ -141,14 +189,12 @@ func Test_AddPodToVolume_Positive_ExistingVolumeNewNode(t *testing.T) { }, }, } - volumeSpec := &volume.Spec{Volume: &pod.Spec.Volumes[0]} - volumeName, err := volumehelper.GetUniqueVolumeNameFromSpec( - plugin, volumeSpec) + generatedVolumeName, err := volumehelper.GetUniqueVolumeNameFromSpec(plugin, volumeSpec) - generatedVolumeName, err := asw.AddVolume(volumeSpec, devicePath) + err = asw.MarkVolumeAsAttached(emptyVolumeName, volumeSpec, "" /* nodeName */, devicePath) if err != nil { - t.Fatalf("AddVolume failed. Expected: Actual: <%v>", err) + t.Fatalf("MarkVolumeAsAttached failed. Expected: Actual: <%v>", err) } podName := volumehelper.GetUniquePodName(pod) @@ -159,7 +205,7 @@ func Test_AddPodToVolume_Positive_ExistingVolumeNewNode(t *testing.T) { // Act err = asw.AddPodToVolume( - podName, pod.UID, volumeName, mounter, volumeSpec.Name(), "" /* volumeGidValue */) + podName, pod.UID, generatedVolumeName, mounter, volumeSpec.Name(), "" /* volumeGidValue */) // Assert if err != nil { @@ -202,12 +248,12 @@ func Test_AddPodToVolume_Positive_ExistingVolumeExistingNode(t *testing.T) { } volumeSpec := &volume.Spec{Volume: &pod.Spec.Volumes[0]} - volumeName, err := volumehelper.GetUniqueVolumeNameFromSpec( + generatedVolumeName, err := volumehelper.GetUniqueVolumeNameFromSpec( plugin, volumeSpec) - generatedVolumeName, err := asw.AddVolume(volumeSpec, devicePath) + err = asw.MarkVolumeAsAttached(emptyVolumeName, volumeSpec, "" /* nodeName */, devicePath) if err != nil { - t.Fatalf("AddVolume failed. Expected: Actual: <%v>", err) + t.Fatalf("MarkVolumeAsAttached failed. Expected: Actual: <%v>", err) } podName := volumehelper.GetUniquePodName(pod) @@ -217,14 +263,14 @@ func Test_AddPodToVolume_Positive_ExistingVolumeExistingNode(t *testing.T) { } err = asw.AddPodToVolume( - podName, pod.UID, volumeName, mounter, volumeSpec.Name(), "" /* volumeGidValue */) + podName, pod.UID, generatedVolumeName, mounter, volumeSpec.Name(), "" /* volumeGidValue */) if err != nil { t.Fatalf("AddPodToVolume failed. Expected: Actual: <%v>", err) } // Act err = asw.AddPodToVolume( - podName, pod.UID, volumeName, mounter, volumeSpec.Name(), "" /* volumeGidValue */) + podName, pod.UID, generatedVolumeName, mounter, volumeSpec.Name(), "" /* volumeGidValue */) // Assert if err != nil { @@ -301,13 +347,13 @@ func Test_AddPodToVolume_Negative_VolumeDoesntExist(t *testing.T) { asw) } -// Calls AddVolume() once to add volume +// Calls MarkVolumeAsAttached() once to add volume // Calls MarkDeviceAsMounted() to mark volume as globally mounted. // Verifies newly added volume exists in GetUnmountedVolumes() // Verifies newly added volume exists in GetGloballyMountedVolumes() func Test_MarkDeviceAsMounted_Positive_NewVolume(t *testing.T) { // Arrange - volumePluginMgr, _ := volumetesting.GetTestVolumePluginMgr(t) + volumePluginMgr, plugin := volumetesting.GetTestVolumePluginMgr(t) asw := NewActualStateOfWorld("mynode" /* nodeName */, volumePluginMgr) pod := &api.Pod{ ObjectMeta: api.ObjectMeta{ @@ -329,9 +375,11 @@ func Test_MarkDeviceAsMounted_Positive_NewVolume(t *testing.T) { } volumeSpec := &volume.Spec{Volume: &pod.Spec.Volumes[0]} devicePath := "fake/device/path" - generatedVolumeName, err := asw.AddVolume(volumeSpec, devicePath) + generatedVolumeName, err := volumehelper.GetUniqueVolumeNameFromSpec(plugin, volumeSpec) + + err = asw.MarkVolumeAsAttached(emptyVolumeName, volumeSpec, "" /* nodeName */, devicePath) if err != nil { - t.Fatalf("AddVolume failed. Expected: Actual: <%v>", err) + t.Fatalf("MarkVolumeAsAttached failed. Expected: Actual: <%v>", err) } // Act diff --git a/pkg/kubelet/volumemanager/cache/desired_state_of_world.go b/pkg/kubelet/volumemanager/cache/desired_state_of_world.go index bdcac83babf..24edd5906c0 100644 --- a/pkg/kubelet/volumemanager/cache/desired_state_of_world.go +++ b/pkg/kubelet/volumemanager/cache/desired_state_of_world.go @@ -183,14 +183,27 @@ func (dsw *desiredStateOfWorld) AddPodToVolume( err) } - volumeName, err := - volumehelper.GetUniqueVolumeNameFromSpec(volumePlugin, volumeSpec) - if err != nil { - return "", fmt.Errorf( - "failed to GetUniqueVolumeNameFromSpec for volumeSpec %q using volume plugin %q err=%v", - volumeSpec.Name(), - volumePlugin.GetPluginName(), - err) + var volumeName api.UniqueVolumeName + + // The unique volume name used depends on whether the volume is attachable + // or not. + attachable := dsw.isAttachableVolume(volumeSpec) + if attachable { + // For attachable volumes, use the unique volume name as reported by + // the plugin. + volumeName, err = + volumehelper.GetUniqueVolumeNameFromSpec(volumePlugin, volumeSpec) + if err != nil { + return "", fmt.Errorf( + "failed to GetUniqueVolumeNameFromSpec for volumeSpec %q using volume plugin %q err=%v", + volumeSpec.Name(), + volumePlugin.GetPluginName(), + err) + } + } else { + // For non-attachable volumes, generate a unique name based on the pod + // namespace and name and the name of the volume within the pod. + volumeName = volumehelper.GetUniqueVolumeNameForNonAttachableVolume(podName, volumePlugin, outerVolumeSpecName) } volumeObj, volumeExists := dsw.volumesToMount[volumeName] @@ -198,7 +211,7 @@ func (dsw *desiredStateOfWorld) AddPodToVolume( volumeObj = volumeToMount{ volumeName: volumeName, podsToMount: make(map[types.UniquePodName]podToMount), - pluginIsAttachable: dsw.isAttachableVolume(volumeSpec), + pluginIsAttachable: attachable, volumeGidValue: volumeGidValue, reportedInUse: false, } diff --git a/pkg/volume/git_repo/git_repo.go b/pkg/volume/git_repo/git_repo.go index 09d9300dacf..754d642cd9c 100644 --- a/pkg/volume/git_repo/git_repo.go +++ b/pkg/volume/git_repo/git_repo.go @@ -61,7 +61,7 @@ func (plugin *gitRepoPlugin) GetPluginName() string { func (plugin *gitRepoPlugin) GetVolumeName(spec *volume.Spec) (string, error) { volumeSource, _ := getVolumeSource(spec) if volumeSource == nil { - return "", fmt.Errorf("Spec does not reference a GCE volume type") + return "", fmt.Errorf("Spec does not reference a Git repo volume type") } return fmt.Sprintf( diff --git a/pkg/volume/util/operationexecutor/operation_executor.go b/pkg/volume/util/operationexecutor/operation_executor.go index 5475c2b7b68..28c1e57d750 100644 --- a/pkg/volume/util/operationexecutor/operation_executor.go +++ b/pkg/volume/util/operationexecutor/operation_executor.go @@ -15,8 +15,9 @@ limitations under the License. */ // Package operationexecutor implements interfaces that enable execution of -// attach, detach, mount, and unmount operations with a nestedpendingoperations -// so that more than one operation is never triggered on the same volume. +// attach, detach, mount, and unmount operations with a +// nestedpendingoperations so that more than one operation is never triggered +// on the same volume for the same pod. package operationexecutor import ( @@ -131,8 +132,14 @@ type ActualStateOfWorldMounterUpdater interface { // ActualStateOfWorldAttacherUpdater defines a set of operations updating the // actual state of the world cache after successful attach/detach/mount/unmount. type ActualStateOfWorldAttacherUpdater interface { - // Marks the specified volume as attached to the specified node - MarkVolumeAsAttached(volumeSpec *volume.Spec, nodeName string, devicePath string) error + // Marks the specified volume as attached to the specified node. If the + // volume name is supplied, that volume name will be used. If not, the + // volume name is computed using the result from querying the plugin. + // + // TODO: in the future, we should be able to remove the volumeName + // argument to this method -- since it is used only for attachable + // volumes. See issue 29695. + MarkVolumeAsAttached(volumeName api.UniqueVolumeName, volumeSpec *volume.Spec, nodeName, devicePath string) error // Marks the specified volume as detached from the specified node MarkVolumeAsDetached(volumeName api.UniqueVolumeName, nodeName string) @@ -370,6 +377,7 @@ func (oe *operationExecutor) MountVolume( } podName := volumetypes.UniquePodName("") + // TODO: remove this -- not necessary if !volumeToMount.PluginIsAttachable { // Non-attachable volume plugins can execute mount for multiple pods // referencing the same volume in parallel @@ -473,7 +481,7 @@ func (oe *operationExecutor) generateAttachVolumeFunc( // Update actual state of world addVolumeNodeErr := actualStateOfWorld.MarkVolumeAsAttached( - volumeToAttach.VolumeSpec, volumeToAttach.NodeName, devicePath) + api.UniqueVolumeName(""), volumeToAttach.VolumeSpec, volumeToAttach.NodeName, devicePath) if addVolumeNodeErr != nil { // On failure, return error. Caller will log and retry. return fmt.Errorf( @@ -931,12 +939,13 @@ func (oe *operationExecutor) generateVerifyControllerAttachedVolumeFunc( // If the volume does not implement the attacher interface, it is // assumed to be attached and the the actual state of the world is // updated accordingly. + addVolumeNodeErr := actualStateOfWorld.MarkVolumeAsAttached( - volumeToMount.VolumeSpec, nodeName, volumeToMount.DevicePath) + volumeToMount.VolumeName, volumeToMount.VolumeSpec, nodeName, "" /* devicePath */) if addVolumeNodeErr != nil { // On failure, return error. Caller will log and retry. return fmt.Errorf( - "VerifyControllerAttachedVolume.MarkVolumeAsAttached failed for volume %q (spec.Name: %q) pod %q (UID: %q) with: %v.", + "VerifyControllerAttachedVolume.MarkVolumeAsAttachedByUniqueVolumeName failed for volume %q (spec.Name: %q) pod %q (UID: %q) with: %v.", volumeToMount.VolumeName, volumeToMount.VolumeSpec.Name(), volumeToMount.PodName, @@ -987,7 +996,7 @@ func (oe *operationExecutor) generateVerifyControllerAttachedVolumeFunc( for _, attachedVolume := range node.Status.VolumesAttached { if attachedVolume.Name == volumeToMount.VolumeName { addVolumeNodeErr := actualStateOfWorld.MarkVolumeAsAttached( - volumeToMount.VolumeSpec, nodeName, attachedVolume.DevicePath) + api.UniqueVolumeName(""), volumeToMount.VolumeSpec, nodeName, attachedVolume.DevicePath) glog.Infof("Controller successfully attached volume %q (spec.Name: %q) pod %q (UID: %q) devicePath: %q", volumeToMount.VolumeName, volumeToMount.VolumeSpec.Name(), diff --git a/pkg/volume/util/volumehelper/volumehelper.go b/pkg/volume/util/volumehelper/volumehelper.go index fd7fa9c5188..2ab765e3cd9 100644 --- a/pkg/volume/util/volumehelper/volumehelper.go +++ b/pkg/volume/util/volumehelper/volumehelper.go @@ -52,6 +52,12 @@ func GetUniqueVolumeName(pluginName, volumeName string) api.UniqueVolumeName { return api.UniqueVolumeName(fmt.Sprintf("%s/%s", pluginName, volumeName)) } +// GetUniqueVolumeNameForNonAttachableVolume returns the unique volume name +// for a non-attachable volume. +func GetUniqueVolumeNameForNonAttachableVolume(podName types.UniquePodName, volumePlugin volume.VolumePlugin, podSpecName string) api.UniqueVolumeName { + return api.UniqueVolumeName(fmt.Sprintf("%s/%v-%s", volumePlugin.GetPluginName(), podName, podSpecName)) +} + // GetUniqueVolumeNameFromSpec uses the given VolumePlugin to generate a unique // name representing the volume defined in the specified volume spec. // This returned name can be used to uniquely reference the actual backing From f15684e0c4aace9fac7e1dbf0ed3ce3d32c3eb8a Mon Sep 17 00:00:00 2001 From: Paul Morie Date: Wed, 27 Jul 2016 12:08:17 -0400 Subject: [PATCH 2/2] Add e2e coverage for mount collisions --- test/e2e/configmap.go | 79 +++++++++++++++++++++++++++++- test/e2e/secrets.go | 111 ++++++++++++++++++++++++++++++++++-------- 2 files changed, 170 insertions(+), 20 deletions(-) diff --git a/test/e2e/configmap.go b/test/e2e/configmap.go index ac216b96790..87490ee0079 100644 --- a/test/e2e/configmap.go +++ b/test/e2e/configmap.go @@ -29,7 +29,6 @@ import ( ) var _ = framework.KubeDescribe("ConfigMap", func() { - f := framework.NewDefaultFramework("configmap") It("should be consumable from pods in volume [Conformance]", func() { @@ -200,6 +199,84 @@ var _ = framework.KubeDescribe("ConfigMap", func() { "CONFIG_DATA_1=value-1", }, f.Namespace.Name) }) + + It("should be consumable in multiple volumes in the same pod", func() { + var ( + name = "configmap-test-volume-" + string(util.NewUUID()) + volumeName = "configmap-volume" + volumeMountPath = "/etc/configmap-volume" + volumeName2 = "configmap-volume-2" + volumeMountPath2 = "/etc/configmap-volume-2" + configMap = newConfigMap(f, name) + ) + + By(fmt.Sprintf("Creating configMap with name %s", configMap.Name)) + defer func() { + By("Cleaning up the configMap") + if err := f.Client.ConfigMaps(f.Namespace.Name).Delete(configMap.Name); err != nil { + framework.Failf("unable to delete configMap %v: %v", configMap.Name, err) + } + }() + var err error + if configMap, err = f.Client.ConfigMaps(f.Namespace.Name).Create(configMap); err != nil { + framework.Failf("unable to create test configMap %s: %v", configMap.Name, err) + } + + pod := &api.Pod{ + ObjectMeta: api.ObjectMeta{ + Name: "pod-configmaps-" + string(util.NewUUID()), + }, + Spec: api.PodSpec{ + Volumes: []api.Volume{ + { + Name: volumeName, + VolumeSource: api.VolumeSource{ + ConfigMap: &api.ConfigMapVolumeSource{ + LocalObjectReference: api.LocalObjectReference{ + Name: name, + }, + }, + }, + }, + { + Name: volumeName2, + VolumeSource: api.VolumeSource{ + ConfigMap: &api.ConfigMapVolumeSource{ + LocalObjectReference: api.LocalObjectReference{ + Name: name, + }, + }, + }, + }, + }, + Containers: []api.Container{ + { + Name: "configmap-volume-test", + Image: "gcr.io/google_containers/mounttest:0.6", + Args: []string{"--file_content=/etc/configmap-volume/data-1"}, + VolumeMounts: []api.VolumeMount{ + { + Name: volumeName, + MountPath: volumeMountPath, + ReadOnly: true, + }, + { + Name: volumeName2, + MountPath: volumeMountPath2, + ReadOnly: true, + }, + }, + }, + }, + RestartPolicy: api.RestartPolicyNever, + }, + } + + framework.TestContainerOutput("consume configMaps", f.Client, pod, 0, []string{ + "content of file \"/etc/configmap-volume/data-1\": value-1", + }, f.Namespace.Name) + + }) }) func newConfigMap(f *framework.Framework, name string) *api.ConfigMap { diff --git a/test/e2e/secrets.go b/test/e2e/secrets.go index f501c81f777..960d9194cde 100644 --- a/test/e2e/secrets.go +++ b/test/e2e/secrets.go @@ -33,18 +33,7 @@ var _ = framework.KubeDescribe("Secrets", func() { name := "secret-test-" + string(util.NewUUID()) volumeName := "secret-volume" volumeMountPath := "/etc/secret-volume" - - secret := &api.Secret{ - ObjectMeta: api.ObjectMeta{ - Namespace: f.Namespace.Name, - Name: name, - }, - Data: map[string][]byte{ - "data-1": []byte("value-1\n"), - "data-2": []byte("value-2\n"), - "data-3": []byte("value-3\n"), - }, - } + secret := secretForTest(f.Namespace.Name, name) By(fmt.Sprintf("Creating secret with name %s", secret.Name)) defer func() { @@ -99,19 +88,89 @@ var _ = framework.KubeDescribe("Secrets", func() { }, f.Namespace.Name) }) - It("should be consumable from pods in env vars [Conformance]", func() { - name := "secret-test-" + string(util.NewUUID()) + It("should be consumable in multiple volumes in a pod [Conformance]", func() { + // This test ensures that the same secret can be mounted in multiple + // volumes in the same pod. This test case exists to prevent + // regressions that break this use-case. + var ( + name = "secret-test-" + string(util.NewUUID()) + volumeName = "secret-volume" + volumeMountPath = "/etc/secret-volume" + volumeName2 = "secret-volume-2" + volumeMountPath2 = "/etc/secret-volume-2" + secret = secretForTest(f.Namespace.Name, name) + ) - secret := &api.Secret{ + By(fmt.Sprintf("Creating secret with name %s", secret.Name)) + defer func() { + By("Cleaning up the secret") + if err := f.Client.Secrets(f.Namespace.Name).Delete(secret.Name); err != nil { + framework.Failf("unable to delete secret %v: %v", secret.Name, err) + } + }() + var err error + if secret, err = f.Client.Secrets(f.Namespace.Name).Create(secret); err != nil { + framework.Failf("unable to create test secret %s: %v", secret.Name, err) + } + + pod := &api.Pod{ ObjectMeta: api.ObjectMeta{ - Namespace: f.Namespace.Name, - Name: name, + Name: "pod-secrets-" + string(util.NewUUID()), }, - Data: map[string][]byte{ - "data-1": []byte("value-1"), + Spec: api.PodSpec{ + Volumes: []api.Volume{ + { + Name: volumeName, + VolumeSource: api.VolumeSource{ + Secret: &api.SecretVolumeSource{ + SecretName: name, + }, + }, + }, + { + Name: volumeName2, + VolumeSource: api.VolumeSource{ + Secret: &api.SecretVolumeSource{ + SecretName: name, + }, + }, + }, + }, + Containers: []api.Container{ + { + Name: "secret-volume-test", + Image: "gcr.io/google_containers/mounttest:0.7", + Args: []string{ + "--file_content=/etc/secret-volume/data-1", + "--file_mode=/etc/secret-volume/data-1"}, + VolumeMounts: []api.VolumeMount{ + { + Name: volumeName, + MountPath: volumeMountPath, + ReadOnly: true, + }, + { + Name: volumeName2, + MountPath: volumeMountPath2, + ReadOnly: true, + }, + }, + }, + }, + RestartPolicy: api.RestartPolicyNever, }, } + framework.TestContainerOutput("consume secrets", f.Client, pod, 0, []string{ + "content of file \"/etc/secret-volume/data-1\": value-1", + "mode of file \"/etc/secret-volume/data-1\": -rw-r--r--", + }, f.Namespace.Name) + }) + + It("should be consumable from pods in env vars [Conformance]", func() { + name := "secret-test-" + string(util.NewUUID()) + secret := secretForTest(f.Namespace.Name, name) + By(fmt.Sprintf("Creating secret with name %s", secret.Name)) defer func() { By("Cleaning up the secret") @@ -158,3 +217,17 @@ var _ = framework.KubeDescribe("Secrets", func() { }, f.Namespace.Name) }) }) + +func secretForTest(namespace, name string) *api.Secret { + return &api.Secret{ + ObjectMeta: api.ObjectMeta{ + Namespace: namespace, + Name: name, + }, + Data: map[string][]byte{ + "data-1": []byte("value-1\n"), + "data-2": []byte("value-2\n"), + "data-3": []byte("value-3\n"), + }, + } +}