Skip to content

Commit 1c0e716

Browse files
moshevaynermoshevayner
committed
refactor(linode): extract MockLinodeClient/MockLinodeCloud to remove test duplication
Signed-off-by: Moshe Vayner <moshe@vayner.me>
1 parent 4b89ddf commit 1c0e716

7 files changed

Lines changed: 408 additions & 564 deletions

File tree

‎upup/pkg/fi/cloudup/linode/cloud_test.go‎

Lines changed: 43 additions & 169 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@ limitations under the License.
1717
package linode
1818

1919
import (
20-
"context"
2120
"errors"
2221
"net"
2322
"reflect"
@@ -33,131 +32,6 @@ import (
3332
"k8s.io/kops/upup/pkg/fi"
3433
)
3534

36-
type fakeLinodeCloudClient struct {
37-
listInstancesResponse []linodego.Instance
38-
listInstancesError error
39-
40-
listNodeBalancersResponse []linodego.NodeBalancer
41-
42-
deleteInstanceErr error
43-
deleteInstanceErrByID map[int]error
44-
deletedInstanceIDs []int
45-
46-
updateInstanceErr error
47-
updateInstanceErrByID map[int]error
48-
updatedInstanceIDs []int
49-
updatedTagsByID map[int][]string
50-
}
51-
52-
func (f *fakeLinodeCloudClient) ListSSHKeys(ctx context.Context, opts *linodego.ListOptions) ([]linodego.SSHKey, error) {
53-
return nil, nil
54-
}
55-
56-
func (f *fakeLinodeCloudClient) CreateSSHKey(ctx context.Context, opts linodego.SSHKeyCreateOptions) (*linodego.SSHKey, error) {
57-
return nil, nil
58-
}
59-
60-
func (f *fakeLinodeCloudClient) DeleteSSHKey(ctx context.Context, keyID int) error {
61-
return nil
62-
}
63-
64-
func (f *fakeLinodeCloudClient) ListInstances(ctx context.Context, opts *linodego.ListOptions) ([]linodego.Instance, error) {
65-
if f.listInstancesError != nil {
66-
return nil, f.listInstancesError
67-
}
68-
return f.listInstancesResponse, nil
69-
}
70-
71-
func (f *fakeLinodeCloudClient) CreateInstance(ctx context.Context, opts linodego.InstanceCreateOptions) (*linodego.Instance, error) {
72-
return nil, nil
73-
}
74-
75-
func (f *fakeLinodeCloudClient) DeleteInstance(ctx context.Context, linodeID int) error {
76-
f.deletedInstanceIDs = append(f.deletedInstanceIDs, linodeID)
77-
if f.deleteInstanceErrByID != nil {
78-
if err := f.deleteInstanceErrByID[linodeID]; err != nil {
79-
return err
80-
}
81-
}
82-
return f.deleteInstanceErr
83-
}
84-
85-
func (f *fakeLinodeCloudClient) UpdateInstance(ctx context.Context, linodeID int, opts linodego.InstanceUpdateOptions) (*linodego.Instance, error) {
86-
f.updatedInstanceIDs = append(f.updatedInstanceIDs, linodeID)
87-
if opts.Tags != nil {
88-
if f.updatedTagsByID == nil {
89-
f.updatedTagsByID = make(map[int][]string)
90-
}
91-
f.updatedTagsByID[linodeID] = append([]string(nil), (*opts.Tags)...)
92-
}
93-
if f.updateInstanceErrByID != nil {
94-
if err := f.updateInstanceErrByID[linodeID]; err != nil {
95-
return nil, err
96-
}
97-
}
98-
if f.updateInstanceErr != nil {
99-
return nil, f.updateInstanceErr
100-
}
101-
102-
updated := &linodego.Instance{ID: linodeID}
103-
if opts.Tags != nil {
104-
updated.Tags = append([]string(nil), (*opts.Tags)...)
105-
}
106-
return updated, nil
107-
}
108-
109-
func (f *fakeLinodeCloudClient) ListVolumes(ctx context.Context, opts *linodego.ListOptions) ([]linodego.Volume, error) {
110-
return nil, nil
111-
}
112-
113-
func (f *fakeLinodeCloudClient) CreateVolume(ctx context.Context, opts linodego.VolumeCreateOptions) (*linodego.Volume, error) {
114-
return nil, nil
115-
}
116-
117-
func (f *fakeLinodeCloudClient) DeleteVolume(ctx context.Context, volumeID int) error {
118-
return nil
119-
}
120-
121-
func (f *fakeLinodeCloudClient) ListNodeBalancers(ctx context.Context, opts *linodego.ListOptions) ([]linodego.NodeBalancer, error) {
122-
return f.listNodeBalancersResponse, nil
123-
}
124-
125-
func (f *fakeLinodeCloudClient) GetNodeBalancer(ctx context.Context, nodebalancerID int) (*linodego.NodeBalancer, error) {
126-
return nil, nil
127-
}
128-
129-
func (f *fakeLinodeCloudClient) CreateNodeBalancer(ctx context.Context, opts linodego.NodeBalancerCreateOptions) (*linodego.NodeBalancer, error) {
130-
return nil, nil
131-
}
132-
133-
func (f *fakeLinodeCloudClient) DeleteNodeBalancer(ctx context.Context, nodebalancerID int) error {
134-
return nil
135-
}
136-
137-
func (f *fakeLinodeCloudClient) ListNodeBalancerConfigs(ctx context.Context, nodebalancerID int, opts *linodego.ListOptions) ([]linodego.NodeBalancerConfig, error) {
138-
return nil, nil
139-
}
140-
141-
func (f *fakeLinodeCloudClient) CreateNodeBalancerConfig(ctx context.Context, nodebalancerID int, opts linodego.NodeBalancerConfigCreateOptions) (*linodego.NodeBalancerConfig, error) {
142-
return nil, nil
143-
}
144-
145-
func (f *fakeLinodeCloudClient) RebuildNodeBalancerConfig(ctx context.Context, nodebalancerID int, configID int, opts linodego.NodeBalancerConfigRebuildOptions) (*linodego.NodeBalancerConfig, error) {
146-
return nil, nil
147-
}
148-
149-
func (f *fakeLinodeCloudClient) ListNodeBalancerNodes(ctx context.Context, nodebalancerID int, configID int, opts *linodego.ListOptions) ([]linodego.NodeBalancerNode, error) {
150-
return nil, nil
151-
}
152-
153-
func (f *fakeLinodeCloudClient) CreateNodeBalancerNode(ctx context.Context, nodebalancerID int, configID int, opts linodego.NodeBalancerNodeCreateOptions) (*linodego.NodeBalancerNode, error) {
154-
return nil, nil
155-
}
156-
157-
func (f *fakeLinodeCloudClient) UpdateNodeBalancerNode(ctx context.Context, nodebalancerID int, configID int, nodeID int, opts linodego.NodeBalancerNodeUpdateOptions) (*linodego.NodeBalancerNode, error) {
158-
return nil, nil
159-
}
160-
16135
func ptrIP(s string) *net.IP {
16236
ip := net.ParseIP(s)
16337
if ip == nil {
@@ -226,21 +100,21 @@ func TestNewCloud(t *testing.T) {
226100

227101
func TestDeleteInstance(t *testing.T) {
228102
t.Run("deletes instance", func(t *testing.T) {
229-
client := &fakeLinodeCloudClient{}
103+
client := &MockLinodeClient{}
230104
cloud := &Cloud{region: "us-east", client: client}
231105

232106
err := cloud.DeleteInstance(&cloudinstances.CloudInstance{ID: "42"})
233107
if err != nil {
234108
t.Fatalf("DeleteInstance returned error: %v", err)
235109
}
236110

237-
if got, want := client.deletedInstanceIDs, []int{42}; !reflect.DeepEqual(got, want) {
111+
if got, want := client.DeletedInstanceIDs, []int{42}; !reflect.DeepEqual(got, want) {
238112
t.Fatalf("unexpected deleted IDs: got %v, want %v", got, want)
239113
}
240114
})
241115

242116
t.Run("rejects invalid id", func(t *testing.T) {
243-
client := &fakeLinodeCloudClient{}
117+
client := &MockLinodeClient{}
244118
cloud := &Cloud{region: "us-east", client: client}
245119

246120
err := cloud.DeleteInstance(&cloudinstances.CloudInstance{ID: "not-a-number"})
@@ -251,13 +125,13 @@ func TestDeleteInstance(t *testing.T) {
251125
t.Fatalf("unexpected error: %v", err)
252126
}
253127

254-
if len(client.deletedInstanceIDs) != 0 {
255-
t.Fatalf("expected no delete calls, got %v", client.deletedInstanceIDs)
128+
if len(client.DeletedInstanceIDs) != 0 {
129+
t.Fatalf("expected no delete calls, got %v", client.DeletedInstanceIDs)
256130
}
257131
})
258132

259133
t.Run("ignores not found", func(t *testing.T) {
260-
client := &fakeLinodeCloudClient{deleteInstanceErrByID: map[int]error{
134+
client := &MockLinodeClient{DeleteInstanceErrByID: map[int]error{
261135
42: &linodego.Error{Code: 404, Message: "not found"},
262136
}}
263137
cloud := &Cloud{region: "us-east", client: client}
@@ -267,13 +141,13 @@ func TestDeleteInstance(t *testing.T) {
267141
t.Fatalf("DeleteInstance returned error: %v", err)
268142
}
269143

270-
if got, want := client.deletedInstanceIDs, []int{42}; !reflect.DeepEqual(got, want) {
144+
if got, want := client.DeletedInstanceIDs, []int{42}; !reflect.DeepEqual(got, want) {
271145
t.Fatalf("unexpected deleted IDs: got %v, want %v", got, want)
272146
}
273147
})
274148

275149
t.Run("returns API errors", func(t *testing.T) {
276-
client := &fakeLinodeCloudClient{deleteInstanceErrByID: map[int]error{
150+
client := &MockLinodeClient{DeleteInstanceErrByID: map[int]error{
277151
42: errors.New("api unavailable"),
278152
}}
279153
cloud := &Cloud{region: "us-east", client: client}
@@ -290,8 +164,8 @@ func TestDeleteInstance(t *testing.T) {
290164

291165
func TestDeleteGroup(t *testing.T) {
292166
t.Run("deletes instances for group and cluster", func(t *testing.T) {
293-
client := &fakeLinodeCloudClient{
294-
listInstancesResponse: []linodego.Instance{
167+
client := &MockLinodeClient{
168+
ListInstancesResponse: []linodego.Instance{
295169
{ID: 101, Tags: []string{"kops.k8s.io/instance-group:nodes-us-east", "kops.k8s.io/cluster:example.k8s.local"}},
296170
{ID: 102, Tags: []string{"kops.k8s.io/instance-group:nodes-us-east", "kops.k8s.io/cluster:example.k8s.local"}},
297171
{ID: 103, Tags: []string{"kops.k8s.io/instance-group:nodes-us-east", "kops.k8s.io/cluster:other.k8s.local"}},
@@ -316,15 +190,15 @@ func TestDeleteGroup(t *testing.T) {
316190
t.Fatalf("DeleteGroup returned error: %v", err)
317191
}
318192

319-
sort.Ints(client.deletedInstanceIDs)
320-
if got, want := client.deletedInstanceIDs, []int{101, 102}; !reflect.DeepEqual(got, want) {
193+
sort.Ints(client.DeletedInstanceIDs)
194+
if got, want := client.DeletedInstanceIDs, []int{101, 102}; !reflect.DeepEqual(got, want) {
321195
t.Fatalf("unexpected deleted IDs: got %v, want %v", got, want)
322196
}
323197
})
324198

325199
t.Run("deletes by instance-group tag when cluster label is missing", func(t *testing.T) {
326-
client := &fakeLinodeCloudClient{
327-
listInstancesResponse: []linodego.Instance{
200+
client := &MockLinodeClient{
201+
ListInstancesResponse: []linodego.Instance{
328202
{ID: 101, Tags: []string{"kops.k8s.io/instance-group:nodes-us-east", "kops.k8s.io/cluster:example.k8s.local"}},
329203
{ID: 102, Tags: []string{"kops.k8s.io/instance-group:nodes-us-east", "kops.k8s.io/cluster:other.k8s.local"}},
330204
{ID: 103, Tags: []string{"kops.k8s.io/instance-group:control-plane-us-east", "kops.k8s.io/cluster:example.k8s.local"}},
@@ -341,14 +215,14 @@ func TestDeleteGroup(t *testing.T) {
341215
t.Fatalf("DeleteGroup returned error: %v", err)
342216
}
343217

344-
sort.Ints(client.deletedInstanceIDs)
345-
if got, want := client.deletedInstanceIDs, []int{101, 102}; !reflect.DeepEqual(got, want) {
218+
sort.Ints(client.DeletedInstanceIDs)
219+
if got, want := client.DeletedInstanceIDs, []int{101, 102}; !reflect.DeepEqual(got, want) {
346220
t.Fatalf("unexpected deleted IDs: got %v, want %v", got, want)
347221
}
348222
})
349223

350224
t.Run("returns list errors", func(t *testing.T) {
351-
client := &fakeLinodeCloudClient{listInstancesError: errors.New("api unavailable")}
225+
client := &MockLinodeClient{ListInstancesError: errors.New("api unavailable")}
352226
cloud := &Cloud{region: "us-east", client: client}
353227

354228
group := &cloudinstances.CloudInstanceGroup{InstanceGroup: &kops.InstanceGroup{ObjectMeta: metav1.ObjectMeta{Name: "nodes-us-east"}}}
@@ -362,9 +236,9 @@ func TestDeleteGroup(t *testing.T) {
362236
})
363237

364238
t.Run("returns delete errors", func(t *testing.T) {
365-
client := &fakeLinodeCloudClient{
366-
listInstancesResponse: []linodego.Instance{{ID: 101, Tags: []string{"kops.k8s.io/instance-group:nodes-us-east"}}},
367-
deleteInstanceErrByID: map[int]error{101: errors.New("api unavailable")},
239+
client := &MockLinodeClient{
240+
ListInstancesResponse: []linodego.Instance{{ID: 101, Tags: []string{"kops.k8s.io/instance-group:nodes-us-east"}}},
241+
DeleteInstanceErrByID: map[int]error{101: errors.New("api unavailable")},
368242
}
369243
cloud := &Cloud{region: "us-east", client: client}
370244

@@ -381,8 +255,8 @@ func TestDeleteGroup(t *testing.T) {
381255

382256
func TestDetachInstance(t *testing.T) {
383257
t.Run("removes instance-role tag", func(t *testing.T) {
384-
client := &fakeLinodeCloudClient{
385-
listInstancesResponse: []linodego.Instance{{
258+
client := &MockLinodeClient{
259+
ListInstancesResponse: []linodego.Instance{{
386260
ID: 42,
387261
Tags: []string{"kops.k8s.io/cluster:example.k8s.local", "kops.k8s.io/instance-group:nodes-us-east", "kops.k8s.io/instance-role:Node", "env:test"},
388262
}},
@@ -394,31 +268,31 @@ func TestDetachInstance(t *testing.T) {
394268
t.Fatalf("DetachInstance returned error: %v", err)
395269
}
396270

397-
if got, want := client.updatedInstanceIDs, []int{42}; !reflect.DeepEqual(got, want) {
271+
if got, want := client.UpdatedInstanceIDs, []int{42}; !reflect.DeepEqual(got, want) {
398272
t.Fatalf("unexpected updated IDs: got %v, want %v", got, want)
399273
}
400274

401275
wantTags := []string{"kops.k8s.io/cluster:example.k8s.local", "kops.k8s.io/instance-group:nodes-us-east", "env:test"}
402-
if got := client.updatedTagsByID[42]; !reflect.DeepEqual(got, wantTags) {
276+
if got := client.UpdatedTagsByID[42]; !reflect.DeepEqual(got, wantTags) {
403277
t.Fatalf("unexpected updated tags: got %v, want %v", got, wantTags)
404278
}
405279
})
406280

407281
t.Run("returns nil when instance already missing", func(t *testing.T) {
408-
client := &fakeLinodeCloudClient{listInstancesResponse: []linodego.Instance{{ID: 101, Tags: []string{"kops.k8s.io/instance-role:Node"}}}}
282+
client := &MockLinodeClient{ListInstancesResponse: []linodego.Instance{{ID: 101, Tags: []string{"kops.k8s.io/instance-role:Node"}}}}
409283
cloud := &Cloud{region: "us-east", client: client}
410284

411285
err := cloud.DetachInstance(&cloudinstances.CloudInstance{ID: "42"})
412286
if err != nil {
413287
t.Fatalf("DetachInstance returned error: %v", err)
414288
}
415-
if len(client.updatedInstanceIDs) != 0 {
416-
t.Fatalf("expected no update calls, got %v", client.updatedInstanceIDs)
289+
if len(client.UpdatedInstanceIDs) != 0 {
290+
t.Fatalf("expected no update calls, got %v", client.UpdatedInstanceIDs)
417291
}
418292
})
419293

420294
t.Run("returns nil when no instance-role tag", func(t *testing.T) {
421-
client := &fakeLinodeCloudClient{listInstancesResponse: []linodego.Instance{{
295+
client := &MockLinodeClient{ListInstancesResponse: []linodego.Instance{{
422296
ID: 42,
423297
Tags: []string{"kops.k8s.io/cluster:example.k8s.local", "kops.k8s.io/instance-group:nodes-us-east"},
424298
}}}
@@ -428,13 +302,13 @@ func TestDetachInstance(t *testing.T) {
428302
if err != nil {
429303
t.Fatalf("DetachInstance returned error: %v", err)
430304
}
431-
if len(client.updatedInstanceIDs) != 0 {
432-
t.Fatalf("expected no update calls, got %v", client.updatedInstanceIDs)
305+
if len(client.UpdatedInstanceIDs) != 0 {
306+
t.Fatalf("expected no update calls, got %v", client.UpdatedInstanceIDs)
433307
}
434308
})
435309

436310
t.Run("rejects invalid id", func(t *testing.T) {
437-
client := &fakeLinodeCloudClient{}
311+
client := &MockLinodeClient{}
438312
cloud := &Cloud{region: "us-east", client: client}
439313

440314
err := cloud.DetachInstance(&cloudinstances.CloudInstance{ID: "not-a-number"})
@@ -447,7 +321,7 @@ func TestDetachInstance(t *testing.T) {
447321
})
448322

449323
t.Run("returns list errors", func(t *testing.T) {
450-
client := &fakeLinodeCloudClient{listInstancesError: errors.New("api unavailable")}
324+
client := &MockLinodeClient{ListInstancesError: errors.New("api unavailable")}
451325
cloud := &Cloud{region: "us-east", client: client}
452326

453327
err := cloud.DetachInstance(&cloudinstances.CloudInstance{ID: "42"})
@@ -460,9 +334,9 @@ func TestDetachInstance(t *testing.T) {
460334
})
461335

462336
t.Run("ignores not found update errors", func(t *testing.T) {
463-
client := &fakeLinodeCloudClient{
464-
listInstancesResponse: []linodego.Instance{{ID: 42, Tags: []string{"kops.k8s.io/instance-role:Node"}}},
465-
updateInstanceErrByID: map[int]error{42: &linodego.Error{Code: 404, Message: "not found"}},
337+
client := &MockLinodeClient{
338+
ListInstancesResponse: []linodego.Instance{{ID: 42, Tags: []string{"kops.k8s.io/instance-role:Node"}}},
339+
UpdateInstanceErrByID: map[int]error{42: &linodego.Error{Code: 404, Message: "not found"}},
466340
}
467341
cloud := &Cloud{region: "us-east", client: client}
468342

@@ -473,9 +347,9 @@ func TestDetachInstance(t *testing.T) {
473347
})
474348

475349
t.Run("returns update errors", func(t *testing.T) {
476-
client := &fakeLinodeCloudClient{
477-
listInstancesResponse: []linodego.Instance{{ID: 42, Tags: []string{"kops.k8s.io/instance-role:Node"}}},
478-
updateInstanceErrByID: map[int]error{42: errors.New("api unavailable")},
350+
client := &MockLinodeClient{
351+
ListInstancesResponse: []linodego.Instance{{ID: 42, Tags: []string{"kops.k8s.io/instance-role:Node"}}},
352+
UpdateInstanceErrByID: map[int]error{42: errors.New("api unavailable")},
479353
}
480354
cloud := &Cloud{region: "us-east", client: client}
481355

@@ -512,8 +386,8 @@ func TestGetApiIngressStatus(t *testing.T) {
512386
{
513387
name: "load balancer ingress",
514388
loadBalancer: true,
515-
client: &fakeLinodeCloudClient{
516-
listNodeBalancersResponse: []linodego.NodeBalancer{
389+
client: &MockLinodeClient{
390+
ListNodeBalancersResponse: []linodego.NodeBalancer{
517391
{
518392
Label: fi.PtrTo("api-example-k8s-local"),
519393
IPv4: fi.PtrTo("203.0.113.20"),
@@ -525,7 +399,7 @@ func TestGetApiIngressStatus(t *testing.T) {
525399
{
526400
name: "empty ingress when load balancer not created yet",
527401
loadBalancer: true,
528-
client: &fakeLinodeCloudClient{listInstancesResponse: []linodego.Instance{
402+
client: &MockLinodeClient{ListInstancesResponse: []linodego.Instance{
529403
{
530404
Tags: []string{"kops.k8s.io/cluster:example.k8s.local", "kops.k8s.io/instance-role:ControlPlane"},
531405
IPv4: []*net.IP{ptrIP("10.0.0.10"), ptrIP("198.51.100.10")},
@@ -539,7 +413,7 @@ func TestGetApiIngressStatus(t *testing.T) {
539413
},
540414
{
541415
name: "control-plane ingress from instances when no load balancer",
542-
client: &fakeLinodeCloudClient{listInstancesResponse: []linodego.Instance{
416+
client: &MockLinodeClient{ListInstancesResponse: []linodego.Instance{
543417
{
544418
Tags: []string{"kops.k8s.io/cluster:example.k8s.local", "kops.k8s.io/instance-role:ControlPlane"},
545419
IPv4: []*net.IP{ptrIP("10.0.0.10"), ptrIP("198.51.100.10")},
@@ -558,7 +432,7 @@ func TestGetApiIngressStatus(t *testing.T) {
558432
{
559433
name: "empty ingress when no api endpoint data",
560434
loadBalancer: true,
561-
client: &fakeLinodeCloudClient{},
435+
client: &MockLinodeClient{},
562436
},
563437
}
564438

0 commit comments

Comments
 (0)