Skip to content

Commit 9f09075

Browse files
jpbetzjpbetz
committed
Change merge rules so that updaters to share ownership with appliers
1 parent 085f5bf commit 9f09075

File tree

2 files changed

+180
-9
lines changed

2 files changed

+180
-9
lines changed

‎merge/multiple_appliers_test.go‎

Lines changed: 176 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -384,7 +384,7 @@ func testMultipleAppliersFieldUnsetting(t *testing.T, v1, v2, v3 fieldpath.APIVe
384384
),
385385
},
386386
},
387-
"unset_scalar_shared_owner": {
387+
"unset_scalar_shared_with_applier": {
388388
Ops: []Operation{
389389
Apply{
390390
Manager: "apply-one",
@@ -435,6 +435,104 @@ func testMultipleAppliersFieldUnsetting(t *testing.T, v1, v2, v3 fieldpath.APIVe
435435
),
436436
},
437437
},
438+
"unset_scalar_shared_with_updater": {
439+
Ops: []Operation{
440+
Update{
441+
Manager: "updater",
442+
APIVersion: v1,
443+
Object: typed.YAMLObject(fmt.Sprintf(`
444+
struct:
445+
name: a
446+
scalarField_%s: a
447+
`, v1)),
448+
},
449+
Apply{
450+
Manager: "applier",
451+
APIVersion: v2,
452+
Object: typed.YAMLObject(fmt.Sprintf(`
453+
struct:
454+
name: a
455+
scalarField_%s: a
456+
`, v2)),
457+
},
458+
Apply{
459+
Manager: "applier",
460+
APIVersion: v3,
461+
Object: `
462+
struct:
463+
name: a
464+
`,
465+
},
466+
},
467+
Object: typed.YAMLObject(fmt.Sprintf(`
468+
struct:
469+
name: a
470+
scalarField_%s: a
471+
`, v3)),
472+
APIVersion: v3,
473+
Managed: fieldpath.ManagedFields{
474+
"updater": fieldpath.NewVersionedSet(
475+
_NS(
476+
_P("struct"),
477+
_P("struct", "name"),
478+
_P("struct", fmt.Sprintf("scalarField_%s", v1)),
479+
),
480+
v1,
481+
false,
482+
),
483+
"applier": fieldpath.NewVersionedSet(
484+
_NS(
485+
_P("struct", "name"),
486+
),
487+
v3,
488+
true,
489+
),
490+
},
491+
},
492+
"updater_claims_field": {
493+
Ops: []Operation{
494+
Apply{
495+
Manager: "applier",
496+
APIVersion: v1,
497+
Object: typed.YAMLObject(fmt.Sprintf(`
498+
struct:
499+
name: a
500+
scalarField_%s: a
501+
`, v1)),
502+
},
503+
Update{
504+
Manager: "updater",
505+
APIVersion: v2,
506+
Object: typed.YAMLObject(fmt.Sprintf(`
507+
struct:
508+
name: a
509+
scalarField_%s: b
510+
`, v2)),
511+
},
512+
},
513+
Object: typed.YAMLObject(fmt.Sprintf(`
514+
struct:
515+
name: a
516+
scalarField_%s: b
517+
`, v3)),
518+
APIVersion: v3,
519+
Managed: fieldpath.ManagedFields{
520+
"updater": fieldpath.NewVersionedSet(
521+
_NS(
522+
_P("struct", fmt.Sprintf("scalarField_%s", v2)),
523+
),
524+
v2,
525+
false,
526+
),
527+
"applier": fieldpath.NewVersionedSet(
528+
_NS(
529+
_P("struct", "name"),
530+
),
531+
v1,
532+
true,
533+
),
534+
},
535+
},
438536
"unset_complex_sole_owner": {
439537
Ops: []Operation{
440538
Apply{
@@ -472,7 +570,7 @@ func testMultipleAppliersFieldUnsetting(t *testing.T, v1, v2, v3 fieldpath.APIVe
472570
),
473571
},
474572
},
475-
"unset_complex_shared_owner": {
573+
"unset_complex_shared_with_applier": {
476574
Ops: []Operation{
477575
Apply{
478576
Manager: "apply-one",
@@ -1206,7 +1304,7 @@ func TestMultipleAppliersRealConversion(t *testing.T) {
12061304
),
12071305
},
12081306
},
1209-
"appliers_remove_from_controller_real_conversion": {
1307+
"applier_updater_shared_ownership_real_conversion": {
12101308
Ops: []Operation{
12111309
Update{
12121310
Manager: "controller",
@@ -1242,6 +1340,8 @@ func TestMultipleAppliersRealConversion(t *testing.T) {
12421340
Object: `
12431341
mapOfMapsRecursive:
12441342
aaa:
1343+
bbb:
1344+
ccc:
12451345
ccc:
12461346
`,
12471347
APIVersion: "v3",
@@ -1250,6 +1350,8 @@ func TestMultipleAppliersRealConversion(t *testing.T) {
12501350
_NS(
12511351
_P("mapOfMapsRecursive"),
12521352
_P("mapOfMapsRecursive", "a"),
1353+
_P("mapOfMapsRecursive", "a", "b"),
1354+
_P("mapOfMapsRecursive", "a", "b", "c"),
12531355
),
12541356
"v1",
12551357
false,
@@ -1275,6 +1377,77 @@ func TestMultipleAppliersRealConversion(t *testing.T) {
12751377
}
12761378
}
12771379

1380+
func TestMultipleAppliersFieldRenameConversions(t *testing.T) {
1381+
versions := []fieldpath.APIVersion{"v1", "v2", "v3"}
1382+
for _, v1 := range versions {
1383+
for _, v2 := range versions {
1384+
for _, v3 := range versions {
1385+
t.Run(fmt.Sprintf("%s-%s-%s", v1, v2, v3), func(t *testing.T) {
1386+
testMultipleAppliersFieldRenameConversions(t, v1, v2, v3)
1387+
})
1388+
}
1389+
}
1390+
}
1391+
}
1392+
1393+
func testMultipleAppliersFieldRenameConversions(t *testing.T, v1, v2, v3 fieldpath.APIVersion) {
1394+
tests := map[string]TestCase{
1395+
"updater_claims_field": {
1396+
Ops: []Operation{
1397+
Apply{
1398+
Manager: "applier",
1399+
APIVersion: v1,
1400+
Object: typed.YAMLObject(fmt.Sprintf(`
1401+
struct:
1402+
name: a
1403+
scalarField_%s: a
1404+
`, v1)),
1405+
},
1406+
Update{
1407+
Manager: "updater",
1408+
APIVersion: v2,
1409+
Object: typed.YAMLObject(fmt.Sprintf(`
1410+
struct:
1411+
name: a
1412+
scalarField_%s: b
1413+
`, v2)),
1414+
},
1415+
},
1416+
Object: typed.YAMLObject(fmt.Sprintf(`
1417+
struct:
1418+
name: a
1419+
scalarField_%s: b
1420+
`, v3)),
1421+
APIVersion: v3,
1422+
Managed: fieldpath.ManagedFields{
1423+
"updater": fieldpath.NewVersionedSet(
1424+
_NS(
1425+
_P("struct", fmt.Sprintf("scalarField_%s", v2)),
1426+
),
1427+
v2,
1428+
false,
1429+
),
1430+
"applier": fieldpath.NewVersionedSet(
1431+
_NS(
1432+
_P("struct", "name"),
1433+
),
1434+
v1,
1435+
true,
1436+
),
1437+
},
1438+
},
1439+
}
1440+
1441+
converter := renamingConverter{structMultiversionParser}
1442+
for name, test := range tests {
1443+
t.Run(name, func(t *testing.T) {
1444+
if err := test.TestWithConverter(structMultiversionParser, converter); err != nil {
1445+
t.Fatal(err)
1446+
}
1447+
})
1448+
}
1449+
}
1450+
12781451
// repeatingConverter repeats a single letterkey v times, where v is the version.
12791452
type repeatingConverter struct {
12801453
parser Parser

‎merge/update.go‎

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -241,17 +241,15 @@ func (s *Updater) prune(merged *typed.TypedValue, managers fieldpath.ManagedFiel
241241
}
242242

243243
// addBackOwnedItems adds back any fields, list and map items that were removed by prune,
244-
// but other appliers (or the current applier's new config) claim to own.
244+
// but other appliers or updaters (or the current applier's new config) claim to own.
245245
func (s *Updater) addBackOwnedItems(merged, pruned *typed.TypedValue, managedFields fieldpath.ManagedFields, applyingManager string) (*typed.TypedValue, error) {
246246
var err error
247247
managedAtVersion := map[fieldpath.APIVersion]*fieldpath.Set{}
248248
for _, managerSet := range managedFields {
249-
if managerSet.Applied() {
250-
if _, ok := managedAtVersion[managerSet.APIVersion()]; !ok {
251-
managedAtVersion[managerSet.APIVersion()] = fieldpath.NewSet()
252-
}
253-
managedAtVersion[managerSet.APIVersion()] = managedAtVersion[managerSet.APIVersion()].Union(managerSet.Set())
249+
if _, ok := managedAtVersion[managerSet.APIVersion()]; !ok {
250+
managedAtVersion[managerSet.APIVersion()] = fieldpath.NewSet()
254251
}
252+
managedAtVersion[managerSet.APIVersion()] = managedAtVersion[managerSet.APIVersion()].Union(managerSet.Set())
255253
}
256254
for version, managed := range managedAtVersion {
257255
merged, err = s.Converter.Convert(merged, version)

0 commit comments

Comments
 (0)