Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

koordlet: add nri remove #2046

Merged
merged 1 commit into from
May 21, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions pkg/koordlet/runtimehooks/hooks/hooks.go
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,7 @@ func init() {
rmconfig.PostStopContainer: make([]*Hook, 0),
rmconfig.PostStopPodSandbox: make([]*Hook, 0),
rmconfig.PreUpdateContainerResources: make([]*Hook, 0),
rmconfig.PreRemoveRunPodSandbox: make([]*Hook, 0),
}
}

Expand Down
20 changes: 19 additions & 1 deletion pkg/koordlet/runtimehooks/nri/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ type NriServer struct {
}

const (
events = "RunPodSandbox,CreateContainer,UpdateContainer"
events = "RunPodSandbox,RemovePodSandbox,CreateContainer,UpdateContainer"
pluginName = "koordlet_nri"
pluginIdx = "00"
)
Expand All @@ -78,6 +78,7 @@ var (
_ = stub.ConfigureInterface(&NriServer{})
_ = stub.SynchronizeInterface(&NriServer{})
_ = stub.RunPodInterface(&NriServer{})
_ = stub.RemovePodInterface(&NriServer{})
_ = stub.CreateContainerInterface(&NriServer{})
_ = stub.UpdateContainerInterface(&NriServer{})
opts []stub.Option
Expand Down Expand Up @@ -227,6 +228,23 @@ func (p *NriServer) UpdateContainer(pod *api.PodSandbox, container *api.Containe
return []*api.ContainerUpdate{update}, nil
}

func (p *NriServer) RemovePodSandbox(pod *api.PodSandbox) error {
saintube marked this conversation as resolved.
Show resolved Hide resolved
podCtx := &protocol.PodContext{}
podCtx.FromNri(pod)
// todo: return error or bypass error based on PluginFailurePolicy
err := hooks.RunHooks(p.options.PluginFailurePolicy, rmconfig.PreRemoveRunPodSandbox, podCtx)
if err != nil {
klog.Errorf("nri hooks run error: %v", err)
if p.options.PluginFailurePolicy == rmconfig.PolicyFail {
return err
}
}
podCtx.NriRemoveDone(p.options.Executor)

klog.V(6).Infof("handle NRI RemovePodSandbox successfully, pod %s/%s", pod.GetNamespace(), pod.GetName())
return nil
}

func (p *NriServer) onClose() {
//TODO: consider the pod status during restart
retryFunc := func() (bool, error) {
Expand Down
98 changes: 98 additions & 0 deletions pkg/koordlet/runtimehooks/nri/server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ limitations under the License.
package nri

import (
"fmt"
"reflect"
"testing"
"time"
Expand All @@ -25,10 +26,22 @@ import (
"github.com/containerd/nri/pkg/stub"

"github.com/koordinator-sh/koordinator/pkg/koordlet/resourceexecutor"
"github.com/koordinator-sh/koordinator/pkg/koordlet/runtimehooks/hooks"
"github.com/koordinator-sh/koordinator/pkg/koordlet/runtimehooks/protocol"
"github.com/koordinator-sh/koordinator/pkg/koordlet/util/system"
"github.com/koordinator-sh/koordinator/pkg/runtimeproxy/config"
)

type mockPlugin struct{}

func (p *mockPlugin) Register(op hooks.Options) {
hooks.Register(config.PreRemoveRunPodSandbox, "mockPlugin", "mockPlugin remove", p.Remove)
}

func (p *mockPlugin) Remove(proto protocol.HooksProtocol) error {
return fmt.Errorf("mock error")
}

func getDisableStagesMap(stagesSlice []string) map[string]struct{} {
stagesMap := map[string]struct{}{}
for _, item := range stagesSlice {
Expand Down Expand Up @@ -508,3 +521,88 @@ func TestNriServer_UpdateContainer(t *testing.T) {
})
}
}

func TestNriServer_RemovePodSandbox(t *testing.T) {
type fields struct {
stub stub.Stub
mask stub.EventMask
options Options
runPodSandbox func(*NriServer, *api.PodSandbox, *api.Container) error
createContainer func(*NriServer, *api.PodSandbox, *api.Container) (*api.ContainerAdjustment, []*api.ContainerUpdate, error)
updateContainer func(*NriServer, *api.PodSandbox, *api.Container) ([]*api.ContainerUpdate, error)
plugin *mockPlugin
}
pod := &api.PodSandbox{
Id: "test",
Name: "test",
Uid: "test",
Namespace: "test",
Labels: nil,
Annotations: nil,
Linux: &api.LinuxPodSandbox{
PodOverhead: nil,
PodResources: nil,
CgroupParent: "",
CgroupsPath: "",
Namespaces: nil,
Resources: nil,
},
Pid: 0,
}
type args struct {
pod *api.PodSandbox
}
tests := []struct {
name string
fields fields
args args
wantErr bool
}{
{
name: "RemovePodSandbox success",
saintube marked this conversation as resolved.
Show resolved Hide resolved
fields: fields{
stub: nil,
options: Options{
PluginFailurePolicy: config.PolicyIgnore,
DisableStages: getDisableStagesMap([]string{"PreRemovePodSandbox"}),
Executor: resourceexecutor.NewTestResourceExecutor(),
},
},
args: args{
pod: pod,
},
wantErr: false,
},
{
name: "RemovePodSandbox fail",
fields: fields{
stub: nil,
options: Options{
PluginFailurePolicy: config.PolicyFail,
DisableStages: getDisableStagesMap([]string{"PreRemovePodSandbox"}),
Executor: resourceexecutor.NewTestResourceExecutor(),
},
plugin: &mockPlugin{},
},
args: args{
pod: pod,
},
wantErr: true,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
p := &NriServer{
stub: tt.fields.stub,
mask: tt.fields.mask,
options: tt.fields.options,
}
if tt.fields.plugin != nil {
tt.fields.plugin.Register(hooks.Options{})
}
if err := p.RemovePodSandbox(tt.args.pod); (err != nil) != tt.wantErr {
t.Errorf("RemovePodSandbox() error = %v, wantErr %v", err, tt.wantErr)
}
})
}
}
12 changes: 12 additions & 0 deletions pkg/koordlet/runtimehooks/protocol/pod_context.go
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,14 @@ func (p *PodContext) NriDone(executor resourceexecutor.ResourceUpdateExecutor) {
p.Update()
}

func (p *PodContext) NriRemoveDone(executor resourceexecutor.ResourceUpdateExecutor) {
if p.executor == nil {
p.executor = executor
}
p.removeForExt()
p.Update()
}

func (p *PodContext) FromReconciler(podMeta *statesinformer.PodMeta) {
p.Request.FromReconciler(podMeta)
}
Expand Down Expand Up @@ -278,3 +286,7 @@ func (p *PodContext) injectForExt() {
}
}
}

