Skip to content

Commit 9f9ad27

Browse files
fira42073fira42073
committed
apply code review suggestions
1 parent 9503900 commit 9f9ad27

File tree

3 files changed

+72
-62
lines changed

3 files changed

+72
-62
lines changed

‎pkg/scalers/gitlab_runner_scaler.go

+4-3
Original file line numberDiff line numberDiff line change
@@ -44,8 +44,9 @@ type gitlabRunnerMetadata struct {
4444
PersonalAccessToken string `keda:"name=personalAccessToken, order=authParams"`
4545
ProjectID string `keda:"name=projectID, order=triggerMetadata"`
4646

47-
TargetPipelineQueueLength int64 `keda:"name=targetPipelineQueueLength, order=triggerMetadata, default=1, optional"`
48-
TriggerIndex int
47+
TargetPipelineQueueLength int64 `keda:"name=targetPipelineQueueLength, order=triggerMetadata, default=1, optional"`
48+
ActivationTargetPipelineQueueLength int64 `keda:"name=activationTargetPipelineQueueLength, order=triggerMetadata, default=0, optional"`
49+
TriggerIndex int
4950
}
5051

5152
// NewGitLabRunnerScaler creates a new GitLab Runner Scaler
@@ -110,7 +111,7 @@ func (s *gitlabRunnerScaler) GetMetricsAndActivity(ctx context.Context, metricNa
110111

111112
metric := GenerateMetricInMili(metricName, float64(queueLen))
112113

113-
return []external_metrics.ExternalMetricValue{metric}, queueLen >= s.metadata.TargetPipelineQueueLength, nil
114+
return []external_metrics.ExternalMetricValue{metric}, queueLen > s.metadata.ActivationTargetPipelineQueueLength, nil
114115
}
115116

