Skip to content

Commit a58d346

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request #7767 from Kamatera/tag-fixes-add-support-for-filter-name-prefix
Kamatera cluster autoscaler fixes
2 parents 06dc935 + 83ce5dd commit a58d346

10 files changed

+251
-97
lines changed

‎cluster-autoscaler/cloudprovider/kamatera/README.md‎

Lines changed: 13 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -30,18 +30,19 @@ You can see an example of the cloud config file at [examples/deployment.yaml](ex
3030

3131
it is an INI file with the following fields:
3232

33-
| Key | Value | Mandatory | Default |
34-
|-----|-------|-----------|---------|
35-
| global/kamatera-api-client-id | Kamatera API Client ID | yes | none |
36-
| global/kamatera-api-secret | Kamatera API Secret | yes | none |
37-
| global/cluster-name | **max 15 characters: english letters, numbers, dash, underscore, space, dot**: distinct string used to set the cluster server tag | yes | none |
38-
| global/default-min-size | default minimum size of a node group (must be > 0) | no | 1 |
39-
| global/default-max-size | default maximum size of a node group | no | 254 |
40-
| global/default-<SERVER_CONFIG_KEY> | replace <SERVER_CONFIG_KEY> with the relevant configuration key | see below | see below |
41-
| nodegroup \"name\" | **max 15 characters: english letters, numbers, dash, underscore, space, dot**: distinct string within the cluster used to set the node group server tag | yes | none |
42-
| nodegroup \"name\"/min-size | minimum size for a specific node group | no | global/defaut-min-size |
43-
| nodegroup \"name\"/max-size | maximum size for a specific node group | no | global/defaut-min-size |
44-
| nodegroup \"name\"/<SERVER_CONFIG_KEY> | replace <SERVER_CONFIG_KEY> with the relevant configuration key | no | global/default-<SERVER_CONFIG_KEY> |
33+
| Key | Value | Mandatory | Default |
34+
|----------------------------------------|---------------------------------------------------------------------------------------------------------------------------------------------------------|-----------|------------------------------------|
35+
| global/kamatera-api-client-id | Kamatera API Client ID | yes | none |
36+
| global/kamatera-api-secret | Kamatera API Secret | yes | none |
37+
| global/cluster-name | **max 15 characters: english letters, numbers, dash, underscore, space, dot**: distinct string used to set the cluster server tag | yes | none |
38+
| global/filter-name-prefix | autoscaler will only handle server names that start with this prefix | no | none |
39+
| global/default-min-size | default minimum size of a node group (must be > 0) | no | 1 |
40+
| global/default-max-size | default maximum size of a node group | no | 254 |
41+
| global/default-<SERVER_CONFIG_KEY> | replace <SERVER_CONFIG_KEY> with the relevant configuration key | see below | see below |
42+
| nodegroup \"name\" | **max 15 characters: english letters, numbers, dash, underscore, space, dot**: distinct string within the cluster used to set the node group server tag | yes | none |
43+
| nodegroup \"name\"/min-size | minimum size for a specific node group | no | global/defaut-min-size |
44+
| nodegroup \"name\"/max-size | maximum size for a specific node group | no | global/defaut-min-size |
45+
| nodegroup \"name\"/<SERVER_CONFIG_KEY> | replace <SERVER_CONFIG_KEY> with the relevant configuration key | no | global/default-<SERVER_CONFIG_KEY> |
4546

4647
### Server configuration keys
4748

‎cluster-autoscaler/cloudprovider/kamatera/kamatera_api_client.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ import (
2222

2323
// kamateraAPIClient is the interface used to call kamatera API
2424
type kamateraAPIClient interface {
25-
ListServers(ctx context.Context, instances map[string]*Instance) ([]Server, error)
25+
ListServers(ctx context.Context, instances map[string]*Instance, namePrefix string) ([]Server, error)
2626
DeleteServer(ctx context.Context, name string) error
2727
CreateServers(ctx context.Context, count int, config ServerConfig) ([]Server, error)
2828
}

‎cluster-autoscaler/cloudprovider/kamatera/kamatera_api_client_rest.go‎

Lines changed: 42 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -32,10 +32,12 @@ const (
3232
// NewKamateraApiClientRest factory to create new Rest API Client struct
3333
func NewKamateraApiClientRest(clientId string, secret string, url string) (client KamateraApiClientRest) {
3434
return KamateraApiClientRest{
35-
userAgent: userAgent,
36-
clientId: clientId,
37-
secret: secret,
38-
url: url,
35+
userAgent: userAgent,
36+
clientId: clientId,
37+
secret: secret,
38+
url: url,
39+
maxRetries: 5,
40+
expSecondsBetweenRetries: 1,
3941
}
4042
}
4143

@@ -75,20 +77,24 @@ type KamateraServerCreatePostRequest struct {
7577

7678
// KamateraApiClientRest is the struct to perform API calls
7779
type KamateraApiClientRest struct {
78-
userAgent string
79-
clientId string
80-
secret string
81-
url string
80+
userAgent string
81+
clientId string
82+
secret string
83+
url string
84+
maxRetries int
85+
expSecondsBetweenRetries int
8286
}
8387

8488
// ListServers returns a list of all servers in the relevant account and fetches their tags
85-
func (c *KamateraApiClientRest) ListServers(ctx context.Context, instances map[string]*Instance) ([]Server, error) {
89+
func (c *KamateraApiClientRest) ListServers(ctx context.Context, instances map[string]*Instance, namePrefix string) ([]Server, error) {
8690
res, err := request(
8791
ctx,
8892
ProviderConfig{ApiUrl: c.url, ApiClientID: c.clientId, ApiSecret: c.secret},
8993
"GET",
9094
"/service/servers",
9195
nil,
96+
c.maxRetries,
97+
c.expSecondsBetweenRetries,
9298
)
9399
if err != nil {
94100
return nil, err
@@ -97,16 +103,18 @@ func (c *KamateraApiClientRest) ListServers(ctx context.Context, instances map[s
97103
for _, server := range res.([]interface{}) {
98104
server := server.(map[string]interface{})
99105
serverName := server["name"].(string)
100-
serverPowerOn := server["power"].(string) == "on"
101-
serverTags, err := c.getServerTags(ctx, serverName, instances)
102-
if err != nil {
103-
return nil, err
106+
if len(namePrefix) == 0 || strings.HasPrefix(serverName, namePrefix) {
107+
serverPowerOn := server["power"].(string) == "on"
108+
serverTags, err := c.getServerTags(ctx, serverName, instances)
109+
if err != nil {
110+
return nil, err
111+
}
112+
servers = append(servers, Server{
113+
Name: serverName,
114+
Tags: serverTags,
115+
PowerOn: serverPowerOn,
116+
})
104117
}
105-
servers = append(servers, Server{
106-
Name: serverName,
107-
Tags: serverTags,
108-
PowerOn: serverPowerOn,
109-
})
110118
}
111119
return servers, nil
112120
}
@@ -119,16 +127,20 @@ func (c *KamateraApiClientRest) DeleteServer(ctx context.Context, name string) e
119127
"POST",
120128
"/service/server/poweroff",
121129
KamateraServerPostRequest{ServerName: name},
130+
c.maxRetries,
131+
c.expSecondsBetweenRetries,
122132
)
123133
if err == nil {
124134
commandId := res.([]interface{})[0].(string)
125135
_, err = waitCommand(
126136
ctx,
127137
ProviderConfig{ApiUrl: c.url, ApiClientID: c.clientId, ApiSecret: c.secret},
128138
commandId,
139+
c.maxRetries,
140+
c.expSecondsBetweenRetries,
129141
)
130142
if err != nil {
131-
return err
143+
klog.V(1).Infof("Failed to validate server power off but will attempt to terminate anyway %s: %v", name, err)
132144
}
133145
} else {
134146
klog.V(1).Infof("Failed to power off server but will attempt to terminate anyway %s: %v", name, err)
@@ -139,6 +151,8 @@ func (c *KamateraApiClientRest) DeleteServer(ctx context.Context, name string) e
139151
"POST",
140152
"/service/server/terminate",
141153
KamateraServerTerminatePostRequest{ServerName: name, Force: true},
154+
c.maxRetries,
155+
c.expSecondsBetweenRetries,
142156
)
143157
if err != nil {
144158
return err
@@ -193,6 +207,8 @@ func (c *KamateraApiClientRest) CreateServers(ctx context.Context, count int, co
193207
UserdataFile: config.UserdataFile,
194208
Tag: Tag,
195209
},
210+
1,
211+
0,
196212
)
197213
if err != nil {
198214
return nil, err
@@ -209,6 +225,8 @@ func (c *KamateraApiClientRest) CreateServers(ctx context.Context, count int, co
209225
ctx,
210226
ProviderConfig{ApiUrl: c.url, ApiClientID: c.clientId, ApiSecret: c.secret},
211227
serverNameCommandIds,
228+
c.maxRetries,
229+
c.expSecondsBetweenRetries,
212230
)
213231
if err != nil {
214232
return nil, err
@@ -231,14 +249,16 @@ func (c *KamateraApiClientRest) getServerTags(ctx context.Context, serverName st
231249
"POST",
232250
"/server/tags",
233251
KamateraServerPostRequest{ServerName: serverName},
252+
c.maxRetries,
253+
c.expSecondsBetweenRetries,
234254
)
235255
if err != nil {
236256
return nil, err
237257
}
238-
var tags []string
258+
tags := make([]string, 0)
239259
for _, row := range res.([]interface{}) {
240260
row := row.(map[string]interface{})
241-
tags = append(tags, row["tag name"].(string))
261+
tags = append(tags, row["tagName"].(string))
242262
}
243263
return tags, nil
244264
}
@@ -265,5 +285,5 @@ func kamateraServerName(namePrefix string) string {
265285
if len(namePrefix) > 0 {
266286
namePrefix = fmt.Sprintf("%s-", namePrefix)
267287
}
268-
return fmt.Sprintf("%s%s", namePrefix, strings.ReplaceAll(uuid.New().String(), "-", ""))
288+
return fmt.Sprintf("%s%s", namePrefix, strings.ReplaceAll(uuid.New().String()[:12], "-", ""))
269289
}

‎cluster-autoscaler/cloudprovider/kamatera/kamatera_api_client_rest_test.go‎

Lines changed: 124 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -31,16 +31,27 @@ const (
3131
mockKamateraSecret = "mock-secret"
3232
)
3333

34+
func NewMockKamateraApiClientRest(url string, maxRetries int, expSecondsBetweenRetries int) (client KamateraApiClientRest) {
35+
return KamateraApiClientRest{
36+
userAgent: userAgent,
37+
clientId: mockKamateraClientId,
38+
secret: mockKamateraSecret,
39+
url: url,
40+
maxRetries: maxRetries,
41+
expSecondsBetweenRetries: expSecondsBetweenRetries,
42+
}
43+
}
44+
3445
func TestApiClientRest_ListServers_NoServers(t *testing.T) {
3546
server := NewHttpServerMock(MockFieldContentType, MockFieldResponse)
3647
defer server.Close()
3748
ctx := context.Background()
38-
client := NewKamateraApiClientRest(mockKamateraClientId, mockKamateraSecret, server.URL)
49+
client := NewMockKamateraApiClientRest(server.URL, 5, 0)
3950
server.On("handle", "/service/servers").Return(
4051
"application/json",
4152
`[]`,
4253
).Once()
43-
servers, err := client.ListServers(ctx, map[string]*Instance{})
54+
servers, err := client.ListServers(ctx, map[string]*Instance{}, "")
4455
assert.NoError(t, err)
4556
assert.Equal(t, 0, len(servers))
4657
mock.AssertExpectationsForObjects(t, server)
@@ -50,20 +61,22 @@ func TestApiClientRest_ListServers(t *testing.T) {
5061
server := NewHttpServerMock(MockFieldContentType, MockFieldResponse)
5162
defer server.Close()
5263
ctx := context.Background()
53-
client := NewKamateraApiClientRest(mockKamateraClientId, mockKamateraSecret, server.URL)
64+
client := NewMockKamateraApiClientRest(server.URL, 5, 0)
5465
newServerName1 := mockKamateraServerName()
5566
cachedServerName2 := mockKamateraServerName()
5667
cachedServerName3 := mockKamateraServerName()
68+
cachedServerName4 := mockKamateraServerName()
5769
server.On("handle", "/service/servers").Return(
5870
"application/json",
5971
fmt.Sprintf(`[
6072
{"name": "%s", "power": "on"},
6173
{"name": "%s", "power": "on"},
62-
{"name": "%s", "power": "off"}
63-
]`, newServerName1, cachedServerName2, cachedServerName3),
74+
{"name": "%s", "power": "off"},
75+
{"name": "%s", "power": "on"}
76+
]`, newServerName1, cachedServerName2, cachedServerName3, cachedServerName4),
6477
).On("handle", "/server/tags").Return(
6578
"application/json",
66-
`[{"tag name": "test-tag"}, {"tag name": "other-test-tag"}]`,
79+
`[{"tagName": "test-tag"}, {"tagName": "other-test-tag"}]`,
6780
)
6881
servers, err := client.ListServers(ctx, map[string]*Instance{
6982
cachedServerName2: {
@@ -78,9 +91,15 @@ func TestApiClientRest_ListServers(t *testing.T) {
7891
PowerOn: true,
7992
Tags: []string{"another-tag", "my-other-tag"},
8093
},
81-
})
94+
cachedServerName4: {
95+
Id: cachedServerName4,
96+
Status: &cloudprovider.InstanceStatus{State: cloudprovider.InstanceRunning},
97+
PowerOn: true,
98+
Tags: []string{},
99+
},
100+
}, "")
82101
assert.NoError(t, err)
83-
assert.Equal(t, 3, len(servers))
102+
assert.Equal(t, 4, len(servers))
84103
assert.Equal(t, servers, []Server{
85104
{
86105
Name: newServerName1,
@@ -97,15 +116,90 @@ func TestApiClientRest_ListServers(t *testing.T) {
97116
Tags: []string{"another-tag", "my-other-tag"},
98117
PowerOn: false,
99118
},
119+
{
120+
Name: cachedServerName4,
121+
Tags: []string{},
122+
PowerOn: true,
123+
},
100124
})
101125
mock.AssertExpectationsForObjects(t, server)
102126
}
103127

128+
func TestApiClientRest_ListServersNamePrefix(t *testing.T) {
129+
server := NewHttpServerMock(MockFieldContentType, MockFieldResponse)
130+
defer server.Close()
131+
ctx := context.Background()
132+
client := NewMockKamateraApiClientRest(server.URL, 5, 0)
133+
newServerName1 := "prefixa" + mockKamateraServerName()
134+
newServerName2 := "prefixb" + mockKamateraServerName()
135+
server.On("handle", "/service/servers").Return(
136+
"application/json",
137+
fmt.Sprintf(`[
138+
{"name": "%s", "power": "on"},
139+
{"name": "%s", "power": "on"}
140+
]`, newServerName1, newServerName2),
141+
).On("handle", "/server/tags").Return(
142+
"application/json",
143+
`[{"tagName": "test-tag"}, {"tagName": "other-test-tag"}]`,
144+
)
145+
servers, err := client.ListServers(ctx, map[string]*Instance{}, "prefixb")
146+
assert.NoError(t, err)
147+
assert.Equal(t, 1, len(servers))
148+
assert.Equal(t, servers, []Server{
149+
{
150+
Name: newServerName2,
151+
Tags: []string{"test-tag", "other-test-tag"},
152+
PowerOn: true,
153+
},
154+
})
155+
mock.AssertExpectationsForObjects(t, server)
156+
}
157+
158+
func TestApiClientRest_ListServersNoTags(t *testing.T) {
159+
server := NewHttpServerMock(MockFieldContentType, MockFieldResponse)
160+
defer server.Close()
161+
ctx := context.Background()
162+
client := NewMockKamateraApiClientRest(server.URL, 5, 0)
163+
newServerName1 := mockKamateraServerName()
164+
server.On("handle", "/service/servers").Return(
165+
"application/json", fmt.Sprintf(`[{"name": "%s", "power": "on"}]`, newServerName1),
166+
).On("handle", "/server/tags").Return(
167+
"application/json", `[]`,
168+
)
169+
servers, err := client.ListServers(ctx, map[string]*Instance{}, "")
170+
assert.NoError(t, err)
171+
assert.Equal(t, 1, len(servers))
172+
assert.Equal(t, servers, []Server{
173+
{
174+
Name: newServerName1,
175+
Tags: []string{},
176+
PowerOn: true,
177+
},
178+
})
179+
mock.AssertExpectationsForObjects(t, server)
180+
}
181+
182+
func TestApiClientRest_ListServersTagsError(t *testing.T) {
183+
server := NewHttpServerMock(MockFieldContentType, MockFieldResponse, MockFieldStatusCode)
184+
defer server.Close()
185+
ctx := context.Background()
186+
client := NewMockKamateraApiClientRest(server.URL, 5, 0)
187+
newServerName1 := mockKamateraServerName()
188+
server.On("handle", "/service/servers").Return(
189+
"application/json", fmt.Sprintf(`[{"name": "%s", "power": "on"}]`, newServerName1), 200,
190+
).On("handle", "/server/tags").Return(
191+
"application/json", `{"message":"Failed to run command method (serverTags)"}`, 500,
192+
)
193+
servers, err := client.ListServers(ctx, map[string]*Instance{}, "")
194+
assert.Error(t, err)
195+
assert.Nil(t, servers)
196+
}
197+
104198
func TestApiClientRest_DeleteServer(t *testing.T) {
105199
server := NewHttpServerMock(MockFieldContentType, MockFieldResponse)
106200
defer server.Close()
107201
ctx := context.Background()
108-
client := NewKamateraApiClientRest(mockKamateraClientId, mockKamateraSecret, server.URL)
202+
client := NewMockKamateraApiClientRest(server.URL, 5, 0)
109203
serverName := mockKamateraServerName()
110204
commandId := "mock-command-id"
111205
server.On("handle", "/service/server/poweroff").Return(
@@ -123,11 +217,31 @@ func TestApiClientRest_DeleteServer(t *testing.T) {
123217
mock.AssertExpectationsForObjects(t, server)
124218
}
125219

220+
func TestApiClientRest_DeleteServer_TerminateError(t *testing.T) {
221+
server := NewHttpServerMock(MockFieldContentType, MockFieldResponse, MockFieldStatusCode)
222+
defer server.Close()
223+
ctx := context.Background()
224+
client := NewMockKamateraApiClientRest(server.URL, 5, 0)
225+
serverName := mockKamateraServerName()
226+
commandId := "mock-command-id"
227+
server.On("handle", "/service/server/poweroff").Return(
228+
"application/json", fmt.Sprintf(`["%s"]`, commandId), 200,
229+
).Once().On("handle", "/service/queue").Return(
230+
"application/json", `[{"status": "complete"}]`, 200,
231+
).Once().On("handle", "/service/server/terminate").Return(
232+
"application/json",
233+
"Gateway Timeout",
234+
504,
235+
).Times(5)
236+
err := client.DeleteServer(ctx, serverName)
237+
assert.Error(t, err)
238+
}
239+
126240
func TestApiClientRest_CreateServers(t *testing.T) {
127241
server := NewHttpServerMock(MockFieldContentType, MockFieldResponse)
128242
defer server.Close()
129243
ctx := context.Background()
130-
client := NewKamateraApiClientRest(mockKamateraClientId, mockKamateraSecret, server.URL)
244+
client := NewMockKamateraApiClientRest(server.URL, 5, 0)
131245
commandId := "command"
132246
server.On("handle", "/service/server").Return(
133247
"application/json",

0 commit comments

Comments
 (0)