func (p *PodContext) removeForExt() {
// TODO: cleanup
}
34 changes: 34 additions & 0 deletions pkg/koordlet/runtimehooks/protocol/pod_context_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -140,3 +140,37 @@ func TestPodContext_NriDone(t *testing.T) {
})
}
}

func TestPodContext_NriRemoveDone(t *testing.T) {
type fields struct {
Request PodRequest
Response PodResponse
executor resourceexecutor.ResourceUpdateExecutor
}
type args struct {
executor resourceexecutor.ResourceUpdateExecutor
}
tests := []struct {
name string
fields fields
args args
}{
{
name: "nri remove done",
fields: fields{
executor: resourceexecutor.NewTestResourceExecutor(),
},
args: args{},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
p := &PodContext{
Request: tt.fields.Request,
Response: tt.fields.Response,
executor: tt.fields.executor,
}
p.NriRemoveDone(tt.args.executor)
})
}
}
1 change: 1 addition & 0 deletions pkg/runtimeproxy/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ const (
PostStartContainer RuntimeHookType = "PostStartContainer"
PreUpdateContainerResources RuntimeHookType = "PreUpdateContainerResources"
PostStopContainer RuntimeHookType = "PostStopContainer"
PreRemoveRunPodSandbox RuntimeHookType = "PreRemoveRunPodSandbox"
NoneRuntimeHookType RuntimeHookType = "NoneRuntimeHookType"
)

Expand Down