From 9dd01d09383607f7f2cbf8df9253638b758e1bda Mon Sep 17 00:00:00 2001 From: Stephanie Date: Thu, 14 Oct 2021 13:33:20 -0400 Subject: [PATCH 1/3] add more resource requirement in generator Signed-off-by: Stephanie --- go.mod | 2 +- go.sum | 2 ++ pkg/devfile/generator/utils.go | 37 +++++++++++++++++++++++++---- pkg/devfile/generator/utils_test.go | 27 ++++++++++++++++----- 4 files changed, 56 insertions(+), 12 deletions(-) diff --git a/go.mod b/go.mod index 367eb9fb..a02ef022 100644 --- a/go.mod +++ b/go.mod @@ -3,7 +3,7 @@ module github.com/devfile/library go 1.15 require ( - github.com/devfile/api/v2 v2.0.0-20210917193329-089a48011460 + github.com/devfile/api/v2 v2.0.0-20211012194348-adf74d82f446 github.com/fatih/color v1.7.0 github.com/gobwas/glob v0.2.3 github.com/golang/mock v1.5.0 diff --git a/go.sum b/go.sum index 11acfdcb..4da199a3 100644 --- a/go.sum +++ b/go.sum @@ -85,6 +85,8 @@ github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/devfile/api/v2 v2.0.0-20210917193329-089a48011460 h1:cmd+3poyUwevcWchYdvE02YT1nQU4SJpA5/wrdLrpWE= github.com/devfile/api/v2 v2.0.0-20210917193329-089a48011460/go.mod h1:kLX/nW93gigOHXK3NLeJL2fSS/sgEe+OHu8bo3aoOi4= +github.com/devfile/api/v2 v2.0.0-20211012194348-adf74d82f446 h1:4/lT2y37QMUQpqF0JnYPy2JkJ58X/dHEwr3goiWbzD4= +github.com/devfile/api/v2 v2.0.0-20211012194348-adf74d82f446/go.mod h1:d99eTN6QxgzihOOFyOZA+VpUyD4Q1pYRYHZ/ci9J96Q= github.com/dgrijalva/jwt-go v3.2.0+incompatible/go.mod h1:E3ru+11k8xSBh+hMPgOLZmtrrCbhqsmaPHjLKYnJCaQ= github.com/dgryski/go-sip13 v0.0.0-20181026042036-e10d5fee7954/go.mod h1:vAd38F8PWV+bWy6jNmig1y/TA+kYO4g3RSRF0IAv0no= github.com/docker/spdystream v0.0.0-20160310174837-449fdfce4d96/go.mod h1:Qh8CwZgvJUkLughtfhJv5dyTYa91l1fOUCrgjqmcifM= diff --git a/pkg/devfile/generator/utils.go b/pkg/devfile/generator/utils.go index f6583bb5..fd0b1480 100644 --- a/pkg/devfile/generator/utils.go +++ b/pkg/devfile/generator/utils.go @@ -3,6 +3,7 @@ package generator import ( "fmt" "path/filepath" + "reflect" "strings" v1 "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2" @@ -61,12 +62,38 @@ func convertPorts(endpoints []v1.Endpoint) []corev1.ContainerPort { func getResourceReqs(comp v1.Component) corev1.ResourceRequirements { reqs := corev1.ResourceRequirements{} limits := make(corev1.ResourceList) - if comp.Container != nil && comp.Container.MemoryLimit != "" { - memoryLimit, err := resource.ParseQuantity(comp.Container.MemoryLimit) - if err == nil { - limits[corev1.ResourceMemory] = memoryLimit + requests := make(corev1.ResourceList) + if comp.Container != nil { + if comp.Container.MemoryLimit != "" { + memoryLimit, err := resource.ParseQuantity(comp.Container.MemoryLimit) + if err == nil { + limits[corev1.ResourceMemory] = memoryLimit + } + } + if comp.Container.CpuLimit != "" { + cpuLimit, err := resource.ParseQuantity(comp.Container.CpuLimit) + if err == nil { + limits[corev1.ResourceCPU] = cpuLimit + } + } + if comp.Container.MemoryRequest != "" { + memoryRequest, err := resource.ParseQuantity(comp.Container.MemoryRequest) + if err == nil { + requests[corev1.ResourceMemory] = memoryRequest + } + } + if comp.Container.CpuLimit != "" { + cpuRequest, err := resource.ParseQuantity(comp.Container.CpuRequest) + if err == nil { + requests[corev1.ResourceCPU] = cpuRequest + } + } + if !reflect.DeepEqual(limits, corev1.ResourceList{}) { + reqs.Limits = limits + } + if !reflect.DeepEqual(requests, corev1.ResourceList{}) { + reqs.Requests = requests } - reqs.Limits = limits } return reqs } diff --git a/pkg/devfile/generator/utils_test.go b/pkg/devfile/generator/utils_test.go index 15f0353b..eda8749d 100644 --- a/pkg/devfile/generator/utils_test.go +++ b/pkg/devfile/generator/utils_test.go @@ -162,8 +162,15 @@ func TestConvertPorts(t *testing.T) { } func TestGetResourceReqs(t *testing.T) { - limit := "1024Mi" - quantity, err := resource.ParseQuantity(limit) + memoryLimit := "1024Mi" + memoryRequest := "1Gi" + cpuRequest := "1m" + cpuLimit := "1m" + + memoryLimitQuantity, err := resource.ParseQuantity(memoryLimit) + memoryRequestQuantity, err := resource.ParseQuantity(memoryRequest) + cpuRequestQuantity, err := resource.ParseQuantity(cpuRequest) + cpuLimitQuantity, err := resource.ParseQuantity(cpuLimit) if err != nil { t.Errorf("TestGetResourceReqs() unexpected error: %v", err) } @@ -173,20 +180,28 @@ func TestGetResourceReqs(t *testing.T) { want corev1.ResourceRequirements }{ { - name: "One Endpoint", + name: "generate resource limit", component: v1.Component{ Name: "testcomponent", ComponentUnion: v1.ComponentUnion{ Container: &v1.ContainerComponent{ Container: v1.Container{ - MemoryLimit: "1024Mi", + MemoryLimit: memoryLimit, + MemoryRequest: memoryRequest, + CpuRequest: cpuRequest, + CpuLimit: cpuLimit, }, }, }, }, want: corev1.ResourceRequirements{ Limits: corev1.ResourceList{ - corev1.ResourceMemory: quantity, + corev1.ResourceMemory: memoryLimitQuantity, + corev1.ResourceCPU: cpuLimitQuantity, + }, + Requests: corev1.ResourceList{ + corev1.ResourceMemory: memoryRequestQuantity, + corev1.ResourceCPU: cpuRequestQuantity, }, }, }, @@ -215,7 +230,7 @@ func TestGetResourceReqs(t *testing.T) { t.Run(tt.name, func(t *testing.T) { req := getResourceReqs(tt.component) if !reflect.DeepEqual(tt.want, req) { - t.Errorf("TestGetResourceReqs() error: expected %v, wanted %v", req, tt.want) + assert.Equal(t, tt.want, req, "TestGetResourceReqs(): The two values should be the same.") } }) } From 2af1e12a36023e674ca815f5c0ee2ef5af95fb14 Mon Sep 17 00:00:00 2001 From: Stephanie Date: Thu, 14 Oct 2021 13:50:43 -0400 Subject: [PATCH 2/3] run go mod Signed-off-by: Stephanie --- go.sum | 2 -- pkg/devfile/generator/utils_test.go | 10 +++++----- 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/go.sum b/go.sum index 4da199a3..faa61676 100644 --- a/go.sum +++ b/go.sum @@ -83,8 +83,6 @@ github.com/creack/pty v1.1.11/go.mod h1:oKZEueFk5CKHvIhNR5MUki03XCEU+Q6VDXinZuGJ github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= -github.com/devfile/api/v2 v2.0.0-20210917193329-089a48011460 h1:cmd+3poyUwevcWchYdvE02YT1nQU4SJpA5/wrdLrpWE= -github.com/devfile/api/v2 v2.0.0-20210917193329-089a48011460/go.mod h1:kLX/nW93gigOHXK3NLeJL2fSS/sgEe+OHu8bo3aoOi4= github.com/devfile/api/v2 v2.0.0-20211012194348-adf74d82f446 h1:4/lT2y37QMUQpqF0JnYPy2JkJ58X/dHEwr3goiWbzD4= github.com/devfile/api/v2 v2.0.0-20211012194348-adf74d82f446/go.mod h1:d99eTN6QxgzihOOFyOZA+VpUyD4Q1pYRYHZ/ci9J96Q= github.com/dgrijalva/jwt-go v3.2.0+incompatible/go.mod h1:E3ru+11k8xSBh+hMPgOLZmtrrCbhqsmaPHjLKYnJCaQ= diff --git a/pkg/devfile/generator/utils_test.go b/pkg/devfile/generator/utils_test.go index eda8749d..63508ef3 100644 --- a/pkg/devfile/generator/utils_test.go +++ b/pkg/devfile/generator/utils_test.go @@ -186,10 +186,10 @@ func TestGetResourceReqs(t *testing.T) { ComponentUnion: v1.ComponentUnion{ Container: &v1.ContainerComponent{ Container: v1.Container{ - MemoryLimit: memoryLimit, + MemoryLimit: memoryLimit, MemoryRequest: memoryRequest, - CpuRequest: cpuRequest, - CpuLimit: cpuLimit, + CpuRequest: cpuRequest, + CpuLimit: cpuLimit, }, }, }, @@ -197,11 +197,11 @@ func TestGetResourceReqs(t *testing.T) { want: corev1.ResourceRequirements{ Limits: corev1.ResourceList{ corev1.ResourceMemory: memoryLimitQuantity, - corev1.ResourceCPU: cpuLimitQuantity, + corev1.ResourceCPU: cpuLimitQuantity, }, Requests: corev1.ResourceList{ corev1.ResourceMemory: memoryRequestQuantity, - corev1.ResourceCPU: cpuRequestQuantity, + corev1.ResourceCPU: cpuRequestQuantity, }, }, }, From fd79c14f5c8cad975f20f21bef30d89167bfc2c7 Mon Sep 17 00:00:00 2001 From: Stephanie Date: Thu, 14 Oct 2021 16:20:25 -0400 Subject: [PATCH 3/3] add error check to getResoureReqs Signed-off-by: Stephanie --- pkg/devfile/generator/utils.go | 33 ++++++++++++++++++++++------- pkg/devfile/generator/utils_test.go | 33 +++++++++++++++++++++++++++-- 2 files changed, 56 insertions(+), 10 deletions(-) diff --git a/pkg/devfile/generator/utils.go b/pkg/devfile/generator/utils.go index fd0b1480..8487a2cf 100644 --- a/pkg/devfile/generator/utils.go +++ b/pkg/devfile/generator/utils.go @@ -2,6 +2,7 @@ package generator import ( "fmt" + "github.com/hashicorp/go-multierror" "path/filepath" "reflect" "strings" @@ -59,32 +60,45 @@ func convertPorts(endpoints []v1.Endpoint) []corev1.ContainerPort { } // getResourceReqs creates a kubernetes ResourceRequirements object based on resource requirements set in the devfile -func getResourceReqs(comp v1.Component) corev1.ResourceRequirements { +func getResourceReqs(comp v1.Component) (corev1.ResourceRequirements, error) { reqs := corev1.ResourceRequirements{} limits := make(corev1.ResourceList) requests := make(corev1.ResourceList) + var returnedErr error if comp.Container != nil { if comp.Container.MemoryLimit != "" { memoryLimit, err := resource.ParseQuantity(comp.Container.MemoryLimit) - if err == nil { + if err != nil { + errMsg := fmt.Errorf("error parsing memoryLimit requirement for component %s: %v", comp.Name, err.Error()) + returnedErr = multierror.Append(returnedErr, errMsg) + } else { limits[corev1.ResourceMemory] = memoryLimit } } if comp.Container.CpuLimit != "" { cpuLimit, err := resource.ParseQuantity(comp.Container.CpuLimit) - if err == nil { + if err != nil { + errMsg := fmt.Errorf("error parsing cpuLimit requirement for component %s: %v", comp.Name, err.Error()) + returnedErr = multierror.Append(returnedErr, errMsg) + } else { limits[corev1.ResourceCPU] = cpuLimit } } if comp.Container.MemoryRequest != "" { memoryRequest, err := resource.ParseQuantity(comp.Container.MemoryRequest) - if err == nil { + if err != nil { + errMsg := fmt.Errorf("error parsing memoryRequest requirement for component %s: %v", comp.Name, err.Error()) + returnedErr = multierror.Append(returnedErr, errMsg) + } else { requests[corev1.ResourceMemory] = memoryRequest } } - if comp.Container.CpuLimit != "" { + if comp.Container.CpuRequest != "" { cpuRequest, err := resource.ParseQuantity(comp.Container.CpuRequest) - if err == nil { + if err != nil { + errMsg := fmt.Errorf("error parsing cpuRequest requirement for component %s: %v", comp.Name, err.Error()) + returnedErr = multierror.Append(returnedErr, errMsg) + } else { requests[corev1.ResourceCPU] = cpuRequest } } @@ -95,7 +109,7 @@ func getResourceReqs(comp v1.Component) corev1.ResourceRequirements { reqs.Requests = requests } } - return reqs + return reqs, returnedErr } // addSyncRootFolder adds the sync root folder to the container env @@ -544,7 +558,10 @@ func getAllContainers(devfileObj parser.DevfileObj, options common.DevfileOption } for _, comp := range containerComponents { envVars := convertEnvs(comp.Container.Env) - resourceReqs := getResourceReqs(comp) + resourceReqs, err := getResourceReqs(comp) + if err != nil { + return containers, err + } ports := convertPorts(comp.Container.Endpoints) containerParams := containerParams{ Name: comp.Name, diff --git a/pkg/devfile/generator/utils_test.go b/pkg/devfile/generator/utils_test.go index 63508ef3..b547c5f5 100644 --- a/pkg/devfile/generator/utils_test.go +++ b/pkg/devfile/generator/utils_test.go @@ -1,6 +1,7 @@ package generator import ( + "github.com/hashicorp/go-multierror" "github.com/stretchr/testify/assert" "path/filepath" "reflect" @@ -178,6 +179,7 @@ func TestGetResourceReqs(t *testing.T) { name string component v1.Component want corev1.ResourceRequirements + wantErr []string }{ { name: "generate resource limit", @@ -224,12 +226,39 @@ func TestGetResourceReqs(t *testing.T) { }, want: corev1.ResourceRequirements{}, }, + { + name: "test error case", + component: v1.Component{ + Name: "testcomponent", + ComponentUnion: v1.ComponentUnion{ + Container: &v1.ContainerComponent{ + Container: v1.Container{ + MemoryLimit: "invalid", + MemoryRequest: "invalid", + CpuRequest: "invalid", + CpuLimit: "invalid", + }, + }, + }, + }, + wantErr: []string{ + "error parsing memoryLimit requirement.*", + "error parsing cpuLimit requirement.*", + "error parsing memoryRequest requirement.*", + "error parsing cpuRequest requirement.*", + }, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - req := getResourceReqs(tt.component) - if !reflect.DeepEqual(tt.want, req) { + req, err := getResourceReqs(tt.component) + if merr, ok := err.(*multierror.Error); ok && tt.wantErr != nil { + assert.Equal(t, len(tt.wantErr), len(merr.Errors), "Error list length should match") + for i := 0; i < len(merr.Errors); i++ { + assert.Regexp(t, tt.wantErr[i], merr.Errors[i].Error(), "Error message should match") + } + } else if !reflect.DeepEqual(tt.want, req) { assert.Equal(t, tt.want, req, "TestGetResourceReqs(): The two values should be the same.") } })