Skip to content

Commit 4473873

Browse files
nikhilkatharenikhilkathare
authored
Merge pull request #2 from kubernetes-sigs/master
Pull request from sigs head to forked head
2 parents ed82c10 + cca3c14 commit 4473873

File tree

8 files changed

+187
-2
lines changed

8 files changed

+187
-2
lines changed

‎OWNERS

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,4 +7,5 @@ reviewers:
77
approvers:
88
- davidz627
99
- saad-ali
10-
- msau42
10+
- msau42
11+
- verult

‎deploy/kubernetes/overlays/prow-gke-release-staging-head/kustomization.yaml

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,3 +11,9 @@ patchesJson6902:
1111
kind: Deployment
1212
name: csi-gce-pd-controller
1313
path: leader-election-flag.yaml
14+
- target:
15+
group: apps
16+
version: v1
17+
kind: Deployment
18+
name: csi-gce-pd-controller
19+
path: volume-inuse-error-handler.yaml

‎deploy/kubernetes/overlays/prow-gke-release-staging-head/volume-inuse-error-handler.yaml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
- op: add
2+
path: /spec/template/spec/containers/2/args/-
3+
value: "--handle-volume-inuse-error=false"

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

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,17 @@ func (d *CloudDisk) GetKind() string {
9292
}
9393
}
9494

95+
func (d *CloudDisk) GetStatus() string {
96+
switch d.Type() {
97+
case Zonal:
98+
return d.ZonalDisk.Status
99+
case Regional:
100+
return d.RegionalDisk.Status
101+
default:
102+
return "Unknown"
103+
}
104+
}
105+
95106
// GetPDType returns the type of the PD as either 'pd-standard' or 'pd-ssd' The
96107
// "Type" field on the compute disk is stored as a url like
97108
// projects/project/zones/zone/diskTypes/pd-standard

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

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,9 @@ type FakeCloudProvider struct {
4848
pageTokens map[string]sets.String
4949
instances map[string]*computev1.Instance
5050
snapshots map[string]*computev1.Snapshot
51+
52+
// marker to set disk status during InsertDisk operation.
53+
mockDiskStatus string
5154
}
5255

