Skip to content

Commit 109006c

Browse files
kishen-vkishen-v
committed
Pass on volume details when disk is in available state
1 parent 75d1c51 commit 109006c

File tree

7 files changed

+128
-56
lines changed

7 files changed

+128
-56
lines changed

‎pkg/cloud/cloud_interface.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,11 +23,11 @@ import (
2323
type Cloud interface {
2424
CreateDisk(volumeName string, diskOptions *DiskOptions) (disk *Disk, err error)
2525
DeleteDisk(volumeID string) (err error)
26-
AttachDisk(volumeID string, nodeID string) (err error)
27-
DetachDisk(volumeID string, nodeID string) (err error)
26+
AttachDisk(volumeID string, nodeID string) (*models.Volume, error)
27+
DetachDisk(volumeID string, nodeID string) (*models.Volume, error)
2828
ResizeDisk(volumeID string, reqSize int64) (newSize int64, err error)
2929
CloneDisk(sourceVolumeName string, cloneVolumeName string) (disk *Disk, err error)
30-
WaitForVolumeState(volumeID, state string) error
30+
WaitForVolumeState(volumeID, state string) (*models.Volume, error)
3131
WaitForCloneStatus(taskId string) error
3232
GetDiskByName(name string) (disk *Disk, err error)
3333
GetDiskByNamePrefix(namePrefix string) (disk *Disk, err error)

‎pkg/cloud/mocks/mock_cloud.go‎

Lines changed: 12 additions & 9 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎pkg/cloud/powervs.go‎

Lines changed: 16 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -128,7 +128,7 @@ func (p *powerVSCloud) CreateDisk(volumeName string, diskOptions *DiskOptions) (
128128
if err != nil {
129129
return nil, err
130130
}
131-
err = p.WaitForVolumeState(*v.VolumeID, VolumeAvailableState)
131+
v, err = p.WaitForVolumeState(*v.VolumeID, VolumeAvailableState)
132132
if err != nil {
133133
return nil, err
134134
}
@@ -151,16 +151,16 @@ func (p *powerVSCloud) DeleteDisk(volumeID string) (err error) {
151151
return nil
152152
}
153153

154-
func (p *powerVSCloud) AttachDisk(volumeID string, nodeID string) (err error) {
154+
func (p *powerVSCloud) AttachDisk(volumeID string, nodeID string) (vol *models.Volume, err error) {
155155
if err = p.volClient.Attach(nodeID, volumeID); err != nil {
156-
return err
156+
return nil, err
157157
}
158158
return p.WaitForVolumeState(volumeID, VolumeInUseState)
159159
}
160160

161-
func (p *powerVSCloud) DetachDisk(volumeID string, nodeID string) (err error) {
161+
func (p *powerVSCloud) DetachDisk(volumeID string, nodeID string) (vol *models.Volume, err error) {
162162
if err = p.volClient.Detach(nodeID, volumeID); err != nil {
163-
return err
163+
return nil, err
164164
}
165165
return p.WaitForVolumeState(volumeID, VolumeAvailableState)
166166
}
@@ -216,24 +216,27 @@ func (p *powerVSCloud) CloneDisk(sourceVolumeID string, cloneVolumeName string)
216216
return nil, errors.New("cloned volume not found")
217217
}
218218
clonedVolumeID := clonedVolumeDetails.ClonedVolumes[0].ClonedVolumeID
219-
err = p.WaitForVolumeState(clonedVolumeID, VolumeAvailableState)
219+
_, err = p.WaitForVolumeState(clonedVolumeID, VolumeAvailableState)
220220
if err != nil {
221221
return nil, err
222222
}
223223
return p.GetDiskByID(clonedVolumeID)
224224
}
225225

226-
func (p *powerVSCloud) WaitForVolumeState(volumeID, state string) error {
226+
func (p *powerVSCloud) WaitForVolumeState(volumeID, state string) (*models.Volume, error) {
227227
ctx := context.Background()
228+
var vol *models.Volume
229+
var err error
228230
klog.V(4).Infof("Waiting for volume %s to be in %q state", volumeID, state)
229-
return wait.PollUntilContextTimeout(ctx, PollInterval, PollTimeout, true, func(ctx context.Context) (bool, error) {
230-
v, err := p.volClient.Get(volumeID)
231+
err = wait.PollUntilContextTimeout(ctx, PollInterval, PollTimeout, true, func(ctx context.Context) (bool, error) {
232+
vol, err = p.volClient.Get(volumeID)
231233
if err != nil {
232234
return false, err
233235
}
234-
spew.Dump(v)
235-
return v.State == state, nil
236+
spew.Dump(vol)
237+
return vol.State == state, nil
236238
})
239+
return vol, err
237240
}
238241

239242
func (p *powerVSCloud) WaitForCloneStatus(cloneTaskId string) error {
@@ -249,12 +252,14 @@ func (p *powerVSCloud) WaitForCloneStatus(cloneTaskId string) error {
249252
}
250253

251254
func (p *powerVSCloud) GetDiskByName(name string) (disk *Disk, err error) {
255+
klog.Infof("Looking for volume by name: %s", name)
252256
vols, err := p.volClient.GetAll()
253257
if err != nil {
254258
return nil, err
255259
}
256260
for _, v := range vols.Volumes {
257261
if name == *v.Name {
262+
klog.Infof("Found volume by name: %s", *v.Name)
258263
return &Disk{
259264
Name: *v.Name,
260265
DiskType: *v.DiskType,

‎pkg/driver/controller.go‎

Lines changed: 17 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ package driver
1818

1919
import (
2020
"context"
21+
"errors"
2122
"fmt"
2223
"os"
2324
"strconv"
@@ -182,7 +183,7 @@ func (d *controllerService) CreateVolume(ctx context.Context, req *csi.CreateVol
182183
}
183184
// Check if the disk already exists
184185
// Disk exists only if previous createVolume request fails due to any network/tcp error
185-
disk, _ := d.cloud.GetDiskByName(volName)
186+
disk, err := d.cloud.GetDiskByName(volName)
186187
if disk != nil {
187188
// wait for volume to be available as the volume already exists
188189
klog.V(3).Infof("CreateVolume: Found an existing volume %s in %q state.", volName, disk.State)
@@ -191,15 +192,24 @@ func (d *controllerService) CreateVolume(ctx context.Context, req *csi.CreateVol
191192
return nil, err
192193
}
193194
if disk.State != cloud.VolumeAvailableState {
194-
err = d.cloud.WaitForVolumeState(disk.VolumeID, cloud.VolumeAvailableState)
195+
vol, err := d.cloud.WaitForVolumeState(disk.VolumeID, cloud.VolumeAvailableState)
195196
if err != nil {
196197
return nil, status.Errorf(codes.Internal, "Disk exists, but not in required state. Current:%s Required:%s", disk.State, cloud.VolumeAvailableState)
197198
}
199+
// When the disk is still in the "Creating" state, the WWN will not be available.
200+
// In such a case, once when the volume is available, assign the WWN to the disk if not already assigned.
201+
if disk.WWN == "" {
202+
disk.WWN = vol.Wwn
203+
}
198204
}
199205
} else {
200-
disk, err = d.cloud.CreateDisk(volName, opts)
201-
if err != nil {
202-
return nil, status.Errorf(codes.Internal, "Could not create volume %q: %v", volName, err)
206+
if errors.Is(err, cloud.ErrNotFound) {
207+
disk, err = d.cloud.CreateDisk(volName, opts)
208+
if err != nil {
209+
return nil, status.Errorf(codes.Internal, "Could not create volume %q: %v", volName, err)
210+
}
211+
} else {
212+
return nil, status.Errorf(codes.Internal, "Could not find volume by name %q: %v", volName, err)
203213
}
204214
}
205215
klog.V(3).Infof("CreateVolume: created volume %s, took %s", volName, time.Since(start))
@@ -256,7 +266,7 @@ func (d *controllerService) ControllerPublishVolume(ctx context.Context, req *cs
256266

257267
pvInfo := map[string]string{WWNKey: req.VolumeContext[WWNKey]}
258268

259-
err := d.cloud.AttachDisk(volumeID, nodeID)
269+
_, err := d.cloud.AttachDisk(volumeID, nodeID)
260270
if err != nil {
261271
if strings.Contains(err.Error(), cloud.ErrConflictVolumeAlreadyExists.Error()) {
262272
return nil, status.Error(codes.AlreadyExists, err.Error())
@@ -288,7 +298,7 @@ func (d *controllerService) ControllerUnpublishVolume(ctx context.Context, req *
288298
return nil, status.Error(codes.InvalidArgument, "Node ID not provided")
289299
}
290300

291-
err := d.cloud.DetachDisk(volumeID, nodeID)
301+
_, err := d.cloud.DetachDisk(volumeID, nodeID)
292302
if err != nil {
293303
if strings.Contains(err.Error(), cloud.ErrVolumeDetachNotFound.Error()) {
294304
klog.V(4).Infof("ControllerUnpublishVolume: volume %s is detached from node %s, took %s", volumeID, nodeID, time.Since(start))

0 commit comments

Comments
 (0)