Skip to content

Commit fd82b8b

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 79f44b8 commit fd82b8b

File tree

2 files changed

+165
-24
lines changed

2 files changed

+165
-24
lines changed

‎pkg/adaptation/adaptation_suite_test.go‎

Lines changed: 49 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -448,7 +448,20 @@ var _ = Describe("Plugin container creation adjustments", func() {
448448
adjust := func(subject string, p *mockPlugin, _ *api.PodSandbox, c *api.Container, overwrite bool) (*api.ContainerAdjustment, []*api.ContainerUpdate, error) {
449449
plugin := p.idx + "-" + p.name
450450
a := &api.ContainerAdjustment{}
451+
if plugin == "00-all-nil-adjust" {
452+
return nil, nil, nil
453+
}
451454
switch subject {
455+
case "all-nil-adjustment":
456+
return nil, nil, nil
457+
458+
case "00-nil-adjustment", "10-nil-adjustment":
459+
if subject[:2] == p.idx {
460+
return nil, nil, nil
461+
} else {
462+
a.AddAnnotation("non-nil-adjustment", p.idx+"-"+p.name)
463+
}
464+
452465
case "annotation":
453466
if overwrite {
454467
a.RemoveAnnotation("key")
@@ -594,14 +607,19 @@ var _ = Describe("Plugin container creation adjustments", func() {
594607

595608
When("there is a single plugin", func() {
596609
BeforeEach(func() {
597-
s.Prepare(&mockRuntime{}, &mockPlugin{idx: "00", name: "test"})
610+
611+
s.Prepare(
612+
&mockRuntime{},
613+
&mockPlugin{idx: "00", name: "all-nil-adjust"},
614+
&mockPlugin{idx: "00", name: "test"},
615+
)
598616
})
599617

600618
DescribeTable("should be successfully collected without conflicts",
601619
func(subject string, expected *api.ContainerAdjustment) {
602620
var (
603621
runtime = s.runtime
604-
plugin = s.plugins[0]
622+
plugins = s.plugins
605623
ctx = context.Background()
606624

607625
pod = &api.PodSandbox{
@@ -635,7 +653,8 @@ var _ = Describe("Plugin container creation adjustments", func() {
635653
return adjust(subject, p, pod, ctr, false)
636654
}
637655

638-
plugin.createContainer = create
656+
plugins[0].createContainer = create
657+
plugins[1].createContainer = create
639658

640659
s.Startup()
641660

@@ -651,6 +670,10 @@ var _ = Describe("Plugin container creation adjustments", func() {
651670
protoDiff(reply.Adjust, expected))
652671
},
653672

673+
Entry("nil adjustment", "all-nil-adjustment",
674+
nil,
675+
),
676+
654677
Entry("adjust annotations", "annotation",
655678
&api.ContainerAdjustment{
656679
Annotations: map[string]string{
@@ -875,6 +898,7 @@ var _ = Describe("Plugin container creation adjustments", func() {
875898
BeforeEach(func() {
876899
s.Prepare(
877900
&mockRuntime{},
901+
&mockPlugin{idx: "00", name: "all-nil-adjust"},
878902
&mockPlugin{idx: "10", name: "foo"},
879903
&mockPlugin{idx: "00", name: "bar"},
880904
)
@@ -908,11 +932,12 @@ var _ = Describe("Plugin container creation adjustments", func() {
908932
)
909933

910934
create := func(p *mockPlugin, pod *api.PodSandbox, ctr *api.Container) (*api.ContainerAdjustment, []*api.ContainerUpdate, error) {
911-
return adjust(subject, p, pod, ctr, p == plugins[0] && remove)
935+
return adjust(subject, p, pod, ctr, p == plugins[1] && remove)
912936
}
913937

914938
plugins[0].createContainer = create
915939
plugins[1].createContainer = create
940+
plugins[2].createContainer = create
916941

917942
s.Startup()
918943

@@ -932,6 +957,25 @@ var _ = Describe("Plugin container creation adjustments", func() {
932957
}
933958
},
934959

960+
Entry("all nil adjustment", "all-nil-adjustment", false, false,
961+
nil,
962+
),
963+
964+
Entry("00 nil adjustment", "00-nil-adjustment", false, false,
965+
&api.ContainerAdjustment{
966+
Annotations: map[string]string{
967+
"non-nil-adjustment": "10-foo",
968+
},
969+
},
970+
),
971+
Entry("10 nil adjustment", "10-nil-adjustment", false, false,
972+
&api.ContainerAdjustment{
973+
Annotations: map[string]string{
974+
"non-nil-adjustment": "00-bar",
975+
},
976+
},
977+
),
978+
935979
Entry("adjust annotations (conflicts)", "annotation", false, true, nil),
936980
Entry("adjust annotations", "annotation", true, false,
937981
&api.ContainerAdjustment{
@@ -1117,6 +1161,7 @@ var _ = Describe("Plugin container creation adjustments", func() {
11171161
),
11181162
},
11191163
},
1164+
&mockPlugin{idx: "00", name: "all-nil-adjust"},
11201165
&mockPlugin{idx: "00", name: "foo"},
11211166
&mockPlugin{idx: "10", name: "validator1"},
11221167
&mockPlugin{idx: "20", name: "validator2"},

‎pkg/adaptation/result.go‎

Lines changed: 116 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -87,26 +87,7 @@ func collectCreateContainerResult(request *CreateContainerRequest) *result {
8787
request: resultRequest{
8888
create: request,
8989
},
90-
reply: resultReply{
91-
adjust: &ContainerAdjustment{
92-
Annotations: map[string]string{},
93-
Mounts: []*Mount{},
94-
Env: []*KeyValue{},
95-
Hooks: &Hooks{},
96-
Rlimits: []*POSIXRlimit{},
97-
CDIDevices: []*CDIDevice{},
98-
Linux: &LinuxContainerAdjustment{
99-
Devices: []*LinuxDevice{},
100-
Resources: &LinuxResources{
101-
Memory: &LinuxMemory{},
102-
Cpu: &LinuxCPU{},
103-
HugepageLimits: []*HugepageLimit{},
104-
Unified: map[string]string{},
105-
},
106-
Namespaces: []*LinuxNamespace{},
107-
},
108-
},
109-
},
90+
reply: resultReply{},
11091
updates: map[string]*ContainerUpdate{},
11192
owners: api.NewOwningPlugins(),
11293
}
@@ -265,6 +246,8 @@ func (r *result) adjustAnnotations(annotations map[string]string, plugin string)
265246
return nil
266247
}
267248

249+
r.initAdjustAnnotations()
250+
268251
create, id := r.request.create, r.request.create.Container.Id
269252
del := map[string]struct{}{}
270253
for k := range annotations {
@@ -300,6 +283,8 @@ func (r *result) adjustMounts(mounts []*Mount, plugin string) error {
300283
return nil
301284
}
302285

286+
r.initAdjustMounts()
287+
303288
create, id := r.request.create, r.request.create.Container.Id
304289

305290
// first split removals from the rest of adjustments
@@ -365,6 +350,8 @@ func (r *result) adjustDevices(devices []*LinuxDevice, plugin string) error {
365350
return nil
366351
}
367352

353+
r.initAdjustDevices()
354+
368355
create, id := r.request.create, r.request.create.Container.Id
369356

370357
// first split removals from the rest of adjustments
@@ -423,6 +410,8 @@ func (r *result) adjustNamespaces(namespaces []*LinuxNamespace, plugin string) e
423410
return nil
424411
}
425412

413+
r.initAdjustNamespaces()
414+
426415
create, id := r.request.create, r.request.create.Container.Id
427416

428417
creatensmap := map[string]*LinuxNamespace{}
@@ -456,6 +445,8 @@ func (r *result) adjustCDIDevices(devices []*CDIDevice, plugin string) error {
456445
return nil
457446
}
458447

448+
r.initAdjustCDIDevices()
449+
459450
// Notes:
460451
// CDI devices are opaque references, typically to vendor specific
461452
// devices. They get resolved to actual devices and potential related
@@ -486,6 +477,8 @@ func (r *result) adjustEnv(env []*KeyValue, plugin string) error {
486477
return nil
487478
}
488479

480+
r.initAdjustEnv()
481+
489482
create, id := r.request.create, r.request.create.Container.Id
490483

491484
// first split removals from the rest of adjustments
@@ -558,6 +551,8 @@ func (r *result) adjustArgs(args []string, plugin string) error {
558551
return nil
559552
}
560553

554+
r.initAdjustArgs()
555+
561556
create, id := r.request.create, r.request.create.Container.Id
562557

563558
if args[0] == "" {
@@ -580,6 +575,8 @@ func (r *result) adjustHooks(hooks *Hooks, plugin string) error {
580575
return nil
581576
}
582577

578+
r.initAdjustHooks()
579+
583580
reply := r.reply.adjust
584581
container := r.request.create.Container
585582
claim := false
@@ -627,6 +624,8 @@ func (r *result) adjustResources(resources *LinuxResources, plugin string) error
627624
return nil
628625
}
629626

627+
r.initAdjustResources()
628+
630629
create, id := r.request.create, r.request.create.Container.Id
631630
container := create.Container.Linux.Resources
632631
reply := r.reply.adjust.Linux.Resources
@@ -796,6 +795,8 @@ func (r *result) adjustCgroupsPath(path, plugin string) error {
796795
return nil
797796
}
798797

798+
r.initAdjustCgroupsPath()
799+
799800
create, id := r.request.create, r.request.create.Container.Id
800801

801802
if err := r.owners.ClaimCgroupsPath(id, plugin); err != nil {
@@ -813,6 +814,8 @@ func (r *result) adjustOomScoreAdj(OomScoreAdj *OptionalInt, plugin string) erro
813814
return nil
814815
}
815816

817+
r.initAdjustOomScoreAdj()
818+
816819
create, id := r.request.create, r.request.create.Container.Id
817820

818821
if err := r.owners.ClaimOomScoreAdj(id, plugin); err != nil {
@@ -830,6 +833,8 @@ func (r *result) adjustIOPriority(priority *LinuxIOPriority, plugin string) erro
830833
return nil
831834
}
832835

836+
r.initAdjustIOPriority()
837+
833838
create, id := r.request.create, r.request.create.Container.Id
834839

835840
if err := r.owners.ClaimIOPriority(id, plugin); err != nil {
@@ -846,6 +851,9 @@ func (r *result) adjustSeccompPolicy(adjustment *LinuxSeccomp, plugin string) er
846851
if adjustment == nil {
847852
return nil
848853
}
854+
855+
r.initAdjustSeccompPolicy()
856+
849857
create, id := r.request.create, r.request.create.Container.Id
850858

851859
if err := r.owners.ClaimSeccompPolicy(id, plugin); err != nil {
@@ -859,6 +867,12 @@ func (r *result) adjustSeccompPolicy(adjustment *LinuxSeccomp, plugin string) er
859867
}
860868

861869
func (r *result) adjustRlimits(rlimits []*POSIXRlimit, plugin string) error {
870+
if len(rlimits) == 0 {
871+
return nil
872+
}
873+
874+
r.initAdjustRlimits()
875+
862876
create, id, adjust := r.request.create, r.request.create.Container.Id, r.reply.adjust
863877
for _, l := range rlimits {
864878
if err := r.owners.ClaimRlimit(id, l.Type, plugin); err != nil {
@@ -871,6 +885,88 @@ func (r *result) adjustRlimits(rlimits []*POSIXRlimit, plugin string) error {
871885
return nil
872886
}
873887

888+
func (r *result) initAdjust() {
889+
if r.reply.adjust == nil {
890+
r.reply.adjust = &ContainerAdjustment{}
891+
}
892+
}
893+
894+
func (r *result) initAdjustAnnotations() {
895+
r.initAdjust()
896+
if r.reply.adjust.Annotations == nil {
897+
r.reply.adjust.Annotations = map[string]string{}
898+
}
899+
}
900+
901+
func (r *result) initAdjustMounts() {
902+
r.initAdjust()
903+
}
904+
905+
func (r *result) initAdjustLinux() {
906+
r.initAdjust()
907+
if r.reply.adjust.Linux == nil {
908+
r.reply.adjust.Linux = &LinuxContainerAdjustment{}
909+
}
910+
}
911+
912+
func (r *result) initAdjustDevices() {
913+
r.initAdjustLinux()
914+
}
915+
916+
func (r *result) initAdjustEnv() {
917+
r.initAdjust()
918+
}
919+
920+
func (r *result) initAdjustArgs() {
921+
r.initAdjust()
922+
}
923+
924+
func (r *result) initAdjustHooks() {
925+
r.initAdjust()
926+
if r.reply.adjust.Hooks == nil {
927+
r.reply.adjust.Hooks = &Hooks{}
928+
}
929+
}
930+
931+
func (r *result) initAdjustResources() {
932+
r.initAdjustLinux()
933+
if r.reply.adjust.Linux.Resources == nil {
934+
r.reply.adjust.Linux.Resources = &LinuxResources{
935+
Memory: &LinuxMemory{},
936+
Cpu: &LinuxCPU{},
937+
Unified: map[string]string{},
938+
}
939+
}
940+
}
941+
942+
func (r *result) initAdjustCgroupsPath() {
943+
r.initAdjustLinux()
944+
}
945+
946+
func (r *result) initAdjustOomScoreAdj() {
947+
r.initAdjustLinux()
948+
}
949+
950+
func (r *result) initAdjustIOPriority() {
951+
r.initAdjustLinux()
952+
}
953+
954+
func (r *result) initAdjustSeccompPolicy() {
955+
r.initAdjustLinux()
956+
}
957+
958+
func (r *result) initAdjustNamespaces() {
959+
r.initAdjustLinux()
960+
}
961+
962+
func (r *result) initAdjustRlimits() {
963+
r.initAdjust()
964+
}
965+
966+
func (r *result) initAdjustCDIDevices() {
967+
r.initAdjust()
968+
}
969+
874970
func (r *result) updateResources(reply, u *ContainerUpdate, plugin string) error {
875971
if u.Linux == nil || u.Linux.Resources == nil {
876972
return nil

0 commit comments

Comments
 (0)