Skip to content

Commit 189801b

Browse files
deadlycoconutsdeadlycoconuts
authored
feat(api): Split UserContainerLimitRequestFactor into two separate config values (#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
1 parent 2728514 commit 189801b

16 files changed

+299
-237
lines changed

‎.golangci.yml‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ run:
77
linters:
88
enable:
99
- bodyclose
10-
- deadcode
1110
- errcheck
1211
- gocyclo
1312
- gofmt
@@ -19,9 +18,7 @@ linters:
1918
- misspell
2019
- revive
2120
- staticcheck
22-
- structcheck
2321
- unused
24-
- varcheck
2522

2623
linters-settings:
2724
gocyclo:

‎api/turing/cluster/knative_service.go‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -50,8 +50,9 @@ type KnativeService struct {
5050
TopologySpreadConstraints []corev1.TopologySpreadConstraint `json:"topologySpreadConstraints"`
5151

5252
// Resource properties
53-
QueueProxyResourcePercentage int `json:"queueProxyResourcePercentage"`
54-
UserContainerLimitRequestFactor float64 `json:"userContainerLimitRequestFactor"`
53+
QueueProxyResourcePercentage int `json:"queueProxyResourcePercentage"`
54+
UserContainerCPULimitRequestFactor float64 `json:"userContainerLimitCPURequestFactor"`
55+
UserContainerMemoryLimitRequestFactor float64 `json:"userContainerLimitMemoryRequestFactor"`
5556
}
5657

5758
// Creates a new config object compatible with the knative serving API, from
@@ -131,7 +132,10 @@ func (cfg *KnativeService) buildSvcSpec(
131132
revisionName := getDefaultRevisionName(cfg.Name)
132133

133134
// Build resource requirements for the user container
134-
resourceReqs := cfg.buildResourceReqs(cfg.UserContainerLimitRequestFactor)
135+
resourceReqs := cfg.buildResourceReqs(
136+
cfg.UserContainerCPULimitRequestFactor,
137+
cfg.UserContainerMemoryLimitRequestFactor,
138+
)
135139

136140
// Build container spec
137141
var portName string

‎api/turing/cluster/knative_service_test.go‎

Lines changed: 35 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -172,15 +172,16 @@ func TestBuildKnativeServiceConfig(t *testing.T) {
172172
}{
173173
"basic": {
174174
serviceCfg: KnativeService{
175-
BaseService: baseSvc,
176-
ContainerPort: 8080,
177-
MinReplicas: 1,
178-
MaxReplicas: 2,
179-
AutoscalingMetric: "concurrency",
180-
AutoscalingTarget: "1",
181-
IsClusterLocal: true,
182-
QueueProxyResourcePercentage: 30,
183-
UserContainerLimitRequestFactor: 1.5,
175+
BaseService: baseSvc,
176+
ContainerPort: 8080,
177+
MinReplicas: 1,
178+
MaxReplicas: 2,
179+
AutoscalingMetric: "concurrency",
180+
AutoscalingTarget: "1",
181+
IsClusterLocal: true,
182+
QueueProxyResourcePercentage: 30,
183+
UserContainerCPULimitRequestFactor: 1.5,
184+
UserContainerMemoryLimitRequestFactor: 1.5,
184185
},
185186
expectedSpec: knservingv1.Service{
186187
ObjectMeta: metav1.ObjectMeta{
@@ -222,16 +223,17 @@ func TestBuildKnativeServiceConfig(t *testing.T) {
222223
// upi has no liveness probe in pod spec and user-container is using h2c
223224
"upi": {
224225
serviceCfg: KnativeService{
225-
BaseService: baseSvc,
226-
ContainerPort: 8080,
227-
MinReplicas: 1,
228-
MaxReplicas: 2,
229-
AutoscalingMetric: "concurrency",
230-
AutoscalingTarget: "1",
231-
IsClusterLocal: true,
232-
QueueProxyResourcePercentage: 30,
233-
UserContainerLimitRequestFactor: 1.5,
234-
Protocol: routerConfig.UPI,
226+
BaseService: baseSvc,
227+
ContainerPort: 8080,
228+
MinReplicas: 1,
229+
MaxReplicas: 2,
230+
AutoscalingMetric: "concurrency",
231+
AutoscalingTarget: "1",
232+
IsClusterLocal: true,
233+
QueueProxyResourcePercentage: 30,
234+
UserContainerCPULimitRequestFactor: 1.5,
235+
UserContainerMemoryLimitRequestFactor: 1.5,
236+
Protocol: routerConfig.UPI,
235237
},
236238
expectedSpec: knservingv1.Service{
237239
ObjectMeta: metav1.ObjectMeta{
@@ -291,14 +293,15 @@ func TestBuildKnativeServiceConfig(t *testing.T) {
291293
},
292294
"annotations": {
293295
serviceCfg: KnativeService{
294-
BaseService: baseSvc,
295-
ContainerPort: 8080,
296-
MinReplicas: 5,
297-
MaxReplicas: 6,
298-
AutoscalingMetric: "memory",
299-
AutoscalingTarget: "70",
300-
IsClusterLocal: false,
301-
UserContainerLimitRequestFactor: 1.5,
296+
BaseService: baseSvc,
297+
ContainerPort: 8080,
298+
MinReplicas: 5,
299+
MaxReplicas: 6,
300+
AutoscalingMetric: "memory",
301+
AutoscalingTarget: "70",
302+
IsClusterLocal: false,
303+
UserContainerCPULimitRequestFactor: 1.5,
304+
UserContainerMemoryLimitRequestFactor: 1.5,
302305
},
303306
expectedSpec: knservingv1.Service{
304307
ObjectMeta: metav1.ObjectMeta{
@@ -381,10 +384,11 @@ func TestBuildKnativeServiceConfig(t *testing.T) {
381384
},
382385
},
383386
},
384-
IsClusterLocal: true,
385-
QueueProxyResourcePercentage: 30,
386-
UserContainerLimitRequestFactor: 1.5,
387-
Protocol: routerConfig.UPI,
387+
IsClusterLocal: true,
388+
QueueProxyResourcePercentage: 30,
389+
UserContainerCPULimitRequestFactor: 1.5,
390+
UserContainerMemoryLimitRequestFactor: 1.5,
391+
Protocol: routerConfig.UPI,
388392
},
389393
expectedSpec: knservingv1.Service{
390394
ObjectMeta: metav1.ObjectMeta{

‎api/turing/cluster/kubernetes_service.go‎

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,14 @@ import (
77
"k8s.io/apimachinery/pkg/util/intstr"
88
)
99

10-
// defaultLimitRequestFactor is the default multiplication factor applied to the resource request,
11-
// to be set as resource limit
12-
const defaultLimitRequestFactor = 1.0
10+
const (
11+
// defaultCPULimitRequestFactor is the default multiplication factor applied to the CPU request,
12+
// to be set as the limit
13+
defaultCPULimitRequestFactor = 1.0
14+
// defaultMemoryLimitRequestFactor is the default multiplication factor applied to the memory request,
15+
// to be set as the limit
16+
defaultMemoryLimitRequestFactor = 2.0
17+
)
1318

1419
// KubernetesService defines the properties for Kubernetes services
1520
type KubernetesService struct {
@@ -68,7 +73,7 @@ func (cfg *KubernetesService) buildDeployment(labels map[string]string) *appsv1.
6873
Args: cfg.Command,
6974
Ports: cfg.buildContainerPorts(),
7075
Env: cfg.Envs,
71-
Resources: cfg.buildResourceReqs(defaultLimitRequestFactor),
76+
Resources: cfg.buildResourceReqs(defaultCPULimitRequestFactor, defaultMemoryLimitRequestFactor),
7277
VolumeMounts: cfg.VolumeMounts,
7378
LivenessProbe: cfg.buildContainerProbe(livenessProbeType, int(cfg.ProbePort)),
7479
ReadinessProbe: cfg.buildContainerProbe(readinessProbeType, int(cfg.ProbePort)),

‎api/turing/cluster/kubernetes_service_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,7 @@ func TestBuildKubernetesServiceConfig(t *testing.T) {
110110
},
111111
Limits: map[corev1.ResourceName]resource.Quantity{
112112
corev1.ResourceCPU: resource.MustParse("1"),
113-
corev1.ResourceMemory: resource.MustParse("1"),
113+
corev1.ResourceMemory: resource.MustParse("2"),
114114
},
115115
},
116116
VolumeMounts: svcConf.VolumeMounts,

‎api/turing/cluster/models.go‎

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -62,16 +62,22 @@ type BaseService struct {
6262
InitContainers []Container `json:"init_containers"`
6363
}
6464

65-
func (cfg *BaseService) buildResourceReqs(userContainerLimitRequestFactor float64) corev1.ResourceRequirements {
65+
func (cfg *BaseService) buildResourceReqs(
66+
UserContainerCPULimitRequestFactor float64,
67+
UserContainerMemoryLimitRequestFactor float64,
68+
) corev1.ResourceRequirements {
6669
reqs := map[corev1.ResourceName]resource.Quantity{
6770
corev1.ResourceCPU: cfg.CPURequests,
6871
corev1.ResourceMemory: cfg.MemoryRequests,
6972
}
7073

71-
// Set resource limits to request * userContainerLimitRequestFactor
72-
limits := map[corev1.ResourceName]resource.Quantity{
73-
corev1.ResourceCPU: ComputeResource(cfg.CPURequests, userContainerLimitRequestFactor),
74-
corev1.ResourceMemory: ComputeResource(cfg.MemoryRequests, userContainerLimitRequestFactor),
74+
// Set resource limits to request * userContainerCPULimitRequestFactor or UserContainerMemoryLimitRequestFactor
75+
limits := map[corev1.ResourceName]resource.Quantity{}
76+
if UserContainerCPULimitRequestFactor != 0 {
77+
limits[corev1.ResourceCPU] = ComputeResource(cfg.CPURequests, UserContainerCPULimitRequestFactor)
78+
}
79+
if UserContainerMemoryLimitRequestFactor != 0 {
80+
limits[corev1.ResourceMemory] = ComputeResource(cfg.MemoryRequests, UserContainerMemoryLimitRequestFactor)
7581
}
7682

7783
return corev1.ResourceRequirements{

‎api/turing/cluster/servicebuilder/router.go‎

Lines changed: 14 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,8 @@ func (sb *clusterSvcBuilder) NewRouterService(
105105
sentryEnabled bool,
106106
sentryDSN string,
107107
knativeQueueProxyResourcePercentage int,
108-
userContainerLimitRequestFactor float64,
108+
userContainerCPULimitRequestFactor float64,
109+
userContainerMemoryLimitRequestFactor float64,
109110
initialScale *int,
110111
) (*cluster.KnativeService, error) {
111112
// Create service name
@@ -150,17 +151,18 @@ func (sb *clusterSvcBuilder) NewRouterService(
150151
VolumeMounts: volumeMounts,
151152
InitContainers: initContainers,
152153
},
153-
IsClusterLocal: false,
154-
ContainerPort: routerPort,
155-
Protocol: routerVersion.Protocol,
156-
MinReplicas: routerVersion.ResourceRequest.MinReplica,
157-
MaxReplicas: routerVersion.ResourceRequest.MaxReplica,
158-
InitialScale: initialScale,
159-
AutoscalingMetric: string(routerVersion.AutoscalingPolicy.Metric),
160-
AutoscalingTarget: routerVersion.AutoscalingPolicy.Target,
161-
TopologySpreadConstraints: topologySpreadConstraints,
162-
QueueProxyResourcePercentage: knativeQueueProxyResourcePercentage,
163-
UserContainerLimitRequestFactor: userContainerLimitRequestFactor,
154+
IsClusterLocal: false,
155+
ContainerPort: routerPort,
156+
Protocol: routerVersion.Protocol,
157+
MinReplicas: routerVersion.ResourceRequest.MinReplica,
158+
MaxReplicas: routerVersion.ResourceRequest.MaxReplica,
159+
InitialScale: initialScale,
160+
AutoscalingMetric: string(routerVersion.AutoscalingPolicy.Metric),
161+
AutoscalingTarget: routerVersion.AutoscalingPolicy.Target,
162+
TopologySpreadConstraints: topologySpreadConstraints,
163+
QueueProxyResourcePercentage: knativeQueueProxyResourcePercentage,
164+
UserContainerCPULimitRequestFactor: userContainerCPULimitRequestFactor,
165+
UserContainerMemoryLimitRequestFactor: userContainerMemoryLimitRequestFactor,
164166
}
165167
return sb.validateKnativeService(svc)
166168
}

0 commit comments

Comments
 (0)