Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Parameterize DSPO max concurrency count #394

Merged
merged 3 commits into from
Oct 18, 2023
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions config/base/kustomization.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -106,5 +106,12 @@ vars:
apiVersion: v1
fieldref:
fieldpath: data.ZAP_LOG_LEVEL
- name: MAX_CONCURRENT_RECONCILES
objref:
kind: ConfigMap
name: dspo-parameters
apiVersion: v1
fieldref:
fieldpath: data.MAX_CONCURRENT_RECONCILES
configurations:
- params.yaml
1 change: 1 addition & 0 deletions config/base/params.env
Original file line number Diff line number Diff line change
Expand Up @@ -11,3 +11,4 @@ IMAGES_MOVERESULTSIMAGE=registry.access.redhat.com/ubi8/ubi-micro:8.8
IMAGES_MARIADB=registry.redhat.io/rhel8/mariadb-103:1
IMAGES_OAUTHPROXY=registry.redhat.io/openshift4/ose-oauth-proxy@sha256:ab112105ac37352a2a4916a39d6736f5db6ab4c29bad4467de8d613e80e9bb33
ZAP_LOG_LEVEL=info
MAX_CONCURRENT_RECONCILES=20
HumairAK marked this conversation as resolved.
Show resolved Hide resolved
3 changes: 3 additions & 0 deletions config/manager/manager.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ spec:
args:
- --leader-elect
- --zap-log-level=$(ZAP_LOG_LEVEL)
- --MaxConcurrentReconciles=$(MAX_CONCURRENT_RECONCILES)
HumairAK marked this conversation as resolved.
Show resolved Hide resolved
- --config
- /home/config
image: $(IMAGES_DSPO)
Expand Down Expand Up @@ -59,6 +60,8 @@ spec:
value: $(IMAGES_MLMDWRITER)
- name: ZAP_LOG_LEVEL
value: $(ZAP_LOG_LEVEL)
- name: MAX_CONCURRENT_RECONCILES
value: $(MAX_CONCURRENT_RECONCILES)
securityContext:
allowPrivilegeEscalation: false
capabilities:
Expand Down
9 changes: 5 additions & 4 deletions controllers/dspipeline_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,9 +48,10 @@ const finalizerName = "datasciencepipelinesapplications.opendatahub.io/finalizer
// DSPAReconciler reconciles a DSPAParams object
type DSPAReconciler struct {
client.Client
Scheme *runtime.Scheme
Log logr.Logger
TemplatesPath string
Scheme *runtime.Scheme
Log logr.Logger
TemplatesPath string
MaxConcurrentReconcilesParam int
}

func (r *DSPAReconciler) Apply(owner mf.Owner, params *DSPAParams, template string, fns ...mf.Transformer) error {
Expand Down Expand Up @@ -576,7 +577,7 @@ func (r *DSPAReconciler) SetupWithManager(mgr ctrl.Manager) error {
})).
// TODO: Add watcher for ui cluster rbac since it has no owner
WithOptions(controller.Options{
MaxConcurrentReconciles: config.DefaultMaxConcurrentReconciles,
MaxConcurrentReconciles: r.MaxConcurrentReconcilesParam,
HumairAK marked this conversation as resolved.
Show resolved Hide resolved
}).
Complete(r)
}
Expand Down
11 changes: 7 additions & 4 deletions main.go
Original file line number Diff line number Diff line change
Expand Up @@ -103,12 +103,14 @@ func main() {
var enableLeaderElection bool
var probeAddr string
var configPath string
var maxConcurrentReconciles int
flag.StringVar(&metricsAddr, "metrics-bind-address", ":8080", "The address the metric endpoint binds to.")
flag.StringVar(&probeAddr, "health-probe-bind-address", ":8081", "The address the probe endpoint binds to.")
flag.StringVar(&configPath, "config", "", "Path to JSON file containing config")
flag.BoolVar(&enableLeaderElection, "leader-elect", false,
"Enable leader election for controller manager. "+
"Enabling this will ensure there is only one active controller manager.")
flag.IntVar(&maxConcurrentReconciles, "MaxConcurrentReconciles", config.DefaultMaxConcurrentReconciles, "Maximum concurrent reconciles")
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@HumairAK flag package is taking care of assigning the default value when there is no param. We are passing maxConcurrentreconciles (which will either have param value or default value to SetupWithManager function in the controller.

opts := zap.Options{
Development: true,
TimeEncoder: zapcore.TimeEncoderOfLayout(time.RFC3339),
Expand Down Expand Up @@ -137,10 +139,11 @@ func main() {
}

if err = (&controllers.DSPAReconciler{
Client: mgr.GetClient(),
Scheme: mgr.GetScheme(),
Log: ctrl.Log,
TemplatesPath: "config/internal/",
Client: mgr.GetClient(),
Scheme: mgr.GetScheme(),
Log: ctrl.Log,
TemplatesPath: "config/internal/",
MaxConcurrentReconcilesParam: maxConcurrentReconciles,
HumairAK marked this conversation as resolved.
Show resolved Hide resolved
}).SetupWithManager(mgr); err != nil {
setupLog.Error(err, "unable to create controller", "controller", "DSPAParams")
os.Exit(1)
Expand Down
Loading