Skip to content

Commit

Permalink
Fix transient condition when device path is not yet available on host (
Browse files Browse the repository at this point in the history
…#415)

We currently fail with an internal error if the device path is not ready
on the host yet. The error occurs when we try to mount the volume. This
leads to unnecessary error events.

In this PR we try to address this issue by return an `Unavailable` error
in case the device on the host is not ready yet. This will trigger a
re-try.
  • Loading branch information
afritzler authored Feb 1, 2024
1 parent b8984aa commit a588d1d
Show file tree
Hide file tree
Showing 5 changed files with 17 additions and 8 deletions.
2 changes: 1 addition & 1 deletion hack/license-header.txt
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
SPDX-FileCopyrightText: 2023 SAP SE or an SAP affiliate company and IronCore contributors
SPDX-FileCopyrightText: 2024 SAP SE or an SAP affiliate company and IronCore contributors
SPDX-License-Identifier: Apache-2.0
13 changes: 12 additions & 1 deletion pkg/driver/node.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"context"
"fmt"
"io"
"os"
"path/filepath"

"github.com/container-storage-interface/spec/lib/go/csi"
Expand Down Expand Up @@ -35,6 +36,16 @@ func (d *driver) NodeStageVolume(_ context.Context, req *csi.NodeStageVolumeRequ
fstype := req.GetVolumeContext()["fstype"]
devicePath := req.PublishContext["device_name"]

klog.InfoS("Check if the device path exists")
if _, err := d.os.Stat(devicePath); err != nil {
if os.IsNotExist(err) {
// We will requeue here since the device path is not ready yet.
return nil, status.Errorf(codes.Unavailable, "Device path %s does not exist: %v", devicePath, err)
} else {
return nil, status.Errorf(codes.Internal, "Failed to determine wether the device path %s exists: %v", devicePath, err)
}
}

readOnly := false
if req.GetVolumeContext()["readOnly"] == "true" {
readOnly = true
Expand All @@ -45,7 +56,7 @@ func (d *driver) NodeStageVolume(_ context.Context, req *csi.NodeStageVolumeRequ
klog.InfoS("Validate mount point", "MountPoint", targetPath)
notMnt, err := d.mounter.IsLikelyNotMountPoint(targetPath)
if err != nil && !d.os.IsNotExist(err) {
return nil, status.Errorf(codes.Internal, "Failed to verify mount point %s: %v", devicePath, err)
return nil, status.Errorf(codes.Internal, "Failed to verify mount point %s: %v", targetPath, err)
}
klog.InfoS("Check if volume is already mounted")
if !notMnt {
Expand Down
6 changes: 2 additions & 4 deletions pkg/driver/node_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,8 @@ var _ = Describe("Node", func() {
},
},
}

mockOS.EXPECT().Stat(devicePath).Return(nil, nil)
})

It("should fail if the volume is already mounted", func(ctx SpecContext) {
Expand Down Expand Up @@ -107,7 +109,6 @@ var _ = Describe("Node", func() {
statusErr, ok := status.FromError(err)
Expect(ok).To(BeTrue())
Expect(statusErr.Code()).To(Equal(codes.Internal))

})

It("should fail if the mount operation fails", func(ctx SpecContext) {
Expand All @@ -129,7 +130,6 @@ var _ = Describe("Node", func() {
_, err := drv.NodeStageVolume(ctx, req)
Expect(err).NotTo(HaveOccurred())
})

})

Describe("NodePublishVolume", func() {
Expand Down Expand Up @@ -598,7 +598,6 @@ var _ = Describe("Node", func() {
Expect(err).NotTo(HaveOccurred())
Expect(res).To(SatisfyAll(
HaveField("Usage", ContainElements([]*csi.VolumeUsage{

{
Available: 0,
Total: 0,
Expand All @@ -615,6 +614,5 @@ var _ = Describe("Node", func() {
)),
))
})

})
})
2 changes: 1 addition & 1 deletion pkg/utils/mount/mock_mountutils_unix.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion pkg/utils/os/mock_osutils_unix.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit a588d1d

Please sign in to comment.