feat(api): Split UserContainerLimitRequestFactor into two separate co…

…nfig values (caraml-dev#367)

* Split UserContainerLimitRequestFactor into two separate config values

* Set UserContainerCPULimitRequestFactor as 0 in unit tests

* Fix lint comment to shorten line length

* Remove outdated golangci linters

* Make setting of resource limits conditional

* Increase defaultMemoryLimitRequestFactor to 2

.golangci.yml

@@ -7,7 +7,6 @@ run:
 linters:
   enable:
     - bodyclose
-    - deadcode
     - errcheck
     - gocyclo
     - gofmt
@@ -19,9 +18,7 @@ linters:
     - misspell
     - revive
     - staticcheck
-    - structcheck
     - unused
-    - varcheck
 
 linters-settings:
   gocyclo:

api/turing/cluster/knative_service.go

@@ -50,8 +50,9 @@ type KnativeService struct {
 	TopologySpreadConstraints []corev1.TopologySpreadConstraint `json:"topologySpreadConstraints"`
 
 	// Resource properties
-	QueueProxyResourcePercentage    int     `json:"queueProxyResourcePercentage"`
-	UserContainerLimitRequestFactor float64 `json:"userContainerLimitRequestFactor"`
+	QueueProxyResourcePercentage          int     `json:"queueProxyResourcePercentage"`
+	UserContainerCPULimitRequestFactor    float64 `json:"userContainerLimitCPURequestFactor"`
+	UserContainerMemoryLimitRequestFactor float64 `json:"userContainerLimitMemoryRequestFactor"`
 }
 
 // Creates a new config object compatible with the knative serving API, from
@@ -131,7 +132,10 @@ func (cfg *KnativeService) buildSvcSpec(
 	revisionName := getDefaultRevisionName(cfg.Name)
 
 	// Build resource requirements for the user container
-	resourceReqs := cfg.buildResourceReqs(cfg.UserContainerLimitRequestFactor)
+	resourceReqs := cfg.buildResourceReqs(
+		cfg.UserContainerCPULimitRequestFactor,
+		cfg.UserContainerMemoryLimitRequestFactor,
+	)
 
 	// Build container spec
 	var portName string

api/turing/cluster/knative_service_test.go

@@ -172,15 +172,16 @@ func TestBuildKnativeServiceConfig(t *testing.T) {
 	}{
 		"basic": {
 			serviceCfg: KnativeService{
-				BaseService:                     baseSvc,
-				ContainerPort:                   8080,
-				MinReplicas:                     1,
-				MaxReplicas:                     2,
-				AutoscalingMetric:               "concurrency",
-				AutoscalingTarget:               "1",
-				IsClusterLocal:                  true,
-				QueueProxyResourcePercentage:    30,
-				UserContainerLimitRequestFactor: 1.5,
+				BaseService:                           baseSvc,
+				ContainerPort:                         8080,
+				MinReplicas:                           1,
+				MaxReplicas:                           2,
+				AutoscalingMetric:                     "concurrency",
+				AutoscalingTarget:                     "1",
+				IsClusterLocal:                        true,
+				QueueProxyResourcePercentage:          30,
+				UserContainerCPULimitRequestFactor:    1.5,
+				UserContainerMemoryLimitRequestFactor: 1.5,
 			},
 			expectedSpec: knservingv1.Service{
 				ObjectMeta: metav1.ObjectMeta{
@@ -222,16 +223,17 @@ func TestBuildKnativeServiceConfig(t *testing.T) {
 		// upi has no liveness probe in pod spec and user-container is using h2c
 		"upi": {
 			serviceCfg: KnativeService{
-				BaseService:                     baseSvc,
-				ContainerPort:                   8080,
-				MinReplicas:                     1,
-				MaxReplicas:                     2,
-				AutoscalingMetric:               "concurrency",
-				AutoscalingTarget:               "1",
-				IsClusterLocal:                  true,
-				QueueProxyResourcePercentage:    30,
-				UserContainerLimitRequestFactor: 1.5,
-				Protocol:                        routerConfig.UPI,
+				BaseService:                           baseSvc,
+				ContainerPort:                         8080,
+				MinReplicas:                           1,
+				MaxReplicas:                           2,
+				AutoscalingMetric:                     "concurrency",
+				AutoscalingTarget:                     "1",
+				IsClusterLocal:                        true,
+				QueueProxyResourcePercentage:          30,
+				UserContainerCPULimitRequestFactor:    1.5,
+				UserContainerMemoryLimitRequestFactor: 1.5,
+				Protocol:                              routerConfig.UPI,
 			},
 			expectedSpec: knservingv1.Service{
 				ObjectMeta: metav1.ObjectMeta{
@@ -291,14 +293,15 @@ func TestBuildKnativeServiceConfig(t *testing.T) {
 		},
 		"annotations": {
 			serviceCfg: KnativeService{
-				BaseService:                     baseSvc,
-				ContainerPort:                   8080,
-				MinReplicas:                     5,
-				MaxReplicas:                     6,
-				AutoscalingMetric:               "memory",
-				AutoscalingTarget:               "70",
-				IsClusterLocal:                  false,
-				UserContainerLimitRequestFactor: 1.5,
+				BaseService:                           baseSvc,
+				ContainerPort:                         8080,
+				MinReplicas:                           5,
+				MaxReplicas:                           6,
+				AutoscalingMetric:                     "memory",
+				AutoscalingTarget:                     "70",
+				IsClusterLocal:                        false,
+				UserContainerCPULimitRequestFactor:    1.5,
+				UserContainerMemoryLimitRequestFactor: 1.5,
 			},
 			expectedSpec: knservingv1.Service{
 				ObjectMeta: metav1.ObjectMeta{
@@ -381,10 +384,11 @@ func TestBuildKnativeServiceConfig(t *testing.T) {
 						},
 					},
 				},
-				IsClusterLocal:                  true,
-				QueueProxyResourcePercentage:    30,
-				UserContainerLimitRequestFactor: 1.5,
-				Protocol:                        routerConfig.UPI,
+				IsClusterLocal:                        true,
+				QueueProxyResourcePercentage:          30,
+				UserContainerCPULimitRequestFactor:    1.5,
+				UserContainerMemoryLimitRequestFactor: 1.5,
+				Protocol:                              routerConfig.UPI,
 			},
 			expectedSpec: knservingv1.Service{
 				ObjectMeta: metav1.ObjectMeta{

api/turing/cluster/kubernetes_service.go

@@ -7,9 +7,14 @@ import (
 	"k8s.io/apimachinery/pkg/util/intstr"
 )
 
-// defaultLimitRequestFactor is the default multiplication factor applied to the resource request,
-// to be set as resource limit
-const defaultLimitRequestFactor = 1.0
+const (
+	// defaultCPULimitRequestFactor is the default multiplication factor applied to the CPU request,
+	// to be set as the limit
+	defaultCPULimitRequestFactor = 1.0
+	// defaultMemoryLimitRequestFactor is the default multiplication factor applied to the memory request,
+	// to be set as the limit
+	defaultMemoryLimitRequestFactor = 2.0
+)
 
 // KubernetesService defines the properties for Kubernetes services
 type KubernetesService struct {
@@ -68,7 +73,7 @@ func (cfg *KubernetesService) buildDeployment(labels map[string]string) *appsv1.
 							Args:            cfg.Command,
 							Ports:           cfg.buildContainerPorts(),
 							Env:             cfg.Envs,
-							Resources:       cfg.buildResourceReqs(defaultLimitRequestFactor),
+							Resources:       cfg.buildResourceReqs(defaultCPULimitRequestFactor, defaultMemoryLimitRequestFactor),
 							VolumeMounts:    cfg.VolumeMounts,
 							LivenessProbe:   cfg.buildContainerProbe(livenessProbeType, int(cfg.ProbePort)),
 							ReadinessProbe:  cfg.buildContainerProbe(readinessProbeType, int(cfg.ProbePort)),

api/turing/cluster/kubernetes_service_test.go

@@ -110,7 +110,7 @@ func TestBuildKubernetesServiceConfig(t *testing.T) {
 								},
 								Limits: map[corev1.ResourceName]resource.Quantity{
 									corev1.ResourceCPU:    resource.MustParse("1"),
-									corev1.ResourceMemory: resource.MustParse("1"),
+									corev1.ResourceMemory: resource.MustParse("2"),
 								},
 							},
 							VolumeMounts: svcConf.VolumeMounts,

api/turing/cluster/models.go

@@ -62,16 +62,22 @@ type BaseService struct {
 	InitContainers []Container `json:"init_containers"`
 }
 
-func (cfg *BaseService) buildResourceReqs(userContainerLimitRequestFactor float64) corev1.ResourceRequirements {
+func (cfg *BaseService) buildResourceReqs(
+	UserContainerCPULimitRequestFactor float64,
+	UserContainerMemoryLimitRequestFactor float64,
+) corev1.ResourceRequirements {
 	reqs := map[corev1.ResourceName]resource.Quantity{
 		corev1.ResourceCPU:    cfg.CPURequests,
 		corev1.ResourceMemory: cfg.MemoryRequests,
 	}
 
-	// Set resource limits to request * userContainerLimitRequestFactor
-	limits := map[corev1.ResourceName]resource.Quantity{
-		corev1.ResourceCPU:    ComputeResource(cfg.CPURequests, userContainerLimitRequestFactor),
-		corev1.ResourceMemory: ComputeResource(cfg.MemoryRequests, userContainerLimitRequestFactor),
+	// Set resource limits to request * userContainerCPULimitRequestFactor or UserContainerMemoryLimitRequestFactor
+	limits := map[corev1.ResourceName]resource.Quantity{}
+	if UserContainerCPULimitRequestFactor != 0 {
+		limits[corev1.ResourceCPU] = ComputeResource(cfg.CPURequests, UserContainerCPULimitRequestFactor)
+	}
+	if UserContainerMemoryLimitRequestFactor != 0 {
+		limits[corev1.ResourceMemory] = ComputeResource(cfg.MemoryRequests, UserContainerMemoryLimitRequestFactor)
 	}
 
 	return corev1.ResourceRequirements{

api/turing/cluster/servicebuilder/router.go

@@ -105,7 +105,8 @@ func (sb *clusterSvcBuilder) NewRouterService(
 	sentryEnabled bool,
 	sentryDSN string,
 	knativeQueueProxyResourcePercentage int,
-	userContainerLimitRequestFactor float64,
+	userContainerCPULimitRequestFactor float64,
+	userContainerMemoryLimitRequestFactor float64,
 	initialScale *int,
 ) (*cluster.KnativeService, error) {
 	// Create service name
@@ -150,17 +151,18 @@ func (sb *clusterSvcBuilder) NewRouterService(
 			VolumeMounts:         volumeMounts,
 			InitContainers:       initContainers,
 		},
-		IsClusterLocal:                  false,
-		ContainerPort:                   routerPort,
-		Protocol:                        routerVersion.Protocol,
-		MinReplicas:                     routerVersion.ResourceRequest.MinReplica,
-		MaxReplicas:                     routerVersion.ResourceRequest.MaxReplica,
-		InitialScale:                    initialScale,
-		AutoscalingMetric:               string(routerVersion.AutoscalingPolicy.Metric),
-		AutoscalingTarget:               routerVersion.AutoscalingPolicy.Target,
-		TopologySpreadConstraints:       topologySpreadConstraints,
-		QueueProxyResourcePercentage:    knativeQueueProxyResourcePercentage,
-		UserContainerLimitRequestFactor: userContainerLimitRequestFactor,
+		IsClusterLocal:                        false,
+		ContainerPort:                         routerPort,
+		Protocol:                              routerVersion.Protocol,
+		MinReplicas:                           routerVersion.ResourceRequest.MinReplica,
+		MaxReplicas:                           routerVersion.ResourceRequest.MaxReplica,
+		InitialScale:                          initialScale,
+		AutoscalingMetric:                     string(routerVersion.AutoscalingPolicy.Metric),
+		AutoscalingTarget:                     routerVersion.AutoscalingPolicy.Target,
+		TopologySpreadConstraints:             topologySpreadConstraints,
+		QueueProxyResourcePercentage:          knativeQueueProxyResourcePercentage,
+		UserContainerCPULimitRequestFactor:    userContainerCPULimitRequestFactor,
+		UserContainerMemoryLimitRequestFactor: userContainerMemoryLimitRequestFactor,
 	}
 	return sb.validateKnativeService(svc)
 }