Skip to content

Commit

Permalink
gha: make MaxConcurrentReconciles for each reconciler configurable
Browse files Browse the repository at this point in the history
  • Loading branch information
tfujiwar committed Nov 19, 2024
1 parent 80d8483 commit 8e824ca
Show file tree
Hide file tree
Showing 13 changed files with 121 additions and 54 deletions.
1 change: 1 addition & 0 deletions .github/workflows/gha-validate-chart.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ on:
pull_request:
branches:
- master
- mercari-master
paths:
- 'charts/**'
- '.github/workflows/gha-validate-chart.yaml'
Expand Down
1 change: 1 addition & 0 deletions .github/workflows/go.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ on:
push:
branches:
- master
- mercari-master
paths:
- '.github/workflows/go.yaml'
- '**.go'
Expand Down
12 changes: 12 additions & 0 deletions charts/gha-runner-scale-set-controller/templates/deployment.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,18 @@ spec:
{{- range .Values.flags.excludeLabelPropagationPrefixes }}
- "--exclude-label-propagation-prefix={{ . }}"
{{- end }}
{{- with .Values.flags.maxConcurrentReconcilesForAutoscalingRunnerSet }}
- "--max-concurrent-reconciles-for-autoscaling-runner-set={{ . }}"
{{- end }}
{{- with .Values.flags.maxConcurrentReconcilesForEphemeralRunnerSet }}
- "--max-concurrent-reconciles-for-ephemeral-runner-set={{ . }}"
{{- end }}
{{- with .Values.flags.maxConcurrentReconcilesForEphemeralRunner }}
- "--max-concurrent-reconciles-for-ephemeral-runner={{ . }}"
{{- end }}
{{- with .Values.flags.maxConcurrentReconcilesForAutoscalingListener }}
- "--max-concurrent-reconciles-for-autoscaling-listener={{ . }}"
{{- end }}
command:
- "/manager"
{{- with .Values.metrics }}
Expand Down
6 changes: 6 additions & 0 deletions charts/gha-runner-scale-set-controller/values.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -130,3 +130,9 @@ flags:
## Labels that match prefix specified in the list are excluded from propagation.
# excludeLabelPropagationPrefixes:
# - "argocd.argoproj.io/instance"

