Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix block volume reconstruction #83451

Merged
merged 3 commits into from
Oct 8, 2019
Merged

Fix block volume reconstruction #83451

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
612fb17
Test global block directory in reconstruction tests
jsafrane Oct 4, 2019
44a006e
Fix volume map path during reconstruction
jsafrane Oct 4, 2019
1c8c861
Reconstruct block PV name in all volume plugins
jsafrane Oct 4, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 3 additions & 1 deletion pkg/kubelet/volumemanager/reconciler/reconciler.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
"io/ioutil"
"os"
"path"
"path/filepath"
"time"

v1 "k8s.io/api/core/v1"
Expand Down Expand Up @@ -518,7 +519,8 @@ func (rc *reconciler) reconstructVolume(volume podVolume) (*reconstructedVolume,
pod.UID,
newMapperErr)
}
checkPath, _ = volumeMapper.GetPodDeviceMapPath()
mapDir, linkName := volumeMapper.GetPodDeviceMapPath()
checkPath = filepath.Join(mapDir, linkName)
} else {
var err error
volumeMounter, err = plugin.NewMounter(
Expand Down
8 changes: 6 additions & 2 deletions pkg/volume/awsebs/aws_ebs_block.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import (
"strings"

v1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/types"
"k8s.io/klog"
"k8s.io/kubernetes/pkg/util/mount"
Expand All @@ -51,10 +52,10 @@ func (plugin *awsElasticBlockStorePlugin) ConstructBlockVolumeSpec(podUID types.
return nil, fmt.Errorf("failed to get volume plugin information from globalMapPathUUID: %v", globalMapPathUUID)
}

return getVolumeSpecFromGlobalMapPath(globalMapPath)
return getVolumeSpecFromGlobalMapPath(volumeName, globalMapPath)
}

func getVolumeSpecFromGlobalMapPath(globalMapPath string) (*volume.Spec, error) {
func getVolumeSpecFromGlobalMapPath(volumeName string, globalMapPath string) (*volume.Spec, error) {
// Get volume spec information from globalMapPath
// globalMapPath example:
// plugins/kubernetes.io/{PluginName}/{DefaultKubeletVolumeDevicesDirName}/{volumeID}
Expand All @@ -68,6 +69,9 @@ func getVolumeSpecFromGlobalMapPath(globalMapPath string) (*volume.Spec, error)
}
block := v1.PersistentVolumeBlock
awsVolume := &v1.PersistentVolume{
ObjectMeta: metav1.ObjectMeta{
Name: volumeName,
},
Spec: v1.PersistentVolumeSpec{
PersistentVolumeSource: v1.PersistentVolumeSource{
AWSElasticBlockStore: &v1.AWSElasticBlockStoreVolumeSource{
Expand Down
7 changes: 5 additions & 2 deletions pkg/volume/awsebs/aws_ebs_block_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,16 +52,19 @@ func TestGetVolumeSpecFromGlobalMapPath(t *testing.T) {
expectedGlobalPath := filepath.Join(tmpVDir, testGlobalPath)

//Bad Path
badspec, err := getVolumeSpecFromGlobalMapPath("")
badspec, err := getVolumeSpecFromGlobalMapPath("", "")
if badspec != nil || err == nil {
t.Fatalf("Expected not to get spec from GlobalMapPath but did")
}

// Good Path
spec, err := getVolumeSpecFromGlobalMapPath(expectedGlobalPath)
spec, err := getVolumeSpecFromGlobalMapPath("myVolume", expectedGlobalPath)
if spec == nil || err != nil {
t.Fatalf("Failed to get spec from GlobalMapPath: %v", err)
}
if spec.PersistentVolume.Name != "myVolume" {
t.Errorf("Invalid PV name from GlobalMapPath spec: %s", spec.PersistentVolume.Name)
}
if spec.PersistentVolume.Spec.AWSElasticBlockStore.VolumeID != testVolName {
t.Errorf("Invalid volumeID from GlobalMapPath spec: %s", spec.PersistentVolume.Spec.AWSElasticBlockStore.VolumeID)
}
Expand Down
10 changes: 7 additions & 3 deletions pkg/volume/cinder/cinder_block.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,8 @@ import (
"fmt"
"path/filepath"

"k8s.io/api/core/v1"
v1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/types"
"k8s.io/klog"
"k8s.io/kubernetes/pkg/util/mount"
Expand Down Expand Up @@ -53,10 +54,10 @@ func (plugin *cinderPlugin) ConstructBlockVolumeSpec(podUID types.UID, volumeNam
return nil, fmt.Errorf("failed to get volume plugin information from globalMapPathUUID: %v", globalMapPathUUID)
}

return getVolumeSpecFromGlobalMapPath(globalMapPath)
return getVolumeSpecFromGlobalMapPath(volumeName, globalMapPath)
}

func getVolumeSpecFromGlobalMapPath(globalMapPath string) (*volume.Spec, error) {
func getVolumeSpecFromGlobalMapPath(volumeName, globalMapPath string) (*volume.Spec, error) {
// Get volume spec information from globalMapPath
// globalMapPath example:
// plugins/kubernetes.io/{PluginName}/{DefaultKubeletVolumeDevicesDirName}/{volumeID}
Expand All @@ -67,6 +68,9 @@ func getVolumeSpecFromGlobalMapPath(globalMapPath string) (*volume.Spec, error)
}
block := v1.PersistentVolumeBlock
cinderVolume := &v1.PersistentVolume{
ObjectMeta: metav1.ObjectMeta{
Name: volumeName,
},
Spec: v1.PersistentVolumeSpec{
PersistentVolumeSource: v1.PersistentVolumeSource{
Cinder: &v1.CinderPersistentVolumeSource{
Expand Down
9 changes: 6 additions & 3 deletions pkg/volume/cinder/cinder_block_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import (
"path/filepath"
"testing"

"k8s.io/api/core/v1"
v1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/types"
utiltesting "k8s.io/client-go/util/testing"
Expand Down Expand Up @@ -52,16 +52,19 @@ func TestGetVolumeSpecFromGlobalMapPath(t *testing.T) {
expectedGlobalPath := filepath.Join(tmpVDir, testGlobalPath)

//Bad Path
badspec, err := getVolumeSpecFromGlobalMapPath("")
badspec, err := getVolumeSpecFromGlobalMapPath("", "")
if badspec != nil || err == nil {
t.Errorf("Expected not to get spec from GlobalMapPath but did")
}

// Good Path
spec, err := getVolumeSpecFromGlobalMapPath(expectedGlobalPath)
spec, err := getVolumeSpecFromGlobalMapPath("myVolume", expectedGlobalPath)
if spec == nil || err != nil {
t.Fatalf("Failed to get spec from GlobalMapPath: %v", err)
}
if spec.PersistentVolume.Name != "myVolume" {
t.Errorf("Invalid PV name from GlobalMapPath spec: %s", spec.PersistentVolume.Name)
}
if spec.PersistentVolume.Spec.Cinder.VolumeID != testVolName {
t.Errorf("Invalid volumeID from GlobalMapPath spec: %s", spec.PersistentVolume.Spec.Cinder.VolumeID)
}
Expand Down
8 changes: 6 additions & 2 deletions pkg/volume/gcepd/gce_pd_block.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
"strconv"

v1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/types"
"k8s.io/klog"
"k8s.io/kubernetes/pkg/util/mount"
Expand Down Expand Up @@ -54,10 +55,10 @@ func (plugin *gcePersistentDiskPlugin) ConstructBlockVolumeSpec(podUID types.UID
return nil, fmt.Errorf("failed to get volume plugin information from globalMapPathUUID: %v", globalMapPathUUID)
}

return getVolumeSpecFromGlobalMapPath(globalMapPath)
return getVolumeSpecFromGlobalMapPath(volumeName, globalMapPath)
}

func getVolumeSpecFromGlobalMapPath(globalMapPath string) (*volume.Spec, error) {
func getVolumeSpecFromGlobalMapPath(volumeName, globalMapPath string) (*volume.Spec, error) {
// Get volume spec information from globalMapPath
// globalMapPath example:
// plugins/kubernetes.io/{PluginName}/{DefaultKubeletVolumeDevicesDirName}/{volumeID}
Expand All @@ -68,6 +69,9 @@ func getVolumeSpecFromGlobalMapPath(globalMapPath string) (*volume.Spec, error)
}
block := v1.PersistentVolumeBlock
gceVolume := &v1.PersistentVolume{
ObjectMeta: metav1.ObjectMeta{
Name: volumeName,
},
Spec: v1.PersistentVolumeSpec{
PersistentVolumeSource: v1.PersistentVolumeSource{
GCEPersistentDisk: &v1.GCEPersistentDiskVolumeSource{
Expand Down
7 changes: 5 additions & 2 deletions pkg/volume/gcepd/gce_pd_block_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,16 +52,19 @@ func TestGetVolumeSpecFromGlobalMapPath(t *testing.T) {
expectedGlobalPath := filepath.Join(tmpVDir, testGlobalPath)

//Bad Path
badspec, err := getVolumeSpecFromGlobalMapPath("")
badspec, err := getVolumeSpecFromGlobalMapPath("", "")
if badspec != nil || err == nil {
t.Errorf("Expected not to get spec from GlobalMapPath but did")
}

// Good Path
spec, err := getVolumeSpecFromGlobalMapPath(expectedGlobalPath)
spec, err := getVolumeSpecFromGlobalMapPath("myVolume", expectedGlobalPath)
if spec == nil || err != nil {
t.Fatalf("Failed to get spec from GlobalMapPath: %v", err)
}
if spec.PersistentVolume.Name != "myVolume" {
t.Errorf("Invalid PV name from GlobalMapPath spec: %s", spec.PersistentVolume.Name)
}
if spec.PersistentVolume.Spec.GCEPersistentDisk.PDName != testPdName {
t.Errorf("Invalid pdName from GlobalMapPath spec: %s", spec.PersistentVolume.Spec.GCEPersistentDisk.PDName)
}
Expand Down
10 changes: 7 additions & 3 deletions pkg/volume/vsphere_volume/vsphere_volume_block.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,8 @@ import (
"path/filepath"
"strings"

"k8s.io/api/core/v1"
v1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/types"
"k8s.io/klog"
"k8s.io/kubernetes/pkg/util/mount"
Expand All @@ -49,10 +50,10 @@ func (plugin *vsphereVolumePlugin) ConstructBlockVolumeSpec(podUID types.UID, vo
if len(globalMapPath) <= 1 {
return nil, fmt.Errorf("failed to get volume plugin information from globalMapPathUUID: %v", globalMapPathUUID)
}
return getVolumeSpecFromGlobalMapPath(globalMapPath)
return getVolumeSpecFromGlobalMapPath(volumeName, globalMapPath)
}

func getVolumeSpecFromGlobalMapPath(globalMapPath string) (*volume.Spec, error) {
func getVolumeSpecFromGlobalMapPath(volumeName, globalMapPath string) (*volume.Spec, error) {
// Construct volume spec from globalMapPath
// globalMapPath example:
// plugins/kubernetes.io/{PluginName}/{DefaultKubeletVolumeDevicesDirName}/{volumeID}
Expand All @@ -64,6 +65,9 @@ func getVolumeSpecFromGlobalMapPath(globalMapPath string) (*volume.Spec, error)
}
block := v1.PersistentVolumeBlock
vsphereVolume := &v1.PersistentVolume{
ObjectMeta: metav1.ObjectMeta{
Name: volumeName,
},
Spec: v1.PersistentVolumeSpec{
PersistentVolumeSource: v1.PersistentVolumeSource{
VsphereVolume: &v1.VsphereVirtualDiskVolumeSource{
Expand Down
9 changes: 6 additions & 3 deletions pkg/volume/vsphere_volume/vsphere_volume_block_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import (
"path/filepath"
"testing"

"k8s.io/api/core/v1"
v1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/types"
utiltesting "k8s.io/client-go/util/testing"
Expand Down Expand Up @@ -51,16 +51,19 @@ func TestGetVolumeSpecFromGlobalMapPath(t *testing.T) {
expectedGlobalPath := filepath.Join(tmpVDir, testGlobalPath)

// Bad Path
badspec, err := getVolumeSpecFromGlobalMapPath("")
badspec, err := getVolumeSpecFromGlobalMapPath("", "")
if badspec != nil || err == nil {
t.Errorf("Expected not to get spec from GlobalMapPath but did")
}

// Good Path
spec, err := getVolumeSpecFromGlobalMapPath(expectedGlobalPath)
spec, err := getVolumeSpecFromGlobalMapPath("myVolume", expectedGlobalPath)
if spec == nil || err != nil {
t.Fatalf("Failed to get spec from GlobalMapPath: %s", err)
}
if spec.PersistentVolume.Name != "myVolume" {
t.Errorf("Invalid PV name from GlobalMapPath spec: %s", spec.PersistentVolume.Name)
}
if spec.PersistentVolume.Spec.VsphereVolume.VolumePath != testVolumePath {
t.Fatalf("Invalid volumePath from GlobalMapPath spec: %s", spec.PersistentVolume.Spec.VsphereVolume.VolumePath)
}
Expand Down
14 changes: 8 additions & 6 deletions test/e2e/storage/testsuites/disruptive.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,8 +140,9 @@ func (s *disruptiveTestSuite) defineTests(driver TestDriver, pattern testpattern
}

for _, test := range disruptiveTestTable {
if test.runTestFile != nil {
func(t disruptiveTest) {
func(t disruptiveTest) {
if (pattern.VolMode == v1.PersistentVolumeBlock && t.runTestBlock != nil) ||
(pattern.VolMode == v1.PersistentVolumeFilesystem && t.runTestFile != nil) {
ginkgo.It(t.testItStmt, func() {
init()
defer cleanup()
Expand All @@ -158,13 +159,14 @@ func (s *disruptiveTestSuite) defineTests(driver TestDriver, pattern testpattern
l.pod, err = e2epod.CreateSecPodWithNodeSelection(l.cs, l.ns.Name, pvcs, inlineSources, false, "", false, false, e2epv.SELinuxLabel, nil, e2epod.NodeSelection{Name: l.config.ClientNodeName}, framework.PodStartTimeout)
framework.ExpectNoError(err, "While creating pods for kubelet restart test")

if pattern.VolMode == v1.PersistentVolumeBlock {
if pattern.VolMode == v1.PersistentVolumeBlock && t.runTestBlock != nil {
t.runTestBlock(l.cs, l.config.Framework, l.pod)
} else {
}
if pattern.VolMode == v1.PersistentVolumeFilesystem && t.runTestFile != nil {
t.runTestFile(l.cs, l.config.Framework, l.pod)
}
})
}(test)
}
}
}(test)
}
}
26 changes: 20 additions & 6 deletions test/e2e/storage/utils/utils.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -318,18 +318,28 @@ func TestVolumeUnmapsFromDeletedPodWithForceOption(c clientset.Interface, f *fra
nodeIP = nodeIP + ":22"

// Creating command to check whether path exists
command := fmt.Sprintf("ls /var/lib/kubelet/pods/%s/volumeDevices/*/ | grep '.'", clientPod.UID)
podDirectoryCmd := fmt.Sprintf("ls /var/lib/kubelet/pods/%s/volumeDevices/*/ | grep '.'", clientPod.UID)
if isSudoPresent(nodeIP, framework.TestContext.Provider) {
command = fmt.Sprintf("sudo sh -c \"%s\"", command)
podDirectoryCmd = fmt.Sprintf("sudo sh -c \"%s\"", podDirectoryCmd)
}
// Directories in the global directory have unpredictable names, however, device symlinks
// have the same name as pod.UID. So just find anything with pod.UID name.
globalBlockDirectoryCmd := fmt.Sprintf("find /var/lib/kubelet/plugins -name %s", clientPod.UID)
if isSudoPresent(nodeIP, framework.TestContext.Provider) {
globalBlockDirectoryCmd = fmt.Sprintf("sudo sh -c \"%s\"", globalBlockDirectoryCmd)
}

ginkgo.By("Expecting the symlinks from PodDeviceMapPath to be found.")
result, err := e2essh.SSH(command, nodeIP, framework.TestContext.Provider)
result, err := e2essh.SSH(podDirectoryCmd, nodeIP, framework.TestContext.Provider)
e2essh.LogResult(result)
framework.ExpectNoError(err, "Encountered SSH error.")
framework.ExpectEqual(result.Code, 0, fmt.Sprintf("Expected grep exit code of 0, got %d", result.Code))

// TODO: Needs to check GetGlobalMapPath and descriptor lock, as well.
ginkgo.By("Expecting the symlinks from global map path to be found.")
result, err = e2essh.SSH(globalBlockDirectoryCmd, nodeIP, framework.TestContext.Provider)
e2essh.LogResult(result)
framework.ExpectNoError(err, "Encountered SSH error.")
framework.ExpectEqual(result.Code, 0, fmt.Sprintf("Expected find exit code of 0, got %d", result.Code))

// This command is to make sure kubelet is started after test finishes no matter it fails or not.
defer func() {
Expand Down Expand Up @@ -358,12 +368,16 @@ func TestVolumeUnmapsFromDeletedPodWithForceOption(c clientset.Interface, f *fra
}

ginkgo.By("Expecting the symlink from PodDeviceMapPath not to be found.")
result, err = e2essh.SSH(command, nodeIP, framework.TestContext.Provider)
result, err = e2essh.SSH(podDirectoryCmd, nodeIP, framework.TestContext.Provider)
e2essh.LogResult(result)
framework.ExpectNoError(err, "Encountered SSH error.")
gomega.Expect(result.Stdout).To(gomega.BeEmpty(), "Expected grep stdout to be empty.")

// TODO: Needs to check GetGlobalMapPath and descriptor lock, as well.
ginkgo.By("Expecting the symlinks from global map path not to be found.")
result, err = e2essh.SSH(globalBlockDirectoryCmd, nodeIP, framework.TestContext.Provider)
e2essh.LogResult(result)
framework.ExpectNoError(err, "Encountered SSH error.")
gomega.Expect(result.Stdout).To(gomega.BeEmpty(), "Expected find stdout to be empty.")

framework.Logf("Volume unmaped on node %s", clientPod.Spec.NodeName)
}
Expand Down