From 08acb1b831378d892f439bdf90335d00d7b586a9 Mon Sep 17 00:00:00 2001 From: Tingluo Huang Date: Wed, 15 Mar 2023 11:10:09 -0400 Subject: [PATCH] Get RunnerScaleSet based on both RunnerGroupId and Name. (#2413) --- cmd/githubrunnerscalesetlistener/main_test.go | 2 +- .../autoscalingrunnerset_controller.go | 31 +++++++++++-------- github/actions/client.go | 6 ++-- .../actions/client_runner_scale_set_test.go | 14 ++++----- github/actions/fake/client.go | 2 +- github/actions/mock_ActionsService.go | 14 ++++----- 6 files changed, 37 insertions(+), 32 deletions(-) diff --git a/cmd/githubrunnerscalesetlistener/main_test.go b/cmd/githubrunnerscalesetlistener/main_test.go index e4c1df03..6d11194f 100644 --- a/cmd/githubrunnerscalesetlistener/main_test.go +++ b/cmd/githubrunnerscalesetlistener/main_test.go @@ -144,7 +144,7 @@ func TestCustomerServerRootCA(t *testing.T) { client, err := newActionsClientFromConfig(config, creds) require.NoError(t, err) - _, err = client.GetRunnerScaleSet(ctx, "test") + _, err = client.GetRunnerScaleSet(ctx, 1, "test") require.NoError(t, err) assert.True(t, serverCalledSuccessfully) } diff --git a/controllers/actions.github.com/autoscalingrunnerset_controller.go b/controllers/actions.github.com/autoscalingrunnerset_controller.go index b279e084..b833e57d 100644 --- a/controllers/actions.github.com/autoscalingrunnerset_controller.go +++ b/controllers/actions.github.com/autoscalingrunnerset_controller.go @@ -316,24 +316,29 @@ func (r *AutoscalingRunnerSetReconciler) createRunnerScaleSet(ctx context.Contex logger.Error(err, "Failed to initialize Actions service client for creating a new runner scale set") return ctrl.Result{}, err } - runnerScaleSet, err := actionsClient.GetRunnerScaleSet(ctx, autoscalingRunnerSet.Spec.RunnerScaleSetName) + + runnerGroupId := 1 + if len(autoscalingRunnerSet.Spec.RunnerGroup) > 0 { + runnerGroup, err := actionsClient.GetRunnerGroupByName(ctx, autoscalingRunnerSet.Spec.RunnerGroup) + if err != nil { + logger.Error(err, "Failed to get runner group by name", "runnerGroup", autoscalingRunnerSet.Spec.RunnerGroup) + return ctrl.Result{}, err + } + + runnerGroupId = int(runnerGroup.ID) + } + + runnerScaleSet, err := actionsClient.GetRunnerScaleSet(ctx, runnerGroupId, autoscalingRunnerSet.Spec.RunnerScaleSetName) if err != nil { - logger.Error(err, "Failed to get runner scale set from Actions service") + logger.Error(err, "Failed to get runner scale set from Actions service", + "runnerGroupId", + strconv.Itoa(runnerGroupId), + "runnerScaleSetName", + autoscalingRunnerSet.Spec.RunnerScaleSetName) return ctrl.Result{}, err } - runnerGroupId := 1 if runnerScaleSet == nil { - if len(autoscalingRunnerSet.Spec.RunnerGroup) > 0 { - runnerGroup, err := actionsClient.GetRunnerGroupByName(ctx, autoscalingRunnerSet.Spec.RunnerGroup) - if err != nil { - logger.Error(err, "Failed to get runner group by name", "runnerGroup", autoscalingRunnerSet.Spec.RunnerGroup) - return ctrl.Result{}, err - } - - runnerGroupId = int(runnerGroup.ID) - } - runnerScaleSet, err = actionsClient.CreateRunnerScaleSet( ctx, &actions.RunnerScaleSet{ diff --git a/github/actions/client.go b/github/actions/client.go index 7d68bd39..51fe75d8 100644 --- a/github/actions/client.go +++ b/github/actions/client.go @@ -31,7 +31,7 @@ const ( //go:generate mockery --inpackage --name=ActionsService type ActionsService interface { - GetRunnerScaleSet(ctx context.Context, runnerScaleSetName string) (*RunnerScaleSet, error) + GetRunnerScaleSet(ctx context.Context, runnerGroupId int, runnerScaleSetName string) (*RunnerScaleSet, error) GetRunnerScaleSetById(ctx context.Context, runnerScaleSetId int) (*RunnerScaleSet, error) GetRunnerGroupByName(ctx context.Context, runnerGroup string) (*RunnerGroup, error) CreateRunnerScaleSet(ctx context.Context, runnerScaleSet *RunnerScaleSet) (*RunnerScaleSet, error) @@ -285,8 +285,8 @@ func (c *Client) NewActionsServiceRequest(ctx context.Context, method, path stri return req, nil } -func (c *Client) GetRunnerScaleSet(ctx context.Context, runnerScaleSetName string) (*RunnerScaleSet, error) { - path := fmt.Sprintf("/%s?name=%s", scaleSetEndpoint, runnerScaleSetName) +func (c *Client) GetRunnerScaleSet(ctx context.Context, runnerGroupId int, runnerScaleSetName string) (*RunnerScaleSet, error) { + path := fmt.Sprintf("/%s?runnerGroupId=%d&name=%s", scaleSetEndpoint, runnerGroupId, runnerScaleSetName) req, err := c.NewActionsServiceRequest(ctx, http.MethodGet, path, nil) if err != nil { return nil, err diff --git a/github/actions/client_runner_scale_set_test.go b/github/actions/client_runner_scale_set_test.go index d313d013..7f74190e 100644 --- a/github/actions/client_runner_scale_set_test.go +++ b/github/actions/client_runner_scale_set_test.go @@ -34,7 +34,7 @@ func TestGetRunnerScaleSet(t *testing.T) { client, err := actions.NewClient(server.configURLForOrg("my-org"), auth) require.NoError(t, err) - got, err := client.GetRunnerScaleSet(ctx, scaleSetName) + got, err := client.GetRunnerScaleSet(ctx, 1, scaleSetName) require.NoError(t, err) assert.Equal(t, want, got) }) @@ -50,7 +50,7 @@ func TestGetRunnerScaleSet(t *testing.T) { client, err := actions.NewClient(server.configURLForOrg("my-org"), auth) require.NoError(t, err) - _, err = client.GetRunnerScaleSet(ctx, scaleSetName) + _, err = client.GetRunnerScaleSet(ctx, 1, scaleSetName) require.NoError(t, err) expectedPath := "/tenant/123/_apis/runtime/runnerscalesets" @@ -67,7 +67,7 @@ func TestGetRunnerScaleSet(t *testing.T) { client, err := actions.NewClient(server.configURLForOrg("my-org"), auth) require.NoError(t, err) - _, err = client.GetRunnerScaleSet(ctx, scaleSetName) + _, err = client.GetRunnerScaleSet(ctx, 1, scaleSetName) assert.NotNil(t, err) }) @@ -80,7 +80,7 @@ func TestGetRunnerScaleSet(t *testing.T) { client, err := actions.NewClient(server.configURLForOrg("my-org"), auth) require.NoError(t, err) - _, err = client.GetRunnerScaleSet(ctx, scaleSetName) + _, err = client.GetRunnerScaleSet(ctx, 1, scaleSetName) assert.NotNil(t, err) }) @@ -102,7 +102,7 @@ func TestGetRunnerScaleSet(t *testing.T) { ) require.NoError(t, err) - _, err = client.GetRunnerScaleSet(ctx, scaleSetName) + _, err = client.GetRunnerScaleSet(ctx, 1, scaleSetName) assert.NotNil(t, err) expectedRetry := retryMax + 1 assert.Equalf(t, actualRetry, expectedRetry, "A retry was expected after the first request but got: %v", actualRetry) @@ -118,7 +118,7 @@ func TestGetRunnerScaleSet(t *testing.T) { client, err := actions.NewClient(server.configURLForOrg("my-org"), auth) require.NoError(t, err) - got, err := client.GetRunnerScaleSet(ctx, scaleSetName) + got, err := client.GetRunnerScaleSet(ctx, 1, scaleSetName) require.NoError(t, err) assert.Equal(t, want, got) }) @@ -133,7 +133,7 @@ func TestGetRunnerScaleSet(t *testing.T) { client, err := actions.NewClient(server.configURLForOrg("my-org"), auth) require.NoError(t, err) - _, err = client.GetRunnerScaleSet(ctx, scaleSetName) + _, err = client.GetRunnerScaleSet(ctx, 1, scaleSetName) require.NotNil(t, err) assert.Equal(t, wantErr.Error(), err.Error()) }) diff --git a/github/actions/fake/client.go b/github/actions/fake/client.go index 0729425d..a4f17e4d 100644 --- a/github/actions/fake/client.go +++ b/github/actions/fake/client.go @@ -215,7 +215,7 @@ func (f *FakeClient) applyDefaults() { f.getRunnerByNameResult.RunnerReference = defaultRunnerReference } -func (f *FakeClient) GetRunnerScaleSet(ctx context.Context, runnerScaleSetName string) (*actions.RunnerScaleSet, error) { +func (f *FakeClient) GetRunnerScaleSet(ctx context.Context, runnerGroupId int, runnerScaleSetName string) (*actions.RunnerScaleSet, error) { return f.getRunnerScaleSetResult.RunnerScaleSet, f.getRunnerScaleSetResult.err } diff --git a/github/actions/mock_ActionsService.go b/github/actions/mock_ActionsService.go index ba10de8d..ad5050bc 100644 --- a/github/actions/mock_ActionsService.go +++ b/github/actions/mock_ActionsService.go @@ -263,13 +263,13 @@ func (_m *MockActionsService) GetRunnerGroupByName(ctx context.Context, runnerGr return r0, r1 } -// GetRunnerScaleSet provides a mock function with given fields: ctx, runnerScaleSetName -func (_m *MockActionsService) GetRunnerScaleSet(ctx context.Context, runnerScaleSetName string) (*RunnerScaleSet, error) { - ret := _m.Called(ctx, runnerScaleSetName) +// GetRunnerScaleSet provides a mock function with given fields: ctx, runnerGroupId, runnerScaleSetName +func (_m *MockActionsService) GetRunnerScaleSet(ctx context.Context, runnerGroupId int, runnerScaleSetName string) (*RunnerScaleSet, error) { + ret := _m.Called(ctx, runnerGroupId, runnerScaleSetName) var r0 *RunnerScaleSet - if rf, ok := ret.Get(0).(func(context.Context, string) *RunnerScaleSet); ok { - r0 = rf(ctx, runnerScaleSetName) + if rf, ok := ret.Get(0).(func(context.Context, int, string) *RunnerScaleSet); ok { + r0 = rf(ctx, runnerGroupId, runnerScaleSetName) } else { if ret.Get(0) != nil { r0 = ret.Get(0).(*RunnerScaleSet) @@ -277,8 +277,8 @@ func (_m *MockActionsService) GetRunnerScaleSet(ctx context.Context, runnerScale } var r1 error - if rf, ok := ret.Get(1).(func(context.Context, string) error); ok { - r1 = rf(ctx, runnerScaleSetName) + if rf, ok := ret.Get(1).(func(context.Context, int, string) error); ok { + r1 = rf(ctx, runnerGroupId, runnerScaleSetName) } else { r1 = ret.Error(1) }