fix: minor refactorings of packages and namings

api/v1alpha1/pool_types.go

@@ -6,6 +6,8 @@ import (
 	commonParams "github.com/cloudbase/garm-provider-common/params"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+
+	"github.com/mercedes-benz/garm-operator/pkg/util/filter"
 )
 
 // EDIT THIS FILE!  THIS IS SCAFFOLDING FOR YOU TO OWN!
@@ -99,54 +101,32 @@ func init() {
 	SchemeBuilder.Register(&Pool{}, &PoolList{})
 }
 
-// +k8s:deepcopy-gen=false
-type Predicate func(p Pool) bool
-
-func MatchesImage(image string) Predicate {
+func MatchesImage(image string) filter.Predicate[Pool] {
 	return func(p Pool) bool {
 		return p.Spec.ImageName == image
 	}
 }
 
-func MatchesFlavor(flavor string) Predicate {
+func MatchesFlavor(flavor string) filter.Predicate[Pool] {
 	return func(p Pool) bool {
 		return p.Spec.Flavor == flavor
 	}
 }
 
-func MatchesProvider(provider string) Predicate {
+func MatchesProvider(provider string) filter.Predicate[Pool] {
 	return func(p Pool) bool {
 		return p.Spec.ProviderName == provider
 	}
 }
 
-func MatchesGitHubScope(name, kind string) Predicate {
+func MatchesGitHubScope(name, kind string) filter.Predicate[Pool] {
 	return func(p Pool) bool {
 		return p.Spec.GitHubScopeRef.Name == name && p.Spec.GitHubScopeRef.Kind == kind
 	}
 }
 
-func MatchesID(id string) Predicate {
+func MatchesID(id string) filter.Predicate[Pool] {
 	return func(p Pool) bool {
 		return p.Status.ID == id
 	}
 }
-
-func (p *PoolList) FilterByFields(predicates ...Predicate) {
-	var filteredItems []Pool
-
-	for _, pool := range p.Items {
-		match := true
-		for _, predicate := range predicates {
-			if !predicate(pool) {
-				match = false
-				break
-			}
-		}
-		if match {
-			filteredItems = append(filteredItems, pool)
-		}
-	}
-
-	p.Items = filteredItems
-}

api/v1alpha1/pool_types_test.go

@@ -8,6 +8,8 @@ import (
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/utils/ptr"
+
+	"github.com/mercedes-benz/garm-operator/pkg/util/filter"
 )
 
 func TestPoolList_FilterByFields(t *testing.T) {
@@ -17,7 +19,7 @@ func TestPoolList_FilterByFields(t *testing.T) {
 		Items    []Pool
 	}
 	type args struct {
-		predicates []Predicate
+		predicates []filter.Predicate[Pool]
 	}
 	tests := []struct {
 		name   string
@@ -68,7 +70,7 @@ func TestPoolList_FilterByFields(t *testing.T) {
 				},
 			},
 			args: args{
-				predicates: []Predicate{
+				predicates: []filter.Predicate[Pool]{
 					MatchesImage("ubuntu-2204"),
 					MatchesFlavor("large"),
 					MatchesProvider("openstack"),
@@ -120,7 +122,7 @@ func TestPoolList_FilterByFields(t *testing.T) {
 				},
 			},
 			args: args{
-				predicates: []Predicate{
+				predicates: []filter.Predicate[Pool]{
 					MatchesImage("ubuntu-2404"),
 					MatchesFlavor("large"),
 					MatchesProvider("openstack"),
@@ -129,6 +131,53 @@ func TestPoolList_FilterByFields(t *testing.T) {
 			},
 			length: 0,
 		},
+		{
+			name: "no predicates provided",
+			fields: fields{
+				TypeMeta: metav1.TypeMeta{},
+				ListMeta: metav1.ListMeta{},
+				Items: []Pool{
+					{
+						TypeMeta: metav1.TypeMeta{},
+						ObjectMeta: metav1.ObjectMeta{
+							Name:      "ubuntu-2004-large",
+							Namespace: "test",
+						},
+						Spec: PoolSpec{
+							ImageName:    "ubuntu-2004",
+							Flavor:       "large",
+							ProviderName: "openstack",
+							GitHubScopeRef: corev1.TypedLocalObjectReference{
+								Name:     "test",
+								Kind:     "Enterprise",
+								APIGroup: ptr.To[string]("github.com"),
+							},
+						},
+					},
+					{
+						TypeMeta: metav1.TypeMeta{},
+						ObjectMeta: metav1.ObjectMeta{
+							Name:      "ubuntu-2204-large",
+							Namespace: "test",
+						},
+						Spec: PoolSpec{
+							ImageName:    "ubuntu-2204",
+							Flavor:       "large",
+							ProviderName: "openstack",
+							GitHubScopeRef: corev1.TypedLocalObjectReference{
+								Name:     "test",
+								Kind:     "Enterprise",
+								APIGroup: ptr.To[string]("github.com"),
+							},
+						},
+					},
+				},
+			},
+			args: args{
+				predicates: []filter.Predicate[Pool]{},
+			},
+			length: 2,
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
@@ -138,10 +187,10 @@ func TestPoolList_FilterByFields(t *testing.T) {
 				Items:    tt.fields.Items,
 			}
 
-			p.FilterByFields(tt.args.predicates...)
+			filteredPools := filter.Match(p.Items, tt.args.predicates...)
 
-			if len(p.Items) != tt.length {
-				t.Errorf("FilterByFields() = %v, want %v", len(p.Items), tt.length)
+			if len(filteredPools) != tt.length {
+				t.Errorf("FilterByFields() = %v, want %v", len(filteredPools), tt.length)
 			}
 		})
 	}

api/v1alpha1/pool_webhook.go

@@ -17,6 +17,8 @@ import (
 	logf "sigs.k8s.io/controller-runtime/pkg/log"
 	"sigs.k8s.io/controller-runtime/pkg/webhook"
 	"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
+
+	"github.com/mercedes-benz/garm-operator/pkg/util/filter"
 )
 
 // log is for logging in this package.
@@ -58,15 +60,15 @@ func (r *Pool) ValidateCreate() (admission.Warnings, error) {
 		return nil, err
 	}
 
-	poolList.FilterByFields(
+	filteredPoolList := filter.Match(poolList.Items,
 		MatchesFlavor(r.Spec.Flavor),
 		MatchesImage(r.Spec.ImageName),
 		MatchesProvider(r.Spec.ProviderName),
 		MatchesGitHubScope(r.Spec.GitHubScopeRef.Name, r.Spec.GitHubScopeRef.Kind),
 	)
 
-	if len(poolList.Items) > 0 {
-		existing := poolList.Items[0]
+	if len(filteredPoolList) > 0 {
+		existing := filteredPoolList[0]
 		return nil, apierrors.NewBadRequest(
 			fmt.Sprintf("can not create pool, pool=%s with same image=%s, flavor=%s and provider=%s already exists for specified GitHubScope=%s", existing.Name, existing.Spec.ImageName, existing.Spec.Flavor, existing.Spec.ProviderName, existing.Spec.GitHubScopeRef.Name))
 	}

config/overlays/debug/manager_patch.yaml

@@ -26,6 +26,8 @@ spec:
             - --garm-username=admin
             - --garm-password=LmrBG1KcBOsDfNKq4cQTGpc0hJ0kejkk
             - --operator-watch-namespace=garm-operator-system
+            - --operator-min-idle-runners-age=1m
+            - --operator-runner-reconciliation=true
           ports:
             - containerPort: 2345
               name: delve
@@ -35,3 +37,7 @@ spec:
               memory: 1Gi
             requests:
               memory: 1Gi
+          livenessProbe:
+            $patch: delete
+          readinessProbe:
+            $patch: delete

internal/controller/pool_controller.go

@@ -34,7 +34,9 @@ import (
 	"github.com/mercedes-benz/garm-operator/pkg/event"
 	"github.com/mercedes-benz/garm-operator/pkg/util/annotations"
 	"github.com/mercedes-benz/garm-operator/pkg/util/conditions"
-	poolUtil "github.com/mercedes-benz/garm-operator/pkg/util/pool"
+	poolUtil "github.com/mercedes-benz/garm-operator/pkg/util/pools"
+	runnerUtil "github.com/mercedes-benz/garm-operator/pkg/util/runners"
+	"github.com/mercedes-benz/garm-operator/pkg/util/tags"
 )
 
 // PoolReconciler reconciles a Pool object
@@ -178,7 +180,7 @@ func (r *PoolReconciler) reconcileUpdate(ctx context.Context, garmClient garmCli
 		}
 	}
 
-	longRunningIdleRunnersCount := len(poolUtil.OldIdleRunners(config.Config.Operator.MinIdleRunnersAge, idleRunners))
+	longRunningIdleRunnersCount := len(runnerUtil.OldIdleRunners(config.Config.Operator.MinIdleRunnersAge, idleRunners))
 
 	switch {
 	case pool.Spec.MinIdleRunners == 0:
@@ -188,7 +190,7 @@ func (r *PoolReconciler) reconcileUpdate(ctx context.Context, garmClient garmCli
 		log.Info("Scaling pool", "pool", pool.Name)
 		event.Scaling(r.Recorder, pool, fmt.Sprintf("scale idle runners down to %d", pool.Spec.MinIdleRunners))
 
-		runners := poolUtil.DeletableRunners(ctx, idleRunners)
+		runners := runnerUtil.DeletableRunners(ctx, idleRunners)
 		for _, runner := range runners {
 			if err := instanceClient.DeleteInstance(instances.NewDeleteInstanceParams().WithInstanceName(runner.Name)); err != nil {
 				log.Error(err, "unable to delete runner", "runner", runner.Name)
@@ -200,13 +202,13 @@ func (r *PoolReconciler) reconcileUpdate(ctx context.Context, garmClient garmCli
 		// the spec, we delete old idle runners
 
 		// get all idle runners that are older than minRunnerAge
-		longRunningIdleRunners := poolUtil.OldIdleRunners(config.Config.Operator.MinIdleRunnersAge, idleRunners)
+		longRunningIdleRunners := runnerUtil.OldIdleRunners(config.Config.Operator.MinIdleRunnersAge, idleRunners)
 
 		// calculate how many old runners need to be deleted to match the desired minIdleRunners
-		alignedRunners := poolUtil.AlignIdleRunners(int(pool.Spec.MinIdleRunners), longRunningIdleRunners)
+		alignedRunners := runnerUtil.AlignIdleRunners(int(pool.Spec.MinIdleRunners), longRunningIdleRunners)
 
 		// extract runners which are deletable
-		runners := poolUtil.DeletableRunners(ctx, alignedRunners)
+		runners := runnerUtil.DeletableRunners(ctx, alignedRunners)
 		for _, runner := range runners {
 			log.Info("Scaling pool", "pool", pool.Name)
 			event.Scaling(r.Recorder, pool, fmt.Sprintf("scale long running idle runners down to %d", pool.Spec.MinIdleRunners))
@@ -259,13 +261,13 @@ func (r *PoolReconciler) reconcileDelete(ctx context.Context, garmClient garmCli
 	}
 
 	// get all runners
-	runners, err := poolUtil.GetAllRunners(ctx, pool, instanceClient)
+	runners, err := runnerUtil.GetRunnersByPoolID(ctx, pool, instanceClient)
 	if err != nil {
 		return ctrl.Result{Requeue: true, RequeueAfter: 1 * time.Minute}, err
 	}
 
 	// get a list of all idle runners to trigger deletion
-	deletableRunners := poolUtil.DeletableRunners(ctx, runners)
+	deletableRunners := runnerUtil.DeletableRunners(ctx, runners)
 	if err != nil {
 		return ctrl.Result{Requeue: true, RequeueAfter: 1 * time.Minute}, err
 	}
@@ -355,7 +357,7 @@ func (r *PoolReconciler) comparePoolSpecs(ctx context.Context, pool *garmoperato
 
 	// as there are some "special" tags, which aren't set by the user and aren't part of the pool spec
 	// we need to "discover" them and add them to the pool spec before comparing
-	poolTags, err := poolUtil.CreateComparableRunnerTags(pool.Spec.Tags, pool.Spec.OSArch, pool.Spec.OSType)
+	poolTags, err := tags.CreateComparableRunnerTags(pool.Spec.Tags, pool.Spec.OSArch, pool.Spec.OSType)
 	if err != nil {
 		return false, []params.Instance{}, err
 	}
@@ -406,7 +408,7 @@ func (r *PoolReconciler) comparePoolSpecs(ctx context.Context, pool *garmoperato
 	}
 
 	// we are only interested in IdleRunners
-	idleInstances := poolUtil.IdleRunners(ctx, garmPool.Payload.Instances)
+	idleInstances := runnerUtil.IdleRunners(ctx, garmPool.Payload.Instances)
 
 	// empty instances for comparison
 	garmPool.Payload.Instances = nil

internal/controller/runner_controller.go

@@ -29,8 +29,8 @@ import (
 	garmClient "github.com/mercedes-benz/garm-operator/pkg/client"
 	"github.com/mercedes-benz/garm-operator/pkg/client/key"
 	"github.com/mercedes-benz/garm-operator/pkg/config"
-	"github.com/mercedes-benz/garm-operator/pkg/filter"
-	instancefilter "github.com/mercedes-benz/garm-operator/pkg/filter/instance"
+	"github.com/mercedes-benz/garm-operator/pkg/util/filter"
+	runnerUtil "github.com/mercedes-benz/garm-operator/pkg/util/runners"
 )
 
 // RunnerReconciler reconciles a Runner object
@@ -51,7 +51,7 @@ func (r *RunnerReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr
 
 func (r *RunnerReconciler) reconcile(ctx context.Context, req ctrl.Request, instanceClient garmClient.InstanceClient) (ctrl.Result, error) {
 	// try fetch runner instance in garm db with events coming from reconcile loop events of RunnerCR or from manually enqueued events of garm api.
-	garmRunner, err := r.getGarmRunnerInstance(instanceClient, req.Name)
+	garmRunner, err := r.getGarmRunnerInstanceByName(instanceClient, req.Name)
 	if err != nil {
 		return ctrl.Result{}, err
 	}
@@ -124,13 +124,13 @@ func (r *RunnerReconciler) reconcileDelete(ctx context.Context, runnerClient gar
 	return ctrl.Result{}, nil
 }
 
-func (r *RunnerReconciler) getGarmRunnerInstance(client garmClient.InstanceClient, name string) (*params.Instance, error) {
+func (r *RunnerReconciler) getGarmRunnerInstanceByName(client garmClient.InstanceClient, name string) (*params.Instance, error) {
 	allInstances, err := client.ListInstances(instances.NewListInstancesParams().WithDefaults())
 	if err != nil {
 		return nil, err
 	}
 
-	filteredInstances := filter.Match(allInstances.Payload, instancefilter.MatchesName(name))
+	filteredInstances := filter.Match(allInstances.Payload, runnerUtil.MatchesName(name))
 	if len(filteredInstances) == 0 {
 		return nil, nil
 	}
@@ -162,10 +162,10 @@ func (r *RunnerReconciler) updateRunnerStatus(ctx context.Context, runner *garmo
 	pools := &garmoperatorv1alpha1.PoolList{}
 	err := r.List(ctx, pools)
 	if err == nil {
-		pools.FilterByFields(garmoperatorv1alpha1.MatchesID(garmRunner.PoolID))
+		filteredPools := filter.Match(pools.Items, garmoperatorv1alpha1.MatchesID(garmRunner.PoolID))
 
-		if len(pools.Items) > 0 {
-			poolName = pools.Items[0].Name
+		if len(filteredPools) > 0 {
+			poolName = filteredPools[0].Name
 		}
 	}
 

pkg/filter/filter.go → pkg/util/filter/filter.go

@@ -5,20 +5,20 @@ package filter
 type Predicate[T any] func(T) bool
 
 func Match[T any](items []T, predicates ...Predicate[T]) []T {
-	var filteredPools []T
+	var resultList []T
 
-	for _, pool := range items {
+	for _, item := range items {
 		match := true
 		for _, predicate := range predicates {
-			if !predicate(pool) {
+			if !predicate(item) {
 				match = false
 				break
 			}
 		}
 		if match {
-			filteredPools = append(filteredPools, pool)
+			resultList = append(resultList, item)
 		}
 	}
 
-	return filteredPools
+	return resultList
 }