Skip to content

Commit a39f4c4

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 <krisztian.litkey@intel.com>
1 parent 0bbc6d0 commit a39f4c4

2 files changed

Lines changed: 147 additions & 19 deletions

File tree

‎pkg/adaptation/adaptation_suite_test.go‎

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,9 +20,11 @@ import (
2020
"context"
2121
"os"
2222
"path/filepath"
23+
"reflect"
2324
"strconv"
2425
"strings"
2526
"time"
27+
"unsafe"
2628

2729
"sigs.k8s.io/yaml"
2830

@@ -439,6 +441,23 @@ var _ = Describe("Plugin container creation adjustments", func() {
439441
plugin := p.idx + "-" + p.name
440442
a := &api.ContainerAdjustment{}
441443
switch subject {
444+
case "all-nil-adjustment":
445+
return nil, nil, nil
446+
447+
case "00-nil-adjustment":
448+
if p.idx == "00" {
449+
return nil, nil, nil
450+
} else {
451+
a.AddAnnotation("non-nil-adjustment", p.idx+"-"+p.name)
452+
}
453+
454+
case "10-nil-adjustment":
455+
if p.idx == "10" {
456+
return nil, nil, nil
457+
} else {
458+
a.AddAnnotation("non-nil-adjustment", p.idx+"-"+p.name)
459+
}
460+
442461
case "annotation":
443462
if overwrite {
444463
a.RemoveAnnotation("key")
@@ -592,6 +611,9 @@ var _ = Describe("Plugin container creation adjustments", func() {
592611
Expect(stripAdjustment(reply.Adjust)).Should(Equal(stripAdjustment(expected)))
593612
},
594613

614+
Entry("nil adjustment", "all-nil-adjustment",
615+
nil,
616+
),
595617
Entry("adjust annotations", "annotation",
596618
&api.ContainerAdjustment{
597619
Annotations: map[string]string{
@@ -807,6 +829,24 @@ var _ = Describe("Plugin container creation adjustments", func() {
807829
}
808830
},
809831

832+
Entry("all nil adjustment", "all-nil-adjustment", false, false,
833+
nil,
834+
),
835+
836+
Entry("00 nil adjustment", "00-nil-adjustment", false, false,
837+
&api.ContainerAdjustment{
838+
Annotations: map[string]string{
839+
"non-nil-adjustment": "10-foo",
840+
},
841+
},
842+
),
843+
Entry("10 nil adjustment", "10-nil-adjustment", false, false,
844+
&api.ContainerAdjustment{
845+
Annotations: map[string]string{
846+
"non-nil-adjustment": "00-bar",
847+
},
848+
},
849+
),
810850
Entry("adjust annotations (conflicts)", "annotation", false, true, nil),
811851
Entry("adjust annotations", "annotation", true, false,
812852
&api.ContainerAdjustment{
@@ -1946,13 +1986,28 @@ var _ = Describe("Plugin configuration request", func() {
19461986
// around we marshal then unmarshal compared objects in offending test cases to
19471987
// clear those unexported fields.
19481988
func strip(obj interface{}, ptr interface{}) interface{} {
1989+
if isNil(obj) {
1990+
return nilPtrFor(ptr)
1991+
}
19491992
bytes, err := yaml.Marshal(obj)
19501993
Expect(err).To(BeNil())
19511994
Expect(yaml.Unmarshal(bytes, ptr)).To(Succeed())
19521995
return ptr
19531996
}
19541997

1998+
func isNil(obj interface{}) bool {
1999+
return (*[2]uintptr)(unsafe.Pointer(&obj))[1] == 0
2000+
}
2001+
2002+
func nilPtrFor(ptr interface{}) interface{} {
2003+
t := reflect.TypeOf(ptr)
2004+
return reflect.Zero(t).Interface()
2005+
}
2006+
19552007
func stripAdjustment(a *api.ContainerAdjustment) *api.ContainerAdjustment {
2008+
if a == nil {
2009+
return nil
2010+
}
19562011
stripAnnotations(a)
19572012
stripMounts(a)
19582013
stripEnv(a)

‎pkg/adaptation/result.go‎

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

234+
r.initAdjustAnnotations()
235+
252236
create, id := r.request.create, r.request.create.Container.Id
253237
del := map[string]struct{}{}
254238
for k := range annotations {
@@ -284,6 +268,8 @@ func (r *result) adjustMounts(mounts []*Mount, plugin string) error {
284268
return nil
285269
}
286270

271+
r.initAdjustMounts()
272+
287273
create, id := r.request.create, r.request.create.Container.Id
288274

289275
// first split removals from the rest of adjustments
@@ -349,6 +335,8 @@ func (r *result) adjustDevices(devices []*LinuxDevice, plugin string) error {
349335
return nil
350336
}
351337

338+
r.initAdjustDevices()
339+
352340
create, id := r.request.create, r.request.create.Container.Id
353341

354342
// first split removals from the rest of adjustments
@@ -407,6 +395,8 @@ func (r *result) adjustCDIDevices(devices []*CDIDevice, plugin string) error {
407395
return nil
408396
}
409397

398+
r.initAdjustCDIDevices()
399+
410400
// Notes:
411401
// CDI devices are opaque references, typically to vendor specific
412402
// devices. They get resolved to actual devices and potential related
@@ -437,6 +427,8 @@ func (r *result) adjustEnv(env []*KeyValue, plugin string) error {
437427
return nil
438428
}
439429

430+
r.initAdjustEnv()
431+
440432
create, id := r.request.create, r.request.create.Container.Id
441433

442434
// first split removals from the rest of adjustments
@@ -509,6 +501,8 @@ func (r *result) adjustHooks(hooks *Hooks) error {
509501
return nil
510502
}
511503

504+
r.initAdjustHooks()
505+
512506
reply := r.reply.adjust
513507
container := r.request.create.Container
514508

@@ -545,6 +539,8 @@ func (r *result) adjustResources(resources *LinuxResources, plugin string) error
545539
return nil
546540
}
547541

542+
r.initAdjustResources()
543+
548544
create, id := r.request.create, r.request.create.Container.Id
549545
container := create.Container.Linux.Resources
550546
reply := r.reply.adjust.Linux.Resources
@@ -709,6 +705,8 @@ func (r *result) adjustCgroupsPath(path, plugin string) error {
709705
return nil
710706
}
711707

708+
r.initAdjustCgroupsPath()
709+
712710
create, id := r.request.create, r.request.create.Container.Id
713711

714712
if err := r.owners.claimCgroupsPath(id, plugin); err != nil {
@@ -726,6 +724,8 @@ func (r *result) adjustOomScoreAdj(OomScoreAdj *OptionalInt, plugin string) erro
726724
return nil
727725
}
728726

727+
r.initAdjustOomScoreAdj()
728+
729729
create, id := r.request.create, r.request.create.Container.Id
730730

731731
if err := r.owners.claimOomScoreAdj(id, plugin); err != nil {
@@ -739,6 +739,12 @@ func (r *result) adjustOomScoreAdj(OomScoreAdj *OptionalInt, plugin string) erro
739739
}
740740

741741
func (r *result) adjustRlimits(rlimits []*POSIXRlimit, plugin string) error {
742+
if len(rlimits) == 0 {
743+
return nil
744+
}
745+
746+
r.initAdjustRlimits()
747+
742748
create, id, adjust := r.request.create, r.request.create.Container.Id, r.reply.adjust
743749
for _, l := range rlimits {
744750
if err := r.owners.claimRlimits(id, l.Type, plugin); err != nil {
@@ -751,6 +757,73 @@ func (r *result) adjustRlimits(rlimits []*POSIXRlimit, plugin string) error {
751757
return nil
752758
}
753759

760+
func (r *result) initAdjust() {
761+
if r.reply.adjust == nil {
762+
r.reply.adjust = &ContainerAdjustment{}
763+
}
764+
}
765+
766+
func (r *result) initAdjustAnnotations() {
767+
r.initAdjust()
768+
if r.reply.adjust.Annotations == nil {
769+
r.reply.adjust.Annotations = map[string]string{}
770+
}
771+
}
772+
773+
func (r *result) initAdjustMounts() {
774+
r.initAdjust()
775+
}
776+
777+
func (r *result) initAdjustEnv() {
778+
r.initAdjust()
779+
}
780+
781+
func (r *result) initAdjustHooks() {
782+
r.initAdjust()
783+
if r.reply.adjust.Hooks == nil {
784+
r.reply.adjust.Hooks = &Hooks{}
785+
}
786+
}
787+
788+
func (r *result) initAdjustLinux() {
789+
r.initAdjust()
790+
if r.reply.adjust.Linux == nil {
791+
r.reply.adjust.Linux = &LinuxContainerAdjustment{}
792+
}
793+
}
794+
795+
func (r *result) initAdjustDevices() {
796+
r.initAdjustLinux()
797+
}
798+
799+
func (r *result) initAdjustResources() {
800+
r.initAdjustLinux()
801+
if r.reply.adjust.Linux.Resources == nil {
802+
r.reply.adjust.Linux.Resources = &LinuxResources{
803+
Memory: &LinuxMemory{},
804+
Cpu: &LinuxCPU{},
805+
HugepageLimits: []*HugepageLimit{},
806+
Unified: map[string]string{},
807+
}
808+
}
809+
}
810+
811+
func (r *result) initAdjustCgroupsPath() {
812+
r.initAdjustLinux()
813+
}
814+
815+
func (r *result) initAdjustOomScoreAdj() {
816+
r.initAdjustLinux()
817+
}
818+
819+
func (r *result) initAdjustRlimits() {
820+
r.initAdjustLinux()
821+
}
822+
823+
func (r *result) initAdjustCDIDevices() {
824+
r.initAdjust()
825+
}
826+
754827
func (r *result) updateResources(reply, u *ContainerUpdate, plugin string) error {
755828
if u.Linux == nil || u.Linux.Resources == nil {
756829
return nil

0 commit comments

Comments
 (0)