Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: allow storage class setting of useragent #999

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions docs/driver-parameters.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ perfProfile | [disk device performance profile](../docs/enhancements/feat-add-ab
networkAccessPolicy | NetworkAccessPolicy property to prevent anybody from generating the SAS URI for a disk or a snapshot | `AllowAll`, `DenyAll`, `AllowPrivate` | No | `AllowAll`
diskAccessID | ARM id of the DiskAccess resource for using private endpoints on disks | | No | ``
enableBursting | [enable on-demand bursting](https://docs.microsoft.com/en-us/azure/virtual-machines/disk-bursting) beyond the provisioned performance target of the disk. On-demand bursting only be applied to Premium disk, disk size > 512GB, Ultra & shared disk is not supported. Bursting is disabled by default. | `true`, `false` | No | `false`
useragent | User agent used for [customer usage attribution](https://docs.microsoft.com/en-us/azure/marketplace/azure-partner-customer-usage-attribution)| | No | Generated Useragent formatted `driverName/driverVersion compiler/version (GOARCH-GOOS) gitCommit/buildDate`


- disk created by dynamic provisioning
- disk name format(example): `pvc-e132d37f-9e8f-434a-b599-15a4ab211b39`
Expand Down Expand Up @@ -49,3 +51,4 @@ volumeAttributes.cachingMode | [disk host cache setting](https://docs.microsoft.
resourceGroup | resource group for storing snapshot shots | EXISTING RESOURCE GROUP | No | If not specified, snapshot will be stored in the same resource group as source Azure disk
incremental | take [full or incremental snapshot](https://docs.microsoft.com/en-us/azure/virtual-machines/windows/incremental-snapshots) | `true`, `false` | No | `true`
tags | azure disk [tags](https://docs.microsoft.com/en-us/azure/azure-resource-manager/management/tag-resources) | tag format: 'key1=val1,key2=val2' | No | ""
useragent | User agent used for [customer usage attribution](https://docs.microsoft.com/en-us/azure/marketplace/azure-partner-customer-usage-attribution) | | No | Generated Useragent formatted `driverName/driverVersion compiler/version (GOARCH-GOOS) gitCommit/buildDate`
1 change: 1 addition & 0 deletions pkg/azureconstants/azure_constants.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ const (
TagsField = "tags"
ThrottlingKey = "throttlingKey"
TrueValue = "true"
UserAgentField = "useragent"
VolumeAttributePartition = "partition"
WellKnownTopologyKey = "topology.kubernetes.io/zone"
WriteAcceleratorEnabled = "writeacceleratorenabled"
Expand Down
2 changes: 2 additions & 0 deletions pkg/azuredisk/azuredisk.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ type DriverCore struct {
cloudConfigSecretNamespace string
customUserAgent string
userAgentSuffix string
kubeconfig string
cloud *azure.Cloud
mounter *mount.SafeFormatAndMount
deviceHelper *optimization.SafeDeviceHelper
Expand Down Expand Up @@ -130,6 +131,7 @@ func (d *Driver) Run(endpoint, kubeconfig string, disableAVSetNodes, testingMock
klog.Fatalf("failed to get Azure Cloud Provider, error: %v", err)
}
d.cloud = cloud
d.kubeconfig = kubeconfig

if d.NodeID == "" {
// Disable UseInstanceMetadata for controller to mitigate a timeout issue using IMDS
Expand Down
27 changes: 21 additions & 6 deletions pkg/azuredisk/controllerserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,7 @@ func (d *Driver) CreateVolume(ctx context.Context, req *csi.CreateVolumeRequest)
enableBursting *bool
)

localCloud := d.cloud
tags := make(map[string]string)
parameters := req.GetParameters()
if parameters == nil {
Expand Down Expand Up @@ -171,12 +172,18 @@ func (d *Driver) CreateVolume(ctx context.Context, req *csi.CreateVolumeRequest)
if strings.EqualFold(v, consts.TrueValue) {
enableBursting = to.BoolPtr(true)
}
case consts.UserAgentField:
newUserAgent := v
localCloud, err = azureutils.GetCloudProvider(d.kubeconfig, d.cloudConfigSecretName, d.cloudConfigSecretNamespace, newUserAgent)
if err != nil {
return nil, fmt.Errorf("create cloud with UserAgent(%s) failed with: (%s)", newUserAgent, err)
}
default:
return nil, fmt.Errorf("invalid parameter %s in storage class", k)
}
}

if azureutils.IsAzureStackCloud(d.cloud.Config.Cloud, d.cloud.Config.DisableAzureStackCloud) {
if azureutils.IsAzureStackCloud(localCloud.Config.Cloud, localCloud.Config.DisableAzureStackCloud) {
kassarl marked this conversation as resolved.
Show resolved Hide resolved
if maxShares > 1 {
return nil, status.Error(codes.InvalidArgument, fmt.Sprintf("Invalid maxShares value: %d as Azure Stack does not support shared disk.", maxShares))
}
Expand All @@ -202,7 +209,7 @@ func (d *Driver) CreateVolume(ctx context.Context, req *csi.CreateVolumeRequest)
}

// normalize values
skuName, err := azureutils.NormalizeStorageAccountType(storageAccountType, d.cloud.Config.Cloud, d.cloud.Config.DisableAzureStackCloud)
skuName, err := azureutils.NormalizeStorageAccountType(storageAccountType, localCloud.Config.Cloud, localCloud.Config.DisableAzureStackCloud)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -321,13 +328,13 @@ func (d *Driver) CreateVolume(ctx context.Context, req *csi.CreateVolumeRequest)
}
volumeOptions.SkipGetDiskOperation = d.isGetDiskThrottled()
// Azure Stack Cloud does not support NetworkAccessPolicy
if !azureutils.IsAzureStackCloud(d.cloud.Config.Cloud, d.cloud.Config.DisableAzureStackCloud) {
if !azureutils.IsAzureStackCloud(localCloud.Config.Cloud, localCloud.Config.DisableAzureStackCloud) {
volumeOptions.NetworkAccessPolicy = networkAccessPolicy
if diskAccessID != "" {
volumeOptions.DiskAccessID = &diskAccessID
}
}
diskURI, err := d.cloud.CreateManagedDisk(volumeOptions)
diskURI, err := localCloud.CreateManagedDisk(volumeOptions)
if err != nil {
if strings.Contains(err.Error(), consts.NotFound) {
return nil, status.Error(codes.NotFound, err.Error())
Expand Down Expand Up @@ -826,6 +833,7 @@ func (d *Driver) CreateSnapshot(ctx context.Context, req *csi.CreateSnapshotRequ
incremental := true
var resourceGroup string
var err error
localCloud := d.cloud

parameters := req.GetParameters()
for k, v := range parameters {
Expand All @@ -838,12 +846,19 @@ func (d *Driver) CreateSnapshot(ctx context.Context, req *csi.CreateSnapshotRequ
}
case consts.ResourceGroupField:
resourceGroup = v
case consts.UserAgentField:
newUserAgent := v
localCloud, err = azureutils.GetCloudProvider(d.kubeconfig, d.cloudConfigSecretName, d.cloudConfigSecretNamespace, newUserAgent)
kassarl marked this conversation as resolved.
Show resolved Hide resolved
if err != nil {
return nil, fmt.Errorf("create cloud with UserAgent(%s) failed with: (%s)", newUserAgent, err)

}
default:
return nil, fmt.Errorf("AzureDisk - invalid option %s in VolumeSnapshotClass", k)
}
}

if azureutils.IsAzureStackCloud(d.cloud.Config.Cloud, d.cloud.Config.DisableAzureStackCloud) {
if azureutils.IsAzureStackCloud(localCloud.Config.Cloud, localCloud.Config.DisableAzureStackCloud) {
klog.V(2).Info("Use full snapshot instead as Azure Stack does not support incremental snapshot.")
incremental = false
}
Expand Down Expand Up @@ -883,7 +898,7 @@ func (d *Driver) CreateSnapshot(ctx context.Context, req *csi.CreateSnapshotRequ
}()

klog.V(2).Infof("begin to create snapshot(%s, incremental: %v) under rg(%s)", snapshotName, incremental, resourceGroup)
rerr := d.cloud.SnapshotsClient.CreateOrUpdate(ctx, resourceGroup, snapshotName, snapshot)
rerr := localCloud.SnapshotsClient.CreateOrUpdate(ctx, resourceGroup, snapshotName, snapshot)
if rerr != nil {
if strings.Contains(rerr.Error().Error(), "existing disk") {
return nil, status.Error(codes.AlreadyExists, fmt.Sprintf("request snapshot(%s) under rg(%s) already exists, but the SourceVolumeId is different, error details: %v", snapshotName, resourceGroup, rerr.Error()))
Expand Down
2 changes: 2 additions & 0 deletions test/e2e/dynamic_provisioning_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,7 @@ func (t *dynamicProvisioningTestSuite) defineTests(isMultiZone bool) {
scParameters := map[string]string{
"skuName": "Standard_LRS",
"networkAccessPolicy": "DenyAll",
"userAgent": "azuredisk-e2e-test",
}
test := testsuites.DynamicallyProvisionedVolumeSubpathTester{
CSIDriver: testDriver,
Expand Down Expand Up @@ -210,6 +211,7 @@ func (t *dynamicProvisioningTestSuite) defineTests(isMultiZone bool) {
"perfProfile": "Basic",
// enableBursting can only be applied to Premium disk, disk size > 512GB, Ultra & shared disk is not supported.
"enableBursting": "true",
"userAgent": "azuredisk-e2e-test",
},
}
test.Run(cs, ns)
Expand Down