From b9bf07a25550bbbf3f83d772e2f17e58c5c31c6a Mon Sep 17 00:00:00 2001 From: "David J. M. Karlsen" Date: Tue, 6 Apr 2021 14:35:33 +0200 Subject: [PATCH] Implement minTtl feature (#222) --- api/v1alpha1/githubactionrunner_types.go | 19 ++++++------------- ...aro.tietoevry.com_githubactionrunners.yaml | 5 +++++ controllers/githubactionrunner_controller.go | 4 ++-- controllers/podrunner_types.go | 5 +++-- controllers/podrunner_types_test.go | 8 ++++---- 5 files changed, 20 insertions(+), 21 deletions(-) diff --git a/api/v1alpha1/githubactionrunner_types.go b/api/v1alpha1/githubactionrunner_types.go index 282e72dd..e8895835 100644 --- a/api/v1alpha1/githubactionrunner_types.go +++ b/api/v1alpha1/githubactionrunner_types.go @@ -2,8 +2,6 @@ package v1alpha1 import ( "errors" - "time" - v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) @@ -33,6 +31,11 @@ type GithubActionRunnerSpec struct { // +operator-sdk:csv:customresourcedefinitions:type=spec,displayName="Maximum Pool Size",xDescriptors={"urn:alm:descriptor:com.tectonic.ui:podCount"} MaxRunners int `json:"maxRunners"` + // Minimum time to live for a runner. This can avoid trashing by keeping pods around longer than required by jobs, keeping caches hot. + // +kubebuilder:default="0m" + // +operator-sdk:csv:customresourcedefinitions:type=spec,displayName="Minimum time to live" + MinTTL metav1.Duration `json:"minTtl"` + // +kubebuilder:validation:Required // +operator-sdk:csv:customresourcedefinitions:type=spec,displayName="Pod Template" PodTemplateSpec v1.PodTemplateSpec `json:"podTemplateSpec"` @@ -46,7 +49,7 @@ type GithubActionRunnerSpec struct { // +kubebuilder:validation:Optional // +kubebuilder:default="1m" // +operator-sdk:csv:customresourcedefinitions:type=spec,displayName="Reconciliation Period" - ReconciliationPeriod string `json:"reconciliationPeriod"` + ReconciliationPeriod metav1.Duration `json:"reconciliationPeriod"` // What order to delete idle pods in // +kubebuilder:default="LeastRecent" @@ -75,16 +78,6 @@ func (r GithubActionRunnerSpec) IsValid() (bool, error) { return true, nil } -// GetReconciliationPeriod returns period as a Duration -func (r GithubActionRunnerSpec) GetReconciliationPeriod() time.Duration { - duration, err := time.ParseDuration(r.ReconciliationPeriod) - if err != nil { - return time.Minute - } - - return duration -} - // GithubActionRunnerStatus defines the observed state of GithubActionRunner type GithubActionRunnerStatus struct { // the current size of the build pool diff --git a/config/crd/bases/garo.tietoevry.com_githubactionrunners.yaml b/config/crd/bases/garo.tietoevry.com_githubactionrunners.yaml index 2ae505ae..a0b7fbf6 100644 --- a/config/crd/bases/garo.tietoevry.com_githubactionrunners.yaml +++ b/config/crd/bases/garo.tietoevry.com_githubactionrunners.yaml @@ -54,6 +54,10 @@ spec: description: Minimum pool-size. Note that you need one runner in order for jobs to be schedulable, else they fail claiming no runners match the selector labels. minimum: 1 type: integer + minTtl: + default: 0m + description: Minimum time to live for a runner. This can avoid trashing by keeping pods around longer than required by jobs, keeping caches hot. + type: string organization: description: Your GitHub organization type: string @@ -3822,6 +3826,7 @@ spec: required: - maxRunners - minRunners + - minTtl - organization - podTemplateSpec type: object diff --git a/controllers/githubactionrunner_controller.go b/controllers/githubactionrunner_controller.go index addec726..1a1402e6 100644 --- a/controllers/githubactionrunner_controller.go +++ b/controllers/githubactionrunner_controller.go @@ -146,7 +146,7 @@ func (r *GithubActionRunnerReconciler) handleScaling(ctx context.Context, instan // scaleDown will scale down an idle runner based on policy in CR func (r *GithubActionRunnerReconciler) scaleDown(ctx context.Context, podRunnerPairs podRunnerPairList, instance *garov1alpha1.GithubActionRunner) error { - idles := podRunnerPairs.getIdles(instance.Spec.DeletionOrder) + idles := podRunnerPairs.getIdles(instance.Spec.DeletionOrder, instance.Spec.MinTTL.Duration) for _, pair := range idles { err := r.unregisterRunner(ctx, instance, pair) if err != nil { // should be improved, here we just assume it's because it's running a job and cannot be removed, skip to next candidate @@ -178,7 +178,7 @@ func shouldScaleDown(podRunnerPairs podRunnerPairList, instance *garov1alpha1.Gi } func (r *GithubActionRunnerReconciler) manageOutcome(ctx context.Context, instance *garov1alpha1.GithubActionRunner, issue error) (reconcile.Result, error) { - return r.ManageOutcomeWithRequeue(ctx, instance, issue, instance.Spec.GetReconciliationPeriod()) + return r.ManageOutcomeWithRequeue(ctx, instance, issue, instance.Spec.ReconciliationPeriod.Duration) } // SetupWithManager configures the controller by using the passed mgr diff --git a/controllers/podrunner_types.go b/controllers/podrunner_types.go index 14f6b99f..9860f56f 100644 --- a/controllers/podrunner_types.go +++ b/controllers/podrunner_types.go @@ -8,6 +8,7 @@ import ( "github.com/thoas/go-funk" corev1 "k8s.io/api/core/v1" "sort" + "time" ) type podRunnerPair struct { @@ -77,9 +78,9 @@ func (r podRunnerPairList) numIdle() int { return r.numRunners() - r.numBusy() } -func (r podRunnerPairList) getIdles(sortOrder v1alpha1.SortOrder) []podRunnerPair { +func (r podRunnerPairList) getIdles(sortOrder v1alpha1.SortOrder, minTTL time.Duration) []podRunnerPair { idles := funk.Filter(r.pairs, func(pair podRunnerPair) bool { - return !(pair.runner.GetBusy() || util.IsBeingDeleted(&pair.pod)) + return !(pair.runner.GetBusy() || util.IsBeingDeleted(&pair.pod)) && time.Now().After(pair.pod.CreationTimestamp.Add(minTTL)) }).([]podRunnerPair) sort.SliceStable(idles, func(i, j int) bool { diff --git a/controllers/podrunner_types_test.go b/controllers/podrunner_types_test.go index 8d5a3f14..0ce4163e 100644 --- a/controllers/podrunner_types_test.go +++ b/controllers/podrunner_types_test.go @@ -19,7 +19,7 @@ var podList = v1.PodList{ TypeMeta: metav1.TypeMeta{}, ObjectMeta: metav1.ObjectMeta{ Name: "name1", - CreationTimestamp: metav1.NewTime(time.Now()), + CreationTimestamp: metav1.NewTime(time.Now().Add(-time.Minute)), }, Spec: v1.PodSpec{}, Status: v1.PodStatus{}, @@ -28,7 +28,7 @@ var podList = v1.PodList{ TypeMeta: metav1.TypeMeta{}, ObjectMeta: metav1.ObjectMeta{ Name: "name2", - CreationTimestamp: metav1.NewTime(time.Now().Add(time.Minute)), + CreationTimestamp: metav1.NewTime(time.Now()), }, Spec: v1.PodSpec{}, Status: v1.PodStatus{}, @@ -87,7 +87,7 @@ func TestSort(t *testing.T) { } for _, tc := range testCases { - podList := tc.podRunnerPairList.getIdles(tc.sortOrder) - assert.Equal(t, podList, tc.podRunnerPair) + podList := tc.podRunnerPairList.getIdles(tc.sortOrder, time.Duration(0)) + assert.Equal(t, tc.podRunnerPair, podList) } }