From f44d1b9b37aa930dda50c239f140f922e3615532 Mon Sep 17 00:00:00 2001 From: Hemant Kumar Date: Wed, 6 Jun 2018 14:06:28 -0400 Subject: [PATCH] Fix setup of configmap/secret/projected/downwardapi Only call setup after they are found; otherwise we are left with orphan directories that are never cleaned up. --- pkg/volume/configmap/configmap.go | 14 +-- pkg/volume/downwardapi/downwardapi.go | 16 +-- pkg/volume/projected/projected.go | 14 +-- pkg/volume/secret/secret.go | 14 +-- test/e2e/storage/BUILD | 1 + test/e2e/storage/ephemeral_volume.go | 138 ++++++++++++++++++++++++++ 6 files changed, 169 insertions(+), 28 deletions(-) create mode 100644 test/e2e/storage/ephemeral_volume.go diff --git a/pkg/volume/configmap/configmap.go b/pkg/volume/configmap/configmap.go index ff041c0ac0..2f108bb72c 100644 --- a/pkg/volume/configmap/configmap.go +++ b/pkg/volume/configmap/configmap.go @@ -191,12 +191,6 @@ func (b *configMapVolumeMounter) SetUpAt(dir string, fsGroup *int64) error { if err != nil { return err } - if err := wrapped.SetUpAt(dir, fsGroup); err != nil { - return err - } - if err := volumeutil.MakeNestedMountpoints(b.volName, dir, b.pod); err != nil { - return err - } optional := b.source.Optional != nil && *b.source.Optional configMap, err := b.getConfigMap(b.pod.Namespace, b.source.Name) @@ -213,6 +207,13 @@ func (b *configMapVolumeMounter) SetUpAt(dir string, fsGroup *int64) error { } } + if err := wrapped.SetUpAt(dir, fsGroup); err != nil { + return err + } + if err := volumeutil.MakeNestedMountpoints(b.volName, dir, b.pod); err != nil { + return err + } + totalBytes := totalBytes(configMap) glog.V(3).Infof("Received configMap %v/%v containing (%v) pieces of data, %v total bytes", b.pod.Namespace, @@ -243,7 +244,6 @@ func (b *configMapVolumeMounter) SetUpAt(dir string, fsGroup *int64) error { glog.Errorf("Error applying volume ownership settings for group: %v", fsGroup) return err } - return nil } diff --git a/pkg/volume/downwardapi/downwardapi.go b/pkg/volume/downwardapi/downwardapi.go index e1c1c536ee..b8a2c8ec57 100644 --- a/pkg/volume/downwardapi/downwardapi.go +++ b/pkg/volume/downwardapi/downwardapi.go @@ -180,13 +180,6 @@ func (b *downwardAPIVolumeMounter) SetUpAt(dir string, fsGroup *int64) error { glog.Errorf("Couldn't setup downwardAPI volume %v for pod %v/%v: %s", b.volName, b.pod.Namespace, b.pod.Name, err.Error()) return err } - if err := wrapped.SetUpAt(dir, fsGroup); err != nil { - glog.Errorf("Unable to setup downwardAPI volume %v for pod %v/%v: %s", b.volName, b.pod.Namespace, b.pod.Name, err.Error()) - return err - } - if err := volumeutil.MakeNestedMountpoints(b.volName, dir, *b.pod); err != nil { - return err - } data, err := CollectData(b.source.Items, b.pod, b.plugin.host, b.source.DefaultMode) if err != nil { @@ -194,6 +187,15 @@ func (b *downwardAPIVolumeMounter) SetUpAt(dir string, fsGroup *int64) error { return err } + if err := wrapped.SetUpAt(dir, fsGroup); err != nil { + glog.Errorf("Unable to setup downwardAPI volume %v for pod %v/%v: %s", b.volName, b.pod.Namespace, b.pod.Name, err.Error()) + return err + } + + if err := volumeutil.MakeNestedMountpoints(b.volName, dir, *b.pod); err != nil { + return err + } + writerContext := fmt.Sprintf("pod %v/%v volume %v", b.pod.Namespace, b.pod.Name, b.volName) writer, err := volumeutil.NewAtomicWriter(dir, writerContext) if err != nil { diff --git a/pkg/volume/projected/projected.go b/pkg/volume/projected/projected.go index 2b55308d47..c17ba4c19f 100644 --- a/pkg/volume/projected/projected.go +++ b/pkg/volume/projected/projected.go @@ -194,18 +194,19 @@ func (s *projectedVolumeMounter) SetUpAt(dir string, fsGroup *int64) error { if err != nil { return err } - if err := wrapped.SetUpAt(dir, fsGroup); err != nil { - return err - } - if err := volumeutil.MakeNestedMountpoints(s.volName, dir, *s.pod); err != nil { - return err - } data, err := s.collectData() if err != nil { glog.Errorf("Error preparing data for projected volume %v for pod %v/%v: %s", s.volName, s.pod.Namespace, s.pod.Name, err.Error()) return err } + if err := wrapped.SetUpAt(dir, fsGroup); err != nil { + return err + } + + if err := volumeutil.MakeNestedMountpoints(s.volName, dir, *s.pod); err != nil { + return err + } writerContext := fmt.Sprintf("pod %v/%v volume %v", s.pod.Namespace, s.pod.Name, s.volName) writer, err := volumeutil.NewAtomicWriter(dir, writerContext) @@ -225,7 +226,6 @@ func (s *projectedVolumeMounter) SetUpAt(dir string, fsGroup *int64) error { glog.Errorf("Error applying volume ownership settings for group: %v", fsGroup) return err } - return nil } diff --git a/pkg/volume/secret/secret.go b/pkg/volume/secret/secret.go index c8146d8442..3ab27c77c4 100644 --- a/pkg/volume/secret/secret.go +++ b/pkg/volume/secret/secret.go @@ -190,12 +190,6 @@ func (b *secretVolumeMounter) SetUpAt(dir string, fsGroup *int64) error { if err != nil { return err } - if err := wrapped.SetUpAt(dir, fsGroup); err != nil { - return err - } - if err := volumeutil.MakeNestedMountpoints(b.volName, dir, b.pod); err != nil { - return err - } optional := b.source.Optional != nil && *b.source.Optional secret, err := b.getSecret(b.pod.Namespace, b.source.SecretName) @@ -212,6 +206,13 @@ func (b *secretVolumeMounter) SetUpAt(dir string, fsGroup *int64) error { } } + if err := wrapped.SetUpAt(dir, fsGroup); err != nil { + return err + } + if err := volumeutil.MakeNestedMountpoints(b.volName, dir, b.pod); err != nil { + return err + } + totalBytes := totalSecretBytes(secret) glog.V(3).Infof("Received secret %v/%v containing (%v) pieces of data, %v total bytes", b.pod.Namespace, @@ -242,7 +243,6 @@ func (b *secretVolumeMounter) SetUpAt(dir string, fsGroup *int64) error { glog.Errorf("Error applying volume ownership settings for group: %v", fsGroup) return err } - return nil } diff --git a/test/e2e/storage/BUILD b/test/e2e/storage/BUILD index fcb91637b0..f0430685ae 100644 --- a/test/e2e/storage/BUILD +++ b/test/e2e/storage/BUILD @@ -6,6 +6,7 @@ go_library( "csi_objects.go", "csi_volumes.go", "empty_dir_wrapper.go", + "ephemeral_volume.go", "flexvolume.go", "generic_persistent_volume-disruptive.go", "mounted_volume_resize.go", diff --git a/test/e2e/storage/ephemeral_volume.go b/test/e2e/storage/ephemeral_volume.go new file mode 100644 index 0000000000..5d1948e4b4 --- /dev/null +++ b/test/e2e/storage/ephemeral_volume.go @@ -0,0 +1,138 @@ +/* +Copyright 2018 The Kubernetes Authors. + +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 storage + +import ( + "fmt" + "strings" + "time" + + "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/rand" + clientset "k8s.io/client-go/kubernetes" + "k8s.io/kubernetes/test/e2e/framework" + "k8s.io/kubernetes/test/e2e/storage/utils" + + . "github.com/onsi/ginkgo" + . "github.com/onsi/gomega" +) + +var _ = utils.SIGDescribe("Ephemeralstorage", func() { + var ( + c clientset.Interface + ) + + f := framework.NewDefaultFramework("pv") + + BeforeEach(func() { + c = f.ClientSet + }) + + Describe("When pod refers to non-existent ephemeral storage", func() { + for _, testSource := range invalidEphemeralSource("pod-ephm-test") { + It(fmt.Sprintf("should allow deletion of pod with invalid volume : %s", testSource.volumeType), func() { + pod := testEphemeralVolumePod(f, testSource.volumeType, testSource.source) + pod, err := c.CoreV1().Pods(f.Namespace.Name).Create(pod) + Expect(err).NotTo(HaveOccurred()) + + // Allow it to sleep for 30 seconds + time.Sleep(30 * time.Second) + framework.Logf("Deleting pod %q/%q", pod.Namespace, pod.Name) + framework.ExpectNoError(framework.DeletePodWithWait(f, c, pod)) + }) + } + }) +}) + +type ephemeralTestInfo struct { + volumeType string + source *v1.VolumeSource +} + +func testEphemeralVolumePod(f *framework.Framework, volumeType string, source *v1.VolumeSource) *v1.Pod { + var ( + suffix = strings.ToLower(fmt.Sprintf("%s-%s", volumeType, rand.String(4))) + ) + return &v1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: fmt.Sprintf("pod-ephm-test-%s", suffix), + Namespace: f.Namespace.Name, + }, + Spec: v1.PodSpec{ + Containers: []v1.Container{ + { + Name: fmt.Sprintf("test-container-subpath-%s", suffix), + Image: mountImage, + VolumeMounts: []v1.VolumeMount{ + { + Name: volumeName, + MountPath: volumePath, + }, + }, + }, + }, + RestartPolicy: v1.RestartPolicyNever, + Volumes: []v1.Volume{ + { + Name: volumeName, + VolumeSource: *source, + }, + }, + }, + } +} + +func invalidEphemeralSource(suffix string) []ephemeralTestInfo { + testInfo := []ephemeralTestInfo{ + { + volumeType: "secret", + source: &v1.VolumeSource{ + Secret: &v1.SecretVolumeSource{ + SecretName: fmt.Sprintf("secert-%s", suffix), + }, + }, + }, + { + volumeType: "configmap", + source: &v1.VolumeSource{ + ConfigMap: &v1.ConfigMapVolumeSource{ + LocalObjectReference: v1.LocalObjectReference{ + Name: fmt.Sprintf("configmap-%s", suffix), + }, + }, + }, + }, + { + volumeType: "projected", + source: &v1.VolumeSource{ + Projected: &v1.ProjectedVolumeSource{ + Sources: []v1.VolumeProjection{ + { + Secret: &v1.SecretProjection{ + LocalObjectReference: v1.LocalObjectReference{ + Name: fmt.Sprintf("secret-%s", suffix), + }, + }, + }, + }, + }, + }, + }, + } + return testInfo +}