## Defines the maximum number of concurrent reconciles for each reconciler.
# maxConcurrentReconcilesForAutoscalingRunnerSet: 1
# maxConcurrentReconcilesForEphemeralRunnerSet: 1
# maxConcurrentReconcilesForEphemeralRunner: 1
# maxConcurrentReconcilesForAutoscalingListener: 1
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import (
"k8s.io/apimachinery/pkg/types"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/controller"
"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
"sigs.k8s.io/controller-runtime/pkg/handler"
"sigs.k8s.io/controller-runtime/pkg/predicate"
Expand Down Expand Up @@ -55,6 +56,8 @@ type AutoscalingListenerReconciler struct {
ListenerMetricsAddr string
ListenerMetricsEndpoint string

MaxConcurrentReconciles int

ResourceBuilder
}

Expand Down Expand Up @@ -730,6 +733,7 @@ func (r *AutoscalingListenerReconciler) SetupWithManager(mgr ctrl.Manager) error
Watches(&rbacv1.Role{}, handler.EnqueueRequestsFromMapFunc(labelBasedWatchFunc)).
Watches(&rbacv1.RoleBinding{}, handler.EnqueueRequestsFromMapFunc(labelBasedWatchFunc)).
WithEventFilter(predicate.ResourceVersionChangedPredicate{}).
WithOptions(controller.Options{MaxConcurrentReconciles: r.MaxConcurrentReconciles}).
Complete(r)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,9 +44,10 @@ var _ = Describe("Test AutoScalingListener controller", func() {
configSecret = createDefaultSecret(GinkgoT(), k8sClient, autoscalingNS.Name)

controller := &AutoscalingListenerReconciler{
Client: mgr.GetClient(),
Scheme: mgr.GetScheme(),
Log: logf.Log,
Client: mgr.GetClient(),
Scheme: mgr.GetScheme(),
Log: logf.Log,
MaxConcurrentReconciles: 1,
}
err := controller.SetupWithManager(mgr)
Expect(err).NotTo(HaveOccurred(), "failed to setup controller")
Expand Down Expand Up @@ -508,9 +509,10 @@ var _ = Describe("Test AutoScalingListener customization", func() {
configSecret = createDefaultSecret(GinkgoT(), k8sClient, autoscalingNS.Name)

controller := &AutoscalingListenerReconciler{
Client: mgr.GetClient(),
Scheme: mgr.GetScheme(),
Log: logf.Log,
Client: mgr.GetClient(),
Scheme: mgr.GetScheme(),
Log: logf.Log,
MaxConcurrentReconciles: 1,
}
err := controller.SetupWithManager(mgr)
Expect(err).NotTo(HaveOccurred(), "failed to setup controller")
Expand Down Expand Up @@ -782,9 +784,10 @@ var _ = Describe("Test AutoScalingListener controller with proxy", func() {
configSecret = createDefaultSecret(GinkgoT(), k8sClient, autoscalingNS.Name)

controller := &AutoscalingListenerReconciler{
Client: mgr.GetClient(),
Scheme: mgr.GetScheme(),
Log: logf.Log,
Client: mgr.GetClient(),
Scheme: mgr.GetScheme(),
Log: logf.Log,
MaxConcurrentReconciles: 1,
}
err := controller.SetupWithManager(mgr)
Expect(err).NotTo(HaveOccurred(), "failed to setup controller")
Expand Down Expand Up @@ -978,9 +981,10 @@ var _ = Describe("Test AutoScalingListener controller with template modification
configSecret = createDefaultSecret(GinkgoT(), k8sClient, autoscalingNS.Name)

controller := &AutoscalingListenerReconciler{
Client: mgr.GetClient(),
Scheme: mgr.GetScheme(),
Log: logf.Log,
Client: mgr.GetClient(),
Scheme: mgr.GetScheme(),
Log: logf.Log,
MaxConcurrentReconciles: 1,
}
err := controller.SetupWithManager(mgr)
Expect(err).NotTo(HaveOccurred(), "failed to setup controller")
Expand Down Expand Up @@ -1094,9 +1098,10 @@ var _ = Describe("Test GitHub Server TLS configuration", func() {
Expect(err).NotTo(HaveOccurred(), "failed to create configmap with root CAs")

controller := &AutoscalingListenerReconciler{
Client: mgr.GetClient(),
Scheme: mgr.GetScheme(),
Log: logf.Log,
Client: mgr.GetClient(),
Scheme: mgr.GetScheme(),
Log: logf.Log,
MaxConcurrentReconciles: 1,
}
err = controller.SetupWithManager(mgr)
Expect(err).NotTo(HaveOccurred(), "failed to setup controller")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ import (
"k8s.io/apimachinery/pkg/types"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/controller"
"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
"sigs.k8s.io/controller-runtime/pkg/handler"
"sigs.k8s.io/controller-runtime/pkg/predicate"
Expand Down Expand Up @@ -79,6 +80,7 @@ type AutoscalingRunnerSetReconciler struct {
DefaultRunnerScaleSetListenerImagePullSecrets []string
UpdateStrategy UpdateStrategy
ActionsClient actions.MultiClient
MaxConcurrentReconciles int
ResourceBuilder
}

Expand Down Expand Up @@ -763,6 +765,7 @@ func (r *AutoscalingRunnerSetReconciler) SetupWithManager(mgr ctrl.Manager) erro
},
)).
WithEventFilter(predicate.ResourceVersionChangedPredicate{}).
WithOptions(controller.Options{MaxConcurrentReconciles: r.MaxConcurrentReconciles}).
Complete(r)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ var _ = Describe("Test AutoScalingRunnerSet controller", Ordered, func() {
ControllerNamespace: autoscalingNS.Name,
DefaultRunnerScaleSetListenerImage: "ghcr.io/actions/arc",
ActionsClient: fake.NewMultiClient(),
MaxConcurrentReconciles: 1,
}
err := controller.SetupWithManager(mgr)
Expect(err).NotTo(HaveOccurred(), "failed to setup controller")
Expand Down Expand Up @@ -704,6 +705,7 @@ var _ = Describe("Test AutoScalingController updates", Ordered, func() {
nil,
),
),
MaxConcurrentReconciles: 1,
}
err := controller.SetupWithManager(mgr)
Expect(err).NotTo(HaveOccurred(), "failed to setup controller")
Expand Down Expand Up @@ -819,6 +821,7 @@ var _ = Describe("Test AutoscalingController creation failures", Ordered, func()
ControllerNamespace: autoscalingNS.Name,
DefaultRunnerScaleSetListenerImage: "ghcr.io/actions/arc",
ActionsClient: fake.NewMultiClient(),
MaxConcurrentReconciles: 1,
}
err := controller.SetupWithManager(mgr)
Expect(err).NotTo(HaveOccurred(), "failed to setup controller")
Expand Down Expand Up @@ -945,6 +948,7 @@ var _ = Describe("Test client optional configuration", Ordered, func() {
ControllerNamespace: autoscalingNS.Name,
DefaultRunnerScaleSetListenerImage: "ghcr.io/actions/arc",
ActionsClient: actions.NewMultiClient(logr.Discard()),
MaxConcurrentReconciles: 1,
}

err := controller.SetupWithManager(mgr)
Expand Down Expand Up @@ -1128,6 +1132,7 @@ var _ = Describe("Test client optional configuration", Ordered, func() {
ControllerNamespace: autoscalingNS.Name,
DefaultRunnerScaleSetListenerImage: "ghcr.io/actions/arc",
ActionsClient: fake.NewMultiClient(),
MaxConcurrentReconciles: 1,
}
err = controller.SetupWithManager(mgr)
Expect(err).NotTo(HaveOccurred(), "failed to setup controller")
Expand Down Expand Up @@ -1362,6 +1367,7 @@ var _ = Describe("Test external permissions cleanup", Ordered, func() {
ControllerNamespace: autoscalingNS.Name,
DefaultRunnerScaleSetListenerImage: "ghcr.io/actions/arc",
ActionsClient: fake.NewMultiClient(),
MaxConcurrentReconciles: 1,
}
err := controller.SetupWithManager(mgr)
Expect(err).NotTo(HaveOccurred(), "failed to setup controller")
Expand Down Expand Up @@ -1520,6 +1526,7 @@ var _ = Describe("Test external permissions cleanup", Ordered, func() {
ControllerNamespace: autoscalingNS.Name,
DefaultRunnerScaleSetListenerImage: "ghcr.io/actions/arc",
ActionsClient: fake.NewMultiClient(),
MaxConcurrentReconciles: 1,
}
err := controller.SetupWithManager(mgr)
Expect(err).NotTo(HaveOccurred(), "failed to setup controller")
Expand Down Expand Up @@ -1728,6 +1735,7 @@ var _ = Describe("Test resource version and build version mismatch", func() {
ControllerNamespace: autoscalingNS.Name,
DefaultRunnerScaleSetListenerImage: "ghcr.io/actions/arc",
ActionsClient: fake.NewMultiClient(),
MaxConcurrentReconciles: 1,
}
err := controller.SetupWithManager(mgr)
Expect(err).NotTo(HaveOccurred(), "failed to setup controller")
Expand Down
9 changes: 6 additions & 3 deletions controllers/actions.github.com/ephemeralrunner_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import (
"k8s.io/apimachinery/pkg/types"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/controller"
"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
"sigs.k8s.io/controller-runtime/pkg/predicate"
)
Expand All @@ -49,9 +50,10 @@ const (
// EphemeralRunnerReconciler reconciles a EphemeralRunner object
type EphemeralRunnerReconciler struct {
client.Client
Log logr.Logger
Scheme *runtime.Scheme
ActionsClient actions.MultiClient
Log logr.Logger
Scheme *runtime.Scheme
ActionsClient actions.MultiClient
MaxConcurrentReconciles int
ResourceBuilder
}

Expand Down Expand Up @@ -828,6 +830,7 @@ func (r *EphemeralRunnerReconciler) SetupWithManager(mgr ctrl.Manager) error {
For(&v1alpha1.EphemeralRunner{}).
Owns(&corev1.Pod{}).
WithEventFilter(predicate.ResourceVersionChangedPredicate{}).
WithOptions(controller.Options{MaxConcurrentReconciles: r.MaxConcurrentReconciles}).
Complete(r)
}

Expand Down
28 changes: 16 additions & 12 deletions controllers/actions.github.com/ephemeralrunner_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -101,10 +101,11 @@ var _ = Describe("EphemeralRunner", func() {
configSecret = createDefaultSecret(GinkgoT(), k8sClient, autoscalingNS.Name)

controller = &EphemeralRunnerReconciler{
Client: mgr.GetClient(),
Scheme: mgr.GetScheme(),
Log: logf.Log,
ActionsClient: fake.NewMultiClient(),
Client: mgr.GetClient(),
Scheme: mgr.GetScheme(),
Log: logf.Log,
ActionsClient: fake.NewMultiClient(),
MaxConcurrentReconciles: 1,
}

err := controller.SetupWithManager(mgr)
Expand Down Expand Up @@ -681,6 +682,7 @@ var _ = Describe("EphemeralRunner", func() {
nil,
),
),
MaxConcurrentReconciles: 1,
}
err := controller.SetupWithManager(mgr)
Expect(err).To(BeNil(), "failed to setup controller")
Expand Down Expand Up @@ -737,10 +739,11 @@ var _ = Describe("EphemeralRunner", func() {
configSecret = createDefaultSecret(GinkgoT(), k8sClient, autoScalingNS.Name)

controller = &EphemeralRunnerReconciler{
Client: mgr.GetClient(),
Scheme: mgr.GetScheme(),
Log: logf.Log,
ActionsClient: fake.NewMultiClient(),
Client: mgr.GetClient(),
Scheme: mgr.GetScheme(),
Log: logf.Log,
ActionsClient: fake.NewMultiClient(),
MaxConcurrentReconciles: 1,
}
err := controller.SetupWithManager(mgr)
Expect(err).To(BeNil(), "failed to setup controller")
Expand Down Expand Up @@ -901,10 +904,11 @@ var _ = Describe("EphemeralRunner", func() {
Expect(err).NotTo(HaveOccurred(), "failed to create configmap with root CAs")

controller = &EphemeralRunnerReconciler{
Client: mgr.GetClient(),
Scheme: mgr.GetScheme(),
Log: logf.Log,
ActionsClient: fake.NewMultiClient(),
Client: mgr.GetClient(),
Scheme: mgr.GetScheme(),
Log: logf.Log,
ActionsClient: fake.NewMultiClient(),
MaxConcurrentReconciles: 1,
}

err = controller.SetupWithManager(mgr)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ import (
"k8s.io/apimachinery/pkg/types"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/controller"
"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
"sigs.k8s.io/controller-runtime/pkg/predicate"
)
Expand All @@ -53,6 +54,8 @@ type EphemeralRunnerSetReconciler struct {

PublishMetrics bool

MaxConcurrentReconciles int

ResourceBuilder
}

Expand Down Expand Up @@ -575,6 +578,7 @@ func (r *EphemeralRunnerSetReconciler) SetupWithManager(mgr ctrl.Manager) error
For(&v1alpha1.EphemeralRunnerSet{}).
Owns(&v1alpha1.EphemeralRunner{}).
WithEventFilter(predicate.ResourceVersionChangedPredicate{}).
WithOptions(controller.Options{MaxConcurrentReconciles: r.MaxConcurrentReconciles}).
Complete(r)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,10 +48,11 @@ var _ = Describe("Test EphemeralRunnerSet controller", func() {
configSecret = createDefaultSecret(GinkgoT(), k8sClient, autoscalingNS.Name)

controller := &EphemeralRunnerSetReconciler{
Client: mgr.GetClient(),
Scheme: mgr.GetScheme(),
Log: logf.Log,
ActionsClient: fake.NewMultiClient(),
Client: mgr.GetClient(),
Scheme: mgr.GetScheme(),
Log: logf.Log,
ActionsClient: fake.NewMultiClient(),
MaxConcurrentReconciles: 1,
}
err := controller.SetupWithManager(mgr)
Expect(err).NotTo(HaveOccurred(), "failed to setup controller")
Expand Down Expand Up @@ -1098,10 +1099,11 @@ var _ = Describe("Test EphemeralRunnerSet controller with proxy settings", func(
configSecret = createDefaultSecret(GinkgoT(), k8sClient, autoscalingNS.Name)

controller := &EphemeralRunnerSetReconciler{
Client: mgr.GetClient(),
Scheme: mgr.GetScheme(),
Log: logf.Log,
ActionsClient: actions.NewMultiClient(logr.Discard()),
Client: mgr.GetClient(),
Scheme: mgr.GetScheme(),
Log: logf.Log,
ActionsClient: actions.NewMultiClient(logr.Discard()),
MaxConcurrentReconciles: 1,
}
err := controller.SetupWithManager(mgr)
Expect(err).NotTo(HaveOccurred(), "failed to setup controller")
Expand Down Expand Up @@ -1397,10 +1399,11 @@ var _ = Describe("Test EphemeralRunnerSet controller with custom root CA", func(
Expect(err).NotTo(HaveOccurred(), "failed to create configmap with root CAs")

controller := &EphemeralRunnerSetReconciler{
Client: mgr.GetClient(),
Scheme: mgr.GetScheme(),
Log: logf.Log,
ActionsClient: actions.NewMultiClient(logr.Discard()),
Client: mgr.GetClient(),
Scheme: mgr.GetScheme(),
Log: logf.Log,
ActionsClient: actions.NewMultiClient(logr.Discard()),
MaxConcurrentReconciles: 1,
}
err = controller.SetupWithManager(mgr)
Expect(err).NotTo(HaveOccurred(), "failed to setup controller")
Expand Down
Loading

0 comments on commit 8e824ca

Please sign in to comment.