Skip to content

Commit a3308c5

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request kubernetes#3148 from brett-elliott/MachineType404
Support caching errors when fetching machine types.
2 parents a00bf59 + 8f7bd5c commit a3308c5

File tree

4 files changed

+139
-11
lines changed

4 files changed

+139
-11
lines changed

‎cluster-autoscaler/cloudprovider/gce/cache.go‎

Lines changed: 29 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,11 @@ type MachineTypeKey struct {
3434
MachineType string
3535
}
3636

37+
type machinesCacheValue struct {
38+
machineType *gce.MachineType
39+
err error
40+
}
41+
3742
// GceCache is used for caching cluster resources state.
3843
//
3944
// It is needed to:
@@ -62,7 +67,7 @@ type GceCache struct {
6267
instanceRefToMigRef map[GceRef]GceRef
6368
instancesFromUnknownMigs map[GceRef]struct{}
6469
resourceLimiter *cloudprovider.ResourceLimiter
65-
machinesCache map[MachineTypeKey]*gce.MachineType
70+
machinesCache map[MachineTypeKey]machinesCacheValue
6671
migTargetSizeCache map[GceRef]int64
6772
migBaseNameCache map[GceRef]string
6873
instanceTemplatesCache map[GceRef]*gce.InstanceTemplate
@@ -77,7 +82,7 @@ func NewGceCache(gceService AutoscalingGceClient) *GceCache {
7782
migs: map[GceRef]Mig{},
7883
instanceRefToMigRef: map[GceRef]GceRef{},
7984
instancesFromUnknownMigs: map[GceRef]struct{}{},
80-
machinesCache: map[MachineTypeKey]*gce.MachineType{},
85+
machinesCache: map[MachineTypeKey]machinesCacheValue{},
8186
migTargetSizeCache: map[GceRef]int64{},
8287
migBaseNameCache: map[GceRef]string{},
8388
instanceTemplatesCache: map[GceRef]*gce.InstanceTemplate{},
@@ -356,27 +361,45 @@ func (gc *GceCache) InvalidateAllMigInstanceTemplates() {
356361
}
357362

358363
// GetMachineFromCache retrieves machine type from cache under lock.
359-
func (gc *GceCache) GetMachineFromCache(machineType string, zone string) *gce.MachineType {
364+
func (gc *GceCache) GetMachineFromCache(machineType string, zone string) (*gce.MachineType, error) {
360365
gc.cacheMutex.Lock()
361366
defer gc.cacheMutex.Unlock()
362367

363-
return gc.machinesCache[MachineTypeKey{zone, machineType}]
368+
cv, ok := gc.machinesCache[MachineTypeKey{zone, machineType}]
369+
if !ok {
370+
return nil, nil
371+
}
372+
if cv.err != nil {
373+
return nil, cv.err
374+
}
375+
return cv.machineType, nil
364376
}
365377

366378
// AddMachineToCache adds machine to cache under lock.
367379
func (gc *GceCache) AddMachineToCache(machineType string, zone string, machine *gce.MachineType) {
368380
gc.cacheMutex.Lock()
369381
defer gc.cacheMutex.Unlock()
370382

371-
gc.machinesCache[MachineTypeKey{zone, machineType}] = machine
383+
gc.machinesCache[MachineTypeKey{zone, machineType}] = machinesCacheValue{machineType: machine}
384+
}
385+
386+
// AddMachineToCache adds machine to cache under lock.
387+
func (gc *GceCache) AddMachineToCacheWithError(machineType string, zone string, err error) {
388+
gc.cacheMutex.Lock()
389+
defer gc.cacheMutex.Unlock()
390+
391+
gc.machinesCache[MachineTypeKey{zone, machineType}] = machinesCacheValue{err: err}
372392
}
373393

374394
// SetMachinesCache sets the machines cache under lock.
375395
func (gc *GceCache) SetMachinesCache(machinesCache map[MachineTypeKey]*gce.MachineType) {
376396
gc.cacheMutex.Lock()
377397
defer gc.cacheMutex.Unlock()
378398

379-
gc.machinesCache = machinesCache
399+
gc.machinesCache = map[MachineTypeKey]machinesCacheValue{}
400+
for k, v := range machinesCache {
401+
gc.machinesCache[k] = machinesCacheValue{machineType: v}
402+
}
380403
}
381404

382405
// SetMigBasename sets basename for given mig in cache

‎cluster-autoscaler/cloudprovider/gce/cache_test.go‎

Lines changed: 105 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,105 @@
1+
package gce
2+
3+
import (
4+
"errors"
5+
"testing"
6+
7+
"github.com/stretchr/testify/assert"
8+
"github.com/stretchr/testify/mock"
9+
gce "google.golang.org/api/compute/v1"
10+
"k8s.io/autoscaler/cluster-autoscaler/cloudprovider"
11+
)
12+
13+
func TestMachineCache(t *testing.T) {
14+
type CacheQuery struct {
15+
machineType string
16+
zone string
17+
machine *gce.MachineType
18+
err error
19+
}
20+
testCases := []struct {
21+
desc string
22+
machines []CacheQuery
23+
want map[MachineTypeKey]uint64
24+
wantErr []MachineTypeKey
25+
}{
26+
{
27+
desc: "replacement",
28+
machines: []CacheQuery{
29+
{
30+
machineType: "e2-standard-2",
31+
zone: "myzone",
32+
machine: &gce.MachineType{Id: 1},
33+
},
34+
{
35+
machineType: "e2-standard-2",
36+
zone: "myzone",
37+
machine: &gce.MachineType{Id: 1},
38+
},
39+
{
40+
machineType: "e2-standard-2",
41+
zone: "myzone",
42+
machine: &gce.MachineType{Id: 2},
43+
},
44+
{
45+
machineType: "e2-standard-2",
46+
zone: "myzone",
47+
machine: &gce.MachineType{Id: 2},
48+
},
49+
{
50+
machineType: "e2-standard-4",
51+
zone: "myzone2",
52+
err: errors.New("error"),
53+
},
54+
},
55+
want: map[MachineTypeKey]uint64{
56+
MachineTypeKey{
57+
MachineType: "e2-standard-2",
58+
Zone: "myzone",
59+
}: 2,
60+
},
61+
wantErr: []MachineTypeKey{
62+
{
63+
Zone: "myzone2",
64+
MachineType: "e2-standard-4",
65+
},
66+
},
67+
},
68+
}
69+
c := NewGceCache(nil)
70+
for _, tc := range testCases {
71+
t.Run(tc.desc, func(t *testing.T) {
72+
for _, m := range tc.machines {
73+
if m.err != nil {
74+
c.AddMachineToCacheWithError(m.machineType, m.zone, m.err)
75+
continue
76+
}
77+
c.AddMachineToCache(m.machineType, m.zone, m.machine)
78+
}
79+
for mt, wantId := range tc.want {
80+
m, err := c.GetMachineFromCache(mt.MachineType, mt.Zone)
81+
if err != nil {
82+
t.Errorf("Did not expect error for machine type = %q, zone = %q", mt.MachineType, mt.Zone)
83+
}
84+
if m.Id != wantId {
85+
t.Errorf("Wanted id %d but got id %d for machine type = %q, zone = %q", wantId, m.Id, mt.MachineType, mt.Zone)
86+
}
87+
}
88+
for _, mt := range tc.wantErr {
89+
_, err := c.GetMachineFromCache(mt.MachineType, mt.Zone)
90+
if err == nil {
91+
t.Errorf("Wanted an error but got no error for machine type = %q, zone = %q", mt.MachineType, mt.Zone)
92+
}
93+
}
94+
})
95+
}
96+
gceManagerMock := &gceManagerMock{}
97+
gce := &GceCloudProvider{
98+
gceManager: gceManagerMock,
99+
}
100+
mig := &gceMig{gceRef: GceRef{Name: "ng1"}}
101+
gceManagerMock.On("GetMigs").Return([]Mig{mig}).Once()
102+
result := gce.NodeGroups()
103+
assert.Equal(t, []cloudprovider.NodeGroup{mig}, result)
104+
mock.AssertExpectationsForObjects(t, gceManagerMock)
105+
}

‎cluster-autoscaler/cloudprovider/gce/gce_manager.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -492,7 +492,7 @@ func (m *gceManagerImpl) getCpuAndMemoryForMachineType(machineType string, zone
492492
if strings.HasPrefix(machineType, "custom-") {
493493
return parseCustomMachineType(machineType)
494494
}
495-
machine := m.cache.GetMachineFromCache(machineType, zone)
495+
machine, _ := m.cache.GetMachineFromCache(machineType, zone)
496496
if machine == nil {
497497
machine, err = m.GceService.FetchMachineType(zone, machineType)
498498
if err != nil {

‎cluster-autoscaler/cloudprovider/gce/gce_manager_test.go‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -336,10 +336,10 @@ func newTestGceManager(t *testing.T, testServerURL string, regional bool) *gceMa
336336
GceService: gceService,
337337
instanceRefToMigRef: make(map[GceRef]GceRef),
338338
instancesFromUnknownMigs: make(map[GceRef]struct{}),
339-
machinesCache: map[MachineTypeKey]*gce.MachineType{
340-
{"us-central1-b", "n1-standard-1"}: {GuestCpus: 1, MemoryMb: 1},
341-
{"us-central1-c", "n1-standard-1"}: {GuestCpus: 1, MemoryMb: 1},
342-
{"us-central1-f", "n1-standard-1"}: {GuestCpus: 1, MemoryMb: 1},
339+
machinesCache: map[MachineTypeKey]machinesCacheValue{
340+
{"us-central1-b", "n1-standard-1"}: machinesCacheValue{&gce.MachineType{GuestCpus: 1, MemoryMb: 1}, nil},
341+
{"us-central1-c", "n1-standard-1"}: machinesCacheValue{&gce.MachineType{GuestCpus: 1, MemoryMb: 1}, nil},
342+
{"us-central1-f", "n1-standard-1"}: machinesCacheValue{&gce.MachineType{GuestCpus: 1, MemoryMb: 1}, nil},
343343
},
344344
migTargetSizeCache: map[GceRef]int64{},
345345
instanceTemplatesCache: map[GceRef]*gce.InstanceTemplate{},

0 commit comments

Comments
 (0)