increase controller cuncurrency and cpu requests

This commit ups the cpu request for for all our installed compopents (cdi-deployment, cdi-apiserver, cdi-uploadproxy, cdi-operator) for 10m (1% of a core) to 100m (10% of a core). The main driver of this is BZ: 2216038. Without this change, it is pretty easy to create a large number of concurrent clone operations and get token timeout errors. Upping resource requests and concurrency addresses the issue in a very direct way. Signed-off-by: Michael Henriksen <mhenriks@redhat.com>
mhenriks · Aug 22, 2023 · fbfd898 · fbfd898
1 parent b0f3da6
commit fbfd898
Show file tree

Hide file tree

Showing 22 changed files with 51 additions and 21 deletions.
diff --git a/pkg/controller/clone-controller.go b/pkg/controller/clone-controller.go
@@ -96,7 +96,8 @@ func NewCloneController(mgr manager.Manager,
 		installerLabels:     installerLabels,
 	}
 	cloneController, err := controller.New("clone-controller", mgr, controller.Options{
-		Reconciler: reconciler,
+		MaxConcurrentReconciles: 3,
+		Reconciler:              reconciler,
 	})
 	if err != nil {
 		return nil, err

diff --git a/pkg/controller/config-controller.go b/pkg/controller/config-controller.go
@@ -480,7 +480,8 @@ func NewConfigController(mgr manager.Manager, log logr.Logger, uploadProxyServic
 	}
 
 	configController, err := controller.New("config-controller", mgr, controller.Options{
-		Reconciler: reconciler,
+		MaxConcurrentReconciles: 3,
+		Reconciler:              reconciler,
 	})
 	if err != nil {
 		return nil, err

diff --git a/pkg/controller/dataimportcron-controller.go b/pkg/controller/dataimportcron-controller.go
@@ -947,7 +947,10 @@ func NewDataImportCronController(mgr manager.Manager, log logr.Logger, importerI
 		cdiNamespace:    util.GetNamespace(),
 		installerLabels: installerLabels,
 	}
-	dataImportCronController, err := controller.New(dataImportControllerName, mgr, controller.Options{Reconciler: reconciler})
+	dataImportCronController, err := controller.New(dataImportControllerName, mgr, controller.Options{
+		MaxConcurrentReconciles: 3,
+		Reconciler:              reconciler,
+	})
 	if err != nil {
 		return nil, err
 	}

diff --git a/pkg/controller/datasource-controller.go b/pkg/controller/datasource-controller.go
@@ -175,7 +175,10 @@ func NewDataSourceController(mgr manager.Manager, log logr.Logger, installerLabe
 		log:             log.WithName(dataSourceControllerName),
 		installerLabels: installerLabels,
 	}
-	DataSourceController, err := controller.New(dataSourceControllerName, mgr, controller.Options{Reconciler: reconciler})
+	DataSourceController, err := controller.New(dataSourceControllerName, mgr, controller.Options{
+		MaxConcurrentReconciles: 3,
+		Reconciler:              reconciler,
+	})
 	if err != nil {
 		return nil, err
 	}

diff --git a/pkg/controller/datavolume/controller-base.go b/pkg/controller/datavolume/controller-base.go
@@ -495,10 +495,17 @@ func (r *ReconcilerBase) syncUpdate(log logr.Logger, syncState *dvSyncState) err
 		return fmt.Errorf("status update is not allowed in sync phase")
 	}
 	if !reflect.DeepEqual(syncState.dv.ObjectMeta, syncState.dvMutated.ObjectMeta) {
+		_, ok := syncState.dv.Annotations[cc.AnnExtendedCloneToken]
+		_, ok2 := syncState.dvMutated.Annotations[cc.AnnExtendedCloneToken]
 		if err := r.updateDataVolume(syncState.dvMutated); err != nil {
 			r.log.Error(err, "Unable to sync update dv meta", "name", syncState.dvMutated.Name)
 			return err
 		}
+		delta := time.Since(syncState.dv.ObjectMeta.CreationTimestamp.Time)
+		if !ok && ok2 {
+			log.V(3).Info("Adding extended DataVolume token took", "delta", delta)
+		}
+		syncState.dv = syncState.dvMutated.DeepCopy()
 	}
 	return nil
 }

diff --git a/pkg/controller/datavolume/external-population-controller.go b/pkg/controller/datavolume/external-population-controller.go
@@ -71,7 +71,10 @@ func NewPopulatorController(ctx context.Context, mgr manager.Manager, log logr.L
 		},
 	}
 
