From 4232c34e4d6292118e706e7040296aa54cdd4c80 Mon Sep 17 00:00:00 2001 From: Walter Fender Date: Tue, 27 Aug 2024 21:55:17 +0000 Subject: [PATCH] Configure pprof on controller-manager. Generated file. Adding cluster mode changes. Updated version for cluster mode. Skipping version check on dev mode controllers. Make pprof support case insensitive. Factored in Justin's feedback. Fixed the pprof field names. --- ...loud.google.com_controllerreconcilers.yaml | 14 +++ ...e.com_namespacedcontrollerreconcilers.yaml | 14 +++ .../v1alpha1/controllerreconciler_types.go | 20 ++++ .../configconnector_controller.go | 4 + .../configconnectorcontext_controller.go | 4 + operator/pkg/controllers/utils.go | 93 ++++++++++++++++++- operator/pkg/preflight/upgrade.go | 7 +- 7 files changed, 154 insertions(+), 2 deletions(-) diff --git a/operator/config/crd/bases/customize.core.cnrm.cloud.google.com_controllerreconcilers.yaml b/operator/config/crd/bases/customize.core.cnrm.cloud.google.com_controllerreconcilers.yaml index 3ae98df8c8..c3a9cb6f06 100644 --- a/operator/config/crd/bases/customize.core.cnrm.cloud.google.com_controllerreconcilers.yaml +++ b/operator/config/crd/bases/customize.core.cnrm.cloud.google.com_controllerreconcilers.yaml @@ -41,6 +41,20 @@ spec: spec: description: ControllerReconcilerSpec is the specification of ControllerReconciler. properties: + pprof: + description: Configures the debug endpoint on the service. + properties: + port: + description: The port that the pprof server binds to if enabled + type: integer + support: + description: Control if pprof should be turned on and which types + should be enabled. + enum: + - none + - all + type: string + type: object rateLimit: description: |- RateLimit configures the token bucket rate limit to the kubernetes client used diff --git a/operator/config/crd/bases/customize.core.cnrm.cloud.google.com_namespacedcontrollerreconcilers.yaml b/operator/config/crd/bases/customize.core.cnrm.cloud.google.com_namespacedcontrollerreconcilers.yaml index 642ce94a22..d55503cd60 100644 --- a/operator/config/crd/bases/customize.core.cnrm.cloud.google.com_namespacedcontrollerreconcilers.yaml +++ b/operator/config/crd/bases/customize.core.cnrm.cloud.google.com_namespacedcontrollerreconcilers.yaml @@ -41,6 +41,20 @@ spec: spec: description: NamespacedControllerReconciler is the specification of NamespacedControllerReconciler. properties: + pprof: + description: Configures the debug endpoint on the service. + properties: + port: + description: The port that the pprof server binds to if enabled + type: integer + support: + description: Control if pprof should be turned on and which types + should be enabled. + enum: + - none + - all + type: string + type: object rateLimit: description: |- RateLimit configures the token bucket rate limit to the kubernetes client used diff --git a/operator/pkg/apis/core/customize/v1alpha1/controllerreconciler_types.go b/operator/pkg/apis/core/customize/v1alpha1/controllerreconciler_types.go index 6e4fb9e078..ac1f171dab 100644 --- a/operator/pkg/apis/core/customize/v1alpha1/controllerreconciler_types.go +++ b/operator/pkg/apis/core/customize/v1alpha1/controllerreconciler_types.go @@ -41,6 +41,9 @@ type NamespacedControllerReconcilerSpec struct { // If not specified, the default will be Token Bucket with qps 20, burst 30. // +optional RateLimit *RateLimit `json:"rateLimit,omitempty"` + // Configures the debug endpoint on the service. + // +optional + Pprof *PprofConfig `json:"pprof,omitempty"` } type RateLimit struct { @@ -52,6 +55,16 @@ type RateLimit struct { Burst int `json:"burst,omitempty"` } +type PprofConfig struct { + // Control if pprof should be turned on and which types should be enabled. + // +kubebuilder:validation:Enum=none;all + // +optional + Support string `json:"support,omitempty"` + // The port that the pprof server binds to if enabled + // +optional + Port int `json:"port,omitempty"` +} + // NamespacedControllerReconcilerStatus defines the observed state of NamespacedControllerReconciler. type NamespacedControllerReconcilerStatus struct { addonv1alpha1.CommonStatus `json:",inline"` @@ -92,6 +105,9 @@ type ControllerReconcilerSpec struct { // If not specified, the default will be Token Bucket with qps 20, burst 30. // +optional RateLimit *RateLimit `json:"rateLimit,omitempty"` + // Configures the debug endpoint on the service. + // +optional + Pprof *PprofConfig `json:"pprof,omitempty"` } // ControllerReconcilerStatus defines the observed state of ControllerReconciler. @@ -116,6 +132,10 @@ var ValidRateLimitControllers = []string{ "cnrm-controller-manager", } +var SupportedPprofControllers = []string{ + "cnrm-controller-manager", +} + func init() { SchemeBuilder.Register( &NamespacedControllerReconciler{}, diff --git a/operator/pkg/controllers/configconnector/configconnector_controller.go b/operator/pkg/controllers/configconnector/configconnector_controller.go index a3c27cf2e1..bd4b4cfe99 100644 --- a/operator/pkg/controllers/configconnector/configconnector_controller.go +++ b/operator/pkg/controllers/configconnector/configconnector_controller.go @@ -861,6 +861,10 @@ func (r *Reconciler) applyControllerReconcilerCR(ctx context.Context, cr *custom r.log.Error(err, errMsg) return r.handleApplyControllerReconcilerFailed(ctx, cr, errMsg) } + if err := controllers.ApplyContainerPprof(m, cr.Name, cr.Spec.Pprof); err != nil { + msg := fmt.Sprintf("failed to apply pprof customization %s: %v", cr.Name, err) + return r.handleApplyControllerReconcilerFailed(ctx, cr, msg) + } return r.handleApplyControllerReconcilerSucceeded(ctx, cr) } diff --git a/operator/pkg/controllers/configconnectorcontext/configconnectorcontext_controller.go b/operator/pkg/controllers/configconnectorcontext/configconnectorcontext_controller.go index 644bb3cc5a..e1a9053372 100644 --- a/operator/pkg/controllers/configconnectorcontext/configconnectorcontext_controller.go +++ b/operator/pkg/controllers/configconnectorcontext/configconnectorcontext_controller.go @@ -551,6 +551,10 @@ func (r *Reconciler) applyNamespacedControllerReconciler(ctx context.Context, cr msg := fmt.Sprintf("failed to apply rate limit customization %s: %v", cr.Name, err) return r.handleApplyNamespacedControllerReconcilerFailed(ctx, cr.Namespace, cr.Name, msg) } + if err := controllers.ApplyContainerPprof(m, cr.Name, cr.Spec.Pprof); err != nil { + msg := fmt.Sprintf("failed to apply pprof customization %s: %v", cr.Name, err) + return r.handleApplyNamespacedControllerReconcilerFailed(ctx, cr.Namespace, cr.Name, msg) + } return r.handleApplyNamespacedControllerReconcilerSucceeded(ctx, cr.Namespace, cr.Name) } diff --git a/operator/pkg/controllers/utils.go b/operator/pkg/controllers/utils.go index 5595b02d3d..3eb54f362d 100644 --- a/operator/pkg/controllers/utils.go +++ b/operator/pkg/controllers/utils.go @@ -530,6 +530,7 @@ func ApplyContainerRateLimit(m *manifest.Objects, targetControllerName string, r targetControllerName, strings.Join(customizev1alpha1.ValidRateLimitControllers, ", ")) } + count := 0 for _, item := range m.Items { if item.GroupVersionKind() != targetControllerGVK { continue @@ -540,7 +541,10 @@ func ApplyContainerRateLimit(m *manifest.Objects, targetControllerName string, r if err := item.MutateContainers(customizeRateLimitFn(targetContainerName, ratelimit)); err != nil { return err } - break // we already found the matching controller, no need to keep looking. + count++ + } + if count != 1 { + return fmt.Errorf("rate limit customization for %s modified %d instances.", targetControllerName, count) } return nil } @@ -586,3 +590,90 @@ func applyRateLimitToContainerArg(container map[string]interface{}, rateLimit *c } return nil } + +func ApplyContainerPprof(m *manifest.Objects, targetControllerName string, pprofConfig *customizev1alpha1.PprofConfig) error { + if pprofConfig == nil { + return nil + } + + var ( + targetContainerName string + targetControllerGVK schema.GroupVersionKind + ) + switch targetControllerName { + case "cnrm-controller-manager": + targetContainerName = "manager" + targetControllerGVK = schema.GroupVersionKind{ + Group: appsv1.SchemeGroupVersion.Group, + Version: appsv1.SchemeGroupVersion.Version, + Kind: "StatefulSet", + } + default: + return fmt.Errorf("pprof config customization for %s is not supported. "+ + "Supported controllers: %s", + targetControllerName, strings.Join(customizev1alpha1.SupportedPprofControllers, ", ")) + } + + count := 0 + for _, item := range m.Items { + if item.GroupVersionKind() != targetControllerGVK { + continue + } + if !strings.HasPrefix(item.GetName(), targetControllerName) { + continue + } + if err := item.MutateContainers(customizePprofConfigFn(targetContainerName, pprofConfig)); err != nil { + return err + } + count++ + } + if count != 1 { + return fmt.Errorf("pprof config customization for %s modified %d instances.", targetControllerName, count) + } + return nil +} + +func customizePprofConfigFn(target string, pprofConfig *customizev1alpha1.PprofConfig) func(container map[string]interface{}) error { + return func(container map[string]interface{}) error { + name, _, err := unstructured.NestedString(container, "name") + if err != nil { + return fmt.Errorf("error reading container name: %w", err) + } + if name != target { + return nil + } + return applyPprofConfigToContainerArg(container, pprofConfig) + } +} + +func applyPprofConfigToContainerArg(container map[string]interface{}, pprofConfig *customizev1alpha1.PprofConfig) error { + if pprofConfig == nil { + return nil + } + origArgs, found, err := unstructured.NestedStringSlice(container, "args") + if err != nil { + return fmt.Errorf("error getting args in container: %w", err) + } + wantArgs := []string{} + if strings.ToUpper(pprofConfig.Support) == "ALL" { + wantArgs = append(wantArgs, "--enable-pprof=true") + } else { + wantArgs = append(wantArgs, "--enable-pprof=false") + } + if pprofConfig.Port > 0 { + wantArgs = append(wantArgs, fmt.Sprintf("--pprof-port=%d", pprofConfig.Port)) + } + if found { + for _, arg := range origArgs { + if strings.Contains(arg, "--enable-pprof") || strings.Contains(arg, "--pprof-port") { + // drop the old value on the floor + continue + } + wantArgs = append(wantArgs, arg) + } + } + if err := unstructured.SetNestedStringSlice(container, wantArgs, "args"); err != nil { + return fmt.Errorf("error setting args in container: %w", err) + } + return nil +} diff --git a/operator/pkg/preflight/upgrade.go b/operator/pkg/preflight/upgrade.go index 1e71ef8ecf..752fdf2bb7 100644 --- a/operator/pkg/preflight/upgrade.go +++ b/operator/pkg/preflight/upgrade.go @@ -31,7 +31,8 @@ import ( ) var ( - ulog = ctrl.Log.WithName("UpgradeChecker") + ulog = ctrl.Log.WithName("UpgradeChecker") + devVersion = semver.MustParse("0.0.0-dev") ) // NewUpgradeChecker provides an implementation of declarative.Preflight that @@ -95,6 +96,10 @@ func (u *UpgradeChecker) Preflight(ctx context.Context, o declarative.Declarativ } func compareMajorOnly(v, w semver.Version) int { + if v.Equals(devVersion) { + // If we are using a dev controller ignore semver drift. + return 0 + } if v.Major != w.Major { if v.Major > w.Major { return 1