Skip to content

Commit 19ed1ed

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request #1257 from k8s-infra-cherrypick-robot/cherry-pick-1254-to-release-1.10
[release-1.10] Add force attach to storage class
2 parents 71eebef + a36b930 commit 19ed1ed

File tree

14 files changed

+510
-144
lines changed

14 files changed

+510
-144
lines changed

‎pkg/common/parameters.go

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ const (
2929
ParameterKeyLabels = "labels"
3030
ParameterKeyProvisionedIOPSOnCreate = "provisioned-iops-on-create"
3131
ParameterKeyProvisionedThroughputOnCreate = "provisioned-throughput-on-create"
32+
ParameterAvailabilityClass = "availability-class"
3233

3334
// Parameters for VolumeSnapshotClass
3435
ParameterKeyStorageLocations = "storage-locations"
@@ -38,6 +39,10 @@ const (
3839
DiskImageType = "images"
3940
replicationTypeNone = "none"
4041

42+
// Parameters for AvailabilityClass
43+
ParameterNoAvailabilityClass = "none"
44+
ParameterRegionalHardFailoverClass = "regional-hard-failover"
45+
4146
// Keys for PV and PVC parameters as reported by external-provisioner
4247
ParameterKeyPVCName = "csi.storage.k8s.io/pvc/name"
4348
ParameterKeyPVCNamespace = "csi.storage.k8s.io/pvc/namespace"
@@ -83,6 +88,8 @@ type DiskParameters struct {
8388
// Values: {int64}
8489
// Default: none
8590
ProvisionedThroughputOnCreate int64
91+
// Default: false
92+
ForceAttach bool
8693
}
8794

8895
// SnapshotParameters contains normalized and defaulted parameters for snapshots
@@ -155,6 +162,14 @@ func ExtractAndDefaultParameters(parameters map[string]string, driverName string
155162
return p, fmt.Errorf("parameters contain invalid provisionedThroughputOnCreate parameter: %w", err)
156163
}
157164
p.ProvisionedThroughputOnCreate = paramProvisionedThroughputOnCreate
165+
case ParameterAvailabilityClass:
166+
paramAvailabilityClass, err := ConvertStringToAvailabilityClass(v)
167+
if err != nil {
168+
return p, fmt.Errorf("parameters contain invalid availability class parameter: %w", err)
169+
}
170+
if paramAvailabilityClass == ParameterRegionalHardFailoverClass {
171+
p.ForceAttach = true
172+
}
158173
default:
159174
return p, fmt.Errorf("parameters contains invalid option %q", k)
160175
}

‎pkg/common/parameters_test.go

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -178,6 +178,27 @@ func TestExtractAndDefaultParameters(t *testing.T) {
178178
Labels: map[string]string{"key1": "value1", "label-1": "value-a", "label-2": "label-value-2"},
179179
},
180180
},
181+
{
182+
name: "availability class parameters",
183+
parameters: map[string]string{ParameterAvailabilityClass: ParameterRegionalHardFailoverClass},
184+
expectParams: DiskParameters{
185+
DiskType: "pd-standard",
186+
ReplicationType: "none",
187+
ForceAttach: true,
188+
Tags: map[string]string{},
189+
Labels: map[string]string{},
190+
},
191+
},
192+
{
193+
name: "no force attach parameters",
194+
parameters: map[string]string{ParameterAvailabilityClass: ParameterNoAvailabilityClass},
195+
expectParams: DiskParameters{
196+
DiskType: "pd-standard",
197+
ReplicationType: "none",
198+
Tags: map[string]string{},
199+
Labels: map[string]string{},
200+
},
201+
},
181202
}
182203

183204
for _, tc := range tests {

‎pkg/common/utils.go

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -284,6 +284,28 @@ func ConvertMiStringToInt64(str string) (int64, error) {
284284
return volumehelpers.RoundUpToMiB(quantity)
285285
}
286286

287+
// ConvertStringToBool converts a string to a boolean.
288+
func ConvertStringToBool(str string) (bool, error) {
289+
switch strings.ToLower(str) {
290+
case "true":
291+
return true, nil
292+
case "false":
293+
return false, nil
294+
}
295+
return false, fmt.Errorf("Unexpected boolean string %s", str)
296+
}
297+
298+
// ConvertStringToAvailabilityClass converts a string to an availability class string.
299+
func ConvertStringToAvailabilityClass(str string) (string, error) {
300+
switch strings.ToLower(str) {
301+
case ParameterNoAvailabilityClass:
302+
return ParameterNoAvailabilityClass, nil
303+
case ParameterRegionalHardFailoverClass:
304+
return ParameterRegionalHardFailoverClass, nil
305+
}
306+
return "", fmt.Errorf("Unexpected boolean string %s", str)
307+
}
308+
287309
// ParseMachineType returns an extracted machineType from a URL, or empty if not found.
288310
// machineTypeUrl: Full or partial URL of the machine type resource, in the format:
289311
//

‎pkg/common/utils_test.go

Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -806,6 +806,114 @@ func TestConvertMiStringToInt64(t *testing.T) {
806806
}
807807
}
808808

