Skip to content

Commit

Permalink
proposal: more comments needed to explain in injectForOrigin function
Browse files Browse the repository at this point in the history 
Signed-off-by: tiana <917803833@qq.com>
  • Loading branch information
@Tiana2018
Tiana2018 committed May 6, 2023
1 parent 36c99b7 commit cbb0d44
Show file tree
Hide file tree
Showing 2 changed files with 112 additions and 4 deletions.
6 changes: 6 additions & 0 deletions pkg/koordlet/runtimehooks/protocol/container_context.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,10 @@ func (c *ContainerContext) ReconcilerDone(executor resourceexecutor.ResourceUpda
c.injectForOrigin()
}

// Inject valid parameters in ContainerContext.Response.Resources,
// such as CPUShares, CPUSet, CFSQuota, MemoryLimit...
func (c *ContainerContext) injectForOrigin() {
// If CPUShares is not nil, set container cpu share
if c.Response.Resources.CPUShares != nil {
eventHelper := audit.V(3).Container(c.Request.ContainerMeta.ID).Reason("runtime-hooks").Message(
"set container cpu share to %v", *c.Response.Resources.CPUShares)
Expand All @@ -205,6 +208,7 @@ func (c *ContainerContext) injectForOrigin() {
"set container cpu share to %v", *c.Response.Resources.CPUShares).Do()
}
}
// If CPUSet is not nil and is not an empty string, set container cpuset
if c.Response.Resources.CPUSet != nil && *c.Response.Resources.CPUSet != "" {
eventHelper := audit.V(3).Container(c.Request.ContainerMeta.ID).Reason("runtime-hooks").Message("set container cpuset to %v", *c.Response.Resources.CPUSet)
err := injectCPUSet(c.Request.CgroupParent, *c.Response.Resources.CPUSet, eventHelper, c.executor)
Expand All @@ -220,6 +224,7 @@ func (c *ContainerContext) injectForOrigin() {
*c.Response.Resources.CPUSet, c.Request.CgroupParent)
}
}
// If CFSQuota is not nil, set container cfs quota
if c.Response.Resources.CFSQuota != nil {
eventHelper := audit.V(3).Container(c.Request.ContainerMeta.ID).Reason("runtime-hooks").Message(
"set container cfs quota to %v", *c.Response.Resources.CFSQuota)
Expand All @@ -232,6 +237,7 @@ func (c *ContainerContext) injectForOrigin() {
*c.Response.Resources.CFSQuota, c.Request.CgroupParent)
}
}
// If MemoryLimit is not nil, set container memory limit
if c.Response.Resources.MemoryLimit != nil {
eventHelper := audit.V(3).Container(c.Request.ContainerMeta.ID).Reason("runtime-hooks").Message(
"set container memory limit to %v", *c.Response.Resources.MemoryLimit)
Expand Down
110 changes: 106 additions & 4 deletions pkg/koordlet/runtimehooks/protocol/protocol_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,14 @@ limitations under the License.
package protocol

import (
"sync"
"testing"

"github.com/stretchr/testify/assert"
"k8s.io/utils/pointer"

runtimeapi "github.com/koordinator-sh/koordinator/apis/runtime/v1alpha1"
"github.com/koordinator-sh/koordinator/pkg/koordlet/resourceexecutor"
)

func TestResources_IsOriginResSet(t *testing.T) {
Expand Down Expand Up @@ -76,7 +78,11 @@ func TestContainerResponse_ProxyDone(t *testing.T) {
resp *runtimeapi.ContainerResourceHookResponse
}
type wants struct {
CPUSet *string
CPUSet *string
CPUShares *int64
CFSQuota *int64
MemoryLimit *int64
CPUBvt *int64
}
tests := []struct {
name string
Expand All @@ -85,19 +91,28 @@ func TestContainerResponse_ProxyDone(t *testing.T) {
wants wants
}{
{
name: "origin resource is nil",
name: "origin resource is not nil",
fields: fields{
Resources: Resources{
CPUSet: pointer.String("0,1,2"),
CPUShares: pointer.Int64(15),
CFSQuota: pointer.Int64(1000),
CPUSet: pointer.String("0,1,2"),
MemoryLimit: pointer.Int64(1048576),
CPUBvt: pointer.Int64(10),
},
ContainerEnvs: make(map[string]string, 0),
},
args: args{
resp: &runtimeapi.ContainerResourceHookResponse{
ContainerResources: nil,
},
},
wants: wants{
CPUSet: pointer.String("0,1,2"),
CPUSet: pointer.String("0,1,2"),
CPUShares: pointer.Int64(15),
CFSQuota: pointer.Int64(1000),
MemoryLimit: pointer.Int64(1048576),
CPUBvt: pointer.Int64(10),
},
},
}
Expand All @@ -109,6 +124,10 @@ func TestContainerResponse_ProxyDone(t *testing.T) {
}
c.ProxyDone(tt.args.resp)
assert.Equal(t, tt.wants.CPUSet, c.Resources.CPUSet, "cpu set equal")
assert.Equal(t, tt.wants.CPUBvt, c.Resources.CPUBvt, "cpu bvt equal")
assert.Equal(t, tt.wants.CPUShares, c.Resources.CPUShares, "cpu shares equal")
assert.Equal(t, tt.wants.CFSQuota, c.Resources.CFSQuota, "cfs quota equal")
assert.Equal(t, tt.wants.MemoryLimit, c.Resources.MemoryLimit, "memory limit equal")
})
}
}
Expand Down Expand Up @@ -156,3 +175,86 @@ func TestPodResponse_ProxyDone(t *testing.T) {
})
}
}

func TestPodResponse_ReconcilerDone(t *testing.T) {

type wants struct {
CPUSet *string
CPUShares *int64
CFSQuota *int64
MemoryLimit *int64
CPUBvt *int64
}
var tests = []struct {
name string
ex *resourceexecutor.ResourceUpdateExecutorImpl
req ContainerRequest
res ContainerResponse
wants wants
}{
{
name: "req's parent is nil",
ex: &resourceexecutor.ResourceUpdateExecutorImpl{
LeveledUpdateLock: sync.Mutex{},
ResourceCache: nil,
Config: nil,
},
req: ContainerRequest{},
},
{
name: "test injectForExt nil",
ex: &resourceexecutor.ResourceUpdateExecutorImpl{
LeveledUpdateLock: sync.Mutex{},
ResourceCache: nil,
Config: nil,
},
req: ContainerRequest{
CgroupParent: "test",
},
res: ContainerResponse{
Resources: Resources{},
AddContainerEnvs: nil,
},
},
{
name: "test injectForExt",
ex: &resourceexecutor.ResourceUpdateExecutorImpl{
LeveledUpdateLock: sync.Mutex{},
ResourceCache: nil,
Config: nil,
},
req: ContainerRequest{
CgroupParent: "test",
},
res: ContainerResponse{
Resources: Resources{
CPUSet: pointer.String(""),
CPUShares: pointer.Int64(15),
CFSQuota: pointer.Int64(1000),
MemoryLimit: pointer.Int64(1048576),
CPUBvt: pointer.Int64(10),
},
AddContainerEnvs: nil,
},
wants: wants{
CPUSet: pointer.String(""),
CPUShares: pointer.Int64(15),
CFSQuota: pointer.Int64(1000),
MemoryLimit: pointer.Int64(1048576),
CPUBvt: pointer.Int64(10),
},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {

c := ContainerContext{
Request: tt.req,
Response: tt.res,
executor: nil,
}
c.ReconcilerDone(tt.ex)

})
}
}

0 comments on commit cbb0d44

Please sign in to comment.