feat: reconcile effective collectors sizing profiles (#2181)

This PR makes it so that we reconcile sizing profiles in schedualer into effective config, instead of modifying the config object itself in cli. It give the following benefits: - works with helm (vs current implementation that only works with cli). - cleaner code - the code that sets values for profile is now part of the profile for easier navigation in the code base - remove complication from cli regarding handling these values
odigos-io · Jan 11, 2025 · 0a7fb69 · 0a7fb69
1 parent b8eedeb
commit 0a7fb69
Show file tree

Hide file tree

Showing 4 changed files with 71 additions and 102 deletions.
diff --git a/cli/cmd/resources/odigosconfig.go b/cli/cmd/resources/odigosconfig.go
@@ -7,7 +7,6 @@ import (
 	"github.com/odigos-io/odigos/cli/pkg/kube"
 	"github.com/odigos-io/odigos/common"
 	"github.com/odigos-io/odigos/common/consts"
-	k8sprofiles "github.com/odigos-io/odigos/k8sutils/pkg/profiles"
 	v1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"sigs.k8s.io/yaml"
@@ -48,20 +47,6 @@ func (a *odigosConfigResourceManager) Name() string { return "OdigosConfig" }
 
 func (a *odigosConfigResourceManager) InstallFromScratch(ctx context.Context) error {
 
-	sizingProfile := k8sprofiles.FilterSizeProfiles(a.config.Profiles)
-
-	collectorGatewayConfig := GetGatewayConfigBasedOnSize(sizingProfile)
-	a.config.CollectorGateway = collectorGatewayConfig
-
-	collectorNodeConfig := GetNodeCollectorConfigBasedOnSize(sizingProfile)
-	if a.config.CollectorNode != nil && a.config.CollectorNode.CollectorOwnMetricsPort != 0 {
-		if collectorNodeConfig == nil {
-			collectorNodeConfig = &common.CollectorNodeConfiguration{}
-		}
-		collectorNodeConfig.CollectorOwnMetricsPort = a.config.CollectorNode.CollectorOwnMetricsPort
-	}
-	a.config.CollectorNode = collectorNodeConfig
-
 	obj, err := NewOdigosConfiguration(a.ns, a.config)
 	if err != nil {
 		return err
@@ -72,73 +57,3 @@ func (a *odigosConfigResourceManager) InstallFromScratch(ctx context.Context) er
 	}
 	return a.client.ApplyResources(ctx, a.config.ConfigVersion, resources)
 }
-
-func GetNodeCollectorConfigBasedOnSize(profile common.ProfileName) *common.CollectorNodeConfiguration {
-	aggregateProfiles := append([]common.ProfileName{profile}, k8sprofiles.ProfilesMap[profile].Dependencies...)
-
-	for _, profile := range aggregateProfiles {
-		switch profile {
-		case k8sprofiles.SizeSProfile.ProfileName:
-			return &common.CollectorNodeConfiguration{
-				RequestMemoryMiB: 150,
-				LimitMemoryMiB:   300,
-				RequestCPUm:      150,
-				LimitCPUm:        300,
-			}
-		case k8sprofiles.SizeMProfile.ProfileName:
-			return &common.CollectorNodeConfiguration{
-				RequestMemoryMiB: 250,
-				LimitMemoryMiB:   500,
-				RequestCPUm:      250,
-				LimitCPUm:        500,
-			}
-		case k8sprofiles.SizeLProfile.ProfileName:
-			return &common.CollectorNodeConfiguration{
-				RequestMemoryMiB: 500,
-				LimitMemoryMiB:   750,
-				RequestCPUm:      500,
-				LimitCPUm:        750,
-			}
-		}
-	}
-	// Return nil if no matching profile is found.
-	return nil
-}
-
-func GetGatewayConfigBasedOnSize(profile common.ProfileName) *common.CollectorGatewayConfiguration {
-	aggregateProfiles := append([]common.ProfileName{profile}, k8sprofiles.ProfilesMap[profile].Dependencies...)
-
-	for _, profile := range aggregateProfiles {
-		switch profile {
-		case k8sprofiles.SizeSProfile.ProfileName:
-			return &common.CollectorGatewayConfiguration{
-				MinReplicas:      1,
-				MaxReplicas:      5,
-				RequestCPUm:      150,
-				LimitCPUm:        300,
-				RequestMemoryMiB: 300,
-				LimitMemoryMiB:   300,
-			}
-		case k8sprofiles.SizeMProfile.ProfileName:
-			return &common.CollectorGatewayConfiguration{
-				MinReplicas:      2,
-				MaxReplicas:      8,
-				RequestCPUm:      500,
-				LimitCPUm:        1000,
-				RequestMemoryMiB: 500,
-				LimitMemoryMiB:   600,
-			}
-		case k8sprofiles.SizeLProfile.ProfileName:
-			return &common.CollectorGatewayConfiguration{
-				MinReplicas:      3,
-				MaxReplicas:      12,
-				RequestCPUm:      750,
-				LimitCPUm:        1250,
-				RequestMemoryMiB: 750,
-				LimitMemoryMiB:   850,
-			}
-		}
-	}
-	// Return nil if no matching profile is found.
-	return nil
-}
diff --git a/k8sutils/pkg/profiles/profile.go b/k8sutils/pkg/profiles/profile.go
@@ -25,14 +25,65 @@ var (
 	SizeSProfile = Profile{
 		ProfileName:      common.ProfileName("size_s"),
 		ShortDescription: "Small size deployment profile",
+		ModifyConfigFunc: func(c *common.OdigosConfiguration) {
+			modifySizingConfig(c,
+				common.CollectorGatewayConfiguration{
+					MinReplicas:      1,
+					MaxReplicas:      5,
+					RequestCPUm:      150,
+					LimitCPUm:        300,
+					RequestMemoryMiB: 300,
+					LimitMemoryMiB:   300,
+				},
+				common.CollectorNodeConfiguration{
+					RequestMemoryMiB: 150,
+					LimitMemoryMiB:   300,
+					RequestCPUm:      150,
+					LimitCPUm:        300,
+				})
+		},
 	}
 	SizeMProfile = Profile{
 		ProfileName:      common.ProfileName("size_m"),
 		ShortDescription: "Medium size deployment profile",
+		ModifyConfigFunc: func(c *common.OdigosConfiguration) {
+			modifySizingConfig(c,
+				common.CollectorGatewayConfiguration{
+					MinReplicas:      2,
+					MaxReplicas:      8,
+					RequestCPUm:      500,
+					LimitCPUm:        1000,
+					RequestMemoryMiB: 500,
+					LimitMemoryMiB:   600,
+				},
+				common.CollectorNodeConfiguration{
+					RequestMemoryMiB: 250,
+					LimitMemoryMiB:   500,
+					RequestCPUm:      250,
+					LimitCPUm:        500,
+				})
+		},
 	}
 	SizeLProfile = Profile{
 		ProfileName:      common.ProfileName("size_l"),
 		ShortDescription: "Large size deployment profile",
+		ModifyConfigFunc: func(c *common.OdigosConfiguration) {
+			modifySizingConfig(c,
+				common.CollectorGatewayConfiguration{
+					MinReplicas:      3,
+					MaxReplicas:      12,
+					RequestCPUm:      750,
+					LimitCPUm:        1250,
+					RequestMemoryMiB: 750,
+					LimitMemoryMiB:   850,
+				},
+				common.CollectorNodeConfiguration{
+					RequestMemoryMiB: 500,
+					LimitMemoryMiB:   750,
+					RequestCPUm:      500,
+					LimitCPUm:        750,
+				})
+		},
 	}
 	AllowConcurrentAgents = Profile{
 		ProfileName:      common.ProfileName("allow_concurrent_agents"),

diff --git a/k8sutils/pkg/profiles/utils.go b/k8sutils/pkg/profiles/utils.go
@@ -2,23 +2,22 @@ package profiles
 
 import "github.com/odigos-io/odigos/common"
 
-func FilterSizeProfiles(profiles []common.ProfileName) common.ProfileName {
-	// In case multiple size profiles are provided, the first one will be used.
-	for _, profile := range profiles {
-		// Check if the profile is a size profile.
-		switch profile {
-		case SizeSProfile.ProfileName, SizeMProfile.ProfileName, SizeLProfile.ProfileName:
-			return profile
-		}
+func modifySizingConfig(c *common.OdigosConfiguration, clusterCollectorConfig common.CollectorGatewayConfiguration, nodeCollectorConfig common.CollectorNodeConfiguration) {
+	// do not modify the configuration if any of the values if they are already set
+	if c.CollectorGateway != nil {
+		return
+	}
+	// the following is not very elegant.
+	// we only care if the sizing parameters are set, if the port is set, we apply it nevertheless
+	if c.CollectorNode != nil && (c.CollectorNode.RequestMemoryMiB != 0 || c.CollectorNode.LimitMemoryMiB != 0 || c.CollectorNode.RequestCPUm != 0 || c.CollectorNode.LimitCPUm != 0) {
+		return
+	}
 
-		// Check if the profile has a dependency which is a size profile.
-		profileDependencies := ProfilesMap[profile].Dependencies
-		for _, dependencyProfile := range profileDependencies {
-			switch dependencyProfile {
-			case SizeSProfile.ProfileName, SizeMProfile.ProfileName, SizeLProfile.ProfileName:
-				return dependencyProfile
-			}
-		}
+	c.CollectorGateway = &clusterCollectorConfig
+	collectorNodeConfig := nodeCollectorConfig
+	if c.CollectorNode != nil {
+		// make sure we keep the port which is unrelated to the sizing
+		collectorNodeConfig.CollectorOwnMetricsPort = c.CollectorNode.CollectorOwnMetricsPort
 	}
-	return ""
+	c.CollectorNode = &collectorNodeConfig
 }
diff --git a/scheduler/controllers/odigosconfig/odigosconfig_controller.go b/scheduler/controllers/odigosconfig/odigosconfig_controller.go
@@ -38,6 +38,10 @@ func (r *odigosConfigController) Reconcile(ctx context.Context, _ ctrl.Request)
 
 	applyProfilesToOdigosConfig(odigosConfig)
 
+	// if none of the profiles set sizing for collectors, use size_s as default, so the values are never nil
+	// if the values were already set (by user or profile) this is a no-op
+	profiles.SizeSProfile.ModifyConfigFunc(odigosConfig)
+
 	err = r.persistEffectiveConfig(ctx, odigosConfig)
 	if err != nil {
 		return ctrl.Result{}, err