Skip to content

Commit

Permalink
Fix setup of configmap/secret/projected/downwardapi
Browse files Browse the repository at this point in the history
Only call setup after they are found; otherwise
we are left with orphan directories that are never
cleaned up.
  • Loading branch information
gnufied committed Jun 7, 2018
1 parent bceb90c commit f44d1b9
Show file tree
Hide file tree
Showing 6 changed files with 167 additions and 26 deletions.
14 changes: 7 additions & 7 deletions pkg/volume/configmap/configmap.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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,
Expand Down Expand Up @@ -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
}

Expand Down
14 changes: 8 additions & 6 deletions pkg/volume/downwardapi/downwardapi.go
Original file line number Diff line number Diff line change
Expand Up @@ -180,17 +180,19 @@ 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())

data, err := CollectData(b.source.Items, b.pod, b.plugin.host, b.source.DefaultMode)
if err != nil {
glog.Errorf("Error preparing data for 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 {

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
}

data, err := CollectData(b.source.Items, b.pod, b.plugin.host, b.source.DefaultMode)
if err != nil {
glog.Errorf("Error preparing data for downwardAPI volume %v for pod %v/%v: %s", b.volName, b.pod.Namespace, b.pod.Name, err.Error())
if err := volumeutil.MakeNestedMountpoints(b.volName, dir, *b.pod); err != nil {
return err
}

Expand Down
12 changes: 6 additions & 6 deletions pkg/volume/projected/projected.go
Original file line number Diff line number Diff line change
Expand Up @@ -194,16 +194,17 @@ func (s *projectedVolumeMounter) SetUpAt(dir string, fsGroup *int64) error {
if err != nil {
return err
}
if err := wrapped.SetUpAt(dir, fsGroup); err != nil {

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 := volumeutil.MakeNestedMountpoints(s.volName, dir, *s.pod); err != nil {
if err := wrapped.SetUpAt(dir, fsGroup); 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())
if err := volumeutil.MakeNestedMountpoints(s.volName, dir, *s.pod); err != nil {
return err
}

Expand All @@ -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
}

Expand Down
14 changes: 7 additions & 7 deletions pkg/volume/secret/secret.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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,
Expand Down Expand Up @@ -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
}

Expand Down
1 change: 1 addition & 0 deletions test/e2e/storage/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
138 changes: 138 additions & 0 deletions test/e2e/storage/ephemeral_volume.go
Original file line number Diff line number Diff line change
@@ -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
}

0 comments on commit f44d1b9

Please sign in to comment.