Skip to content

Commit

Permalink
fix: autoscaling tests related to callback removal
Browse files Browse the repository at this point in the history
  • Loading branch information
Langleu committed Jun 27, 2023
1 parent f274067 commit 45c975b
Show file tree
Hide file tree
Showing 2 changed files with 68 additions and 14 deletions.
3 changes: 3 additions & 0 deletions controllers/actions.summerwind.net/autoscaling.go
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,8 @@ func (r *HorizontalRunnerAutoscalerReconciler) suggestReplicasByQueuedAndInProgr
var total, inProgress, queued, completed, unknown int
listWorkflowJobs := func(user string, repoName string, runID int64) {
if runID == 0 {
// should not happen in reality
r.Log.Info("Detected run with no runID of 0, ignoring the case and not scaling.", "repo_name", repoName, "run_id", runID)
return
}
opt := github.ListWorkflowJobsOptions{ListOptions: github.ListOptions{PerPage: 50}}
Expand All @@ -137,6 +139,7 @@ func (r *HorizontalRunnerAutoscalerReconciler) suggestReplicasByQueuedAndInProgr
opt.Page = resp.NextPage
}
if len(allJobs) == 0 {
// GitHub API can return run with empty job array - should be ignored
r.Log.Info("Detected run with no jobs, ignoring the case and not scaling.", "repo_name", repoName, "run_id", runID)
} else {
JOB:
Expand Down
79 changes: 65 additions & 14 deletions controllers/actions.summerwind.net/autoscaling_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,30 +61,38 @@ func TestDetermineDesiredReplicas_RepositoryRunner(t *testing.T) {
want int
err string
}{
// case_0
// Legacy functionality
// 3 demanded, max at 3
// 2 demanded, max at 3
{
repo: "test/valid",
min: intPtr(2),
max: intPtr(3),
workflowRuns: `{"total_count": 4, "workflow_runs":[{"status":"queued"}, {"status":"in_progress"}, {"status":"in_progress"}, {"status":"completed"}]}"`,
workflowRuns_queued: `{"total_count": 1, "workflow_runs":[{"status":"queued"}]}"`,
workflowRuns_in_progress: `{"total_count": 2, "workflow_runs":[{"status":"in_progress"}, {"status":"in_progress"}]}"`,
want: 3,
want: 2,
},
// Explicitly speified the default `self-hosted` label which is ignored by the simulator,
// case_1
// Explicitly specified the default `self-hosted` label which is ignored by the simulator,
// as we assume that GitHub Actions automatically associates the `self-hosted` label to every self-hosted runner.
// 3 demanded, max at 3
// 2 demanded, max at 3
{
repo: "test/valid",
labels: []string{"self-hosted"},
min: intPtr(2),
max: intPtr(3),
workflowRuns: `{"total_count": 4, "workflow_runs":[{"status":"queued"}, {"status":"in_progress"}, {"status":"in_progress"}, {"status":"completed"}]}"`,
workflowRuns_queued: `{"total_count": 1, "workflow_runs":[{"status":"queued"}]}"`,
workflowRuns_in_progress: `{"total_count": 2, "workflow_runs":[{"status":"in_progress"}, {"status":"in_progress"}]}"`,
want: 3,
workflowRuns: `{"total_count": 4, "workflow_runs":[{"id": 1, "status":"queued"}, {"id": 2, "status":"in_progress"}, {"id": 3, "status":"in_progress"}, {"status":"completed"}]}"`,
workflowRuns_queued: `{"total_count": 1, "workflow_runs":[{"id": 1, "status":"queued"}]}"`,
workflowRuns_in_progress: `{"total_count": 2, "workflow_runs":[{"id": 2, "status":"in_progress"}, {"id": 3, "status":"in_progress"}]}"`,
workflowJobs: map[int]string{
1: `{"jobs": [{"status": "queued", "labels":["self-hosted"]}]}`,
2: `{"jobs": [{"status": "in_progress", "labels":["self-hosted"]}]}`,
3: `{"jobs": [{"status": "in_progress", "labels":["self-hosted"]}]}`,
},
want: 3,
},
// case_2
// 2 demanded, max at 3, currently 3, delay scaling down due to grace period
{
repo: "test/valid",
Expand All @@ -97,6 +105,7 @@ func TestDetermineDesiredReplicas_RepositoryRunner(t *testing.T) {
workflowRuns_in_progress: `{"total_count": 1, "workflow_runs":[{"status":"in_progress"}]}"`,
want: 3,
},
// case_3
// 3 demanded, max at 2
{
repo: "test/valid",
Expand All @@ -107,6 +116,7 @@ func TestDetermineDesiredReplicas_RepositoryRunner(t *testing.T) {
workflowRuns_in_progress: `{"total_count": 2, "workflow_runs":[{"status":"in_progress"}, {"status":"in_progress"}]}"`,
want: 2,
},
// case_4
// 2 demanded, min at 2
{
repo: "test/valid",
Expand All @@ -117,6 +127,7 @@ func TestDetermineDesiredReplicas_RepositoryRunner(t *testing.T) {
workflowRuns_in_progress: `{"total_count": 1, "workflow_runs":[{"status":"in_progress"}]}"`,
want: 2,
},
// case_5
// 1 demanded, min at 2
{
repo: "test/valid",
Expand All @@ -127,6 +138,7 @@ func TestDetermineDesiredReplicas_RepositoryRunner(t *testing.T) {
workflowRuns_in_progress: `{"total_count": 0, "workflow_runs":[]}"`,
want: 2,
},
// case_6
// 1 demanded, min at 2
{
repo: "test/valid",
Expand All @@ -137,6 +149,7 @@ func TestDetermineDesiredReplicas_RepositoryRunner(t *testing.T) {
workflowRuns_in_progress: `{"total_count": 1, "workflow_runs":[{"status":"in_progress"}]}"`,
want: 2,
},
// case_7
// 1 demanded, min at 1
{
repo: "test/valid",
Expand All @@ -147,6 +160,7 @@ func TestDetermineDesiredReplicas_RepositoryRunner(t *testing.T) {
workflowRuns_in_progress: `{"total_count": 0, "workflow_runs":[]}"`,
want: 1,
},
// case_8
// 1 demanded, min at 1
{
repo: "test/valid",
Expand All @@ -157,6 +171,7 @@ func TestDetermineDesiredReplicas_RepositoryRunner(t *testing.T) {
workflowRuns_in_progress: `{"total_count": 1, "workflow_runs":[{"status":"in_progress"}]}"`,
want: 1,
},
// case_9
// fixed at 3
{
repo: "test/valid",
Expand All @@ -166,9 +181,36 @@ func TestDetermineDesiredReplicas_RepositoryRunner(t *testing.T) {
workflowRuns: `{"total_count": 4, "workflow_runs":[{"status":"in_progress"}, {"status":"in_progress"}, {"status":"in_progress"}, {"status":"completed"}]}"`,
workflowRuns_queued: `{"total_count": 0, "workflow_runs":[]}"`,
workflowRuns_in_progress: `{"total_count": 3, "workflow_runs":[{"status":"in_progress"}, {"status":"in_progress"}, {"status":"in_progress"}]}"`,
want: 3,
want: 1,
},
// Case for empty GitHub Actions reponse - should not trigger scale up
{
description: "GitHub Actions Jobs Array is empty - no scale up",
repo: "test/valid",
min: intPtr(0),
max: intPtr(3),
workflowRuns: `{"total_count": 2, "workflow_runs":[{"status":"queued"}, {"status":"completed"}]}"`,
workflowRuns_queued: `{"total_count": 1, "workflow_runs":[{"status":"queued"}]}"`,
workflowRuns_in_progress: `{"total_count": 0, "workflow_runs":[]}"`,
workflowJobs: map[int]string{
1: `{"jobs": []}`,
},
want: 0,
},
// Case for hosted GitHub Actions run
{
description: "Hosted GitHub Actions run - no scale up",
repo: "test/valid",
min: intPtr(0),
max: intPtr(3),
workflowRuns: `{"total_count": 2, "workflow_runs":[{"id": 1, "status":"queued"}, {"status":"completed"}]}"`,
workflowRuns_queued: `{"total_count": 1, "workflow_runs":[{"id": 1, "status":"queued"}]}"`,
workflowRuns_in_progress: `{"total_count": 0, "workflow_runs":[]}"`,
workflowJobs: map[int]string{
1: `{"jobs": [{"status":"queued"}]}`,
},
want: 0,
},

{
description: "Job-level autoscaling with no explicit runner label (runners have implicit self-hosted, requested self-hosted, 5 jobs from 3 workflows)",
repo: "test/valid",
Expand Down Expand Up @@ -422,7 +464,8 @@ func TestDetermineDesiredReplicas_OrganizationalRunner(t *testing.T) {
want int
err string
}{
// 3 demanded, max at 3
// case_0
// 2 demanded, max at 3
{
org: "test",
repos: []string{"valid"},
Expand All @@ -431,8 +474,9 @@ func TestDetermineDesiredReplicas_OrganizationalRunner(t *testing.T) {
workflowRuns: `{"total_count": 4, "workflow_runs":[{"status":"queued"}, {"status":"in_progress"}, {"status":"in_progress"}, {"status":"completed"}]}"`,
workflowRuns_queued: `{"total_count": 1, "workflow_runs":[{"status":"queued"}]}"`,
workflowRuns_in_progress: `{"total_count": 2, "workflow_runs":[{"status":"in_progress"}, {"status":"in_progress"}]}"`,
want: 3,
want: 2,
},
// case_1
// 2 demanded, max at 3, currently 3, delay scaling down due to grace period
{
org: "test",
Expand All @@ -446,6 +490,7 @@ func TestDetermineDesiredReplicas_OrganizationalRunner(t *testing.T) {
workflowRuns_in_progress: `{"total_count": 1, "workflow_runs":[{"status":"in_progress"}]}"`,
want: 3,
},
// case_2
// 3 demanded, max at 2
{
org: "test",
Expand All @@ -457,6 +502,7 @@ func TestDetermineDesiredReplicas_OrganizationalRunner(t *testing.T) {
workflowRuns_in_progress: `{"total_count": 2, "workflow_runs":[{"status":"in_progress"}, {"status":"in_progress"}]}"`,
want: 2,
},
// case_3
// 2 demanded, min at 2
{
org: "test",
Expand All @@ -468,6 +514,7 @@ func TestDetermineDesiredReplicas_OrganizationalRunner(t *testing.T) {
workflowRuns_in_progress: `{"total_count": 1, "workflow_runs":[{"status":"in_progress"}]}"`,
want: 2,
},
// case_4
// 1 demanded, min at 2
{
org: "test",
Expand All @@ -479,6 +526,7 @@ func TestDetermineDesiredReplicas_OrganizationalRunner(t *testing.T) {
workflowRuns_in_progress: `{"total_count": 0, "workflow_runs":[]}"`,
want: 2,
},
// case_5
// 1 demanded, min at 2
{
org: "test",
Expand Down Expand Up @@ -512,6 +560,7 @@ func TestDetermineDesiredReplicas_OrganizationalRunner(t *testing.T) {
workflowRuns_in_progress: `{"total_count": 1, "workflow_runs":[{"status":"in_progress"}]}"`,
want: 1,
},
// case_6
// fixed at 3
{
org: "test",
Expand All @@ -522,8 +571,9 @@ func TestDetermineDesiredReplicas_OrganizationalRunner(t *testing.T) {
workflowRuns: `{"total_count": 4, "workflow_runs":[{"status":"in_progress"}, {"status":"in_progress"}, {"status":"in_progress"}, {"status":"completed"}]}"`,
workflowRuns_queued: `{"total_count": 0, "workflow_runs":[]}"`,
workflowRuns_in_progress: `{"total_count": 3, "workflow_runs":[{"status":"in_progress"},{"status":"in_progress"},{"status":"in_progress"}]}"`,
want: 3,
want: 1,
},
// case_7
// org runner, fixed at 3
{
org: "test",
Expand All @@ -534,8 +584,9 @@ func TestDetermineDesiredReplicas_OrganizationalRunner(t *testing.T) {
workflowRuns: `{"total_count": 4, "workflow_runs":[{"status":"in_progress"}, {"status":"in_progress"}, {"status":"in_progress"}, {"status":"completed"}]}"`,
workflowRuns_queued: `{"total_count": 0, "workflow_runs":[]}"`,
workflowRuns_in_progress: `{"total_count": 3, "workflow_runs":[{"status":"in_progress"},{"status":"in_progress"},{"status":"in_progress"}]}"`,
want: 3,
want: 1,
},
// case_8
// org runner, 1 demanded, min at 1, no repos
{
org: "test",
Expand Down

0 comments on commit 45c975b

Please sign in to comment.