From 65ca8e6a10527dcb2ee9165d8eb7ee415686f02b Mon Sep 17 00:00:00 2001 From: "David J. M. Karlsen" Date: Sat, 2 Jan 2021 23:45:44 +0100 Subject: [PATCH 1/4] Implement deletion order. Fixes #138 --- api/v1alpha1/githubactionrunner_types.go | 13 ++++++++++ ...aro.tietoevry.com_githubactionrunners.yaml | 7 ++++++ controllers/githubactionrunner_controller.go | 2 +- controllers/podrunner_types.go | 15 ++++++++++-- controllers/podrunner_types_test.go | 24 +++++++++++++++++-- 5 files changed, 56 insertions(+), 5 deletions(-) diff --git a/api/v1alpha1/githubactionrunner_types.go b/api/v1alpha1/githubactionrunner_types.go index 42a3459a..1aa68319 100644 --- a/api/v1alpha1/githubactionrunner_types.go +++ b/api/v1alpha1/githubactionrunner_types.go @@ -41,8 +41,21 @@ type GithubActionRunnerSpec struct { // +kubebuilder:validation:Optional // +kubebuilder:default="1m" ReconciliationPeriod string `json:"reconciliationPeriod"` + + // What order to delete idle pods in + // +kubebuilder:default="LeastRecent" + // +kubebuilder:validation:Optional + DeletionOrder SortOrder `json:"deletionOrder"` } +const ( + LeastRecent SortOrder = "LeastRecent" + MostRecent = "MostRecent" +) + +// +kubebuilder:validation:Enum=MostRecent;LeastRecent +type SortOrder string + // IsValid validates conditions not covered by basic OpenAPI constraints func (r GithubActionRunnerSpec) IsValid() (bool, error) { if r.MaxRunners < r.MinRunners { diff --git a/config/crd/bases/garo.tietoevry.com_githubactionrunners.yaml b/config/crd/bases/garo.tietoevry.com_githubactionrunners.yaml index bdec99e4..b0521899 100644 --- a/config/crd/bases/garo.tietoevry.com_githubactionrunners.yaml +++ b/config/crd/bases/garo.tietoevry.com_githubactionrunners.yaml @@ -43,6 +43,13 @@ spec: spec: description: GithubActionRunnerSpec defines the desired state of GithubActionRunner properties: + deletionOrder: + default: LeastRecent + description: What order to delete idle pods in + enum: + - MostRecent + - LeastRecent + type: string maxRunners: description: Maximum pool-size. Must be greater or equal to minRunners minimum: 1 diff --git a/controllers/githubactionrunner_controller.go b/controllers/githubactionrunner_controller.go index b14aa7ba..368af963 100644 --- a/controllers/githubactionrunner_controller.go +++ b/controllers/githubactionrunner_controller.go @@ -135,7 +135,7 @@ func (r *GithubActionRunnerReconciler) handleScaling(ctx context.Context, instan } else if shouldScaleDown(podRunnerPairs, instance) { logger.Info("Scaling down", "runners at github", podRunnerPairs.numRunners(), "maxrunners in CR", instance.Spec.MaxRunners) - pod := podRunnerPairs.getIdlePods()[0] + pod := podRunnerPairs.getIdlePods(instance.Spec.DeletionOrder)[0] err := r.DeleteResourceIfExists(ctx, &pod) if err == nil { r.GetRecorder().Event(instance, corev1.EventTypeNormal, "Scaling", fmt.Sprintf("Deleted pod %s/%s", pod.Namespace, pod.Name)) diff --git a/controllers/podrunner_types.go b/controllers/podrunner_types.go index c019a9ec..bcbd3ab4 100644 --- a/controllers/podrunner_types.go +++ b/controllers/podrunner_types.go @@ -1,10 +1,12 @@ package controllers import ( + "github.com/evryfs/github-actions-runner-operator/api/v1alpha1" "github.com/google/go-github/v33/github" "github.com/redhat-cop/operator-utils/pkg/util" "github.com/thoas/go-funk" corev1 "k8s.io/api/core/v1" + "sort" ) type podRunnerPair struct { @@ -66,13 +68,22 @@ func (r podRunnerPairList) inSync() bool { return r.numPods() == r.numRunners() } -func (r podRunnerPairList) getIdlePods() []corev1.Pod { +func (r podRunnerPairList) getIdlePods(sortOrder v1alpha1.SortOrder) []corev1.Pod { idles := funk.Filter(r.pairs, func(pair podRunnerPair) bool { return !(pair.runner.GetBusy() || util.IsBeingDeleted(&pair.pod)) }).([]podRunnerPair) - return funk.Map(idles, func(pair podRunnerPair) corev1.Pod { + pods := funk.Map(idles, func(pair podRunnerPair) corev1.Pod { return pair.pod }).([]corev1.Pod) + + sort.SliceStable(pods, func(i, j int) bool { + if sortOrder == v1alpha1.LeastRecent { + return pods[i].CreationTimestamp.Unix() < pods[j].CreationTimestamp.Unix() + } + return pods[i].CreationTimestamp.Unix() > pods[j].CreationTimestamp.Unix() + }) + + return pods } func (r podRunnerPairList) getPodsBeingDeleted() []podRunnerPair { diff --git a/controllers/podrunner_types_test.go b/controllers/podrunner_types_test.go index 3d85436d..89178b9a 100644 --- a/controllers/podrunner_types_test.go +++ b/controllers/podrunner_types_test.go @@ -1,12 +1,14 @@ package controllers import ( + "github.com/evryfs/github-actions-runner-operator/api/v1alpha1" "github.com/google/go-github/v33/github" "github.com/stretchr/testify/assert" v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/utils/pointer" "testing" + "time" ) var podList = v1.PodList{ @@ -16,7 +18,8 @@ var podList = v1.PodList{ { TypeMeta: metav1.TypeMeta{}, ObjectMeta: metav1.ObjectMeta{ - Name: "name1", + Name: "name1", + CreationTimestamp: metav1.NewTime(time.Now()), }, Spec: v1.PodSpec{}, Status: v1.PodStatus{}, @@ -24,7 +27,8 @@ var podList = v1.PodList{ { TypeMeta: metav1.TypeMeta{}, ObjectMeta: metav1.ObjectMeta{ - Name: "name2", + Name: "name2", + CreationTimestamp: metav1.NewTime(time.Now().Add(time.Minute)), }, Spec: v1.PodSpec{}, Status: v1.PodStatus{}, @@ -67,3 +71,19 @@ func TestPodRunnerPairList(t *testing.T) { assert.Equal(t, podRunnerPairList.inSync(), tc.inSync) } } + +func TestSort(t *testing.T) { + testCases := []struct { + sortOrder v1alpha1.SortOrder + podRunnerPairList podRunnerPairList + podList []v1.Pod + }{ + {v1alpha1.LeastRecent, from(&podList, runners), []v1.Pod{podList.Items[0], podList.Items[1]}}, + {v1alpha1.MostRecent, from(&podList, runners), []v1.Pod{podList.Items[1], podList.Items[0]}}, + } + + for _, tc := range testCases { + podList := tc.podRunnerPairList.getIdlePods(tc.sortOrder) + assert.Equal(t, podList, tc.podList) + } +} From ece78136ca11dee28d3d9e4510ac29e5ad88f2c0 Mon Sep 17 00:00:00 2001 From: "David J. M. Karlsen" Date: Sat, 2 Jan 2021 23:50:15 +0100 Subject: [PATCH 2/4] docs --- api/v1alpha1/githubactionrunner_types.go | 1 + 1 file changed, 1 insertion(+) diff --git a/api/v1alpha1/githubactionrunner_types.go b/api/v1alpha1/githubactionrunner_types.go index 1aa68319..0d808c32 100644 --- a/api/v1alpha1/githubactionrunner_types.go +++ b/api/v1alpha1/githubactionrunner_types.go @@ -54,6 +54,7 @@ const ( ) // +kubebuilder:validation:Enum=MostRecent;LeastRecent +// SortOrder defines order to sort by when sorting on creation timestamp. type SortOrder string // IsValid validates conditions not covered by basic OpenAPI constraints From 945dc6285b94adfd0d916042570b91ec00f39808 Mon Sep 17 00:00:00 2001 From: "David J. M. Karlsen" Date: Sat, 2 Jan 2021 23:55:18 +0100 Subject: [PATCH 3/4] statickcheck --- api/v1alpha1/githubactionrunner_types.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/v1alpha1/githubactionrunner_types.go b/api/v1alpha1/githubactionrunner_types.go index 0d808c32..4b09ebae 100644 --- a/api/v1alpha1/githubactionrunner_types.go +++ b/api/v1alpha1/githubactionrunner_types.go @@ -50,7 +50,7 @@ type GithubActionRunnerSpec struct { const ( LeastRecent SortOrder = "LeastRecent" - MostRecent = "MostRecent" + MostRecent SortOrder = "MostRecent" ) // +kubebuilder:validation:Enum=MostRecent;LeastRecent From 06b32cddecb5dd94b37a113858a18b75f35efbf6 Mon Sep 17 00:00:00 2001 From: "David J. M. Karlsen" Date: Sat, 2 Jan 2021 23:58:44 +0100 Subject: [PATCH 4/4] docs --- api/v1alpha1/githubactionrunner_types.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/api/v1alpha1/githubactionrunner_types.go b/api/v1alpha1/githubactionrunner_types.go index 4b09ebae..1b60a240 100644 --- a/api/v1alpha1/githubactionrunner_types.go +++ b/api/v1alpha1/githubactionrunner_types.go @@ -49,12 +49,14 @@ type GithubActionRunnerSpec struct { } const ( + // LeastRecent first. LeastRecent SortOrder = "LeastRecent" + // MostRecent first. MostRecent SortOrder = "MostRecent" ) -// +kubebuilder:validation:Enum=MostRecent;LeastRecent // SortOrder defines order to sort by when sorting on creation timestamp. +// +kubebuilder:validation:Enum=MostRecent;LeastRecent type SortOrder string // IsValid validates conditions not covered by basic OpenAPI constraints