Skip to content

Commit b6c8bcf

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 d1418ab commit b6c8bcf

2 files changed

Lines changed: 179 additions & 25 deletions

File tree

‎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")
@@ -619,14 +631,19 @@ var _ = Describe("Plugin container creation adjustments", func() {
619631

620632
When("there is a single plugin", func() {
621633
BeforeEach(func() {
622-
s.Prepare(&mockRuntime{}, &mockPlugin{idx: "00", name: "test"})
634+
635+
s.Prepare(
636+
&mockRuntime{},
637+
&mockPlugin{idx: "00", name: "all-nil-adjust"},
638+
&mockPlugin{idx: "00", name: "test"},
639+
)
623640
})
624641

625642
DescribeTable("should be successfully collected without conflicts",
626643
func(subject string, expected *api.ContainerAdjustment) {
627644
var (
628645
runtime = s.runtime
629-
plugin = s.plugins[0]
646+
plugins = s.plugins
630647
ctx = context.Background()
631648

632649
pod = &api.PodSandbox{
@@ -679,7 +696,8 @@ var _ = Describe("Plugin container creation adjustments", func() {
679696
return adjust(subject, p, pod, ctr, false)
680697
}
681698

682-
plugin.createContainer = create
699+
plugins[0].createContainer = create
700+
plugins[1].createContainer = create
683701

684702
s.Startup()
685703

@@ -695,6 +713,10 @@ var _ = Describe("Plugin container creation adjustments", func() {
695713
protoDiff(reply.Adjust, expected))
696714
},
697715

716+
Entry("nil adjustment", "all-nil-adjustment",
717+
nil,
718+
),
719+
698720
Entry("adjust annotations", "annotation",
699721
&api.ContainerAdjustment{
700722
Annotations: map[string]string{
@@ -955,6 +977,7 @@ var _ = Describe("Plugin container creation adjustments", func() {
955977
BeforeEach(func() {
956978
s.Prepare(
957979
&mockRuntime{},
980+
&mockPlugin{idx: "00", name: "all-nil-adjust"},
958981
&mockPlugin{idx: "10", name: "foo"},
959982
&mockPlugin{idx: "00", name: "bar"},
960983
)
@@ -1007,11 +1030,12 @@ var _ = Describe("Plugin container creation adjustments", func() {
10071030
)
10081031

10091032
create := func(p *mockPlugin, pod *api.PodSandbox, ctr *api.Container) (*api.ContainerAdjustment, []*api.ContainerUpdate, error) {
1010-
return adjust(subject, p, pod, ctr, p == plugins[0] && remove)
1033+
return adjust(subject, p, pod, ctr, p == plugins[1] && remove)
10111034
}
10121035

10131036
plugins[0].createContainer = create
10141037
plugins[1].createContainer = create
1038+
plugins[2].createContainer = create
10151039

10161040
s.Startup()
10171041

@@ -1031,6 +1055,25 @@ var _ = Describe("Plugin container creation adjustments", func() {
10311055
}
10321056
},
10331057

1058+
Entry("all nil adjustment", "all-nil-adjustment", false, false,
1059+
nil,
1060+
),
1061+
1062+
Entry("00 nil adjustment", "00-nil-adjustment", false, false,
1063+
&api.ContainerAdjustment{
1064+
Annotations: map[string]string{
1065+
"non-nil-adjustment": "10-foo",
1066+
},
1067+
},
1068+
),
1069+
Entry("10 nil adjustment", "10-nil-adjustment", false, false,
1070+
&api.ContainerAdjustment{
1071+
Annotations: map[string]string{
1072+
"non-nil-adjustment": "00-bar",
1073+
},
1074+
},
1075+
),
1076+
10341077
Entry("adjust annotations (conflicts)", "annotation", false, true, nil),
10351078
Entry("adjust annotations", "annotation", true, false,
10361079
&api.ContainerAdjustment{
@@ -1232,6 +1275,7 @@ var _ = Describe("Plugin container creation adjustments", func() {
12321275
),
12331276
},
12341277
},
1278+
&mockPlugin{idx: "00", name: "all-nil-adjust"},
12351279
&mockPlugin{idx: "00", name: "foo"},
12361280
&mockPlugin{idx: "10", name: "validator1"},
12371281
&mockPlugin{idx: "20", name: "validator2"},

‎pkg/adaptation/result.go‎

Lines changed: 131 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -90,27 +90,7 @@ func collectCreateContainerResult(request *CreateContainerRequest) *result {
9090
request: resultRequest{
9191
create: request,
9292
},
93-
reply: resultReply{
94-
adjust: &ContainerAdjustment{
95-
Annotations: map[string]string{},
96-
Mounts: []*Mount{},
97-
Env: []*KeyValue{},
98-
Hooks: &Hooks{},
99-
Rlimits: []*POSIXRlimit{},
100-
CDIDevices: []*CDIDevice{},
101-
Linux: &LinuxContainerAdjustment{
102-
Devices: []*LinuxDevice{},
103-
Resources: &LinuxResources{
104-
Memory: &LinuxMemory{},
105-
Cpu: &LinuxCPU{},
106-
HugepageLimits: []*HugepageLimit{},
107-
Unified: map[string]string{},
108-
},
109-
Namespaces: []*LinuxNamespace{},
110-
NetDevices: map[string]*LinuxNetDevice{},
111-
},
112-
},
113-
},
93+
reply: resultReply{},
11494
updates: map[string]*ContainerUpdate{},
11595
owners: api.NewOwningPlugins(),
11696
}
@@ -278,6 +258,8 @@ func (r *result) adjustAnnotations(annotations map[string]string, plugin string)
278258
return nil
279259
}
280260

261+
r.initAdjustAnnotations()
262+
281263
create, id := r.request.create, r.request.create.Container.Id
282264
del := map[string]struct{}{}
283265
for k := range annotations {
@@ -313,6 +295,8 @@ func (r *result) adjustMounts(mounts []*Mount, plugin string) error {
313295
return nil
314296
}
315297

298+
r.initAdjustMounts()
299+
316300
create, id := r.request.create, r.request.create.Container.Id
317301

318302
// first split removals from the rest of adjustments
@@ -378,6 +362,8 @@ func (r *result) adjustDevices(devices []*LinuxDevice, plugin string) error {
378362
return nil
379363
}
380364

365+
r.initAdjustDevices()
366+
381367
create, id := r.request.create, r.request.create.Container.Id
382368

383369
// first split removals from the rest of adjustments
@@ -436,6 +422,8 @@ func (r *result) adjustNamespaces(namespaces []*LinuxNamespace, plugin string) e
436422
return nil
437423
}
438424

425+
r.initAdjustNamespaces()
426+
439427
create, id := r.request.create, r.request.create.Container.Id
440428

441429
creatensmap := map[string]*LinuxNamespace{}
@@ -504,6 +492,8 @@ func (r *result) adjustCDIDevices(devices []*CDIDevice, plugin string) error {
504492
return nil
505493
}
506494

495+
r.initAdjustCDIDevices()
496+
507497
// Notes:
508498
// CDI devices are opaque references, typically to vendor specific
509499
// devices. They get resolved to actual devices and potential related
@@ -534,6 +524,8 @@ func (r *result) adjustEnv(env []*KeyValue, plugin string) error {
534524
return nil
535525
}
536526

527+
r.initAdjustEnv()
528+
537529
create, id := r.request.create, r.request.create.Container.Id
538530

539531
// first split removals from the rest of adjustments
@@ -606,6 +598,8 @@ func (r *result) adjustArgs(args []string, plugin string) error {
606598
return nil
607599
}
608600

601+
r.initAdjustArgs()
602+
609603
create, id := r.request.create, r.request.create.Container.Id
610604

611605
if args[0] == "" {
@@ -628,6 +622,8 @@ func (r *result) adjustHooks(hooks *Hooks, plugin string) error {
628622
return nil
629623
}
630624

625+
r.initAdjustHooks()
626+
631627
reply := r.reply.adjust
632628
container := r.request.create.Container
633629
claim := false
@@ -828,6 +824,8 @@ func (r *result) adjustResources(resources *LinuxResources, plugin string) error
828824
return nil
829825
}
830826

827+
r.initAdjustResources()
828+
831829
create, id := r.request.create, r.request.create.Container.Id
832830
container := create.Container.Linux.Resources
833831
reply := r.reply.adjust.Linux.Resources
@@ -895,6 +893,8 @@ func (r *result) adjustCgroupsPath(path, plugin string) error {
895893
return nil
896894
}
897895

896+
r.initAdjustCgroupsPath()
897+
898898
create, id := r.request.create, r.request.create.Container.Id
899899

900900
if err := r.owners.ClaimCgroupsPath(id, plugin); err != nil {
@@ -912,6 +912,8 @@ func (r *result) adjustOomScoreAdj(OomScoreAdj *OptionalInt, plugin string) erro
912912
return nil
913913
}
914914

915+
r.initAdjustOomScoreAdj()
916+
915917
create, id := r.request.create, r.request.create.Container.Id
916918

917919
if err := r.owners.ClaimOomScoreAdj(id, plugin); err != nil {
@@ -929,6 +931,8 @@ func (r *result) adjustIOPriority(priority *LinuxIOPriority, plugin string) erro
929931
return nil
930932
}
931933

934+
r.initAdjustIOPriority()
935+
932936
create, id := r.request.create, r.request.create.Container.Id
933937

934938
if err := r.owners.ClaimIOPriority(id, plugin); err != nil {
@@ -945,6 +949,9 @@ func (r *result) adjustSeccompPolicy(adjustment *LinuxSeccomp, plugin string) er
945949
if adjustment == nil {
946950
return nil
947951
}
952+
953+
r.initAdjustLinuxSeccompPolicy()
954+
948955
create, id := r.request.create, r.request.create.Container.Id
949956

950957
if err := r.owners.ClaimSeccompPolicy(id, plugin); err != nil {
@@ -962,6 +969,8 @@ func (r *result) adjustLinuxScheduler(sch *LinuxScheduler, plugin string) error
962969
return nil
963970
}
964971

972+
r.initAdjustLinuxScheduler()
973+
965974
create, id := r.request.create, r.request.create.Container.Id
966975

967976
if err := r.owners.ClaimLinuxScheduler(id, plugin); err != nil {
@@ -975,6 +984,12 @@ func (r *result) adjustLinuxScheduler(sch *LinuxScheduler, plugin string) error
975984
}
976985

977986
func (r *result) adjustRlimits(rlimits []*POSIXRlimit, plugin string) error {
987+
if len(rlimits) == 0 {
988+
return nil
989+
}
990+
991+
r.initAdjustRlimits()
992+
978993
create, id, adjust := r.request.create, r.request.create.Container.Id, r.reply.adjust
979994
for _, l := range rlimits {
980995
if err := r.owners.ClaimRlimit(id, l.Type, plugin); err != nil {
@@ -992,6 +1007,8 @@ func (r *result) adjustLinuxNetDevices(devices map[string]*LinuxNetDevice, plugi
9921007
return nil
9931008
}
9941009

1010+
r.initAdjustLinuxNetDevices()
1011+
9951012
create, id := r.request.create, r.request.create.Container.Id
9961013
del := map[string]struct{}{}
9971014
for k := range devices {
@@ -1022,6 +1039,99 @@ func (r *result) adjustLinuxNetDevices(devices map[string]*LinuxNetDevice, plugi
10221039
return nil
10231040
}
10241041

1042+
func (r *result) initAdjust() {
1043+
if r.reply.adjust == nil {
1044+
r.reply.adjust = &ContainerAdjustment{}
1045+
}
1046+
}
1047+
1048+
func (r *result) initAdjustAnnotations() {
1049+
r.initAdjust()
1050+
if r.reply.adjust.Annotations == nil {
1051+
r.reply.adjust.Annotations = map[string]string{}
1052+
}
1053+
}
1054+
1055+
func (r *result) initAdjustMounts() {
1056+
r.initAdjust()
1057+
}
1058+
1059+
func (r *result) initAdjustLinux() {
1060+
r.initAdjust()
1061+
if r.reply.adjust.Linux == nil {
1062+
r.reply.adjust.Linux = &LinuxContainerAdjustment{}
1063+
}
1064+
}
1065+
1066+
func (r *result) initAdjustDevices() {
1067+
r.initAdjustLinux()
1068+
}
1069+
1070+
func (r *result) initAdjustEnv() {
1071+
r.initAdjust()
1072+
}
1073+
1074+
func (r *result) initAdjustArgs() {
1075+
r.initAdjust()
1076+
}
1077+
1078+
func (r *result) initAdjustHooks() {
1079+
r.initAdjust()
1080+
if r.reply.adjust.Hooks == nil {
1081+
r.reply.adjust.Hooks = &Hooks{}
1082+
}
1083+
}
1084+
1085+
func (r *result) initAdjustResources() {
1086+
r.initAdjustLinux()
1087+
if r.reply.adjust.Linux.Resources == nil {
1088+
r.reply.adjust.Linux.Resources = &LinuxResources{
1089+
Memory: &LinuxMemory{},
1090+
Cpu: &LinuxCPU{},
1091+
Unified: map[string]string{},
1092+
}
1093+
}
1094+
}
1095+
1096+
func (r *result) initAdjustCgroupsPath() {
1097+
r.initAdjustLinux()
1098+
}
1099+
1100+
func (r *result) initAdjustOomScoreAdj() {
1101+
r.initAdjustLinux()
1102+
}
1103+
1104+
func (r *result) initAdjustIOPriority() {
1105+
r.initAdjustLinux()
1106+
}
1107+
1108+
func (r *result) initAdjustLinuxSeccompPolicy() {
1109+
r.initAdjustLinux()
1110+
}
1111+
1112+
func (r *result) initAdjustLinuxScheduler() {
1113+
r.initAdjustLinux()
1114+
}
1115+
1116+
func (r *result) initAdjustNamespaces() {
1117+
r.initAdjustLinux()
1118+
}
1119+
1120+
func (r *result) initAdjustRlimits() {
1121+
r.initAdjust()
1122+
}
1123+
1124+
func (r *result) initAdjustLinuxNetDevices() {
1125+
r.initAdjustLinux()
1126+
if r.reply.adjust.Linux.NetDevices == nil {
1127+
r.reply.adjust.Linux.NetDevices = map[string]*LinuxNetDevice{}
1128+
}
1129+
}
1130+
1131+
func (r *result) initAdjustCDIDevices() {
1132+
r.initAdjust()
1133+
}
1134+
10251135
func (r *result) updateResources(reply, u *ContainerUpdate, plugin string) error {
10261136
if u.Linux == nil || u.Linux.Resources == nil {
10271137
return nil

0 commit comments

Comments
 (0)