Skip to content

Commit

Permalink
fix: reduce GetDisk in AttachDisk
Browse files Browse the repository at this point in the history
  • Loading branch information
andyzhangx committed Jun 27, 2021
1 parent d5cd233 commit afa780b
Show file tree
Hide file tree
Showing 15 changed files with 145 additions and 80 deletions.
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -68,5 +68,5 @@ replace (
k8s.io/sample-cli-plugin => k8s.io/sample-cli-plugin v0.21.0
k8s.io/sample-controller => k8s.io/sample-controller v0.21.0

sigs.k8s.io/cloud-provider-azure => sigs.k8s.io/cloud-provider-azure v0.7.4-0.20210611045613-f7491146d89d
sigs.k8s.io/cloud-provider-azure => sigs.k8s.io/cloud-provider-azure v0.7.4-0.20210627125724-fb498b8d847a
)
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -1206,8 +1206,8 @@ rsc.io/quote/v3 v3.1.0/go.mod h1:yEA65RcK8LyAZtP9Kv3t0HmxON59tX3rD+tICJqUlj0=
rsc.io/sampler v1.3.0/go.mod h1:T1hPZKmBbMNahiBKFy5HrXp6adAjACjK9JXDnKaTXpA=
sigs.k8s.io/apiserver-network-proxy/konnectivity-client v0.0.15 h1:4uqm9Mv+w2MmBYD+F4qf/v6tDFUdPOk29C095RbU5mY=
sigs.k8s.io/apiserver-network-proxy/konnectivity-client v0.0.15/go.mod h1:LEScyzhFmoF5pso/YSeBstl57mOzx9xlU9n85RGrDQg=
sigs.k8s.io/cloud-provider-azure v0.7.4-0.20210611045613-f7491146d89d h1:4rGP3LN7nL6cVClNAWZzkFdhXoj6kTK3XD+ZXMgZhBQ=
sigs.k8s.io/cloud-provider-azure v0.7.4-0.20210611045613-f7491146d89d/go.mod h1:Y5dSIj1lXNzGSRePDD/WpB0uZb1aCwdoHew+Oqt6M90=
sigs.k8s.io/cloud-provider-azure v0.7.4-0.20210627125724-fb498b8d847a h1:CNyZKx+bEZe8Y4s1w6yazvMFdLZfyDGyu5HfZCqADFE=
sigs.k8s.io/cloud-provider-azure v0.7.4-0.20210627125724-fb498b8d847a/go.mod h1:Y5dSIj1lXNzGSRePDD/WpB0uZb1aCwdoHew+Oqt6M90=
sigs.k8s.io/kustomize v2.0.3+incompatible/go.mod h1:MkjgH3RdOWrievjo6c9T245dYlB5QeXV4WCbnt/PEpU=
sigs.k8s.io/kustomize/api v0.8.5/go.mod h1:M377apnKT5ZHJS++6H4rQoCHmWtt6qTpp3mbe7p6OLY=
sigs.k8s.io/kustomize/cmd/config v0.9.7/go.mod h1:MvXCpHs77cfyxRmCNUQjIqCmZyYsbn5PyQpWiq44nW0=
Expand Down
17 changes: 9 additions & 8 deletions pkg/azuredisk/azuredisk.go
Original file line number Diff line number Diff line change
Expand Up @@ -259,32 +259,33 @@ func (d *Driver) isGetDiskThrottled() bool {
return cache != nil
}

