Skip to content

Commit

Permalink
clean up parm in func
Browse files Browse the repository at this point in the history
  • Loading branch information
umagnus committed Nov 20, 2023
1 parent ef565c7 commit d341a05
Show file tree
Hide file tree
Showing 6 changed files with 21 additions and 29 deletions.
4 changes: 2 additions & 2 deletions pkg/azurefile/azurefile.go
Original file line number Diff line number Diff line change
Expand Up @@ -903,8 +903,8 @@ func (d *Driver) ResizeFileShare(ctx context.Context, subsID, resourceGroup, acc
})
}

// CopyFileShare copies a fileshare in the same storage account
func (d *Driver) copyFileShare(_ context.Context, req *csi.CreateVolumeRequest, accountKey string, shareOptions *fileclient.ShareOptions, storageEndpointSuffix string) error {
// copyFileShare copies a fileshare in the same storage account
func (d *Driver) copyFileShare(req *csi.CreateVolumeRequest, accountKey string, shareOptions *fileclient.ShareOptions, storageEndpointSuffix string) error {
if shareOptions.Protocol == storage.EnabledProtocolsNFS {
return fmt.Errorf("protocol nfs is not supported for volume cloning")
}
Expand Down
7 changes: 4 additions & 3 deletions pkg/azurefile/controllerserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -575,7 +575,7 @@ func (d *Driver) CreateVolume(ctx context.Context, req *csi.CreateVolumeRequest)
if err != nil {
return nil, status.Errorf(codes.Internal, "failed to GetStorageAccesskey on account(%s) rg(%s), error: %v", accountOptions.Name, accountOptions.ResourceGroup, err)
}
if err := d.copyVolume(ctx, req, accountKeyCopy, shareOptions, storageEndpointSuffix); err != nil {
if err := d.copyVolume(req, accountKeyCopy, shareOptions, storageEndpointSuffix); err != nil {
return nil, err
}
// storeAccountKey is not needed here since copy volume is only using SAS token
Expand Down Expand Up @@ -725,13 +725,14 @@ func (d *Driver) DeleteVolume(ctx context.Context, req *csi.DeleteVolumeRequest)
return &csi.DeleteVolumeResponse{}, nil
}

func (d *Driver) copyVolume(ctx context.Context, req *csi.CreateVolumeRequest, accountKey string, shareOptions *fileclient.ShareOptions, storageEndpointSuffix string) error {
// copyVolume copy an azure file
func (d *Driver) copyVolume(req *csi.CreateVolumeRequest, accountKey string, shareOptions *fileclient.ShareOptions, storageEndpointSuffix string) error {
vs := req.VolumeContentSource
switch vs.Type.(type) {
case *csi.VolumeContentSource_Snapshot:
return status.Errorf(codes.InvalidArgument, "copy volume from volumeSnapshot is not supported")
case *csi.VolumeContentSource_Volume:
return d.copyFileShare(ctx, req, accountKey, shareOptions, storageEndpointSuffix)
return d.copyFileShare(req, accountKey, shareOptions, storageEndpointSuffix)
default:
return status.Errorf(codes.InvalidArgument, "%v is not a proper volume source", vs)
}
Expand Down
23 changes: 7 additions & 16 deletions pkg/azurefile/controllerserver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1646,10 +1646,9 @@ func TestCopyVolume(t *testing.T) {
}

d := NewFakeDriver()
ctx := context.Background()

expectedErr := status.Errorf(codes.InvalidArgument, "copy volume from volumeSnapshot is not supported")
err := d.copyVolume(ctx, req, "", nil, "core.windows.net")
err := d.copyVolume(req, "", nil, "core.windows.net")
if !reflect.DeepEqual(err, expectedErr) {
t.Errorf("Unexpected error: %v", err)
}
Expand Down Expand Up @@ -1679,10 +1678,9 @@ func TestCopyVolume(t *testing.T) {
}

d := NewFakeDriver()
ctx := context.Background()

expectedErr := fmt.Errorf("protocol nfs is not supported for volume cloning")
err := d.copyVolume(ctx, req, "", &fileclient.ShareOptions{Protocol: storage.EnabledProtocolsNFS}, "core.windows.net")
err := d.copyVolume(req, "", &fileclient.ShareOptions{Protocol: storage.EnabledProtocolsNFS}, "core.windows.net")
if !reflect.DeepEqual(err, expectedErr) {
t.Errorf("Unexpected error: %v", err)
}
Expand Down Expand Up @@ -1712,10 +1710,9 @@ func TestCopyVolume(t *testing.T) {
}

d := NewFakeDriver()
ctx := context.Background()

expectedErr := status.Errorf(codes.NotFound, "error parsing volume id: \"unit-test\", should at least contain two #")
err := d.copyVolume(ctx, req, "", &fileclient.ShareOptions{Name: "dstFileshare"}, "core.windows.net")
err := d.copyVolume(req, "", &fileclient.ShareOptions{Name: "dstFileshare"}, "core.windows.net")
if !reflect.DeepEqual(err, expectedErr) {
t.Errorf("Unexpected error: %v", err)
}
Expand Down Expand Up @@ -1745,10 +1742,9 @@ func TestCopyVolume(t *testing.T) {
}

d := NewFakeDriver()
ctx := context.Background()

expectedErr := fmt.Errorf("srcFileShareName() or dstFileShareName(dstFileshare) is empty")
err := d.copyVolume(ctx, req, "", &fileclient.ShareOptions{Name: "dstFileshare"}, "core.windows.net")
err := d.copyVolume(req, "", &fileclient.ShareOptions{Name: "dstFileshare"}, "core.windows.net")
if !reflect.DeepEqual(err, expectedErr) {
t.Errorf("Unexpected error: %v", err)
}
Expand Down Expand Up @@ -1778,10 +1774,9 @@ func TestCopyVolume(t *testing.T) {
}

d := NewFakeDriver()
ctx := context.Background()

expectedErr := fmt.Errorf("srcFileShareName(fileshare) or dstFileShareName() is empty")
err := d.copyVolume(ctx, req, "", &fileclient.ShareOptions{}, "core.windows.net")
err := d.copyVolume(req, "", &fileclient.ShareOptions{}, "core.windows.net")
if !reflect.DeepEqual(err, expectedErr) {
t.Errorf("Unexpected error: %v", err)
}
Expand Down Expand Up @@ -1822,10 +1817,8 @@ func TestCopyVolume(t *testing.T) {

d.azcopy.ExecCmd = m

ctx := context.Background()

var expectedErr error
err := d.copyVolume(ctx, req, "", &fileclient.ShareOptions{Name: "dstFileshare"}, "core.windows.net")
err := d.copyVolume(req, "", &fileclient.ShareOptions{Name: "dstFileshare"}, "core.windows.net")
if !reflect.DeepEqual(err, expectedErr) {
t.Errorf("Unexpected error: %v", err)
}
Expand Down Expand Up @@ -1867,10 +1860,8 @@ func TestCopyVolume(t *testing.T) {

d.azcopy.ExecCmd = m

ctx := context.Background()

var expectedErr error
err := d.copyVolume(ctx, req, "", &fileclient.ShareOptions{Name: "dstFileshare"}, "core.windows.net")
err := d.copyVolume(req, "", &fileclient.ShareOptions{Name: "dstFileshare"}, "core.windows.net")
if !reflect.DeepEqual(err, expectedErr) {
t.Errorf("Unexpected error: %v", err)
}
Expand Down
4 changes: 2 additions & 2 deletions test/e2e/testsuites/dynamically_provisioned_inline_volume.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,9 +44,9 @@ func (t *DynamicallyProvisionedInlineVolumeTest) Run(ctx context.Context, client
var tpod *TestPod
var cleanup []func()
if t.CSIInlineVolume {
tpod, cleanup = pod.SetupWithCSIInlineVolumes(client, namespace, t.CSIDriver, t.SecretName, t.ShareName, t.Server, t.ReadOnly)
tpod, cleanup = pod.SetupWithCSIInlineVolumes(client, namespace, t.SecretName, t.ShareName, t.Server, t.ReadOnly)
} else {
tpod, cleanup = pod.SetupWithInlineVolumes(client, namespace, t.CSIDriver, t.SecretName, t.ShareName, t.ReadOnly)
tpod, cleanup = pod.SetupWithInlineVolumes(client, namespace, t.SecretName, t.ShareName, t.ReadOnly)
}

// defer must be called here for resources not get removed before using them
Expand Down
4 changes: 2 additions & 2 deletions test/e2e/testsuites/specs.go
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ func (pod *PodDetails) SetupWithDynamicVolumesWithSubpath(ctx context.Context, c
return tpod, cleanupFuncs
}

func (pod *PodDetails) SetupWithInlineVolumes(client clientset.Interface, namespace *v1.Namespace, _ driver.DynamicPVTestDriver, secretName, shareName string, readOnly bool) (*TestPod, []func()) {
func (pod *PodDetails) SetupWithInlineVolumes(client clientset.Interface, namespace *v1.Namespace, secretName, shareName string, readOnly bool) (*TestPod, []func()) {
tpod := NewTestPod(client, namespace, pod.Cmd, pod.IsWindows, pod.WinServerVer)
cleanupFuncs := make([]func(), 0)
for n, v := range pod.Volumes {
Expand All @@ -150,7 +150,7 @@ func (pod *PodDetails) SetupWithInlineVolumes(client clientset.Interface, namesp
return tpod, cleanupFuncs
}

func (pod *PodDetails) SetupWithCSIInlineVolumes(client clientset.Interface, namespace *v1.Namespace, _ driver.DynamicPVTestDriver, secretName, shareName, server string, readOnly bool) (*TestPod, []func()) {
func (pod *PodDetails) SetupWithCSIInlineVolumes(client clientset.Interface, namespace *v1.Namespace, secretName, shareName, server string, readOnly bool) (*TestPod, []func()) {
tpod := NewTestPod(client, namespace, pod.Cmd, pod.IsWindows, pod.WinServerVer)
cleanupFuncs := make([]func(), 0)
for n, v := range pod.Volumes {
Expand Down
8 changes: 4 additions & 4 deletions test/utils/azure/azure_helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,14 +65,14 @@ func GetAzureClient(cloud, subscriptionID, clientID, tenantID, clientSecret stri
return nil, err
}

return getClient(env, subscriptionID, tenantID, cred, env.TokenAudience), nil
return getClient(env, subscriptionID, cred, env.TokenAudience), nil
}

func (az *Client) GetAzureFilesClient() (storage.FileSharesClient, error) {
return az.filesharesClient, nil
}

func (az *Client) EnsureSSHPublicKey(ctx context.Context, _, resourceGroupName, location, keyName string) (publicKey string, err error) {
func (az *Client) EnsureSSHPublicKey(ctx context.Context, resourceGroupName, location, keyName string) (publicKey string, err error) {
_, err = az.sshPublicKeysClient.Create(ctx, resourceGroupName, keyName, compute.SSHPublicKeyResource{Location: &location})
if err != nil {
return "", err
Expand Down Expand Up @@ -135,7 +135,7 @@ func (az *Client) EnsureVirtualMachine(ctx context.Context, groupName, location,
return vm, err
}

publicKey, err := az.EnsureSSHPublicKey(ctx, az.subscriptionID, groupName, location, "test-key")
publicKey, err := az.EnsureSSHPublicKey(ctx, groupName, location, "test-key")
if err != nil {
return vm, err
}
Expand Down Expand Up @@ -313,7 +313,7 @@ func getCloudConfig(env azure.Environment) cloud.Configuration {
}
}

func getClient(env azure.Environment, subscriptionID, _ string, cred *azidentity.ClientSecretCredential, scope string) *Client {
func getClient(env azure.Environment, subscriptionID string, cred *azidentity.ClientSecretCredential, scope string) *Client {
c := &Client{
environment: env,
subscriptionID: subscriptionID,
Expand Down

0 comments on commit d341a05

Please sign in to comment.