Skip to content

Commit 4eab409

Browse files
klihubklihub
committed
adaptation: return nil as no-op adjustment.
Don't create an empty adjustment a priori. Only create once a plugin makes an adjustment to the container being created. In particular, this should result in a nil adjustment if no plugin touches a container. Signed-off-by: Krisztian Litkey <[email protected]>
1 parent d640b80 commit 4eab409

File tree

2 files changed

+153
-19
lines changed

2 files changed

+153
-19
lines changed

‎pkg/adaptation/adaptation_suite_test.go‎

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,10 +20,12 @@ import (
2020
"context"
2121
"os"
2222
"path/filepath"
23+
"reflect"
2324
"slices"
2425
"strconv"
2526
"strings"
2627
"time"
28+
"unsafe"
2729

2830
"sigs.k8s.io/yaml"
2931

@@ -442,6 +444,23 @@ var _ = Describe("Plugin container creation adjustments", func() {
442444
plugin := p.idx + "-" + p.name
443445
a := &api.ContainerAdjustment{}
444446
switch subject {
447+
case "all-nil-adjustment":
448+
return nil, nil, nil
449+
450+
case "00-nil-adjustment":
451+
if p.idx == "00" {
452+
return nil, nil, nil
453+
} else {
454+
a.AddAnnotation("non-nil-adjustment", p.idx+"-"+p.name)
455+
}
456+
457+
case "10-nil-adjustment":
458+
if p.idx == "10" {
459+
return nil, nil, nil
460+
} else {
461+
a.AddAnnotation("non-nil-adjustment", p.idx+"-"+p.name)
462+
}
463+
445464
case "annotation":
446465
if overwrite {
447466
a.RemoveAnnotation("key")
@@ -608,6 +627,9 @@ var _ = Describe("Plugin container creation adjustments", func() {
608627
Expect(stripAdjustment(reply.Adjust)).Should(Equal(stripAdjustment(expected)))
609628
},
610629

630+
Entry("nil adjustment", "all-nil-adjustment",
631+
nil,
632+
),
611633
Entry("adjust annotations", "annotation",
612634
&api.ContainerAdjustment{
613635
Annotations: map[string]string{
@@ -839,6 +861,24 @@ var _ = Describe("Plugin container creation adjustments", func() {
839861
}
840862
},
841863

864+
Entry("all nil adjustment", "all-nil-adjustment", false, false,
865+
nil,
866+
),
867+
868+
Entry("00 nil adjustment", "00-nil-adjustment", false, false,
869+
&api.ContainerAdjustment{
870+
Annotations: map[string]string{
871+
"non-nil-adjustment": "10-foo",
872+
},
873+
},
874+
),
875+
Entry("10 nil adjustment", "10-nil-adjustment", false, false,
876+
&api.ContainerAdjustment{
877+
Annotations: map[string]string{
878+
"non-nil-adjustment": "00-bar",
879+
},
880+
},
881+
),
842882
Entry("adjust annotations (conflicts)", "annotation", false, true, nil),
843883
Entry("adjust annotations", "annotation", true, false,
844884
&api.ContainerAdjustment{
@@ -1992,13 +2032,28 @@ var _ = Describe("Plugin configuration request", func() {
19922032
// around we marshal then unmarshal compared objects in offending test cases to
19932033
// clear those unexported fields.
19942034
func strip(obj interface{}, ptr interface{}) interface{} {
2035+
if isNil(obj) {
2036+
return nilPtrFor(ptr)
2037+
}
19952038
bytes, err := yaml.Marshal(obj)
19962039
Expect(err).To(BeNil())
19972040
Expect(yaml.Unmarshal(bytes, ptr)).To(Succeed())
19982041
return ptr
19992042
}
20002043

2044+
func isNil(obj interface{}) bool {
2045+
return (*[2]uintptr)(unsafe.Pointer(&obj))[1] == 0
2046+
}
2047+
2048+
func nilPtrFor(ptr interface{}) interface{} {
2049+
t := reflect.TypeOf(ptr)
2050+
return reflect.Zero(t).Interface()
2051+
}
2052+
20012053
func stripAdjustment(a *api.ContainerAdjustment) *api.ContainerAdjustment {
2054+
if a == nil {
2055+
return nil
2056+
}
20022057
stripAnnotations(a)
20032058
stripMounts(a)
20042059
stripEnv(a)

‎pkg/adaptation/result.go‎

Lines changed: 98 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -85,25 +85,7 @@ func collectCreateContainerResult(request *CreateContainerRequest) *result {
8585
request: resultRequest{
8686
create: request,
8787
},
88-
reply: resultReply{
89-
adjust: &ContainerAdjustment{
90-
Annotations: map[string]string{},
91-
Mounts: []*Mount{},
92-
Env: []*KeyValue{},
93-
Hooks: &Hooks{},
94-
Rlimits: []*POSIXRlimit{},
95-
CDIDevices: []*CDIDevice{},
96-
Linux: &LinuxContainerAdjustment{
97-
Devices: []*LinuxDevice{},
98-
Resources: &LinuxResources{
99-
Memory: &LinuxMemory{},
100-
Cpu: &LinuxCPU{},
101-
HugepageLimits: []*HugepageLimit{},
102-
Unified: map[string]string{},
103-
},
104-
},
105-
},
106-
},
88+
reply: resultReply{},
10789
updates: map[string]*ContainerUpdate{},
10890
owners: resultOwners{},
10991
}
@@ -253,6 +235,8 @@ func (r *result) adjustAnnotations(annotations map[string]string, plugin string)
253235
return nil
254236
}
255237

238+
r.initAdjustAnnotations()
239+
256240
create, id := r.request.create, r.request.create.Container.Id
257241
del := map[string]struct{}{}
258242
for k := range annotations {
@@ -288,6 +272,8 @@ func (r *result) adjustMounts(mounts []*Mount, plugin string) error {
288272
return nil
289273
}
290274

275+
r.initAdjustMounts()
276+
291277
create, id := r.request.create, r.request.create.Container.Id
292278

293279
// first split removals from the rest of adjustments
@@ -353,6 +339,8 @@ func (r *result) adjustDevices(devices []*LinuxDevice, plugin string) error {
353339
return nil
354340
}
355341

342+
r.initAdjustDevices()
343+
356344
create, id := r.request.create, r.request.create.Container.Id
357345

358346
// first split removals from the rest of adjustments
@@ -411,6 +399,8 @@ func (r *result) adjustCDIDevices(devices []*CDIDevice, plugin string) error {
411399
return nil
412400
}
413401

402+
r.initAdjustCDIDevices()
403+
414404
// Notes:
415405
// CDI devices are opaque references, typically to vendor specific
416406
// devices. They get resolved to actual devices and potential related
@@ -441,6 +431,8 @@ func (r *result) adjustEnv(env []*KeyValue, plugin string) error {
441431
return nil
442432
}
443433

434+
r.initAdjustEnv()
435+
444436
create, id := r.request.create, r.request.create.Container.Id
445437

446438
// first split removals from the rest of adjustments
@@ -513,6 +505,8 @@ func (r *result) adjustArgs(args []string, plugin string) error {
513505
return nil
514506
}
515507

508+
r.initAdjustArgs()
509+
516510
create, id := r.request.create, r.request.create.Container.Id
517511

518512
if args[0] == "" {
@@ -535,6 +529,8 @@ func (r *result) adjustHooks(hooks *Hooks) error {
535529
return nil
536530
}
537531

532+
r.initAdjustHooks()
533+
538534
reply := r.reply.adjust
539535
container := r.request.create.Container
540536

@@ -571,6 +567,8 @@ func (r *result) adjustResources(resources *LinuxResources, plugin string) error
571567
return nil
572568
}
573569

570+
r.initAdjustResources()
571+
574572
create, id := r.request.create, r.request.create.Container.Id
575573
container := create.Container.Linux.Resources
576574
reply := r.reply.adjust.Linux.Resources
@@ -735,6 +733,8 @@ func (r *result) adjustCgroupsPath(path, plugin string) error {
735733
return nil
736734
}
737735

736+
r.initAdjustCgroupsPath()
737+
738738
create, id := r.request.create, r.request.create.Container.Id
739739

740740
if err := r.owners.claimCgroupsPath(id, plugin); err != nil {
@@ -752,6 +752,8 @@ func (r *result) adjustOomScoreAdj(OomScoreAdj *OptionalInt, plugin string) erro
752752
return nil
753753
}
754754

755+
r.initAdjustOomScoreAdj()
756+
755757
create, id := r.request.create, r.request.create.Container.Id
756758

757759
if err := r.owners.claimOomScoreAdj(id, plugin); err != nil {
@@ -765,6 +767,12 @@ func (r *result) adjustOomScoreAdj(OomScoreAdj *OptionalInt, plugin string) erro
765767
}
766768

767769
func (r *result) adjustRlimits(rlimits []*POSIXRlimit, plugin string) error {
770+
if len(rlimits) == 0 {
771+
return nil
772+
}
773+
774+
r.initAdjustRlimits()
775+
768776
create, id, adjust := r.request.create, r.request.create.Container.Id, r.reply.adjust
769777
for _, l := range rlimits {
770778
if err := r.owners.claimRlimits(id, l.Type, plugin); err != nil {
@@ -777,6 +785,77 @@ func (r *result) adjustRlimits(rlimits []*POSIXRlimit, plugin string) error {
777785
return nil
778786
}
779787

788+
func (r *result) initAdjust() {
789+
if r.reply.adjust == nil {
790+
r.reply.adjust = &ContainerAdjustment{}
791+
}
792+
}
793+
794+
func (r *result) initAdjustAnnotations() {
795+
r.initAdjust()
796+
if r.reply.adjust.Annotations == nil {
797+
r.reply.adjust.Annotations = map[string]string{}
798+
}
799+
}
800+
801+
func (r *result) initAdjustMounts() {
802+
r.initAdjust()
803+
}
804+
805+
func (r *result) initAdjustEnv() {
806+
r.initAdjust()
807+
}
808+
809+
func (r *result) initAdjustArgs() {
810+
r.initAdjust()
811+
}
812+
813+
func (r *result) initAdjustHooks() {
814+
r.initAdjust()
815+
if r.reply.adjust.Hooks == nil {
816+
r.reply.adjust.Hooks = &Hooks{}
817+
}
818+
}
819+
820+
func (r *result) initAdjustLinux() {
821+
r.initAdjust()
822+
if r.reply.adjust.Linux == nil {
823+
r.reply.adjust.Linux = &LinuxContainerAdjustment{}
824+
}
825+
}
826+
827+
func (r *result) initAdjustDevices() {
828+
r.initAdjustLinux()
829+
}
830+
831+
func (r *result) initAdjustResources() {
832+
r.initAdjustLinux()
833+
if r.reply.adjust.Linux.Resources == nil {
834+
r.reply.adjust.Linux.Resources = &LinuxResources{
835+
Memory: &LinuxMemory{},
836+
Cpu: &LinuxCPU{},
837+
HugepageLimits: []*HugepageLimit{},
838+
Unified: map[string]string{},
839+
}
840+
}
841+
}
842+
843+
func (r *result) initAdjustCgroupsPath() {
844+
r.initAdjustLinux()
845+
}
846+
847+
func (r *result) initAdjustOomScoreAdj() {
848+
r.initAdjustLinux()
849+
}
850+
851+
func (r *result) initAdjustRlimits() {
852+
r.initAdjustLinux()
853+
}
854+
855+
func (r *result) initAdjustCDIDevices() {
856+
r.initAdjust()
857+
}
858+
780859
func (r *result) updateResources(reply, u *ContainerUpdate, plugin string) error {
781860
if u.Linux == nil || u.Linux.Resources == nil {
782861
return nil

0 commit comments

Comments
 (0)