5356
var _ GCECompute = &FakeCloudProvider{}
@@ -60,6 +63,8 @@ func CreateFakeCloudProvider(project, zone string, cloudDisks []*CloudDisk) (*Fa
6063
instances: map[string]*computev1.Instance{},
6164
snapshots: map[string]*computev1.Snapshot{},
6265
pageTokens: map[string]sets.String{},
66+
// A newly created disk is marked READY by default.
67+
mockDiskStatus: "READY",
6368
}
6469
for _, d := range cloudDisks {
6570
fcp.disks[d.GetName()] = d
@@ -250,6 +255,7 @@ func (cloud *FakeCloudProvider) InsertDisk(ctx context.Context, volKey *meta.Key
250255
Type: cloud.GetDiskTypeURI(volKey, params.DiskType),
251256
SelfLink: fmt.Sprintf("projects/%s/zones/%s/disks/%s", cloud.project, volKey.Zone, volKey.Name),
252257
SourceSnapshotId: snapshotID,
258+
Status: cloud.mockDiskStatus,
253259
}
254260
if params.DiskEncryptionKMSKey != "" {
255261
diskToCreateGA.DiskEncryptionKey = &computev1.CustomerEncryptionKey{
@@ -265,6 +271,7 @@ func (cloud *FakeCloudProvider) InsertDisk(ctx context.Context, volKey *meta.Key
265271
Type: cloud.GetDiskTypeURI(volKey, params.DiskType),
266272
SelfLink: fmt.Sprintf("projects/%s/regions/%s/disks/%s", cloud.project, volKey.Region, volKey.Name),
267273
SourceSnapshotId: snapshotID,
274+
Status: cloud.mockDiskStatus,
268275
}
269276
if params.DiskEncryptionKMSKey != "" {
270277
diskToCreateV1.DiskEncryptionKey = &computev1.CustomerEncryptionKey{
@@ -466,6 +473,10 @@ func (cloud *FakeCloudProvider) getGlobalSnapshotURI(snapshotName string) string
466473
snapshotName)
467474
}
468475

476+
func (cloud *FakeCloudProvider) UpdateDiskStatus(s string) {
477+
cloud.mockDiskStatus = s
478+
}
479+
469480
type FakeBlockingCloudProvider struct {
470481
*FakeCloudProvider
471482
ReadyToExecute chan chan struct{}

‎pkg/gce-pd-csi-driver/controller.go

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,29 @@ const (
6262
replicationTypeRegionalPD = "regional-pd"
6363
)
6464

65+
func isDiskReady(disk *gce.CloudDisk) (bool, error) {
66+
status := disk.GetStatus()
67+
switch status {
68+
case "READY":
69+
return true, nil
70+
case "FAILED":
71+
return false, fmt.Errorf("Disk %s status is FAILED", disk.GetName())
72+
case "CREATING":
73+
klog.V(4).Infof("Disk %s status is CREATING", disk.GetName())
74+
return false, nil
75+
case "DELETING":
76+
klog.V(4).Infof("Disk %s status is DELETING", disk.GetName())
77+
return false, nil
78+
case "RESTORING":
79+
klog.V(4).Infof("Disk %s status is RESTORING", disk.GetName())
80+
return false, nil
81+
default:
82+
klog.V(4).Infof("Disk %s status is: %s", disk.GetName(), status)
83+
}
84+
85+
return false, nil
86+
}
87+
6588
func (gceCS *GCEControllerServer) CreateVolume(ctx context.Context, req *csi.CreateVolumeRequest) (*csi.CreateVolumeResponse, error) {
6689
var err error
6790
// Validate arguments
@@ -143,6 +166,16 @@ func (gceCS *GCEControllerServer) CreateVolume(ctx context.Context, req *csi.Cre
143166
if err != nil {
144167
return nil, status.Error(codes.AlreadyExists, fmt.Sprintf("CreateVolume disk already exists with same name and is incompatible: %v", err))
145168
}
169+
170+
ready, err := isDiskReady(existingDisk)
171+
if err != nil {
172+
return nil, status.Error(codes.Internal, fmt.Sprintf("CreateVolume disk %v had error checking ready status: %v", volKey, err))
173+
}
174+
175+
if !ready {
176+
return nil, status.Error(codes.Internal, fmt.Sprintf("CreateVolume disk %v is not ready", volKey))
177+
}
178+
146179
// If there is no validation error, immediately return success
147180
klog.V(4).Infof("CreateVolume succeeded for disk %v, it already exists and was compatible", volKey)
148181
return generateCreateVolumeResponse(existingDisk, capBytes, zones), nil
@@ -187,6 +220,15 @@ func (gceCS *GCEControllerServer) CreateVolume(ctx context.Context, req *csi.Cre
187220
default:
188221
return nil, status.Error(codes.InvalidArgument, fmt.Sprintf("CreateVolume replication type '%s' is not supported", params.ReplicationType))
189222
}
223+
224+
ready, err := isDiskReady(disk)
225+
if err != nil {
226+
return nil, status.Error(codes.Internal, fmt.Sprintf("CreateVolume disk %v had error checking ready status: %v", volKey, err))
227+
}
228+
if !ready {
229+
return nil, status.Error(codes.Internal, fmt.Sprintf("CreateVolume disk %v is not ready", volKey))
230+
}
231+
190232
klog.V(4).Infof("CreateVolume succeeded for disk %v", volKey)
191233
return generateCreateVolumeResponse(disk, capBytes, zones), nil
192234

‎pkg/gce-pd-csi-driver/controller_test.go

Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1549,3 +1549,111 @@ func TestVolumeOperationConcurrency(t *testing.T) {
15491549
t.Errorf("Unexpected error: %v", err)
15501550
}
15511551
}
1552+
1553+
func TestCreateVolumeDiskReady(t *testing.T) {
1554+
// Define test cases
1555+
testCases := []struct {
1556+
name string
1557+
diskStatus string
1558+
req *csi.CreateVolumeRequest
1559+
expVol *csi.Volume
1560+
expErrCode codes.Code
1561+
}{
1562+
{
1563+
name: "disk status RESTORING",
1564+
diskStatus: "RESTORING",
1565+
req: &csi.CreateVolumeRequest{
1566+
Name: "test-name",
1567+
CapacityRange: stdCapRange,
1568+
VolumeCapabilities: stdVolCaps,
1569+
Parameters: stdParams,
1570+
},
1571+
expErrCode: codes.Internal,
1572+
},
1573+
{
1574+
name: "disk status CREATING",
1575+
diskStatus: "CREATING",
1576+
req: &csi.CreateVolumeRequest{
1577+
Name: "test-name",
1578+
CapacityRange: stdCapRange,
1579+
VolumeCapabilities: stdVolCaps,
1580+
Parameters: stdParams,
1581+
},
1582+
expErrCode: codes.Internal,
1583+
},
1584+
{
1585+
name: "disk status DELETING",
1586+
diskStatus: "DELETING",
1587+
req: &csi.CreateVolumeRequest{
1588+
Name: "test-name",
1589+
CapacityRange: stdCapRange,
1590+
VolumeCapabilities: stdVolCaps,
1591+
Parameters: stdParams,
1592+
},
1593+
expErrCode: codes.Internal,
1594+
},
1595+
{
1596+
name: "disk status FAILED",
1597+
diskStatus: "FAILED",
1598+
req: &csi.CreateVolumeRequest{
1599+
Name: "test-name",
1600+
CapacityRange: stdCapRange,
1601+
VolumeCapabilities: stdVolCaps,
1602+
Parameters: stdParams,
1603+
},
1604+
expErrCode: codes.Internal,
1605+
},
1606+
{
1607+
name: "success default",
1608+
diskStatus: "READY",
1609+
req: &csi.CreateVolumeRequest{
1610+
Name: "test-name",
1611+
CapacityRange: stdCapRange,
1612+
VolumeCapabilities: stdVolCaps,
1613+
Parameters: stdParams,
1614+
},
1615+
expVol: &csi.Volume{
1616+
CapacityBytes: common.GbToBytes(20),
1617+
VolumeId: testVolumeID,
1618+
VolumeContext: nil,
1619+
AccessibleTopology: stdTopology,
1620+
},
1621+
},
1622+
}
1623+
1624+
// Run test cases
1625+
for _, tc := range testCases {
1626+
t.Run(tc.name, func(t *testing.T) {
1627+
fcp, err := gce.CreateFakeCloudProvider(project, zone, nil)
1628+
if err != nil {
1629+
t.Fatalf("Failed to create fake cloud provider: %v", err)
1630+
}
1631+
1632+
// Setup hook to create new disks with given status.
1633+
fcp.UpdateDiskStatus(tc.diskStatus)
1634+
// Setup new driver each time so no interference
1635+
gceDriver := initGCEDriverWithCloudProvider(t, fcp)
1636+
// Start Test
1637+
resp, err := gceDriver.cs.CreateVolume(context.Background(), tc.req)
1638+
//check response
1639+
if err != nil {
1640+
serverError, ok := status.FromError(err)
1641+
if !ok {
1642+
t.Fatalf("Could not get error status code from err: %v", serverError)
1643+
}
1644+
if serverError.Code() != tc.expErrCode {
1645+
t.Fatalf("Expected error code: %v, got: %v. err : %v", tc.expErrCode, serverError.Code(), err)
1646+
}
1647+
return
1648+
}
1649+
if tc.expErrCode != codes.OK {
1650+
t.Fatalf("Expected error: %v, got no error", tc.expErrCode)
1651+
}
1652+
1653+
vol := resp.GetVolume()
1654+
if !reflect.DeepEqual(vol, tc.expVol) {
1655+
t.Fatalf("Mismatch in expected vol %v, current volume: %v\n", tc.expVol, vol)
1656+
}
1657+
})
1658+
}
1659+
}

‎test/k8s-integration/cluster.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -155,11 +155,14 @@ func clusterUpGKE(gceZone, gceRegion string, numNodes int, imageType string, use
155155

156156
var cmd *exec.Cmd
157157
cmdParams := []string{"container", "clusters", "create", gkeTestClusterName,
158-
locationArg, locationVal, "--num-nodes", strconv.Itoa(numNodes)}
158+
locationArg, locationVal, "--num-nodes", strconv.Itoa(numNodes),
159+
"--quiet", "--machine-type", "n1-standard-2", "--image-type", imageType}
159160
if isVariableSet(gkeClusterVer) {
160161
cmdParams = append(cmdParams, "--cluster-version", *gkeClusterVer)
161162
} else {
162163
cmdParams = append(cmdParams, "--release-channel", *gkeReleaseChannel)
164+
// release channel based GKE clusters require autorepair to be enabled.
165+
cmdParams = append(cmdParams, "--enable-autorepair")
163166
}
164167

165168
if useManagedDriver {

0 commit comments

Comments
 (0)