Skip to content

Commit

Permalink
Fix panic on scaling organizational runners (#381)
Browse files Browse the repository at this point in the history
  • Loading branch information
mumoshu authored Mar 9, 2021
1 parent c0b8f9d commit 728829b
Show file tree
Hide file tree
Showing 2 changed files with 94 additions and 0 deletions.
7 changes: 7 additions & 0 deletions controllers/autoscaling.go
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,13 @@ func (r *HorizontalRunnerAutoscalerReconciler) calculateReplicasByQueuedAndInPro
return nil, fmt.Errorf("asserting runner deployment spec to detect bug: spec.template.organization should not be empty on this code path")
}

// In case it's an organizational runners deployment without any scaling metrics defined,
// we assume that the desired replicas should always be `minReplicas + capacityReservedThroughWebhook`.
// See https://github.com/summerwind/actions-runner-controller/issues/377#issuecomment-793372693
if len(metrics) == 0 {
return hra.Spec.MinReplicas, nil
}

if len(metrics[0].RepositoryNames) == 0 {
return nil, errors.New("validating autoscaling metrics: spec.autoscaling.metrics[].repositoryNames is required and must have one more more entries for organizational runner deployment")
}
Expand Down
87 changes: 87 additions & 0 deletions controllers/integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,93 @@ var _ = Context("INTEGRATION: Inside of a new namespace", func() {

Describe("when no existing resources exist", func() {

It("should create and scale organizational runners without any scaling metrics on pull_request event", func() {
name := "example-runnerdeploy"

{
rd := &actionsv1alpha1.RunnerDeployment{
ObjectMeta: metav1.ObjectMeta{
Name: name,
Namespace: ns.Name,
},
Spec: actionsv1alpha1.RunnerDeploymentSpec{
Replicas: intPtr(1),
Selector: &metav1.LabelSelector{
MatchLabels: map[string]string{
"foo": "bar",
},
},
Template: actionsv1alpha1.RunnerTemplate{
ObjectMeta: metav1.ObjectMeta{
Labels: map[string]string{
"foo": "bar",
},
},
Spec: actionsv1alpha1.RunnerSpec{
Organization: "test",
Image: "bar",
Group: "baz",
Env: []corev1.EnvVar{
{Name: "FOO", Value: "FOOVALUE"},
},
},
},
},
}

ExpectCreate(ctx, rd, "test RunnerDeployment")
ExpectRunnerSetsCountEventuallyEquals(ctx, ns.Name, 1)
ExpectRunnerSetsManagedReplicasCountEventuallyEquals(ctx, ns.Name, 1)
}

// Scale-up to 2 replicas
{
hra := &actionsv1alpha1.HorizontalRunnerAutoscaler{
ObjectMeta: metav1.ObjectMeta{
Name: name,
Namespace: ns.Name,
},
Spec: actionsv1alpha1.HorizontalRunnerAutoscalerSpec{
ScaleTargetRef: actionsv1alpha1.ScaleTargetRef{
Name: name,
},
MinReplicas: intPtr(2),
MaxReplicas: intPtr(5),
ScaleDownDelaySecondsAfterScaleUp: intPtr(1),
Metrics: nil,
ScaleUpTriggers: []actionsv1alpha1.ScaleUpTrigger{
{
GitHubEvent: &actionsv1alpha1.GitHubEventScaleUpTriggerSpec{
PullRequest: &actionsv1alpha1.PullRequestSpec{
Types: []string{"created"},
Branches: []string{"main"},
},
},
Amount: 1,
Duration: metav1.Duration{Duration: time.Minute},
},
},
},
}

ExpectCreate(ctx, hra, "test HorizontalRunnerAutoscaler")

ExpectRunnerSetsCountEventuallyEquals(ctx, ns.Name, 1)
ExpectRunnerSetsManagedReplicasCountEventuallyEquals(ctx, ns.Name, 2)
ExpectHRAStatusCacheEntryLengthEventuallyEquals(ctx, ns.Name, name, 1)
}

{
env.ExpectRegisteredNumberCountEventuallyEquals(2, "count of fake runners after HRA creation")
}

// Scale-up to 3 replicas on second pull_request create webhook event
{
env.SendOrgPullRequestEvent("test", "valid", "main", "created")
ExpectRunnerSetsManagedReplicasCountEventuallyEquals(ctx, ns.Name, 3, "runners after second webhook event")
}
})

It("should create and scale organization's repository runners on pull_request event", func() {
name := "example-runnerdeploy"

Expand Down

0 comments on commit 728829b

Please sign in to comment.