From 42780102c3e08d1447a61ea863a590926a7c3c45 Mon Sep 17 00:00:00 2001 From: Kevin Delgado Date: Wed, 31 Mar 2021 22:56:01 +0000 Subject: [PATCH] Unsupport atomic to granular --- merge/schema_change_test.go | 26 ++++++++++---------- typed/reconcile_schema.go | 43 ++++++++++------------------------ typed/reconcile_schema_test.go | 33 -------------------------- 3 files changed, 26 insertions(+), 76 deletions(-) diff --git a/merge/schema_change_test.go b/merge/schema_change_test.go index 82dd5b76..f98fdf73 100644 --- a/merge/schema_change_test.go +++ b/merge/schema_change_test.go @@ -204,6 +204,7 @@ func TestAtomicToGranularSchemaChanges(t *testing.T) { }, }, ChangeParser{Parser: structParser}, + // No conflict after changing struct to a granular schema Apply{ Manager: "two", Object: ` @@ -211,17 +212,6 @@ func TestAtomicToGranularSchemaChanges(t *testing.T) { string: "b" `, APIVersion: "v1", - Conflicts: merge.Conflicts{ - merge.Conflict{Manager: "one", Path: _P("struct", "string")}, - }, - }, - ForceApply{ - Manager: "two", - Object: ` - struct: - string: "b" - `, - APIVersion: "v1", }, }, Object: ` @@ -231,9 +221,21 @@ func TestAtomicToGranularSchemaChanges(t *testing.T) { `, APIVersion: "v1", Managed: fieldpath.ManagedFields{ + // Note that manager one previously owned + // the top level _P("struct") + // which included all of its subfields + // when the struct field was atomic. + // + // Upon changing the schema of struct from + // atomic to granular, manager one continues + // to own the same fieldset as before, + // but does not retain ownership of any of the subfields. + // + // This is a known limitation due to the inability + // to accurately determine whether an empty field + // was previously atomic or not. "one": fieldpath.NewVersionedSet(_NS( _P("struct"), - _P("struct", "numeric"), ), "v1", true), "two": fieldpath.NewVersionedSet(_NS( _P("struct", "string"), diff --git a/typed/reconcile_schema.go b/typed/reconcile_schema.go index 5a8214ae..27b663e3 100644 --- a/typed/reconcile_schema.go +++ b/typed/reconcile_schema.go @@ -187,19 +187,17 @@ func (v *reconcileWithSchemaWalker) visitListItems(t *schema.List, element *fiel } func (v *reconcileWithSchemaWalker) doList(t *schema.List) (errs ValidationErrors) { - // reconcile lists changed from granular to atomic + // reconcile lists changed from granular to atomic. + // Note that migrations from atomic to granular are not recommended and will + // be treated as if they were always granular. + // + // In this case, the manager that owned the previously atomic field (and all subfields), + // will now own just the top-level field and none of the subfields. if !v.isAtomic && t.ElementRelationship == schema.Atomic { v.toRemove = fieldpath.NewSet(v.path) // remove all root and all children fields v.toAdd = fieldpath.NewSet(v.path) // add the root of the atomic return errs } - // reconcile lists changed from atomic to granular - if v.isAtomic && t.ElementRelationship == schema.Associative { - v.toAdd, errs = buildGranularFieldSet(v.path, v.value) - if errs != nil { - return errs - } - } if v.fieldSet != nil { errs = v.visitListItems(t, v.fieldSet) } @@ -231,7 +229,12 @@ func (v *reconcileWithSchemaWalker) visitMapItems(t *schema.Map, element *fieldp } func (v *reconcileWithSchemaWalker) doMap(t *schema.Map) (errs ValidationErrors) { - // reconcile maps and structs changed from granular to atomic + // reconcile maps and structs changed from granular to atomic. + // Note that migrations from atomic to granular are not recommended and will + // be treated as if they were always granular. + // + // In this case the manager that owned the previously atomic field (and all subfields), + // will now own just the top-level field and none of the subfields. if !v.isAtomic && t.ElementRelationship == schema.Atomic { if v.fieldSet != nil && v.fieldSet.Size() > 0 { v.toRemove = fieldpath.NewSet(v.path) // remove all root and all children fields @@ -239,34 +242,12 @@ func (v *reconcileWithSchemaWalker) doMap(t *schema.Map) (errs ValidationErrors) } return errs } - // reconcile maps changed from atomic to granular - if v.isAtomic && (t.ElementRelationship == schema.Separable || t.ElementRelationship == "") { - v.toAdd, errs = buildGranularFieldSet(v.path, v.value) - if errs != nil { - return errs - } - } if v.fieldSet != nil { errs = v.visitMapItems(t, v.fieldSet) } return errs } -func buildGranularFieldSet(path fieldpath.Path, value *TypedValue) (*fieldpath.Set, ValidationErrors) { - - valueFieldSet, err := value.ToFieldSet() - if err != nil { - return nil, errorf("toFieldSet: %v", err) - } - if valueFieldSetAtPath, ok := fieldSetAtPath(valueFieldSet, path); ok { - result := fieldpath.NewSet(path) - resultAtPath := descendToPath(result, path) - *resultAtPath = *valueFieldSetAtPath - return result, nil - } - return nil, nil -} - func fieldSetAtPath(node *fieldpath.Set, path fieldpath.Path) (*fieldpath.Set, bool) { ok := true for _, pe := range path { diff --git a/typed/reconcile_schema_test.go b/typed/reconcile_schema_test.go index c4171412..03bfd49c 100644 --- a/typed/reconcile_schema_test.go +++ b/typed/reconcile_schema_test.go @@ -207,39 +207,6 @@ var reconcileCases = []reconcileTestCase{{ _P("stringMap"), _P("unchanged", "numeric"), ), -}, { - name: "atomic-to-granular", - rootTypeName: "v1", - oldSchema: atomicSchema("v1"), - newSchema: granularSchema("v1"), - liveObject: basicLiveObject, - oldFields: _NS( - _P("struct"), - _P("list"), - _P("objectList"), - _P("stringMap"), - _P("unchanged", "numeric"), - ), - fixedFields: _NS( - _P("struct"), - _P("struct", "numeric"), - _P("struct", "string"), - _P("list"), - _P("list", _V("one")), - _P("list", _V("two")), - _P("objectList"), - _P("objectList", _KBF("keyA", "a1", "keyB", "b1")), - _P("objectList", _KBF("keyA", "a1", "keyB", "b1"), "value"), - _P("objectList", _KBF("keyA", "a1", "keyB", "b1"), "keyA"), - _P("objectList", _KBF("keyA", "a1", "keyB", "b1"), "keyB"), - _P("objectList", _KBF("keyA", "a2", "keyB", "b2")), - _P("objectList", _KBF("keyA", "a2", "keyB", "b2"), "value"), - _P("objectList", _KBF("keyA", "a2", "keyB", "b2"), "keyA"), - _P("objectList", _KBF("keyA", "a2", "keyB", "b2"), "keyB"), - _P("stringMap"), - _P("stringMap", "key1"), - _P("unchanged", "numeric"), - ), }, { name: "no-change-granular", rootTypeName: "v1",