diff --git a/pkg/adaptation/adaptation_suite_test.go b/pkg/adaptation/adaptation_suite_test.go index 9df22b3a..1e9323bf 100644 --- a/pkg/adaptation/adaptation_suite_test.go +++ b/pkg/adaptation/adaptation_suite_test.go @@ -20,10 +20,12 @@ import ( "context" "os" "path/filepath" + "reflect" "slices" "strconv" "strings" "time" + "unsafe" "sigs.k8s.io/yaml" @@ -442,6 +444,23 @@ var _ = Describe("Plugin container creation adjustments", func() { plugin := p.idx + "-" + p.name a := &api.ContainerAdjustment{} switch subject { + case "all-nil-adjustment": + return nil, nil, nil + + case "00-nil-adjustment": + if p.idx == "00" { + return nil, nil, nil + } else { + a.AddAnnotation("non-nil-adjustment", p.idx+"-"+p.name) + } + + case "10-nil-adjustment": + if p.idx == "10" { + return nil, nil, nil + } else { + a.AddAnnotation("non-nil-adjustment", p.idx+"-"+p.name) + } + case "annotation": if overwrite { a.RemoveAnnotation("key") @@ -608,6 +627,9 @@ var _ = Describe("Plugin container creation adjustments", func() { Expect(stripAdjustment(reply.Adjust)).Should(Equal(stripAdjustment(expected))) }, + Entry("nil adjustment", "all-nil-adjustment", + nil, + ), Entry("adjust annotations", "annotation", &api.ContainerAdjustment{ Annotations: map[string]string{ @@ -839,6 +861,24 @@ var _ = Describe("Plugin container creation adjustments", func() { } }, + Entry("all nil adjustment", "all-nil-adjustment", false, false, + nil, + ), + + Entry("00 nil adjustment", "00-nil-adjustment", false, false, + &api.ContainerAdjustment{ + Annotations: map[string]string{ + "non-nil-adjustment": "10-foo", + }, + }, + ), + Entry("10 nil adjustment", "10-nil-adjustment", false, false, + &api.ContainerAdjustment{ + Annotations: map[string]string{ + "non-nil-adjustment": "00-bar", + }, + }, + ), Entry("adjust annotations (conflicts)", "annotation", false, true, nil), Entry("adjust annotations", "annotation", true, false, &api.ContainerAdjustment{ @@ -1992,13 +2032,28 @@ var _ = Describe("Plugin configuration request", func() { // around we marshal then unmarshal compared objects in offending test cases to // clear those unexported fields. func strip(obj interface{}, ptr interface{}) interface{} { + if isNil(obj) { + return nilPtrFor(ptr) + } bytes, err := yaml.Marshal(obj) Expect(err).To(BeNil()) Expect(yaml.Unmarshal(bytes, ptr)).To(Succeed()) return ptr } +func isNil(obj interface{}) bool { + return (*[2]uintptr)(unsafe.Pointer(&obj))[1] == 0 +} + +func nilPtrFor(ptr interface{}) interface{} { + t := reflect.TypeOf(ptr) + return reflect.Zero(t).Interface() +} + func stripAdjustment(a *api.ContainerAdjustment) *api.ContainerAdjustment { + if a == nil { + return nil + } stripAnnotations(a) stripMounts(a) stripEnv(a) diff --git a/pkg/adaptation/result.go b/pkg/adaptation/result.go index da84110a..90501a8f 100644 --- a/pkg/adaptation/result.go +++ b/pkg/adaptation/result.go @@ -85,25 +85,7 @@ func collectCreateContainerResult(request *CreateContainerRequest) *result { request: resultRequest{ create: request, }, - reply: resultReply{ - adjust: &ContainerAdjustment{ - Annotations: map[string]string{}, - Mounts: []*Mount{}, - Env: []*KeyValue{}, - Hooks: &Hooks{}, - Rlimits: []*POSIXRlimit{}, - CDIDevices: []*CDIDevice{}, - Linux: &LinuxContainerAdjustment{ - Devices: []*LinuxDevice{}, - Resources: &LinuxResources{ - Memory: &LinuxMemory{}, - Cpu: &LinuxCPU{}, - HugepageLimits: []*HugepageLimit{}, - Unified: map[string]string{}, - }, - }, - }, - }, + reply: resultReply{}, updates: map[string]*ContainerUpdate{}, owners: resultOwners{}, } @@ -253,6 +235,8 @@ func (r *result) adjustAnnotations(annotations map[string]string, plugin string) return nil } + r.initAdjustAnnotations() + create, id := r.request.create, r.request.create.Container.Id del := map[string]struct{}{} for k := range annotations { @@ -288,6 +272,8 @@ func (r *result) adjustMounts(mounts []*Mount, plugin string) error { return nil } + r.initAdjustMounts() + create, id := r.request.create, r.request.create.Container.Id // first split removals from the rest of adjustments @@ -353,6 +339,8 @@ func (r *result) adjustDevices(devices []*LinuxDevice, plugin string) error { return nil } + r.initAdjustDevices() + create, id := r.request.create, r.request.create.Container.Id // first split removals from the rest of adjustments @@ -411,6 +399,8 @@ func (r *result) adjustCDIDevices(devices []*CDIDevice, plugin string) error { return nil } + r.initAdjustCDIDevices() + // Notes: // CDI devices are opaque references, typically to vendor specific // devices. They get resolved to actual devices and potential related @@ -441,6 +431,8 @@ func (r *result) adjustEnv(env []*KeyValue, plugin string) error { return nil } + r.initAdjustEnv() + create, id := r.request.create, r.request.create.Container.Id // first split removals from the rest of adjustments @@ -513,6 +505,8 @@ func (r *result) adjustArgs(args []string, plugin string) error { return nil } + r.initAdjustArgs() + create, id := r.request.create, r.request.create.Container.Id if args[0] == "" { @@ -535,6 +529,8 @@ func (r *result) adjustHooks(hooks *Hooks) error { return nil } + r.initAdjustHooks() + reply := r.reply.adjust container := r.request.create.Container @@ -571,6 +567,8 @@ func (r *result) adjustResources(resources *LinuxResources, plugin string) error return nil } + r.initAdjustResources() + create, id := r.request.create, r.request.create.Container.Id container := create.Container.Linux.Resources reply := r.reply.adjust.Linux.Resources @@ -735,6 +733,8 @@ func (r *result) adjustCgroupsPath(path, plugin string) error { return nil } + r.initAdjustCgroupsPath() + create, id := r.request.create, r.request.create.Container.Id if err := r.owners.claimCgroupsPath(id, plugin); err != nil { @@ -752,6 +752,8 @@ func (r *result) adjustOomScoreAdj(OomScoreAdj *OptionalInt, plugin string) erro return nil } + r.initAdjustOomScoreAdj() + create, id := r.request.create, r.request.create.Container.Id if err := r.owners.claimOomScoreAdj(id, plugin); err != nil { @@ -765,6 +767,12 @@ func (r *result) adjustOomScoreAdj(OomScoreAdj *OptionalInt, plugin string) erro } func (r *result) adjustRlimits(rlimits []*POSIXRlimit, plugin string) error { + if len(rlimits) == 0 { + return nil + } + + r.initAdjustRlimits() + create, id, adjust := r.request.create, r.request.create.Container.Id, r.reply.adjust for _, l := range rlimits { if err := r.owners.claimRlimits(id, l.Type, plugin); err != nil { @@ -777,6 +785,77 @@ func (r *result) adjustRlimits(rlimits []*POSIXRlimit, plugin string) error { return nil } +func (r *result) initAdjust() { + if r.reply.adjust == nil { + r.reply.adjust = &ContainerAdjustment{} + } +} + +func (r *result) initAdjustAnnotations() { + r.initAdjust() + if r.reply.adjust.Annotations == nil { + r.reply.adjust.Annotations = map[string]string{} + } +} + +func (r *result) initAdjustMounts() { + r.initAdjust() +} + +func (r *result) initAdjustEnv() { + r.initAdjust() +} + +func (r *result) initAdjustArgs() { + r.initAdjust() +} + +func (r *result) initAdjustHooks() { + r.initAdjust() + if r.reply.adjust.Hooks == nil { + r.reply.adjust.Hooks = &Hooks{} + } +} + +func (r *result) initAdjustLinux() { + r.initAdjust() + if r.reply.adjust.Linux == nil { + r.reply.adjust.Linux = &LinuxContainerAdjustment{} + } +} + +func (r *result) initAdjustDevices() { + r.initAdjustLinux() +} + +func (r *result) initAdjustResources() { + r.initAdjustLinux() + if r.reply.adjust.Linux.Resources == nil { + r.reply.adjust.Linux.Resources = &LinuxResources{ + Memory: &LinuxMemory{}, + Cpu: &LinuxCPU{}, + HugepageLimits: []*HugepageLimit{}, + Unified: map[string]string{}, + } + } +} + +func (r *result) initAdjustCgroupsPath() { + r.initAdjustLinux() +} + +func (r *result) initAdjustOomScoreAdj() { + r.initAdjustLinux() +} + +func (r *result) initAdjustRlimits() { + r.initAdjustLinux() +} + +func (r *result) initAdjustCDIDevices() { + r.initAdjust() +} + func (r *result) updateResources(reply, u *ContainerUpdate, plugin string) error { if u.Linux == nil || u.Linux.Resources == nil { return nil