From fa7a4f584ef8e3c7ea9f134f6ba4b1c3ec32d03e Mon Sep 17 00:00:00 2001 From: Nikola Jokic Date: Fri, 17 May 2024 14:42:46 +0200 Subject: [PATCH] Extract single place to set up indexers (#3454) --- .golangci.yaml | 4 +- Makefile | 2 +- .../autoscalinglistener_controller.go | 24 ------- .../autoscalingrunnerset_controller.go | 21 ------ .../ephemeralrunner_controller.go | 1 - .../ephemeralrunnerset_controller.go | 22 ------ .../actions.github.com/helpers_test.go | 3 + controllers/actions.github.com/indexer.go | 71 +++++++++++++++++++ main.go | 4 ++ 9 files changed, 82 insertions(+), 70 deletions(-) create mode 100644 controllers/actions.github.com/indexer.go diff --git a/.golangci.yaml b/.golangci.yaml index 35520223..eca46937 100644 --- a/.golangci.yaml +++ b/.golangci.yaml @@ -1,7 +1,9 @@ run: timeout: 3m output: - format: github-actions + formats: + - format: github-actions + path: stdout linters-settings: errcheck: exclude-functions: diff --git a/Makefile b/Makefile index 684f13e7..5f1302af 100644 --- a/Makefile +++ b/Makefile @@ -68,7 +68,7 @@ endif all: manager lint: - docker run --rm -v $(PWD):/app -w /app golangci/golangci-lint:v1.55.2 golangci-lint run + docker run --rm -v $(PWD):/app -w /app golangci/golangci-lint:v1.57.2 golangci-lint run GO_TEST_ARGS ?= -short diff --git a/controllers/actions.github.com/autoscalinglistener_controller.go b/controllers/actions.github.com/autoscalinglistener_controller.go index 50803c91..f35c85e9 100644 --- a/controllers/actions.github.com/autoscalinglistener_controller.go +++ b/controllers/actions.github.com/autoscalinglistener_controller.go @@ -690,30 +690,6 @@ func (r *AutoscalingListenerReconciler) publishRunningListener(autoscalingListen // SetupWithManager sets up the controller with the Manager. func (r *AutoscalingListenerReconciler) SetupWithManager(mgr ctrl.Manager) error { - groupVersionIndexer := func(rawObj client.Object) []string { - groupVersion := v1alpha1.GroupVersion.String() - owner := metav1.GetControllerOf(rawObj) - if owner == nil { - return nil - } - - // ...make sure it is owned by this controller - if owner.APIVersion != groupVersion || owner.Kind != "AutoscalingListener" { - return nil - } - - // ...and if so, return it - return []string{owner.Name} - } - - if err := mgr.GetFieldIndexer().IndexField(context.Background(), &corev1.Pod{}, resourceOwnerKey, groupVersionIndexer); err != nil { - return err - } - - if err := mgr.GetFieldIndexer().IndexField(context.Background(), &corev1.ServiceAccount{}, resourceOwnerKey, groupVersionIndexer); err != nil { - return err - } - labelBasedWatchFunc := func(_ context.Context, obj client.Object) []reconcile.Request { var requests []reconcile.Request labels := obj.GetLabels() diff --git a/controllers/actions.github.com/autoscalingrunnerset_controller.go b/controllers/actions.github.com/autoscalingrunnerset_controller.go index 2ed654e8..f87a11af 100644 --- a/controllers/actions.github.com/autoscalingrunnerset_controller.go +++ b/controllers/actions.github.com/autoscalingrunnerset_controller.go @@ -30,7 +30,6 @@ import ( corev1 "k8s.io/api/core/v1" rbacv1 "k8s.io/api/rbac/v1" kerrors "k8s.io/apimachinery/pkg/api/errors" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" ctrl "sigs.k8s.io/controller-runtime" @@ -759,26 +758,6 @@ func (r *AutoscalingRunnerSetReconciler) actionsClientOptionsFor(ctx context.Con // SetupWithManager sets up the controller with the Manager. func (r *AutoscalingRunnerSetReconciler) SetupWithManager(mgr ctrl.Manager) error { - groupVersionIndexer := func(rawObj client.Object) []string { - groupVersion := v1alpha1.GroupVersion.String() - owner := metav1.GetControllerOf(rawObj) - if owner == nil { - return nil - } - - // ...make sure it is owned by this controller - if owner.APIVersion != groupVersion || owner.Kind != "AutoscalingRunnerSet" { - return nil - } - - // ...and if so, return it - return []string{owner.Name} - } - - if err := mgr.GetFieldIndexer().IndexField(context.Background(), &v1alpha1.EphemeralRunnerSet{}, resourceOwnerKey, groupVersionIndexer); err != nil { - return err - } - return ctrl.NewControllerManagedBy(mgr). For(&v1alpha1.AutoscalingRunnerSet{}). Owns(&v1alpha1.EphemeralRunnerSet{}). diff --git a/controllers/actions.github.com/ephemeralrunner_controller.go b/controllers/actions.github.com/ephemeralrunner_controller.go index 8765c1a0..6da084b7 100644 --- a/controllers/actions.github.com/ephemeralrunner_controller.go +++ b/controllers/actions.github.com/ephemeralrunner_controller.go @@ -815,7 +815,6 @@ func (r *EphemeralRunnerReconciler) deleteRunnerFromService(ctx context.Context, // SetupWithManager sets up the controller with the Manager. func (r *EphemeralRunnerReconciler) SetupWithManager(mgr ctrl.Manager) error { - // TODO(nikola-jokic): Add indexing and filtering fields on corev1.Pod{} return ctrl.NewControllerManagedBy(mgr). For(&v1alpha1.EphemeralRunner{}). Owns(&corev1.Pod{}). diff --git a/controllers/actions.github.com/ephemeralrunnerset_controller.go b/controllers/actions.github.com/ephemeralrunnerset_controller.go index b462e177..4c3692b8 100644 --- a/controllers/actions.github.com/ephemeralrunnerset_controller.go +++ b/controllers/actions.github.com/ephemeralrunnerset_controller.go @@ -574,28 +574,6 @@ func (r *EphemeralRunnerSetReconciler) actionsClientOptionsFor(ctx context.Conte // SetupWithManager sets up the controller with the Manager. func (r *EphemeralRunnerSetReconciler) SetupWithManager(mgr ctrl.Manager) error { - // Index EphemeralRunner owned by EphemeralRunnerSet so we can perform faster look ups. - if err := mgr.GetFieldIndexer().IndexField(context.Background(), &v1alpha1.EphemeralRunner{}, resourceOwnerKey, func(rawObj client.Object) []string { - groupVersion := v1alpha1.GroupVersion.String() - - // grab the job object, extract the owner... - ephemeralRunner := rawObj.(*v1alpha1.EphemeralRunner) - owner := metav1.GetControllerOf(ephemeralRunner) - if owner == nil { - return nil - } - - // ...make sure it is owned by this controller - if owner.APIVersion != groupVersion || owner.Kind != "EphemeralRunnerSet" { - return nil - } - - // ...and if so, return it - return []string{owner.Name} - }); err != nil { - return err - } - return ctrl.NewControllerManagedBy(mgr). For(&v1alpha1.EphemeralRunnerSet{}). Owns(&v1alpha1.EphemeralRunner{}). diff --git a/controllers/actions.github.com/helpers_test.go b/controllers/actions.github.com/helpers_test.go index 99f5d411..5594280f 100644 --- a/controllers/actions.github.com/helpers_test.go +++ b/controllers/actions.github.com/helpers_test.go @@ -18,6 +18,9 @@ const defaultGitHubToken = "gh_token" func startManagers(t ginkgo.GinkgoTInterface, first manager.Manager, others ...manager.Manager) { for _, mgr := range append([]manager.Manager{first}, others...) { + if err := SetupIndexers(mgr); err != nil { + t.Fatalf("failed to setup indexers: %v", err) + } ctx, cancel := context.WithCancel(context.Background()) g, ctx := errgroup.WithContext(ctx) diff --git a/controllers/actions.github.com/indexer.go b/controllers/actions.github.com/indexer.go new file mode 100644 index 00000000..466c9f14 --- /dev/null +++ b/controllers/actions.github.com/indexer.go @@ -0,0 +1,71 @@ +package actionsgithubcom + +import ( + "context" + "slices" + + v1alpha1 "github.com/actions/actions-runner-controller/apis/actions.github.com/v1alpha1" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +func SetupIndexers(mgr ctrl.Manager) error { + if err := mgr.GetFieldIndexer().IndexField( + context.Background(), + &corev1.Pod{}, + resourceOwnerKey, + newGroupVersionOwnerKindIndexer("AutoscalingListener", "EphemeralRunner"), + ); err != nil { + return err + } + + if err := mgr.GetFieldIndexer().IndexField( + context.Background(), + &corev1.ServiceAccount{}, + resourceOwnerKey, + newGroupVersionOwnerKindIndexer("AutoscalingListener"), + ); err != nil { + return err + } + + if err := mgr.GetFieldIndexer().IndexField( + context.Background(), + &v1alpha1.EphemeralRunnerSet{}, + resourceOwnerKey, + newGroupVersionOwnerKindIndexer("AutoscalingRunnerSet"), + ); err != nil { + return err + } + + if err := mgr.GetFieldIndexer().IndexField( + context.Background(), + &v1alpha1.EphemeralRunner{}, + resourceOwnerKey, + newGroupVersionOwnerKindIndexer("EphemeralRunnerSet"), + ); err != nil { + return err + } + + return nil +} + +func newGroupVersionOwnerKindIndexer(ownerKind string, otherOwnerKinds ...string) client.IndexerFunc { + owners := append([]string{ownerKind}, otherOwnerKinds...) + return func(o client.Object) []string { + groupVersion := v1alpha1.GroupVersion.String() + owner := metav1.GetControllerOfNoCopy(o) + if owner == nil { + return nil + } + + // ...make sure it is owned by this controller + if owner.APIVersion != groupVersion || !slices.Contains(owners, owner.Kind) { + return nil + } + + // ...and if so, return it + return []string{owner.Name} + } +} diff --git a/main.go b/main.go index cfe14c6c..3392377c 100644 --- a/main.go +++ b/main.go @@ -239,6 +239,10 @@ func main() { } if autoScalingRunnerSetOnly { + if err := actionsgithubcom.SetupIndexers(mgr); err != nil { + log.Error(err, "unable to setup indexers") + os.Exit(1) + } managerImage := os.Getenv("CONTROLLER_MANAGER_CONTAINER_IMAGE") if managerImage == "" { log.Error(err, "unable to obtain listener image")