Skip to content

Commit

Permalink
Refactor cpu limit as nullable field
Browse files Browse the repository at this point in the history
  • Loading branch information
deadlycoconuts committed May 24, 2024
1 parent a255f43 commit ac9cb63
Show file tree
Hide file tree
Showing 5 changed files with 9 additions and 25 deletions.
8 changes: 4 additions & 4 deletions api/cluster/resource/templater.go
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,7 @@ func (t *InferenceServiceTemplater) createPredictorSpec(modelService *models.Ser
limits := map[corev1.ResourceName]resource.Quantity{}

// Set cpu resource limits automatically if they have not been set
if modelService.ResourceRequest.CPULimit.IsZero() {
if modelService.ResourceRequest.CPULimit == nil || modelService.ResourceRequest.CPULimit.IsZero() {
if t.deploymentConfig.UserContainerCPULimitRequestFactor != 0 {
limits[corev1.ResourceCPU] = ScaleQuantity(
modelService.ResourceRequest.CPURequest, t.deploymentConfig.UserContainerCPULimitRequestFactor,
Expand All @@ -214,7 +214,7 @@ func (t *InferenceServiceTemplater) createPredictorSpec(modelService *models.Ser
envVars = models.MergeEnvVars(ParseEnvVars(t.deploymentConfig.DefaultEnvVarsWithoutCPULimits), envVars)
}
} else {
limits[corev1.ResourceCPU] = modelService.ResourceRequest.CPULimit
limits[corev1.ResourceCPU] = *modelService.ResourceRequest.CPULimit
}

if t.deploymentConfig.UserContainerMemoryLimitRequestFactor != 0 {
Expand Down Expand Up @@ -406,7 +406,7 @@ func (t *InferenceServiceTemplater) createTransformerSpec(
// Set resource limits to request * userContainerCPULimitRequestFactor or UserContainerMemoryLimitRequestFactor
limits := map[corev1.ResourceName]resource.Quantity{}
// Set cpu resource limits automatically if they have not been set
if transformer.ResourceRequest.CPULimit.IsZero() {
if transformer.ResourceRequest.CPULimit == nil || transformer.ResourceRequest.CPULimit.IsZero() {
if t.deploymentConfig.UserContainerCPULimitRequestFactor != 0 {
limits[corev1.ResourceCPU] = ScaleQuantity(
transformer.ResourceRequest.CPURequest, t.deploymentConfig.UserContainerCPULimitRequestFactor,
Expand All @@ -422,7 +422,7 @@ func (t *InferenceServiceTemplater) createTransformerSpec(
envVars = models.MergeEnvVars(ParseEnvVars(t.deploymentConfig.DefaultEnvVarsWithoutCPULimits), envVars)
}
} else {
limits[corev1.ResourceCPU] = transformer.ResourceRequest.CPULimit
limits[corev1.ResourceCPU] = *transformer.ResourceRequest.CPULimit
}

if t.deploymentConfig.UserContainerMemoryLimitRequestFactor != 0 {
Expand Down
7 changes: 4 additions & 3 deletions api/cluster/resource/templater_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,12 +93,13 @@ var (
MemoryRequest: resource.MustParse("1Gi"),
}

cpuLimit = resource.MustParse("8")
userResourceRequestsWithCPULimits = &models.ResourceRequest{
MinReplica: 1,
MaxReplica: 10,
CPURequest: resource.MustParse("1"),
MemoryRequest: resource.MustParse("1Gi"),
CPULimit: resource.MustParse("8"),
CPULimit: &cpuLimit,
}

expUserResourceRequests = corev1.ResourceRequirements{
Expand All @@ -118,7 +119,7 @@ var (
corev1.ResourceMemory: userResourceRequestsWithCPULimits.MemoryRequest,
},
Limits: corev1.ResourceList{
corev1.ResourceCPU: userResourceRequestsWithCPULimits.CPULimit,
corev1.ResourceCPU: *userResourceRequestsWithCPULimits.CPULimit,
corev1.ResourceMemory: ScaleQuantity(userResourceRequestsWithCPULimits.MemoryRequest, 2),
},
}
Expand Down Expand Up @@ -4627,7 +4628,7 @@ func TestCreateTransformerSpec(t *testing.T) {
MaxReplica: 1,
CPURequest: cpuRequest,
MemoryRequest: memoryRequest,
CPULimit: customCPULimit,
CPULimit: &customCPULimit,
},
EnvVars: models.EnvVars{
{Name: transformerpkg.JaegerCollectorURL, Value: "NEW_HOST"}, // test user overwrite
Expand Down
2 changes: 1 addition & 1 deletion api/models/resource_request.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ type ResourceRequest struct {
// CPU request of inference service
CPURequest resource.Quantity `json:"cpu_request"`
// CPU limit of inference service
CPULimit resource.Quantity `json:"cpu_limit"`
CPULimit *resource.Quantity `json:"cpu_limit,omitempty"`
// Memory request of inference service
MemoryRequest resource.Quantity `json:"memory_request"`
// GPU name
Expand Down
3 changes: 0 additions & 3 deletions api/queue/work/model_service_deployment_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ func TestExecuteDeployment(t *testing.T) {
MinReplica: 0,
MaxReplica: 1,
CPURequest: resource.MustParse("1"),
CPULimit: resource.MustParse("0"),
MemoryRequest: resource.MustParse("1Gi"),
GPURequest: resource.MustParse("0"),
},
Expand Down Expand Up @@ -535,7 +534,6 @@ func TestExecuteDeployment(t *testing.T) {
MinReplica: 0,
MaxReplica: 1,
CPURequest: resource.MustParse("1"),
CPULimit: resource.MustParse("0"),
MemoryRequest: resource.MustParse("1Gi"),
GPUName: "NVIDIA P4",
GPURequest: resource.MustParse("1"),
Expand Down Expand Up @@ -743,7 +741,6 @@ func TestExecuteRedeployment(t *testing.T) {
MinReplica: 0,
MaxReplica: 1,
CPURequest: resource.MustParse("1"),
CPULimit: resource.MustParse("0"),
MemoryRequest: resource.MustParse("1Gi"),
GPURequest: resource.MustParse("0"),
},
Expand Down
14 changes: 0 additions & 14 deletions api/service/environment_service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,6 @@ func TestGetEnvironment(t *testing.T) {
MinReplica: 1,
MaxReplica: 4,
CPURequest: resource.MustParse("1"),
CPULimit: resource.MustParse("0"),
MemoryRequest: resource.MustParse("1Gi"),
GPURequest: resource.MustParse("0"),
},
Expand All @@ -224,7 +223,6 @@ func TestGetEnvironment(t *testing.T) {
MinReplica: 1,
MaxReplica: 4,
CPURequest: resource.MustParse("1"),
CPULimit: resource.MustParse("0"),
MemoryRequest: resource.MustParse("1Gi"),
GPURequest: resource.MustParse("0"),
},
Expand Down Expand Up @@ -255,7 +253,6 @@ func TestGetEnvironment(t *testing.T) {
MinReplica: 1,
MaxReplica: 4,
CPURequest: resource.MustParse("1"),
CPULimit: resource.MustParse("0"),
MemoryRequest: resource.MustParse("1Gi"),
},
CreatedUpdated: models.CreatedUpdated{
Expand Down Expand Up @@ -323,7 +320,6 @@ func TestGetDefaultEnvironment(t *testing.T) {
MinReplica: 1,
MaxReplica: 4,
CPURequest: resource.MustParse("1"),
CPULimit: resource.MustParse("0"),
MemoryRequest: resource.MustParse("1Gi"),
GPURequest: resource.MustParse("0"),
},
Expand All @@ -350,7 +346,6 @@ func TestGetDefaultEnvironment(t *testing.T) {
MinReplica: 1,
MaxReplica: 4,
CPURequest: resource.MustParse("1"),
CPULimit: resource.MustParse("0"),
MemoryRequest: resource.MustParse("1Gi"),
GPURequest: resource.MustParse("0"),
},
Expand Down Expand Up @@ -381,7 +376,6 @@ func TestGetDefaultEnvironment(t *testing.T) {
MinReplica: 1,
MaxReplica: 4,
CPURequest: resource.MustParse("1"),
CPULimit: resource.MustParse("0"),
MemoryRequest: resource.MustParse("1Gi"),
},
CreatedUpdated: models.CreatedUpdated{
Expand Down Expand Up @@ -449,7 +443,6 @@ func TestGetDefaultPredictionJobEnvironment(t *testing.T) {
MinReplica: 1,
MaxReplica: 4,
CPURequest: resource.MustParse("1"),
CPULimit: resource.MustParse("0"),
MemoryRequest: resource.MustParse("1Gi"),
GPURequest: resource.MustParse("0"),
},
Expand All @@ -476,7 +469,6 @@ func TestGetDefaultPredictionJobEnvironment(t *testing.T) {
MinReplica: 1,
MaxReplica: 4,
CPURequest: resource.MustParse("1"),
CPULimit: resource.MustParse("0"),
MemoryRequest: resource.MustParse("1Gi"),
GPURequest: resource.MustParse("0"),
},
Expand Down Expand Up @@ -507,7 +499,6 @@ func TestGetDefaultPredictionJobEnvironment(t *testing.T) {
MinReplica: 1,
MaxReplica: 4,
CPURequest: resource.MustParse("1"),
CPULimit: resource.MustParse("0"),
MemoryRequest: resource.MustParse("1Gi"),
},
CreatedUpdated: models.CreatedUpdated{
Expand Down Expand Up @@ -576,7 +567,6 @@ func TestListEnvironment(t *testing.T) {
MinReplica: 1,
MaxReplica: 4,
CPURequest: resource.MustParse("1"),
CPULimit: resource.MustParse("0"),
MemoryRequest: resource.MustParse("1Gi"),
GPURequest: resource.MustParse("0"),
},
Expand Down Expand Up @@ -604,7 +594,6 @@ func TestListEnvironment(t *testing.T) {
MinReplica: 1,
MaxReplica: 4,
CPURequest: resource.MustParse("1"),
CPULimit: resource.MustParse("0"),
MemoryRequest: resource.MustParse("1Gi"),
GPURequest: resource.MustParse("0"),
},
Expand Down Expand Up @@ -638,7 +627,6 @@ func TestListEnvironment(t *testing.T) {
MinReplica: 1,
MaxReplica: 4,
CPURequest: resource.MustParse("1"),
CPULimit: resource.MustParse("0"),
MemoryRequest: resource.MustParse("1Gi"),
GPURequest: resource.MustParse("0"),
},
Expand Down Expand Up @@ -666,7 +654,6 @@ func TestListEnvironment(t *testing.T) {
MinReplica: 1,
MaxReplica: 4,
CPURequest: resource.MustParse("1"),
CPULimit: resource.MustParse("0"),
MemoryRequest: resource.MustParse("1Gi"),
GPURequest: resource.MustParse("0"),
},
Expand Down Expand Up @@ -700,7 +687,6 @@ func TestListEnvironment(t *testing.T) {
MinReplica: 1,
MaxReplica: 4,
CPURequest: resource.MustParse("1"),
CPULimit: resource.MustParse("0"),
MemoryRequest: resource.MustParse("1Gi"),
},
CreatedUpdated: models.CreatedUpdated{
Expand Down

0 comments on commit ac9cb63

Please sign in to comment.