Skip to content

Commit 119875e

Browse files
author
Stephen Schmitt
committed
Adds multi-writer validation, reduces code duplication, add E2E test
1 parent d32b658 commit 119875e

File tree

11 files changed

+446
-217
lines changed

11 files changed

+446
-217
lines changed

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

Lines changed: 76 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,12 +15,15 @@ limitations under the License.
1515
package gcecloudprovider
1616

1717
import (
18+
computealpha "google.golang.org/api/compute/v0.alpha"
1819
computev1 "google.golang.org/api/compute/v1"
1920
)
2021

2122
type CloudDisk struct {
22-
ZonalDisk *computev1.Disk
23-
RegionalDisk *computev1.Disk
23+
ZonalDisk *computev1.Disk
24+
RegionalDisk *computev1.Disk
25+
ZonalAlphaDisk *computealpha.Disk
26+
RegionalAlphaDisk *computealpha.Disk
2427
}
2528

2629
type CloudDiskType string
@@ -30,6 +33,10 @@ const (
3033
Zonal = "zonal"
3134
// Regional key type.
3235
Regional = "regional"
36+
// ZonalAlpha key type.
37+
ZonalAlpha = "zonalAlpha"
38+
// RegionalAlpha key type.
39+
RegionalAlpha = "regionalAlpha"
3340
// Global key type.
3441
Global = "global"
3542
)
@@ -46,12 +53,28 @@ func RegionalCloudDisk(disk *computev1.Disk) *CloudDisk {
4653
}
4754
}
4855

56+
func ZonalAlphaCloudDisk(disk *computealpha.Disk) *CloudDisk {
57+
return &CloudDisk{
58+
ZonalAlphaDisk: disk,
59+
}
60+
}
61+
62+
func RegionalAlphaCloudDisk(disk *computealpha.Disk) *CloudDisk {
63+
return &CloudDisk{
64+
RegionalAlphaDisk: disk,
65+
}
66+
}
67+
4968
func (d *CloudDisk) Type() CloudDiskType {
5069
switch {
5170
case d.ZonalDisk != nil:
5271
return Zonal
5372
case d.RegionalDisk != nil:
5473
return Regional
74+
case d.ZonalAlphaDisk != nil:
75+
return ZonalAlpha
76+
case d.RegionalAlphaDisk != nil:
77+
return RegionalAlpha
5578
default:
5679
return Global
5780
}
@@ -63,6 +86,10 @@ func (d *CloudDisk) GetUsers() []string {
6386
return d.ZonalDisk.Users
6487
case Regional:
6588
return d.RegionalDisk.Users
89+
case ZonalAlpha:
90+
return d.ZonalAlphaDisk.Users
91+
case RegionalAlpha:
92+
return d.RegionalAlphaDisk.Users
6693
default:
6794
return nil
6895
}
@@ -74,6 +101,10 @@ func (d *CloudDisk) GetName() string {
74101
return d.ZonalDisk.Name
75102
case Regional:
76103
return d.RegionalDisk.Name
104+
case ZonalAlpha:
105+
return d.ZonalAlphaDisk.Name
106+
case RegionalAlpha:
107+
return d.RegionalAlphaDisk.Name
77108
default:
78109
return ""
79110
}
@@ -85,6 +116,10 @@ func (d *CloudDisk) GetKind() string {
85116
return d.ZonalDisk.Kind
86117
case Regional:
87118
return d.RegionalDisk.Kind
119+
case ZonalAlpha:
120+
return d.ZonalAlphaDisk.Kind
121+
case RegionalAlpha:
122+
return d.RegionalAlphaDisk.Kind
88123
default:
89124
return ""
90125
}
@@ -96,6 +131,10 @@ func (d *CloudDisk) GetType() string {
96131
return d.ZonalDisk.Type
97132
case Regional:
98133
return d.RegionalDisk.Type
134+
case ZonalAlpha:
135+
return d.ZonalAlphaDisk.Type
136+
case RegionalAlpha:
137+
return d.RegionalAlphaDisk.Type
99138
default:
100139
return ""
101140
}
@@ -107,6 +146,10 @@ func (d *CloudDisk) GetSelfLink() string {
107146
return d.ZonalDisk.SelfLink
108147
case Regional:
109148
return d.RegionalDisk.SelfLink
149+
case ZonalAlpha:
150+
return d.ZonalAlphaDisk.SelfLink
151+
case RegionalAlpha:
152+
return d.RegionalAlphaDisk.SelfLink
110153
default:
111154
return ""
112155
}
@@ -118,6 +161,10 @@ func (d *CloudDisk) GetSizeGb() int64 {
118161
return d.ZonalDisk.SizeGb
119162
case Regional:
120163
return d.RegionalDisk.SizeGb
164+
case ZonalAlpha:
165+
return d.ZonalAlphaDisk.SizeGb
166+
case RegionalAlpha:
167+
return d.RegionalAlphaDisk.SizeGb
121168
default:
122169
return -1
123170
}
@@ -131,6 +178,10 @@ func (d *CloudDisk) setSizeGb(size int64) {
131178
d.ZonalDisk.SizeGb = size
132179
case Regional:
133180
d.RegionalDisk.SizeGb = size
181+
case ZonalAlpha:
182+
d.ZonalAlphaDisk.SizeGb = size
183+
case RegionalAlpha:
184+
d.RegionalAlphaDisk.SizeGb = size
134185
}
135186
}
136187

