Repository: shipwright-io/build
Pull request #623 (2021-02-26 10:44:16)

Allow to configure MaxConcurrentReconciles for our controllers
# Changes

With the Git validation being added to the build reconciliation, we are observing increases in the time it takes to reconcile a build. This time is critical at the startup of the controller when all builds are reconciled. New builds that are getting created will then not be reconciled within a reasonable time. Example:

With 1000 builds in the system that do not use Git validation, we see the controller reconciling all of them in a little more than a minute. With Git validation enabled, it takes more than six minutes.

In this PR, I am exposing the MaxConcurrentReconciles controller setting as environment variable for our four controllers. The default is still 1.

# Submitter Checklist

- [X] Includes tests if functionality changed/was added
- [X] Includes docs if changes are user-facing
- [X] [Set a kind label on this PR](https://prow.k8s.io/command-help#kind)  
- [X] Release notes block has been filled in, or marked NONE

# Release Notes

```release-note
Exposing MaxConcurrentReconciles configuration of controllers to allow increasing them from the default (1) to be able to handle a larger amount of objects in the system
```


openshift-ci-robot: [APPROVALNOTIFIER] This PR is **NOT APPROVED**

This pull-request has been approved by:
To complete the [pull request process](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process), please assign **saschaschwarze0** after the PR has been reviewed.
You can assign the PR to them by writing `/assign @saschaschwarze0` in a comment when ready.

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=shipwright-io%2Fbuild).

<details open>
Needs approval from an approver in each of these files:

- **[OWNERS](https://github.com/shipwright-io/build/blob/master/OWNERS)**

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":["saschaschwarze0"]} -->
SaschaSchwarze0: Hm, the sanity check failed in files I did not change. Correcting this in a separate commit.
openshift-ci-robot: @SaschaSchwarze0: PR needs rebase.

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.
</details>
zhangtbj: Hi Sascha,

Please fix the code conflict.
gabemontero: /approve

@zhangtbj feel free to put a slash lgtm in this PR if you happy with @SaschaSchwarze0 's responses to your comments and we can merge this one

fwiw LGTM
openshift-ci-robot: [APPROVALNOTIFIER] This PR is **APPROVED**

This pull-request has been approved by: *<a href="https://github.com/shipwright-io/build/pull/623#issuecomment-788097544" title="Approved">gabemontero</a>*

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=shipwright-io%2Fbuild).

The pull request process is described [here](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process)

<details >
Needs approval from an approver in each of these files:

