Skip to content

Commit 4e4519a

Browse files
committed
[cinder-csi-plugin] Wait for volume availability before attach
ControllerPublishVolume now waits for the volume to reach 'available' or 'in-use' status before calling the Cinder attachment API. Previously, if the CO called ControllerPublishVolume immediately after CreateVolume, the volume could still be in 'creating' state on the backend (especially for NetApp-backed volumes). This caused Cinder to reject the attachment with a 409 Conflict ('status must be available or downloading to reserve, but the current status is creating'), forcing the CO to retry blindly. The new behavior uses a context-aware poll (WaitVolumeTargetStatusWithContext) that respects the gRPC request deadline. The volume status is checked every 3 seconds until it reaches a target state, enters an error state, or the context expires. This eliminates unnecessary 409 errors against Cinder and reduces time-to-attach for volumes still being provisioned. Production data from eu-de-2 showed ~78% of attachment_create calls arrived while volumes were still in 'creating' state, with a median creation time of 12s and a max of 132s. Signed-off-by: Walter Boring <waboring@hemna.com>
1 parent 46c2161 commit 4e4519a

5 files changed

Lines changed: 66 additions & 0 deletions

File tree

pkg/csi/cinder/controllerserver.go

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import (
2323
"slices"
2424
"sort"
2525
"strconv"
26+
"time"
2627

2728
"github.com/container-storage-interface/spec/lib/go/csi"
2829
"github.com/gophercloud/gophercloud/v2/openstack/blockstorage/v3/backups"
@@ -51,6 +52,10 @@ const (
5152
cinderCSIClusterIDKey = "cinder.csi.openstack.org/cluster"
5253
affinityKey = "cinder.csi.openstack.org/affinity"
5354
antiAffinityKey = "cinder.csi.openstack.org/anti-affinity"
55+
56+
// volumeReadyPollInterval is the interval at which the volume status is
57+
// polled when waiting for a volume to become available before attaching.
58+
volumeReadyPollInterval = 3 * time.Second
5459
)
5560

5661
func (cs *controllerServer) CreateVolume(ctx context.Context, req *csi.CreateVolumeRequest) (*csi.CreateVolumeResponse, error) {
@@ -321,6 +326,18 @@ func (cs *controllerServer) ControllerPublishVolume(ctx context.Context, req *cs
321326
return nil, status.Errorf(codes.Internal, "[ControllerPublishVolume] get volume failed with error %v", err)
322327
}
323328

329+
// Wait for the volume to be available before attempting attach.
330+
// Cinder rejects attachment requests for volumes not in "available" (or
331+
// "in-use" for multiattach) state. Without this wait, the driver would
332+
// immediately hit a 409 Conflict if CreateVolume has just been called and
333+
// the volume is still being provisioned on the backend.
334+
targetStatus := []string{openstack.VolumeAvailableStatus, openstack.VolumeInUseStatus}
335+
err = cloud.WaitVolumeTargetStatusWithContext(ctx, volumeID, targetStatus, volumeReadyPollInterval)
336+
if err != nil {
337+
klog.Errorf("Failed waiting for volume %s to become available: %v", volumeID, err)
338+
return nil, status.Errorf(codes.FailedPrecondition, "[ControllerPublishVolume] Volume %s is not available for attach: %v", volumeID, err)
339+
}
340+
324341
_, err = cloud.GetInstanceByID(ctx, instanceID)
325342
if err != nil {
326343
if cpoerrors.IsNotFound(err) {

pkg/csi/cinder/controllerserver_test.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -529,6 +529,7 @@ func TestDeleteVolume(t *testing.T) {
529529
func TestControllerPublishVolume(t *testing.T) {
530530
fakeCs, osmock := fakeControllerServer()
531531

532+
osmock.On("WaitVolumeTargetStatusWithContext", FakeVolID, mock.AnythingOfType("[]string"), mock.AnythingOfType("time.Duration")).Return(nil)
532533
osmock.On("AttachVolume", FakeNodeID, FakeVolID).Return(FakeVolID, nil)
533534
osmock.On("WaitDiskAttached", FakeNodeID, FakeVolID).Return(nil)
534535
osmock.On("GetAttachmentDiskPath", FakeNodeID, FakeVolID).Return(FakeDevicePath, nil)

pkg/csi/cinder/openstack/openstack.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import (
2121
"fmt"
2222
"net/http"
2323
"os"
24+
"time"
2425

2526
"github.com/gophercloud/gophercloud/v2"
2627
"github.com/gophercloud/gophercloud/v2/openstack"
@@ -54,6 +55,7 @@ type IOpenStack interface {
5455
DetachVolume(ctx context.Context, instanceID, volumeID string) error
5556
WaitDiskDetached(ctx context.Context, instanceID string, volumeID string) error
5657
WaitVolumeTargetStatus(ctx context.Context, volumeID string, tStatus []string) error
58+
WaitVolumeTargetStatusWithContext(ctx context.Context, volumeID string, tStatus []string, interval time.Duration) error
5759
GetAttachmentDiskPath(ctx context.Context, instanceID, volumeID string) (string, error)
5860
GetVolume(ctx context.Context, volumeID string) (*volumes.Volume, error)
5961
GetVolumesByName(ctx context.Context, name string) ([]volumes.Volume, error)

pkg/csi/cinder/openstack/openstack_mock.go

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ package openstack
1919
import (
2020
"context"
2121
"fmt"
22+
"time"
2223

2324
"github.com/gophercloud/gophercloud/v2/openstack/blockstorage/v3/backups"
2425
"github.com/gophercloud/gophercloud/v2/openstack/blockstorage/v3/snapshots"
@@ -200,6 +201,20 @@ func (_m *OpenStackMock) WaitVolumeTargetStatus(ctx context.Context, volumeID st
200201
return r0
201202
}
202203

204+
// WaitVolumeTargetStatusWithContext provides a mock function with given fields: volumeID, tStatus, interval
205+
func (_m *OpenStackMock) WaitVolumeTargetStatusWithContext(ctx context.Context, volumeID string, tStatus []string, interval time.Duration) error {
206+
ret := _m.Called(volumeID, tStatus, interval)
207+
208+
var r0 error
209+
if rf, ok := ret.Get(0).(func(string, []string, time.Duration) error); ok {
210+
r0 = rf(volumeID, tStatus, interval)
211+
} else {
212+
r0 = ret.Error(0)
213+
}
214+
215+
return r0
216+
}
217+
203218
// WaitDiskDetached provides a mock function with given fields: instanceID, volumeID
204219
func (_m *OpenStackMock) WaitDiskDetached(ctx context.Context, instanceID string, volumeID string) error {
205220
ret := _m.Called(instanceID, volumeID)

pkg/csi/cinder/openstack/openstack_volumes.go

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -298,6 +298,37 @@ func (os *OpenStack) WaitVolumeTargetStatus(ctx context.Context, volumeID string
298298
return waitErr
299299
}
300300

301+
// WaitVolumeTargetStatusWithContext waits for volume to be in target state,
302+
// respecting the context deadline. This is useful when the caller has a
303+
// context with a deadline (e.g., gRPC request context) and wants the wait
304+
// to be bounded by that deadline rather than a fixed number of steps.
305+
// The interval parameter controls how often the volume status is polled.
306+
func (os *OpenStack) WaitVolumeTargetStatusWithContext(ctx context.Context, volumeID string, tStatus []string, interval time.Duration) error {
307+
err := wait.PollUntilContextCancel(ctx, interval, true, func(ctx context.Context) (bool, error) {
308+
vol, err := os.GetVolume(ctx, volumeID)
309+
if err != nil {
310+
return false, err
311+
}
312+
for _, t := range tStatus {
313+
if vol.Status == t {
314+
return true, nil
315+
}
316+
}
317+
for _, eState := range volumeErrorStates {
318+
if vol.Status == eState {
319+
return false, fmt.Errorf("volume %s is in error state: %s", volumeID, vol.Status)
320+
}
321+
}
322+
return false, nil
323+
})
324+
325+
if err != nil && ctx.Err() != nil {
326+
return fmt.Errorf("timeout waiting for volume %s to reach status %v: %w", volumeID, tStatus, err)
327+
}
328+
329+
return err
330+
}
331+
301332
// DetachVolume detaches given cinder volume from the compute
302333
func (os *OpenStack) DetachVolume(ctx context.Context, instanceID, volumeID string) error {
303334
volume, err := os.GetVolume(ctx, volumeID)

0 commit comments

Comments
 (0)