-	datavolumeController, err := controller.New(populatorControllerName, mgr, controller.Options{Reconciler: reconciler})
+	datavolumeController, err := controller.New(populatorControllerName, mgr, controller.Options{
+		MaxConcurrentReconciles: 3,
+		Reconciler:              reconciler,
+	})
 	if err != nil {
 		return nil, err
 	}

diff --git a/pkg/controller/datavolume/import-controller.go b/pkg/controller/datavolume/import-controller.go
@@ -91,7 +91,8 @@ func NewImportController(
 	}
 
 	datavolumeController, err := controller.New(importControllerName, mgr, controller.Options{
-		Reconciler: reconciler,
+		MaxConcurrentReconciles: 3,
+		Reconciler:              reconciler,
 	})
 	if err != nil {
 		return nil, err

diff --git a/pkg/controller/datavolume/pvc-clone-controller.go b/pkg/controller/datavolume/pvc-clone-controller.go
@@ -97,7 +97,8 @@ func NewPvcCloneController(
 	}
 
 	dataVolumeCloneController, err := controller.New(pvcCloneControllerName, mgr, controller.Options{
-		Reconciler: reconciler,
+		MaxConcurrentReconciles: 3,
+		Reconciler:              reconciler,
 	})
 	if err != nil {
 		return nil, err

diff --git a/pkg/controller/datavolume/snapshot-clone-controller.go b/pkg/controller/datavolume/snapshot-clone-controller.go
@@ -96,7 +96,8 @@ func NewSnapshotCloneController(
 	}
 
 	dataVolumeCloneController, err := controller.New(snapshotCloneControllerName, mgr, controller.Options{
-		Reconciler: reconciler,
+		MaxConcurrentReconciles: 3,
+		Reconciler:              reconciler,
 	})
 	if err != nil {
 		return nil, err

diff --git a/pkg/controller/datavolume/upload-controller.go b/pkg/controller/datavolume/upload-controller.go
@@ -90,7 +90,8 @@ func NewUploadController(
 	}
 
 	datavolumeController, err := controller.New(uploadControllerName, mgr, controller.Options{
-		Reconciler: reconciler,
+		MaxConcurrentReconciles: 3,
+		Reconciler:              reconciler,
 	})
 	if err != nil {
 		return nil, err

diff --git a/pkg/controller/import-controller.go b/pkg/controller/import-controller.go
@@ -140,7 +140,8 @@ func NewImportController(mgr manager.Manager, log logr.Logger, importerImage, pu
 		installerLabels: installerLabels,
 	}
 	importController, err := controller.New("import-controller", mgr, controller.Options{
-		Reconciler: reconciler,
+		MaxConcurrentReconciles: 3,
+		Reconciler:              reconciler,
 	})
 	if err != nil {
 		return nil, err

diff --git a/pkg/controller/populators/clone-populator.go b/pkg/controller/populators/clone-populator.go
@@ -104,7 +104,7 @@ func NewClonePopulator(
 	}
 
 	clonePopulator, err := controller.New(clonePopulatorName, mgr, controller.Options{
-		MaxConcurrentReconciles: 5,
+		MaxConcurrentReconciles: 3,
 		Reconciler:              reconciler,
 	})
 	if err != nil {

diff --git a/pkg/controller/populators/import-populator.go b/pkg/controller/populators/import-populator.go
@@ -83,7 +83,8 @@ func NewImportPopulator(
 	}
 
 	importPopulator, err := controller.New(importPopulatorName, mgr, controller.Options{
-		Reconciler: reconciler,
+		MaxConcurrentReconciles: 3,
+		Reconciler:              reconciler,
 	})
 	if err != nil {
 		return nil, err

diff --git a/pkg/controller/populators/upload-populator.go b/pkg/controller/populators/upload-populator.go
@@ -74,7 +74,8 @@ func NewUploadPopulator(
 	}
 
 	uploadPopulator, err := controller.New(uploadPopulatorName, mgr, controller.Options{
-		Reconciler: reconciler,
+		MaxConcurrentReconciles: 3,
+		Reconciler:              reconciler,
 	})
 	if err != nil {
 		return nil, err

diff --git a/pkg/controller/storageprofile-controller.go b/pkg/controller/storageprofile-controller.go
@@ -324,7 +324,7 @@ func NewStorageProfileController(mgr manager.Manager, log logr.Logger, installer
 	storageProfileController, err := controller.New(
 		"storageprofile-controller",
 		mgr,
-		controller.Options{Reconciler: reconciler})
+		controller.Options{Reconciler: reconciler, MaxConcurrentReconciles: 3})
 	if err != nil {
 		return nil, err
 	}

diff --git a/pkg/controller/transfer/controller.go b/pkg/controller/transfer/controller.go
@@ -97,7 +97,8 @@ func NewObjectTransferController(mgr manager.Manager, log logr.Logger, installer
 	}
 
 	ctrl, err := controller.New(name, mgr, controller.Options{
-		Reconciler: reconciler,
+		MaxConcurrentReconciles: 3,
+		Reconciler:              reconciler,
 	})
 	if err != nil {
 		return nil, err

diff --git a/pkg/controller/upload-controller.go b/pkg/controller/upload-controller.go
@@ -662,7 +662,8 @@ func NewUploadController(mgr manager.Manager, log logr.Logger, uploadImage, pull
 		installerLabels:     installerLabels,
 	}
 	uploadController, err := controller.New("upload-controller", mgr, controller.Options{
-		Reconciler: reconciler,
+		MaxConcurrentReconciles: 3,
+		Reconciler:              reconciler,
 	})
 	if err != nil {
 		return nil, err

diff --git a/pkg/operator/controller/controller.go b/pkg/operator/controller/controller.go
@@ -236,7 +236,10 @@ func (r *ReconcileCDI) Reconcile(_ context.Context, request reconcile.Request) (
 
 func (r *ReconcileCDI) add(mgr manager.Manager) error {
 	// Create a new controller
-	c, err := controller.New("cdi-operator-controller", mgr, controller.Options{Reconciler: r})
+	c, err := controller.New("cdi-operator-controller", mgr, controller.Options{
+		MaxConcurrentReconciles: 3,
+		Reconciler:              r,
+	})
 	if err != nil {
 		return err
 	}

diff --git a/pkg/operator/resources/namespaced/apiserver.go b/pkg/operator/resources/namespaced/apiserver.go
@@ -150,7 +150,7 @@ func createAPIServerDeployment(image, verbosity, pullPolicy string, imagePullSec
 	}
 	container.Resources = corev1.ResourceRequirements{
 		Requests: corev1.ResourceList{
-			corev1.ResourceCPU:    resource.MustParse("10m"),
+			corev1.ResourceCPU:    resource.MustParse("100m"),
 			corev1.ResourceMemory: resource.MustParse("150Mi"),
 		},
 	}

diff --git a/pkg/operator/resources/namespaced/controller.go b/pkg/operator/resources/namespaced/controller.go
@@ -253,7 +253,7 @@ func createControllerDeployment(controllerImage, importerImage, clonerImage, upl
 	}
 	container.Resources = corev1.ResourceRequirements{
 		Requests: corev1.ResourceList{
-			corev1.ResourceCPU:    resource.MustParse("10m"),
+			corev1.ResourceCPU:    resource.MustParse("100m"),
 			corev1.ResourceMemory: resource.MustParse("150Mi"),
 		},
 	}

diff --git a/pkg/operator/resources/namespaced/uploadproxy.go b/pkg/operator/resources/namespaced/uploadproxy.go
@@ -135,7 +135,7 @@ func createUploadProxyDeployment(image, verbosity, pullPolicy string, imagePullS
 	}
 	container.Resources = corev1.ResourceRequirements{
 		Requests: corev1.ResourceList{
-			corev1.ResourceCPU:    resource.MustParse("10m"),
+			corev1.ResourceCPU:    resource.MustParse("100m"),
 			corev1.ResourceMemory: resource.MustParse("150Mi"),
 		},
 	}

diff --git a/pkg/operator/resources/operator/operator.go b/pkg/operator/resources/operator/operator.go
@@ -361,7 +361,7 @@ func createOperatorDeployment(operatorVersion, namespace, deployClusterResources
 	container := utils.CreatePortsContainer("cdi-operator", operatorImage, pullPolicy, createPrometheusPorts())
 	container.Resources = corev1.ResourceRequirements{
 		Requests: corev1.ResourceList{
-			corev1.ResourceCPU:    resource.MustParse("10m"),
+			corev1.ResourceCPU:    resource.MustParse("100m"),
 			corev1.ResourceMemory: resource.MustParse("150Mi"),
 		},
 	}