Skip to content

Commit

Permalink
Merge pull request #120645 from rohitssingh/automated-cherry-pick-of-…
Browse files Browse the repository at this point in the history 
…#120330-upstream-release-1.26

Automated cherry pick of #120330: Retry operations if CSI Driver Isn't Found by Treating this
  • Loading branch information
@k8s-ci-robot
k8s-ci-robot committed Oct 23, 2023
2 parents 83bf7dc + 892f4dc commit eccb1fb
Show file tree
Hide file tree
Showing 10 changed files with 443 additions and 19 deletions.
8 changes: 6 additions & 2 deletions pkg/volume/csi/csi_attacher.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -287,7 +287,9 @@ func (c *csiAttacher) MountDevice(spec *volume.Spec, devicePath string, deviceMo
if c.csiClient == nil {
c.csiClient, err = newCsiDriverClient(csiDriverName(csiSource.Driver))
if err != nil {
return errors.New(log("attacher.MountDevice failed to create newCsiDriverClient: %v", err))
// Treat the absence of the CSI driver as a transient error
// See https://github.com/kubernetes/kubernetes/issues/120268
return volumetypes.NewTransientOperationFailure(log("attacher.MountDevice failed to create newCsiDriverClient: %v", err))
}
}
csi := c.csiClient
Expand Down Expand Up @@ -607,7 +609,9 @@ func (c *csiAttacher) UnmountDevice(deviceMountPath string) error {
if c.csiClient == nil {
c.csiClient, err = newCsiDriverClient(csiDriverName(driverName))
if err != nil {
return errors.New(log("attacher.UnmountDevice failed to create newCsiDriverClient: %v", err))
// Treat the absence of the CSI driver as a transient error
// See https://github.com/kubernetes/kubernetes/issues/120268
return volumetypes.NewTransientOperationFailure(log("attacher.UnmountDevice failed to create newCsiDriverClient: %v", err))
}
}
csi := c.csiClient
Expand Down
48 changes: 43 additions & 5 deletions pkg/volume/csi/csi_attacher_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1110,6 +1110,7 @@ func TestAttacherMountDevice(t *testing.T) {
exitError error
spec *volume.Spec
watchTimeout time.Duration
skipClientSetup bool
}{
{
testName: "normal PV",
Expand Down Expand Up @@ -1250,6 +1251,20 @@ func TestAttacherMountDevice(t *testing.T) {
createAttachment: true,
spec: volume.NewSpecFromPersistentVolume(makeTestPV(pvName, 10, testDriver, "test-vol1"), false),
},
{
testName: "driver not specified",
volName: "test-vol1",
devicePath: "path1",
deviceMountPath: "path2",
fsGroup: &testFSGroup,
stageUnstageSet: true,
createAttachment: true,
populateDeviceMountPath: false,
spec: volume.NewSpecFromPersistentVolume(makeTestPV(pvName, 10, "not-found", "test-vol1"), false),
exitError: transientError,
shouldFail: true,
skipClientSetup: true,
},
}

for _, tc := range testCases {
Expand Down Expand Up @@ -1278,7 +1293,9 @@ func TestAttacherMountDevice(t *testing.T) {
t.Fatalf("failed to create new attacher: %v", err0)
}
csiAttacher := getCsiAttacherFromVolumeAttacher(attacher, tc.watchTimeout)
csiAttacher.csiClient = setupClientWithVolumeMountGroup(t, tc.stageUnstageSet, tc.driverSupportsVolumeMountGroup)
if !tc.skipClientSetup {
csiAttacher.csiClient = setupClientWithVolumeMountGroup(t, tc.stageUnstageSet, tc.driverSupportsVolumeMountGroup)
}

if tc.deviceMountPath != "" {
tc.deviceMountPath = filepath.Join(tmpDir, tc.deviceMountPath)
Expand Down Expand Up @@ -1343,16 +1360,15 @@ func TestAttacherMountDevice(t *testing.T) {
t.Errorf("failed to modify permissions after test: %v", err)
}
}
if tc.exitError != nil && reflect.TypeOf(tc.exitError) != reflect.TypeOf(err) {
t.Fatalf("expected exitError type: %v got: %v (%v)", reflect.TypeOf(tc.exitError), reflect.TypeOf(err), err)
}
return
}
if tc.shouldFail {
t.Errorf("test should fail, but no error occurred")
}

if tc.exitError != nil && reflect.TypeOf(tc.exitError) != reflect.TypeOf(err) {
t.Fatalf("expected exitError: %v got: %v", tc.exitError, err)
}

// Verify call goes through all the way
numStaged := 1
if !tc.stageUnstageSet {
Expand Down Expand Up @@ -1570,6 +1586,7 @@ func TestAttacherMountDeviceWithInline(t *testing.T) {
}

func TestAttacherUnmountDevice(t *testing.T) {
transientError := volumetypes.NewTransientOperationFailure("")
testCases := []struct {
testName string
volID string
Expand All @@ -1579,6 +1596,8 @@ func TestAttacherUnmountDevice(t *testing.T) {
stageUnstageSet bool
shouldFail bool
watchTimeout time.Duration
exitError error
unsetClient bool
}{
// PV agnostic path positive test cases
{
Expand Down Expand Up @@ -1610,6 +1629,17 @@ func TestAttacherUnmountDevice(t *testing.T) {
stageUnstageSet: true,
shouldFail: true,
},
// Ensure that a transient error is returned if the client is not established
{
testName: "fail with transient error, json file exists but client not found",
volID: "project/zone/test-vol1",
deviceMountPath: "plugins/csi/" + generateSha("project/zone/test-vol1") + "/globalmount",
jsonFile: `{"driverName": "unknown-driver", "volumeHandle":"project/zone/test-vol1"}`,
stageUnstageSet: true,
shouldFail: true,
exitError: transientError,
unsetClient: true,
},
}

for _, tc := range testCases {
Expand Down Expand Up @@ -1657,6 +1687,11 @@ func TestAttacherUnmountDevice(t *testing.T) {
t.Fatalf("Failed to create PV: %v", err)
}
}
// Clear out the client if specified
// The lookup to generate a new client will fail
if tc.unsetClient {
csiAttacher.csiClient = nil
}

// Run
err := csiAttacher.UnmountDevice(tc.deviceMountPath)
Expand All @@ -1665,6 +1700,9 @@ func TestAttacherUnmountDevice(t *testing.T) {
if !tc.shouldFail {
t.Errorf("test should not fail, but error occurred: %v", err)
}
if tc.exitError != nil && reflect.TypeOf(tc.exitError) != reflect.TypeOf(err) {
t.Fatalf("expected exitError type: %v got: %v (%v)", reflect.TypeOf(tc.exitError), reflect.TypeOf(err), err)
}
return
}
if tc.shouldFail {
Expand Down
16 changes: 12 additions & 4 deletions pkg/volume/csi/csi_block.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -319,7 +319,9 @@ func (m *csiBlockMapper) SetUpDevice() (string, error) {

csiClient, err := m.csiClientGetter.Get()
if err != nil {
return "", errors.New(log("blockMapper.SetUpDevice failed to get CSI client: %v", err))
// Treat the absence of the CSI driver as a transient error
// See https://github.com/kubernetes/kubernetes/issues/120268
return "", volumetypes.NewTransientOperationFailure(log("blockMapper.SetUpDevice failed to get CSI client: %v", err))
}

// Call NodeStageVolume
Expand Down Expand Up @@ -379,7 +381,9 @@ func (m *csiBlockMapper) MapPodDevice() (string, error) {

csiClient, err := m.csiClientGetter.Get()
if err != nil {
return "", errors.New(log("blockMapper.MapPodDevice failed to get CSI client: %v", err))
// Treat the absence of the CSI driver as a transient error
// See https://github.com/kubernetes/kubernetes/issues/120268
return "", volumetypes.NewTransientOperationFailure(log("blockMapper.MapPodDevice failed to get CSI client: %v", err))
}

// Call NodePublishVolume
Expand Down Expand Up @@ -444,7 +448,9 @@ func (m *csiBlockMapper) TearDownDevice(globalMapPath, devicePath string) error

csiClient, err := m.csiClientGetter.Get()
if err != nil {
return errors.New(log("blockMapper.TearDownDevice failed to get CSI client: %v", err))
// Treat the absence of the CSI driver as a transient error
// See https://github.com/kubernetes/kubernetes/issues/120268
return volumetypes.NewTransientOperationFailure(log("blockMapper.TearDownDevice failed to get CSI client: %v", err))
}

// Call NodeUnstageVolume
Expand Down Expand Up @@ -506,7 +512,9 @@ func (m *csiBlockMapper) UnmapPodDevice() error {

csiClient, err := m.csiClientGetter.Get()
if err != nil {
return errors.New(log("blockMapper.UnmapPodDevice failed to get CSI client: %v", err))
// Treat the absence of the CSI driver as a transient error
// See https://github.com/kubernetes/kubernetes/issues/120268
return volumetypes.NewTransientOperationFailure(log("blockMapper.UnmapPodDevice failed to get CSI client: %v", err))
}

ctx, cancel := createCSIOperationContext(m.spec, csiTimeout)
Expand Down

0 comments on commit eccb1fb

Please sign in to comment.