Skip to content

Commit f065384

Browse files
YashasG98l-technicore
authored andcommitted
Changed CSI controller GRPCs to use parent contexts, added unit tests and logging improvements
1 parent 0e3619d commit f065384

File tree

10 files changed

+1628
-126
lines changed

10 files changed

+1628
-126
lines changed

pkg/csi/driver/bv_controller.go

Lines changed: 14 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -351,7 +351,7 @@ func (d *BlockVolumeControllerDriver) CreateVolume(ctx context.Context, req *csi
351351
}
352352

353353
//make sure this method is idempotent by checking existence of volume with same name.
354-
volumes, err := d.client.BlockStorage().GetVolumesByName(context.Background(), volumeName, d.config.CompartmentID)
354+
volumes, err := d.client.BlockStorage().GetVolumesByName(ctx, volumeName, d.config.CompartmentID)
355355
if err != nil {
356356
log.With("service", "blockstorage", "verb", "get", "resource", "volume", "statusCode", util.GetHttpStatusCode(err)).
357357
With(zap.Error(err)).Error("Failed to find existence of volume.")
@@ -389,7 +389,7 @@ func (d *BlockVolumeControllerDriver) CreateVolume(ctx context.Context, req *csi
389389

390390
} else {
391391
// Creating new volume
392-
ad, err := d.client.Identity().GetAvailabilityDomainByName(context.Background(), d.config.CompartmentID, availableDomainShortName)
392+
ad, err := d.client.Identity().GetAvailabilityDomainByName(ctx, d.config.CompartmentID, availableDomainShortName)
393393
if err != nil {
394394
log.With("Compartment Id", d.config.CompartmentID, "service", "identity", "verb", "get", "resource", "AD", "statusCode", util.GetHttpStatusCode(err)).
395395
With(zap.Error(err)).Error("Failed to get available domain.")
@@ -417,8 +417,8 @@ func (d *BlockVolumeControllerDriver) CreateVolume(ctx context.Context, req *csi
417417
bvTags = scTags
418418
}
419419

420-
provisionedVolume, err = provision(log, d.client, volumeName, size, *ad.Name, d.config.CompartmentID, srcSnapshotId, srcVolumeId,
421-
volumeParams.diskEncryptionKey, volumeParams.vpusPerGB, timeout, bvTags)
420+
provisionedVolume, err = provision(ctx, log, d.client, volumeName, size, *ad.Name, d.config.CompartmentID, srcSnapshotId, srcVolumeId,
421+
volumeParams.diskEncryptionKey, volumeParams.vpusPerGB, bvTags)
422422
if err != nil {
423423
log.With("Ad name", *ad.Name, "Compartment Id", d.config.CompartmentID).With(zap.Error(err)).Error("New volume creation failed.")
424424
errorType = util.GetError(err)
@@ -428,8 +428,6 @@ func (d *BlockVolumeControllerDriver) CreateVolume(ctx context.Context, req *csi
428428
return nil, status.Errorf(codes.Internal, "New volume creation failed %v", err.Error())
429429
}
430430
}
431-
ctx, cancel := context.WithTimeout(ctx, timeout)
432-
defer cancel()
433431
log.Info("Waiting for volume to become available.")
434432

435433
if srcVolumeId != "" {
@@ -495,9 +493,6 @@ func (d *BlockVolumeControllerDriver) DeleteVolume(ctx context.Context, req *csi
495493
return nil, status.Error(codes.InvalidArgument, "DeleteVolume Volume ID must be provided")
496494
}
497495

498-
ctx, cancel := context.WithTimeout(ctx, timeout)
499-
defer cancel()
500-
501496
log.Info("Deleting Volume")
502497
err := d.client.BlockStorage().DeleteVolume(ctx, req.VolumeId)
503498
if err != nil {
@@ -559,7 +554,7 @@ func (d *BlockVolumeControllerDriver) ControllerPublishVolume(ctx context.Contex
559554
if !ok {
560555
attachType = attachmentTypeISCSI
561556
}
562-
volumeAttachmentOptions, err := getAttachmentOptions(context.Background(), d.client.Compute(), attachType, id)
557+
volumeAttachmentOptions, err := getAttachmentOptions(ctx, d.client.Compute(), attachType, id)
563558
if err != nil {
564559
log.With("service", "compute", "verb", "get", "resource", "instance", "statusCode", util.GetHttpStatusCode(err)).
565560
With(zap.Error(err)).With("attachmentType", attachType, "instanceID", id).Error("failed to get the attachment options")
@@ -588,7 +583,7 @@ func (d *BlockVolumeControllerDriver) ControllerPublishVolume(ctx context.Contex
588583
return nil, status.Errorf(codes.Unknown, "failed to get compartmentID from node annotation:. error : %s", err)
589584
}
590585

591-
volumeAttached, err := d.client.Compute().FindActiveVolumeAttachment(context.Background(), compartmentID, req.VolumeId)
586+
volumeAttached, err := d.client.Compute().FindActiveVolumeAttachment(ctx, compartmentID, req.VolumeId)
592587

593588
if err != nil && !client.IsNotFound(err) {
594589
log.With("service", "compute", "verb", "get", "resource", "volumeAttachment", "statusCode", util.GetHttpStatusCode(err)).
@@ -654,7 +649,7 @@ func (d *BlockVolumeControllerDriver) ControllerPublishVolume(ctx context.Contex
654649
log.Info("Attaching volume to instance")
655650

656651
if volumeAttachmentOptions.useParavirtualizedAttachment {
657-
volumeAttached, err = d.client.Compute().AttachParavirtualizedVolume(context.Background(), id, req.VolumeId, volumeAttachmentOptions.enableInTransitEncryption)
652+
volumeAttached, err = d.client.Compute().AttachParavirtualizedVolume(ctx, id, req.VolumeId, volumeAttachmentOptions.enableInTransitEncryption)
658653
if err != nil {
659654
log.With("service", "compute", "verb", "create", "resource", "volumeAttachment", "statusCode", util.GetHttpStatusCode(err)).
660655
With("instanceID", id).With(zap.Error(err)).Info("failed paravirtualized attachment instance to volume.")
@@ -665,7 +660,7 @@ func (d *BlockVolumeControllerDriver) ControllerPublishVolume(ctx context.Contex
665660
return nil, status.Errorf(codes.Internal, "failed paravirtualized attachment instance to volume. error : %s", err)
666661
}
667662
} else {
668-
volumeAttached, err = d.client.Compute().AttachVolume(context.Background(), id, req.VolumeId)
663+
volumeAttached, err = d.client.Compute().AttachVolume(ctx, id, req.VolumeId)
669664
if err != nil {
670665
log.With("service", "compute", "verb", "create", "resource", "volumeAttachment", "statusCode", util.GetHttpStatusCode(err)).
671666
With("instanceID", id).With(zap.Error(err)).Info("failed iscsi attachment instance to volume.")
@@ -756,7 +751,7 @@ func (d *BlockVolumeControllerDriver) ControllerUnpublishVolume(ctx context.Cont
756751
return nil, status.Errorf(codes.Unknown, "failed to get compartmentID from node annotation:: error : %s", err)
757752
}
758753
log = log.With("compartmentID", compartmentID)
759-
attachedVolume, err := d.client.Compute().FindVolumeAttachment(context.Background(), compartmentID, req.VolumeId)
754+
attachedVolume, err := d.client.Compute().FindVolumeAttachment(ctx, compartmentID, req.VolumeId)
760755
if attachedVolume != nil && attachedVolume.GetId() != nil {
761756
log = log.With("volumeAttachedId", *attachedVolume.GetId())
762757
}
@@ -779,7 +774,7 @@ func (d *BlockVolumeControllerDriver) ControllerUnpublishVolume(ctx context.Cont
779774
}
780775
if attachedVolume.GetLifecycleState() != core.VolumeAttachmentLifecycleStateDetaching {
781776
log.With("instanceID", *attachedVolume.GetInstanceId()).Info("Detaching Volume")
782-
err = d.client.Compute().DetachVolume(context.Background(), *attachedVolume.GetId())
777+
err = d.client.Compute().DetachVolume(ctx, *attachedVolume.GetId())
783778
if err != nil {
784779
log.With("service", "compute", "verb", "delete", "resource", "volumeAttachment", "statusCode", util.GetHttpStatusCode(err)).
785780
With("instanceID", *attachedVolume.GetInstanceId()).With(zap.Error(err)).Error("Volume can not be detached")
@@ -791,7 +786,7 @@ func (d *BlockVolumeControllerDriver) ControllerUnpublishVolume(ctx context.Cont
791786
}
792787
}
793788
log.With("instanceID", *attachedVolume.GetInstanceId()).Info("Waiting for Volume to Detach")
794-
err = d.client.Compute().WaitForVolumeDetached(context.Background(), *attachedVolume.GetId())
789+
err = d.client.Compute().WaitForVolumeDetached(ctx, *attachedVolume.GetId())
795790
if err != nil {
796791
log.With("service", "compute", "verb", "get", "resource", "volumeAttachment", "statusCode", util.GetHttpStatusCode(err)).
797792
With("instanceID", *attachedVolume.GetInstanceId()).With(zap.Error(err)).Error("timed out waiting for volume to be detached")
@@ -1057,8 +1052,6 @@ func (d *BlockVolumeControllerDriver) CreateSnapshot(ctx context.Context, req *c
10571052

10581053
ts := timestamppb.New(snapshot.TimeCreated.Time)
10591054

1060-
ctx, cancel := context.WithTimeout(ctx, newBackupAvailableTimeout)
1061-
defer cancel()
10621055
_, err = d.client.BlockStorage().AwaitVolumeBackupAvailableOrTimeout(ctx, *snapshot.Id)
10631056
if err != nil {
10641057
if strings.Contains(err.Error(), "timed out") {
@@ -1121,9 +1114,6 @@ func (d *BlockVolumeControllerDriver) DeleteSnapshot(ctx context.Context, req *c
11211114
return nil, status.Error(codes.InvalidArgument, "SnapshotId must be provided")
11221115
}
11231116

1124-
ctx, cancel := context.WithTimeout(ctx, timeout)
1125-
defer cancel()
1126-
11271117
err := d.client.BlockStorage().DeleteVolumeBackup(ctx, req.SnapshotId)
11281118
if err != nil && !k8sapierrors.IsNotFound(err) {
11291119
errorType = util.GetError(err)
@@ -1172,7 +1162,7 @@ func (d *BlockVolumeControllerDriver) ControllerExpandVolume(ctx context.Context
11721162
}
11731163

11741164
//make sure this method is idempotent by checking existence of volume with same name.
1175-
volume, err := d.client.BlockStorage().GetVolume(context.Background(), volumeId)
1165+
volume, err := d.client.BlockStorage().GetVolume(ctx, volumeId)
11761166
if err != nil {
11771167
log.With("service", "blockstorage", "verb", "get", "resource", "volume", "statusCode", util.GetHttpStatusCode(err)).
11781168
With(zap.Error(err)).Error("Failed to find existence of volume")
@@ -1229,10 +1219,8 @@ func (d *BlockVolumeControllerDriver) ControllerGetVolume(ctx context.Context, r
12291219
return nil, status.Error(codes.Unimplemented, "ControllerGetVolume is not supported yet")
12301220
}
12311221

1232-
func provision(log *zap.SugaredLogger, c client.Interface, volName string, volSize int64, availDomainName, compartmentID,
1233-
backupID, srcVolumeID, kmsKeyID string, vpusPerGB int64, timeout time.Duration, bvTags *config.TagConfig) (core.Volume, error) {
1234-
1235-
ctx := context.Background()
1222+
func provision(ctx context.Context, log *zap.SugaredLogger, c client.Interface, volName string, volSize int64, availDomainName, compartmentID,
1223+
backupID, srcVolumeID, kmsKeyID string, vpusPerGB int64, bvTags *config.TagConfig) (core.Volume, error) {
12361224

12371225
volSizeGB, minSizeGB := csi_util.RoundUpSize(volSize, 1*client.GiB), csi_util.RoundUpMinSize()
12381226

@@ -1264,9 +1252,6 @@ func provision(log *zap.SugaredLogger, c client.Interface, volName string, volSi
12641252
volumeDetails.DefinedTags = bvTags.DefinedTags
12651253
}
12661254

1267-
ctx, cancel := context.WithTimeout(ctx, timeout)
1268-
defer cancel()
1269-
12701255
newVolume, err := c.BlockStorage().CreateVolume(ctx, volumeDetails)
12711256

12721257
if err != nil {

0 commit comments

Comments
 (0)