- ~~[OWNERS](https://github.com/shipwright-io/build/blob/master/OWNERS)~~ [gabemontero]

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":[]} -->

Patch:
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
index 5af70eab..22b381ec 100644
--- a/.github/workflows/ci.yml
+++ b/.github/workflows/ci.yml
@@ -125,12 +125,12 @@ jobs:
           run: |
             echo "# Pods:"
             kubectl -n shipwright-build get pod
-            PODS=$(kubectl -n shipwright-build get pod -o json)
+            PODS=$(kubectl -n build-operator get pod -o json)
             POD_NAME=$(echo "${PODS}" | jq -r '.items[] | select(.metadata.name | startswith("shipwright-build-controller-")) | .metadata.name')
             RESTART_COUNT=$(echo "${PODS}" | jq -r ".items[] | select(.metadata.name == \"${POD_NAME}\") | .status.containerStatuses[0].restartCount")
             if [ "${RESTART_COUNT}" != "0" ]; then
               echo "# Previous logs:"
-              kubectl -n shipwright-build logs "${POD_NAME}" --previous || true
+              kubectl -n build-operator logs "${POD_NAME}" --previous || true
             fi
             echo "# Logs:"
             kubectl -n shipwright-build logs "${POD_NAME}"
diff --git a/OWNERS b/OWNERS
index d554897b..f5c3ebca 100644
--- a/OWNERS
+++ b/OWNERS
@@ -8,7 +8,6 @@ approvers:
 - adambkaplan
 - gabemontero
 - SaschaSchwarze0
-- HeavyWombat
 
 reviewers:
 - sbose78
@@ -25,5 +24,3 @@ reviewers:
 - adambkaplan
 - gabemontero
 - HeavyWombat
-- ImJasonH
-- mattcui
diff --git a/docs/configuration.md b/docs/configuration.md
index 970ef557..d64d3d71 100644
--- a/docs/configuration.md
+++ b/docs/configuration.md
@@ -18,3 +18,7 @@ The following environment variables are available:
 | `BUILD_CONTROLLER_LEASE_DURATION` |  Override the `LeaseDuration`, which is the duration that non-leader candidates will wait to force acquire leadership. |
 | `BUILD_CONTROLLER_RENEW_DEADLINE` |  Override the `RenewDeadline`, which is the duration that the acting master will retry refreshing leadership before giving up. |
 | `BUILD_CONTROLLER_RETRY_PERIOD` |  Override the `RetryPeriod`, which is the duration the LeaderElector clients should wait between tries of actions. |
+| `BUILD_MAX_CONCURRENT_RECONCILES` | The number of concurrent reconciles by the build controller. A value of 0 or lower will use the default from the [controller-runtime controller Options](https://pkg.go.dev/sigs.k8s.io/controller-runtime/pkg/controller#Options). Default is 0. |
+| `BUILDRUN_MAX_CONCURRENT_RECONCILES` | The number of concurrent reconciles by the buildrun controller. A value of 0 or lower will use the default from the [controller-runtime controller Options](https://pkg.go.dev/sigs.k8s.io/controller-runtime/pkg/controller#Options). Default is 0. |
+| `BUILDSTRATEGY_MAX_CONCURRENT_RECONCILES` | The number of concurrent reconciles by the buildstrategy controller. A value of 0 or lower will use the default from the [controller-runtime controller Options](https://pkg.go.dev/sigs.k8s.io/controller-runtime/pkg/controller#Options). Default is 0. |
+| `CLUSTERBUILDSTRATEGY_MAX_CONCURRENT_RECONCILES` | The number of concurrent reconciles by the clusterbuildstrategy controller. A value of 0 or lower will use the default from the [controller-runtime controller Options](https://pkg.go.dev/sigs.k8s.io/controller-runtime/pkg/controller#Options). Default is 0. |
diff --git a/pkg/config/config.go b/pkg/config/config.go
index b49cd135..98e30b3b 100644
--- a/pkg/config/config.go
+++ b/pkg/config/config.go
@@ -36,6 +36,12 @@ const (
 	leaseDurationEnvVar = "BUILD_CONTROLLER_LEASE_DURATION"
 	renewDeadlineEnvVar = "BUILD_CONTROLLER_RENEW_DEADLINE"
 	retryPeriodEnvVar   = "BUILD_CONTROLLER_RETRY_PERIOD"
+
+	// environment variable for the controllers
+	controllerBuildMaxConcurrentReconciles                = "BUILD_MAX_CONCURRENT_RECONCILES"
+	controllerBuildRunMaxConcurrentReconciles             = "BUILDRUN_MAX_CONCURRENT_RECONCILES"
+	controllerBuildStrategyMaxConcurrentReconciles        = "BUILDSTRATEGY_MAX_CONCURRENT_RECONCILES"
+	controllerClusterBuildStrategyMaxConcurrentReconciles = "CLUSTERBUILDSTRATEGY_MAX_CONCURRENT_RECONCILES"
 )
 
 var (
@@ -52,6 +58,7 @@ type Config struct {
 	KanikoContainerImage string
 	Prometheus           PrometheusConfig
 	ManagerOptions       ManagerOptions
+	Controllers          Controllers
 }
 
 // PrometheusConfig contains the specific configuration for the
@@ -70,6 +77,19 @@ type ManagerOptions struct {
 	RetryPeriod             *time.Duration
 }
 
+// Controllers contains the options for the different controllers
+type Controllers struct {
+	Build                ControllerOptions
+	BuildRun             ControllerOptions
+	BuildStrategy        ControllerOptions
+	ClusterBuildStrategy ControllerOptions
+}
+
+// ControllerOptions contains configurable options for a controller
+type ControllerOptions struct {
+	MaxConcurrentReconciles int
+}
+
 // NewDefaultConfig returns a new Config, with context timeout and default Kaniko image.
 func NewDefaultConfig() *Config {
 	return &Config{
@@ -83,6 +103,20 @@ func NewDefaultConfig() *Config {
 		ManagerOptions: ManagerOptions{
 			LeaderElectionNamespace: leaderElectionNamespaceDefault,
 		},
+		Controllers: Controllers{
+			Build: ControllerOptions{
+				MaxConcurrentReconciles: 0,
+			},
+			BuildRun: ControllerOptions{
+				MaxConcurrentReconciles: 0,
+			},
+			BuildStrategy: ControllerOptions{
+				MaxConcurrentReconciles: 0,
+			},
+			ClusterBuildStrategy: ControllerOptions{
+				MaxConcurrentReconciles: 0,
+			},
+		},
 	}
 }
 
@@ -130,6 +164,19 @@ func (c *Config) SetConfigFromEnv() error {
 		return err
 	}
 
+	if err := updateIntOption(&c.Controllers.Build.MaxConcurrentReconciles, controllerBuildMaxConcurrentReconciles); err != nil {
+		return err
+	}
+	if err := updateIntOption(&c.Controllers.BuildRun.MaxConcurrentReconciles, controllerBuildRunMaxConcurrentReconciles); err != nil {
+		return err
+	}
+	if err := updateIntOption(&c.Controllers.BuildStrategy.MaxConcurrentReconciles, controllerBuildStrategyMaxConcurrentReconciles); err != nil {
+		return err
+	}
+	if err := updateIntOption(&c.Controllers.ClusterBuildStrategy.MaxConcurrentReconciles, controllerClusterBuildStrategyMaxConcurrentReconciles); err != nil {
+		return err
+	}
+
 	return nil
 }
 
@@ -172,3 +219,15 @@ func updateBuildOperatorDurationOption(d **time.Duration, envVarName string) err
 
 	return nil
 }
+
+func updateIntOption(i *int, envVarName string) error {
+	if value := os.Getenv(envVarName); value != "" {
+		intValue, err := strconv.ParseInt(value, 10, 0)
+		if err != nil {
+			return err
+		}
+		*i = int(intValue)
+	}
+
+	return nil
+}
diff --git a/pkg/reconciler/build/controller.go b/pkg/reconciler/build/controller.go
index 1acfce0d..671f8d5e 100644
--- a/pkg/reconciler/build/controller.go
+++ b/pkg/reconciler/build/controller.go
@@ -38,13 +38,21 @@ type setOwnerReferenceFunc func(owner, object metav1.Object, scheme *runtime.Sch
 // and Start it when the Manager is Started.
 func Add(ctx context.Context, c *config.Config, mgr manager.Manager) error {
 	ctx = ctxlog.NewContext(ctx, "build-controller")
-	return add(ctx, mgr, NewReconciler(ctx, c, mgr, controllerutil.SetControllerReference))
+	return add(ctx, mgr, NewReconciler(ctx, c, mgr, controllerutil.SetControllerReference), c.Controllers.Build.MaxConcurrentReconciles)
 }
 
 // add adds a new Controller to mgr with r as the reconcile.Reconciler
-func add(ctx context.Context, mgr manager.Manager, r reconcile.Reconciler) error {
+func add(ctx context.Context, mgr manager.Manager, r reconcile.Reconciler, maxConcurrentReconciles int) error {
+	// Create the controller options
+	options := controller.Options{
+		Reconciler: r,
+	}
+	if maxConcurrentReconciles > 0 {
+		options.MaxConcurrentReconciles = maxConcurrentReconciles
+	}
+
 	// Create a new controller
-	c, err := controller.New("build-controller", mgr, controller.Options{Reconciler: r})
+	c, err := controller.New("build-controller", mgr, options)
 	if err != nil {
 		return err
 	}
@@ -80,8 +88,7 @@ func add(ctx context.Context, mgr manager.Manager, r reconcile.Reconciler) error
 	}
 
 	// Watch for changes to primary resource Build
-	err = c.Watch(&source.Kind{Type: &build.Build{}}, &handler.EnqueueRequestForObject{}, pred)
-	if err != nil {
+	if err = c.Watch(&source.Kind{Type: &build.Build{}}, &handler.EnqueueRequestForObject{}, pred); err != nil {
 		return err
 	}
 
diff --git a/pkg/reconciler/buildrun/controller.go b/pkg/reconciler/buildrun/controller.go
index e4aec599..de3b350e 100644
--- a/pkg/reconciler/buildrun/controller.go
+++ b/pkg/reconciler/buildrun/controller.go
@@ -33,13 +33,21 @@ type setOwnerReferenceFunc func(owner, object metav1.Object, scheme *runtime.Sch
 // and Start it when the Manager is Started.
 func Add(ctx context.Context, c *config.Config, mgr manager.Manager) error {
 	ctx = ctxlog.NewContext(ctx, "buildrun-controller")
-	return add(ctx, mgr, NewReconciler(ctx, c, mgr, controllerutil.SetControllerReference))
+	return add(ctx, mgr, NewReconciler(ctx, c, mgr, controllerutil.SetControllerReference), c.Controllers.BuildRun.MaxConcurrentReconciles)
 }
 
 // add adds a new Controller to mgr with r as the reconcile.Reconciler
-func add(ctx context.Context, mgr manager.Manager, r reconcile.Reconciler) error {
+func add(ctx context.Context, mgr manager.Manager, r reconcile.Reconciler, maxConcurrentReconciles int) error {
+	// Create the controller options
+	options := controller.Options{
+		Reconciler: r,
+	}
+	if maxConcurrentReconciles > 0 {
+		options.MaxConcurrentReconciles = maxConcurrentReconciles
+	}
+
 	// Create a new controller
-	c, err := controller.New("buildrun-controller", mgr, controller.Options{Reconciler: r})
+	c, err := controller.New("buildrun-controller", mgr, options)
 	if err != nil {
 		return err
 	}
@@ -98,8 +106,7 @@ func add(ctx context.Context, mgr manager.Manager, r reconcile.Reconciler) error
 	}
 
 	// Watch for changes to primary resource BuildRun
-	err = c.Watch(&source.Kind{Type: &buildv1alpha1.BuildRun{}}, &handler.EnqueueRequestForObject{}, predBuildRun)
-	if err != nil {
+	if err = c.Watch(&source.Kind{Type: &buildv1alpha1.BuildRun{}}, &handler.EnqueueRequestForObject{}, predBuildRun); err != nil {
 		return err
 	}
 
diff --git a/pkg/reconciler/buildstrategy/controller.go b/pkg/reconciler/buildstrategy/controller.go
index 95ff5521..61394e49 100644
--- a/pkg/reconciler/buildstrategy/controller.go
+++ b/pkg/reconciler/buildstrategy/controller.go
@@ -22,22 +22,25 @@ import (
 // and Start it when the Manager is Started.
 func Add(ctx context.Context, c *config.Config, mgr manager.Manager) error {
 	ctx = ctxlog.NewContext(ctx, "buildstrategy-controller")
-	return add(ctx, mgr, NewReconciler(ctx, c, mgr))
+	return add(ctx, mgr, NewReconciler(ctx, c, mgr), c.Controllers.BuildStrategy.MaxConcurrentReconciles)
 }
 
 // add adds a new Controller to mgr with r as the reconcile.Reconciler
-func add(ctx context.Context, mgr manager.Manager, r reconcile.Reconciler) error {
-	// Create a new controller
-	c, err := controller.New("buildstrategy-controller", mgr, controller.Options{Reconciler: r})
-	if err != nil {
-		return err
+func add(ctx context.Context, mgr manager.Manager, r reconcile.Reconciler, maxConcurrentReconciles int) error {
+	// Create the controller options
+	options := controller.Options{
+		Reconciler: r,
+	}
+	if maxConcurrentReconciles > 0 {
+		options.MaxConcurrentReconciles = maxConcurrentReconciles
 	}
 
-	// Watch for changes to primary resource BuildStrategy
-	err = c.Watch(&source.Kind{Type: &buildv1alpha1.BuildStrategy{}}, &handler.EnqueueRequestForObject{})
+	// Create a new controller
+	c, err := controller.New("buildstrategy-controller", mgr, options)
 	if err != nil {
 		return err
 	}
 
-	return nil
+	// Watch for changes to primary resource BuildStrategy
+	return c.Watch(&source.Kind{Type: &buildv1alpha1.BuildStrategy{}}, &handler.EnqueueRequestForObject{})
 }
diff --git a/pkg/reconciler/clusterbuildstrategy/controller.go b/pkg/reconciler/clusterbuildstrategy/controller.go
index 363b9cf0..7508dfe8 100644
--- a/pkg/reconciler/clusterbuildstrategy/controller.go
+++ b/pkg/reconciler/clusterbuildstrategy/controller.go
@@ -22,22 +22,25 @@ import (
 // and Start it when the Manager is Started.
 func Add(ctx context.Context, c *config.Config, mgr manager.Manager) error {
 	ctx = ctxlog.NewContext(ctx, "clusterbuildstrategy-controller")
-	return add(ctx, mgr, NewReconciler(ctx, c, mgr))
+	return add(ctx, mgr, NewReconciler(ctx, c, mgr), c.Controllers.ClusterBuildStrategy.MaxConcurrentReconciles)
 }
 
 // add adds a new Controller to mgr with r as the reconcile.Reconciler
-func add(ctx context.Context, mgr manager.Manager, r reconcile.Reconciler) error {
-	// Create a new controller
-	c, err := controller.New("clusterbuildstrategy-controller", mgr, controller.Options{Reconciler: r})
-	if err != nil {
-		return err
+func add(ctx context.Context, mgr manager.Manager, r reconcile.Reconciler, maxConcurrentReconciles int) error {
+	// Create the controller options
+	options := controller.Options{
+		Reconciler: r,
+	}
+	if maxConcurrentReconciles > 0 {
+		options.MaxConcurrentReconciles = maxConcurrentReconciles
 	}
 
-	// Watch for changes to primary resource ClusterBuildStrategy
-	err = c.Watch(&source.Kind{Type: &buildv1alpha1.ClusterBuildStrategy{}}, &handler.EnqueueRequestForObject{})
+	// Create a new controller
+	c, err := controller.New("clusterbuildstrategy-controller", mgr, options)
 	if err != nil {
 		return err
 	}
 
-	return nil
+	// Watch for changes to primary resource ClusterBuildStrategy
+	return c.Watch(&source.Kind{Type: &buildv1alpha1.ClusterBuildStrategy{}}, &handler.EnqueueRequestForObject{})
 }
diff --git a/samples/build/build_ko_cr.yaml b/samples/build/build_ko_cr.yaml
index 63e55283..f2d00bac 100644
--- a/samples/build/build_ko_cr.yaml
+++ b/samples/build/build_ko_cr.yaml
@@ -14,4 +14,4 @@ spec:
     name: ko
     kind: ClusterBuildStrategy
   output:
-    image: image-registry.openshift-image-registry.svc:5000/build-examples/shipwright-build
+    image: image-registry.openshift-image-registry.svc:5000/build-examples/build-operator


Test patch:
diff --git a/pkg/config/config_test.go b/pkg/config/config_test.go
index 666e6ac2..e0fe9b4a 100644
--- a/pkg/config/config_test.go
+++ b/pkg/config/config_test.go
@@ -76,6 +76,22 @@ var _ = Describe("Config", func() {
 				Expect(*config.ManagerOptions.RetryPeriod).To(Equal(time.Duration(10 * time.Second)))
 			})
 		})
+
+		It("should allow for an override of concurrent reconciles of the controllers", func() {
+			var overrides = map[string]string{
+				"BUILD_MAX_CONCURRENT_RECONCILES":                "2",
+				"BUILDRUN_MAX_CONCURRENT_RECONCILES":             "3",
+				"BUILDSTRATEGY_MAX_CONCURRENT_RECONCILES":        "4",
+				"CLUSTERBUILDSTRATEGY_MAX_CONCURRENT_RECONCILES": "5",
+			}
+
+			configWithEnvVariableOverrides(overrides, func(config *Config) {
+				Expect(config.Controllers.Build.MaxConcurrentReconciles).To(Equal(2))
+				Expect(config.Controllers.BuildRun.MaxConcurrentReconciles).To(Equal(3))
+				Expect(config.Controllers.BuildStrategy.MaxConcurrentReconciles).To(Equal(4))
+				Expect(config.Controllers.ClusterBuildStrategy.MaxConcurrentReconciles).To(Equal(5))
+			})
+		})
 	})
 })