@@ -140,6 +191,10 @@ func (d *CloudDisk) GetZone() string {
140191
return d.ZonalDisk.Zone
141192
case Regional:
142193
return d.RegionalDisk.Zone
194+
case ZonalAlpha:
195+
return d.ZonalAlphaDisk.Zone
196+
case RegionalAlpha:
197+
return d.RegionalAlphaDisk.Zone
143198
default:
144199
return ""
145200
}
@@ -151,7 +206,26 @@ func (d *CloudDisk) GetSnapshotId() string {
151206
return d.ZonalDisk.SourceSnapshotId
152207
case Regional:
153208
return d.RegionalDisk.SourceSnapshotId
209+
case ZonalAlpha:
210+
return d.ZonalAlphaDisk.SourceSnapshotId
211+
case RegionalAlpha:
212+
return d.RegionalAlphaDisk.SourceSnapshotId
154213
default:
155214
return ""
156215
}
157216
}
217+
218+
func (d *CloudDisk) GetMultiWriter() bool {
219+
switch d.Type() {
220+
case Zonal:
221+
return false
222+
case Regional:
223+
return false
224+
case ZonalAlpha:
225+
return d.ZonalAlphaDisk.MultiWriter
226+
case RegionalAlpha:
227+
return d.RegionalAlphaDisk.MultiWriter
228+
default:
229+
return false
230+
}
231+
}

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

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -155,15 +155,15 @@ func (cloud *FakeCloudProvider) ListSnapshots(ctx context.Context, filter string
155155
}
156156

157157
// Disk Methods
158-
func (cloud *FakeCloudProvider) GetDisk(ctx context.Context, volKey *meta.Key) (*CloudDisk, error) {
158+
func (cloud *FakeCloudProvider) GetDisk(ctx context.Context, volKey *meta.Key, api ApiVersion) (*CloudDisk, error) {
159159
disk, ok := cloud.disks[volKey.Name]
160160
if !ok {
161161
return nil, notFoundError()
162162
}
163163
return disk, nil
164164
}
165165

166-
func (cloud *FakeCloudProvider) ValidateExistingDisk(ctx context.Context, resp *CloudDisk, diskType string, reqBytes, limBytes int64) error {
166+
func (cloud *FakeCloudProvider) ValidateExistingDisk(ctx context.Context, resp *CloudDisk, diskType string, reqBytes, limBytes int64, multiWriter bool) error {
167167
if resp == nil {
168168
return fmt.Errorf("disk does not exist")
169169
}
@@ -182,6 +182,12 @@ func (cloud *FakeCloudProvider) ValidateExistingDisk(ctx context.Context, resp *
182182
return fmt.Errorf("disk already exists with incompatible type. Need %v. Got %v",
183183
diskType, respType[len(respType)-1])
184184
}
185+
186+
// We are assuming here that a multiWriter disk could be used as non-multiWriter
187+
if multiWriter && !resp.GetMultiWriter() {
188+
return fmt.Errorf("disk already exists with incompatible capability. Need MultiWriter. Got non-MultiWriter")
189+
}
190+
185191
klog.V(4).Infof("Compatible disk already exists")
186192
return nil
187193
}
@@ -190,7 +196,8 @@ func (cloud *FakeCloudProvider) InsertDisk(ctx context.Context, volKey *meta.Key
190196
if disk, ok := cloud.disks[volKey.Name]; ok {
191197
err := cloud.ValidateExistingDisk(ctx, disk, diskType,
192198
int64(capacityRange.GetRequiredBytes()),
193-
int64(capacityRange.GetLimitBytes()))
199+
int64(capacityRange.GetLimitBytes()),
200+
multiWriter)
194201
if err != nil {
195202
return err
196203
}

0 commit comments

Comments
 (0)