809+
func TestConvertStringToBool(t *testing.T) {
810+
tests := []struct {
811+
desc string
812+
inputStr string
813+
expected bool
814+
expectError bool
815+
}{
816+
{
817+
desc: "valid true",
818+
inputStr: "true",
819+
expected: true,
820+
expectError: false,
821+
},
822+
{
823+
desc: "valid mixed case true",
824+
inputStr: "True",
825+
expected: true,
826+
expectError: false,
827+
},
828+
{
829+
desc: "valid false",
830+
inputStr: "false",
831+
expected: false,
832+
expectError: false,
833+
},
834+
{
835+
desc: "valid mixed case false",
836+
inputStr: "False",
837+
expected: false,
838+
expectError: false,
839+
},
840+
{
841+
desc: "invalid",
842+
inputStr: "yes",
843+
expected: false,
844+
expectError: true,
845+
},
846+
}
847+
for _, tc := range tests {
848+
t.Run(tc.desc, func(t *testing.T) {
849+
got, err := ConvertStringToBool(tc.inputStr)
850+
if err != nil && !tc.expectError {
851+
t.Errorf("Got error %v converting string to bool %s; expect no error", err, tc.inputStr)
852+
}
853+
if err == nil && tc.expectError {
854+
t.Errorf("Got no error converting string to bool %s; expect an error", tc.inputStr)
855+
}
856+
if err == nil && got != tc.expected {
857+
t.Errorf("Got %v for converting string to bool; expect %v", got, tc.expected)
858+
}
859+
})
860+
}
861+
}
862+
863+
func TestConvertStringToAvailabilityClass(t *testing.T) {
864+
tests := []struct {
865+
desc string
866+
inputStr string
867+
expected string
868+
expectError bool
869+
}{
870+
{
871+
desc: "valid none",
872+
inputStr: "none",
873+
expected: ParameterNoAvailabilityClass,
874+
expectError: false,
875+
},
876+
{
877+
desc: "valid mixed case none",
878+
inputStr: "None",
879+
expected: ParameterNoAvailabilityClass,
880+
expectError: false,
881+
},
882+
{
883+
desc: "valid failover",
884+
inputStr: "regional-hard-failover",
885+
expected: ParameterRegionalHardFailoverClass,
886+
expectError: false,
887+
},
888+
{
889+
desc: "valid mixed case failover",
890+
inputStr: "Regional-Hard-Failover",
891+
expected: ParameterRegionalHardFailoverClass,
892+
expectError: false,
893+
},
894+
{
895+
desc: "invalid",
896+
inputStr: "yes",
897+
expected: "",
898+
expectError: true,
899+
},
900+
}
901+
for _, tc := range tests {
902+
t.Run(tc.desc, func(t *testing.T) {
903+
got, err := ConvertStringToAvailabilityClass(tc.inputStr)
904+
if err != nil && !tc.expectError {
905+
t.Errorf("Got error %v converting string to availablity class %s; expect no error", err, tc.inputStr)
906+
}
907+
if err == nil && tc.expectError {
908+
t.Errorf("Got no error converting string to availablity class %s; expect an error", tc.inputStr)
909+
}
910+
if err == nil && got != tc.expected {
911+
t.Errorf("Got %v for converting string to availablity class; expect %v", got, tc.expected)
912+
}
913+
})
914+
}
915+
}
916+
809917
func TestParseMachineType(t *testing.T) {
810918
tests := []struct {
811919
desc string

‎pkg/gce-cloud-provider/compute/fake-gce.go

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -246,15 +246,16 @@ func (cloud *FakeCloudProvider) DeleteDisk(ctx context.Context, project string,
246246
return nil
247247
}
248248

249-
func (cloud *FakeCloudProvider) AttachDisk(ctx context.Context, project string, volKey *meta.Key, readWrite, diskType, instanceZone, instanceName string) error {
249+
func (cloud *FakeCloudProvider) AttachDisk(ctx context.Context, project string, volKey *meta.Key, readWrite, diskType, instanceZone, instanceName string, forceAttach bool) error {
250250
source := cloud.GetDiskSourceURI(project, volKey)
251251

252252
attachedDiskV1 := &computev1.AttachedDisk{
253-
DeviceName: volKey.Name,
254-
Kind: diskKind,
255-
Mode: readWrite,
256-
Source: source,
257-
Type: diskType,
253+
DeviceName: volKey.Name,
254+
Kind: diskKind,
255+
Mode: readWrite,
256+
Source: source,
257+
Type: diskType,
258+
ForceAttach: forceAttach,
258259
}
259260
instance, ok := cloud.instances[instanceName]
260261
if !ok {
@@ -538,14 +539,14 @@ func (cloud *FakeBlockingCloudProvider) DetachDisk(ctx context.Context, project,
538539
return cloud.FakeCloudProvider.DetachDisk(ctx, project, deviceName, instanceZone, instanceName)
539540
}
540541

541-
func (cloud *FakeBlockingCloudProvider) AttachDisk(ctx context.Context, project string, volKey *meta.Key, readWrite, diskType, instanceZone, instanceName string) error {
542+
func (cloud *FakeBlockingCloudProvider) AttachDisk(ctx context.Context, project string, volKey *meta.Key, readWrite, diskType, instanceZone, instanceName string, forceAttach bool) error {
542543
execute := make(chan Signal)
543544
cloud.ReadyToExecute <- execute
544545
val := <-execute
545546
if val.ReportError {
546547
return fmt.Errorf("force mock error for AttachDisk: volkey %s", volKey)
547548
}
548-
return cloud.FakeCloudProvider.AttachDisk(ctx, project, volKey, readWrite, diskType, instanceZone, instanceName)
549+
return cloud.FakeCloudProvider.AttachDisk(ctx, project, volKey, readWrite, diskType, instanceZone, instanceName, forceAttach)
549550
}
550551

551552
func notFoundError() *googleapi.Error {

‎pkg/gce-cloud-provider/compute/gce-compute.go

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,7 @@ type GCECompute interface {
9292
ValidateExistingDisk(ctx context.Context, disk *CloudDisk, params common.DiskParameters, reqBytes, limBytes int64, multiWriter bool) error
9393
InsertDisk(ctx context.Context, project string, volKey *meta.Key, params common.DiskParameters, capBytes int64, capacityRange *csi.CapacityRange, replicaZones []string, snapshotID string, volumeContentSourceVolumeID string, multiWriter bool) error
9494
DeleteDisk(ctx context.Context, project string, volumeKey *meta.Key) error
95-
AttachDisk(ctx context.Context, project string, volKey *meta.Key, readWrite, diskType, instanceZone, instanceName string) error
95+
AttachDisk(ctx context.Context, project string, volKey *meta.Key, readWrite, diskType, instanceZone, instanceName string, forceAttach bool) error
9696
DetachDisk(ctx context.Context, project, deviceName, instanceZone, instanceName string) error
9797
GetDiskSourceURI(project string, volKey *meta.Key) string
9898
GetDiskTypeURI(project string, volKey *meta.Key, diskType string) string
@@ -700,7 +700,7 @@ func (cloud *CloudProvider) deleteRegionalDisk(ctx context.Context, project, reg
700700
return nil
701701
}
702702

703-
func (cloud *CloudProvider) AttachDisk(ctx context.Context, project string, volKey *meta.Key, readWrite, diskType, instanceZone, instanceName string) error {
703+
func (cloud *CloudProvider) AttachDisk(ctx context.Context, project string, volKey *meta.Key, readWrite, diskType, instanceZone, instanceName string, forceAttach bool) error {
704704
klog.V(5).Infof("Attaching disk %v to %s", volKey, instanceName)
705705
source := cloud.GetDiskSourceURI(project, volKey)
706706

@@ -714,9 +714,13 @@ func (cloud *CloudProvider) AttachDisk(ctx context.Context, project string, volK
714714
Mode: readWrite,
715715
Source: source,
716716
Type: diskType,
717+
// This parameter is ignored in the call, the ForceAttach decorator
718+
// (query parameter) is the important one. We'll set it in both places
719+
// in case that behavior changes.
720+
ForceAttach: forceAttach,
717721
}
718722

719-
op, err := cloud.service.Instances.AttachDisk(project, instanceZone, instanceName, attachedDiskV1).Context(ctx).Do()
723+
op, err := cloud.service.Instances.AttachDisk(project, instanceZone, instanceName, attachedDiskV1).Context(ctx).ForceAttach(forceAttach).Do()
720724
if err != nil {
721725
return fmt.Errorf("failed cloud service attach disk call: %w", err)
722726
}

0 commit comments

Comments
 (0)