116117
func (s *gitlabRunnerScaler) GetMetricSpecForScaling(_ context.Context) []v2.MetricSpec {

‎pkg/scalers/gitlab_runner_scaler_test.go

+37-30
Original file line numberDiff line numberDiff line change
@@ -207,10 +207,11 @@ func TestGitLabRunnerScaler_GetMetricsAndActivity(t *testing.T) {
207207
pipelineWaitingForResourceQueueLength int64
208208
pipelineRunningQueueLength int64
209209

210-
targetPipelineQueueLength int64
211-
expectedMetricValue int64
212-
expectedActive bool
213-
expectError bool
210+
targetPipelineQueueLength int64
211+
activationTargetPipelineQueueLength int64
212+
expectedMetricValue int64
213+
expectedActive bool
214+
expectError bool
214215
}{
215216
{
216217
name: "Queue length below target",
@@ -219,10 +220,11 @@ func TestGitLabRunnerScaler_GetMetricsAndActivity(t *testing.T) {
219220
pipelineWaitingForResourceQueueLength: 0,
220221
pipelineRunningQueueLength: 0,
221222

222-
targetPipelineQueueLength: 5,
223-
expectedMetricValue: 2,
224-
expectedActive: false,
225-
expectError: false,
223+
targetPipelineQueueLength: 5,
224+
activationTargetPipelineQueueLength: 3,
225+
expectedMetricValue: 2,
226+
expectedActive: false,
227+
expectError: false,
226228
},
227229
{
228230
name: "Queue length equal to target",
@@ -231,10 +233,11 @@ func TestGitLabRunnerScaler_GetMetricsAndActivity(t *testing.T) {
231233
pipelineWaitingForResourceQueueLength: 0,
232234
pipelineRunningQueueLength: 0,
233235

234-
targetPipelineQueueLength: 5,
235-
expectedMetricValue: 5,
236-
expectedActive: true,
237-
expectError: false,
236+
targetPipelineQueueLength: 5,
237+
activationTargetPipelineQueueLength: 3,
238+
expectedMetricValue: 5,
239+
expectedActive: true,
240+
expectError: false,
238241
},
239242
{
240243
name: "Queue length above target",
@@ -243,10 +246,11 @@ func TestGitLabRunnerScaler_GetMetricsAndActivity(t *testing.T) {
243246
pipelineWaitingForResourceQueueLength: 0,
244247
pipelineRunningQueueLength: 0,
245248

246-
targetPipelineQueueLength: 5,
247-
expectedMetricValue: 10,
248-
expectedActive: true,
249-
expectError: false,
249+
targetPipelineQueueLength: 5,
250+
activationTargetPipelineQueueLength: 3,
251+
expectedMetricValue: 10,
252+
expectedActive: true,
253+
expectError: false,
250254
},
251255
{
252256
name: "Queue length is sum of statuses",
@@ -255,10 +259,11 @@ func TestGitLabRunnerScaler_GetMetricsAndActivity(t *testing.T) {
255259
pipelineWaitingForResourceQueueLength: 3,
256260
pipelineRunningQueueLength: 5,
257261

258-
targetPipelineQueueLength: 5,
259-
expectedMetricValue: 9,
260-
expectedActive: true,
261-
expectError: false,
262+
targetPipelineQueueLength: 5,
263+
activationTargetPipelineQueueLength: 3,
264+
expectedMetricValue: 9,
265+
expectedActive: true,
266+
expectError: false,
262267
},
263268
{
264269
name: "Error retrieving queue length",
@@ -267,10 +272,11 @@ func TestGitLabRunnerScaler_GetMetricsAndActivity(t *testing.T) {
267272
pipelineWaitingForResourceQueueLength: 0,
268273
pipelineRunningQueueLength: 0,
269274

270-
targetPipelineQueueLength: 5,
271-
expectedMetricValue: 0,
272-
expectedActive: false,
273-
expectError: true,
275+
targetPipelineQueueLength: 5,
276+
activationTargetPipelineQueueLength: 3,
277+
expectedMetricValue: 0,
278+
expectedActive: false,
279+
expectError: true,
274280
},
275281
}
276282

@@ -323,10 +329,11 @@ func TestGitLabRunnerScaler_GetMetricsAndActivity(t *testing.T) {
323329
defer server.Close()
324330

325331
meta := &gitlabRunnerMetadata{
326-
GitLabAPIURL: mustParseURL(server.URL),
327-
PersonalAccessToken: "test-token",
328-
TargetPipelineQueueLength: tc.targetPipelineQueueLength,
329-
ProjectID: "12345",
332+
GitLabAPIURL: mustParseURL(server.URL),
333+
PersonalAccessToken: "test-token",
334+
TargetPipelineQueueLength: tc.targetPipelineQueueLength,
335+
ActivationTargetPipelineQueueLength: tc.activationTargetPipelineQueueLength,
336+
ProjectID: "12345",
330337
}
331338

332339
scaler := gitlabRunnerScaler{
@@ -354,7 +361,7 @@ func TestGitLabRunnerScaler_GetMetricSpecForScaling(t *testing.T) {
354361
meta := &gitlabRunnerMetadata{
355362
ProjectID: "12345",
356363
TargetPipelineQueueLength: 5,
357-
TriggerIndex: 0,
364+
TriggerIndex: 1,
358365
}
359366

360367
scaler := gitlabRunnerScaler{
@@ -367,7 +374,7 @@ func TestGitLabRunnerScaler_GetMetricSpecForScaling(t *testing.T) {
367374

368375
metricSpec := metricSpecs[0]
369376
assert.Equal(t, v2.ExternalMetricSourceType, metricSpec.Type)
370-
assert.Equal(t, "s0-gitlab-runner-12345", metricSpec.External.Metric.Name)
377+
assert.Equal(t, "s1-gitlab-runner-12345", metricSpec.External.Metric.Name)
371378
assert.Equal(t, int64(5), metricSpec.External.Target.AverageValue.Value())
372379
}
373380

‎tests/scalers/gitlab_runner/gitlab_runner_test.go

+31-29
Original file line numberDiff line numberDiff line change
@@ -51,17 +51,18 @@ var (
5151
)
5252

5353
type templateData struct {
54-
TestNamespace string
55-
SecretName string
56-
DeploymentName string
57-
ScaledObjectName string
58-
ScaledJobName string
59-
MinReplicaCount string
60-
MaxReplicaCount string
61-
Pat string
62-
GitlabAPIURL string
63-
ProjectID string
64-
TargetPipelineQueueLength string
54+
TestNamespace string
55+
SecretName string
56+
DeploymentName string
57+
ScaledObjectName string
58+
ScaledJobName string
59+
MinReplicaCount string
60+
MaxReplicaCount string
61+
Pat string
62+
GitlabAPIURL string
63+
ProjectID string
64+
TargetPipelineQueueLength string
65+
ActivationTargetPipelineQueueLength string
6566
}
6667

6768
const (
@@ -129,7 +130,7 @@ spec:
129130
gitlabAPIURL: {{.GitlabAPIURL}}
130131
projectID: "{{.ProjectID}}"
131132
targetPipelineQueueLength: "{{.TargetPipelineQueueLength}}"
132-
labels: "e2eSOtester"
133+
activationTargetPipelineQueueLength: "{{.ActivationTargetPipelineQueueLength}}"
133134
authenticationRef:
134135
name: gitlab-trigger-auth
135136
`
@@ -167,10 +168,10 @@ func TestScaler(t *testing.T) {
167168

168169
// test scaling Scaled Object
169170
KubectlApplyWithTemplate(t, data, "scaledObjectTemplate", scaledObjectTemplate)
170-
testSONotActivated(t, kc)
171+
testActivation(t, kc)
171172

172-
testSOScaleOut(t, kc, projectID)
173-
testSOScaleIn(t, kc, projectID)
173+
testScaleOut(t, kc, projectID)
174+
testScaleIn(t, kc, projectID)
174175

175176
// cleanup
176177
DeleteKubernetesResources(t, testNamespace, data, templates)
@@ -185,16 +186,17 @@ func getTemplateData(projectID string) (templateData, []Template) {
185186
base64Pat := base64.StdEncoding.EncodeToString([]byte(personalAccessToken))
186187

187188
return templateData{
188-
TestNamespace: testNamespace,
189-
SecretName: secretName,
190-
DeploymentName: deploymentName,
191-
ScaledObjectName: scaledObjectName,
192-
MinReplicaCount: fmt.Sprintf("%v", minReplicaCount),
193-
MaxReplicaCount: fmt.Sprintf("%v", maxReplicaCount),
194-
Pat: base64Pat,
195-
GitlabAPIURL: "https://gitlab.com",
196-
ProjectID: projectID,
197-
TargetPipelineQueueLength: "1",
189+
TestNamespace: testNamespace,
190+
SecretName: secretName,
191+
DeploymentName: deploymentName,
192+
ScaledObjectName: scaledObjectName,
193+
MinReplicaCount: fmt.Sprintf("%v", minReplicaCount),
194+
MaxReplicaCount: fmt.Sprintf("%v", maxReplicaCount),
195+
Pat: base64Pat,
196+
GitlabAPIURL: "https://gitlab.com",
197+
ProjectID: projectID,
198+
TargetPipelineQueueLength: "1",
199+
ActivationTargetPipelineQueueLength: "1",
198200
}, []Template{
199201
{Name: "secretTemplate", Config: secretTemplate},
200202
{Name: "authTemplate", Config: triggerAuthTemplate},
@@ -203,20 +205,20 @@ func getTemplateData(projectID string) (templateData, []Template) {
203205
}
204206
}
205207

206-
func testSONotActivated(t *testing.T, kc *kubernetes.Clientset) {
208+
func testActivation(t *testing.T, kc *kubernetes.Clientset) {
207209
t.Log("--- testing none activation ---")
208210
AssertReplicaCountNotChangeDuringTimePeriod(t, kc, deploymentName, testNamespace, minReplicaCount, 60)
209211
}
210212

211-
func testSOScaleOut(t *testing.T, kc *kubernetes.Clientset, projectID string) {
213+
func testScaleOut(t *testing.T, kc *kubernetes.Clientset, projectID string) {
212214
t.Log("--- testing scale out ---")
213215
queueRun(t, projectID)
214216

215217
assert.True(t, WaitForDeploymentReplicaReadyCount(t, kc, deploymentName, testNamespace, maxReplicaCount, 60, 1),
216-
"replica count should be 2 after 1 minute")
218+
"replica count should be 1 after 1 minute")
217219
}
218220

219-
func testSOScaleIn(t *testing.T, kc *kubernetes.Clientset, projectID string) {
221+
func testScaleIn(t *testing.T, kc *kubernetes.Clientset, projectID string) {
220222
t.Log("--- testing scale in ---")
221223
deleteAllPipelines(t, gitlabBaseURL, projectID)
222224

0 commit comments

Comments
 (0)