Skip to content

Commit 633857b

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 633857b

File tree

2 files changed

+164
-24
lines changed

2 files changed

+164
-24
lines changed

‎pkg/adaptation/adaptation_suite_test.go‎

Lines changed: 48 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -448,7 +448,19 @@ 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+
}
462+
a.AddAnnotation("non-nil-adjustment", p.idx+"-"+p.name)
463+
452464
case "annotation":
453465
if overwrite {
454466
a.RemoveAnnotation("key")
@@ -594,14 +606,19 @@ var _ = Describe("Plugin container creation adjustments", func() {
594606

595607
When("there is a single plugin", func() {
596608
BeforeEach(func() {
597-
s.Prepare(&mockRuntime{}, &mockPlugin{idx: "00", name: "test"})
609+
610+
s.Prepare(
611+
&mockRuntime{},
612+
&mockPlugin{idx: "00", name: "all-nil-adjust"},
613+
&mockPlugin{idx: "00", name: "test"},
614+
)
598615
})
599616

600617
DescribeTable("should be successfully collected without conflicts",
601618
func(subject string, expected *api.ContainerAdjustment) {
602619
var (
603620
runtime = s.runtime
604-
plugin = s.plugins[0]
621+
plugins = s.plugins
605622
ctx = context.Background()
606623

607624
pod = &api.PodSandbox{
@@ -635,7 +652,8 @@ var _ = Describe("Plugin container creation adjustments", func() {
635652
return adjust(subject, p, pod, ctr, false)
636653
}
637654

638-
plugin.createContainer = create
655+
plugins[0].createContainer = create
656+
plugins[1].createContainer = create
639657

640658
s.Startup()
641659

@@ -651,6 +669,10 @@ var _ = Describe("Plugin container creation adjustments", func() {
651669
protoDiff(reply.Adjust, expected))
652670
},
653671

672+
Entry("nil adjustment", "all-nil-adjustment",
673+
nil,
674+
),
675+
654676
Entry("adjust annotations", "annotation",
655677
&api.ContainerAdjustment{
656678
Annotations: map[string]string{
@@ -875,6 +897,7 @@ var _ = Describe("Plugin container creation adjustments", func() {
875897
BeforeEach(func() {
876898
s.Prepare(
877899
&mockRuntime{},
900+
&mockPlugin{idx: "00", name: "all-nil-adjust"},
878901
&mockPlugin{idx: "10", name: "foo"},
879902
&mockPlugin{idx: "00", name: "bar"},
880903
)
@@ -908,11 +931,12 @@ var _ = Describe("Plugin container creation adjustments", func() {
908931
)
909932

910933
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)
934+
return adjust(subject, p, pod, ctr, p == plugins[1] && remove)
912935
}
913936

914937
plugins[0].createContainer = create
915938
plugins[1].createContainer = create
939+
plugins[2].createContainer = create
916940

917941
s.Startup()
918942

@@ -932,6 +956,25 @@ var _ = Describe("Plugin container creation adjustments", func() {
932956
}
933957
},
934958

959+
Entry("all nil adjustment", "all-nil-adjustment", false, false,
960+
nil,
961+
),
962+
963+
Entry("00 nil adjustment", "00-nil-adjustment", false, false,
964+
&api.ContainerAdjustment{
965+
Annotations: map[string]string{
966+
"non-nil-adjustment": "10-foo",
967+
},
968+
},
969+
),
970+
Entry("10 nil adjustment", "10-nil-adjustment", false, false,
971+
&api.ContainerAdjustment{
972+
Annotations: map[string]string{
973+
"non-nil-adjustment": "00-bar",
974+
},
975+
},
976+
),
977+
935978
Entry("adjust annotations (conflicts)", "annotation", false, true, nil),
936979
Entry("adjust annotations", "annotation", true, false,
937980
&api.ContainerAdjustment{
@@ -1117,6 +1160,7 @@ var _ = Describe("Plugin container creation adjustments", func() {
11171160
),
11181161
},
11191162
},
1163+
&mockPlugin{idx: "00", name: "all-nil-adjust"},
11201164
&mockPlugin{idx: "00", name: "foo"},
11211165
&mockPlugin{idx: "10", name: "validator1"},
11221166
&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)