func (d *Driver) checkDiskExists(ctx context.Context, diskURI string) error {
func (d *Driver) checkDiskExists(ctx context.Context, diskURI string) (*compute.Disk, error) {
diskName, err := GetDiskName(diskURI)
if err != nil {
return err
return nil, err
}

resourceGroup, err := GetResourceGroupFromURI(diskURI)
if err != nil {
return err
return nil, err
}

if d.isGetDiskThrottled() {
klog.Warningf("skip checkDiskExists(%s) since it's still in throttling", diskURI)
return nil
return nil, nil
}

if _, rerr := d.cloud.DisksClient.Get(ctx, resourceGroup, diskName); rerr != nil {
disk, rerr := d.cloud.DisksClient.Get(ctx, resourceGroup, diskName)
if rerr != nil {
if strings.Contains(rerr.RawError.Error(), rateLimited) {
klog.Warningf("checkDiskExists(%s) is throttled with error: %v", diskURI, rerr.Error())
d.getDiskThrottlingCache.Set(throttlingKey, "")
return nil
return &disk, nil
}
return rerr.Error()
return nil, rerr.Error()
}

return nil
return &disk, nil
}

func (d *Driver) checkDiskCapacity(ctx context.Context, resourceGroup, diskName string, requestGiB int) (bool, error) {
Expand Down
2 changes: 1 addition & 1 deletion pkg/azuredisk/azuredisk_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -695,6 +695,6 @@ func TestRun(t *testing.T) {

func TestDriver_checkDiskExists(t *testing.T) {
d, _ := NewFakeDriver(t)
err := d.checkDiskExists(context.TODO(), "testurl/subscriptions/12/providers/Microsoft.Compute/disks/name")
_, err := d.checkDiskExists(context.TODO(), "testurl/subscriptions/12/providers/Microsoft.Compute/disks/name")
assert.NotEqual(t, err, nil)
}
2 changes: 1 addition & 1 deletion pkg/azuredisk/azuredisk_v1_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,6 @@ func TestCheckDiskCapacity_V1(t *testing.T) {
func TestDriver_checkDiskExists_V1(t *testing.T) {
d, _ := NewFakeDriver(t)
d.setDiskThrottlingCache(throttlingKey, "")
err := d.checkDiskExists(context.TODO(), "testurl/subscriptions/12/resourceGroups/23/providers/Microsoft.Compute/disks/name")
_, err := d.checkDiskExists(context.TODO(), "testurl/subscriptions/12/resourceGroups/23/providers/Microsoft.Compute/disks/name")
assert.Equal(t, err, nil)
}
9 changes: 5 additions & 4 deletions pkg/azuredisk/controllerserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -414,7 +414,8 @@ func (d *Driver) ControllerPublishVolume(ctx context.Context, req *csi.Controlle
return nil, status.Error(codes.InvalidArgument, "Volume capability not supported")
}

if err := d.checkDiskExists(ctx, diskURI); err != nil {
disk, err := d.checkDiskExists(ctx, diskURI)
if err != nil {
return nil, status.Error(codes.NotFound, fmt.Sprintf("Volume not found, failed with error: %v", err))
}

Expand Down Expand Up @@ -458,7 +459,7 @@ func (d *Driver) ControllerPublishVolume(ctx context.Context, req *csi.Controlle
}
klog.V(2).Infof("Trying to attach volume %q to node %q", diskURI, nodeName)

lun, err = d.cloud.AttachDisk(true, diskName, diskURI, nodeName, cachingMode)
lun, err = d.cloud.AttachDisk(true, diskName, diskURI, nodeName, cachingMode, disk)
if err == nil {
klog.V(2).Infof("Attach operation successful: volume %q attached to node %q.", diskURI, nodeName)
} else {
Expand All @@ -473,7 +474,7 @@ func (d *Driver) ControllerPublishVolume(ctx context.Context, req *csi.Controlle
return nil, status.Errorf(codes.Internal, "Could not detach volume %q from node %q: %v", diskURI, derr.CurrentNode, err)
}
klog.V(2).Infof("Trying to attach volume %q to node %q again", diskURI, nodeName)
lun, err = d.cloud.AttachDisk(true, diskName, diskURI, nodeName, cachingMode)
lun, err = d.cloud.AttachDisk(true, diskName, diskURI, nodeName, cachingMode, disk)
}
if err != nil {
klog.Errorf("Attach volume %q to instance %q failed with %v", diskURI, nodeName, err)
Expand Down Expand Up @@ -537,7 +538,7 @@ func (d *Driver) ValidateVolumeCapabilities(ctx context.Context, req *csi.Valida
return nil, status.Error(codes.InvalidArgument, "Volume capabilities missing in request")
}

err := d.checkDiskExists(ctx, diskURI)
_, err := d.checkDiskExists(ctx, diskURI)
if err != nil {
return nil, status.Error(codes.NotFound, fmt.Sprintf("Volume not found, failed with error: %v", err))
}
Expand Down
3 changes: 2 additions & 1 deletion pkg/azuredisk/fake_azuredisk.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"testing"
"time"

"github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2020-12-01/compute"
"github.com/container-storage-interface/spec/lib/go/csi"
"github.com/golang/mock/gomock"
"github.com/stretchr/testify/assert"
Expand Down Expand Up @@ -77,7 +78,7 @@ type FakeDriver interface {
setMounter(*mount.SafeFormatAndMount)

checkDiskCapacity(context.Context, string, string, int) (bool, error)
checkDiskExists(ctx context.Context, diskURI string) error
checkDiskExists(ctx context.Context, diskURI string) (*compute.Disk, error)
getSnapshotInfo(string) (string, string, error)
getSnapshotByID(context.Context, string, string, string) (*csi.Snapshot, error)
ensureMountPoint(string) (bool, error)
Expand Down
4 changes: 2 additions & 2 deletions vendor/modules.txt
Original file line number Diff line number Diff line change
Expand Up @@ -867,7 +867,7 @@ k8s.io/utils/trace
# sigs.k8s.io/apiserver-network-proxy/konnectivity-client v0.0.15
sigs.k8s.io/apiserver-network-proxy/konnectivity-client/pkg/client
sigs.k8s.io/apiserver-network-proxy/konnectivity-client/proto/client
# sigs.k8s.io/cloud-provider-azure v0.7.4 => sigs.k8s.io/cloud-provider-azure v0.7.4-0.20210611045613-f7491146d89d
# sigs.k8s.io/cloud-provider-azure v0.7.4 => sigs.k8s.io/cloud-provider-azure v0.7.4-0.20210627125724-fb498b8d847a
## explicit
sigs.k8s.io/cloud-provider-azure/pkg/auth
sigs.k8s.io/cloud-provider-azure/pkg/azureclients
Expand Down Expand Up @@ -947,4 +947,4 @@ sigs.k8s.io/yaml
# k8s.io/sample-apiserver => k8s.io/sample-apiserver v0.21.0
# k8s.io/sample-cli-plugin => k8s.io/sample-cli-plugin v0.21.0
# k8s.io/sample-controller => k8s.io/sample-controller v0.21.0
# sigs.k8s.io/cloud-provider-azure => sigs.k8s.io/cloud-provider-azure v0.7.4-0.20210611045613-f7491146d89d
# sigs.k8s.io/cloud-provider-azure => sigs.k8s.io/cloud-provider-azure v0.7.4-0.20210627125724-fb498b8d847a
14 changes: 9 additions & 5 deletions vendor/sigs.k8s.io/cloud-provider-azure/pkg/auth/azure_auth.go

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

7 changes: 7 additions & 0 deletions vendor/sigs.k8s.io/cloud-provider-azure/pkg/consts/consts.go

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

52 changes: 34 additions & 18 deletions vendor/sigs.k8s.io/cloud-provider-azure/pkg/provider/azure.go

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

Loading

0 comments on commit afa780b

Please sign in to comment.