Skip to content

Commit 0bcdf88

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request #170 from jpbetz/test-atomic-introduction
Support granular<->atomic schema changes
2 parents b84068c + 9a22ec6 commit 0bcdf88

File tree

6 files changed

+929
-10
lines changed

6 files changed

+929
-10
lines changed

‎internal/fixture/state.go‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -385,6 +385,33 @@ func (f UpdateObject) preprocess(parser Parser) (Operation, error) {
385385
return f, nil
386386
}
387387

388+
// ChangeParser is a type of operation. It simulates making changes a schema without versioning
389+
// the schema. This can be used to test the behavior of making backward compatible schema changes,
390+
// e.g. setting "elementRelationship: atomic" on an existing struct. It also may be used to ensure
391+
// that backward incompatible changes are detected appropriately.
392+
type ChangeParser struct {
393+
Parser *typed.Parser
394+
}
395+
396+
var _ Operation = &ChangeParser{}
397+
398+
func (cs ChangeParser) run(state *State) error {
399+
state.Parser = cs.Parser
400+
// Swap the schema in for use with the live object so it merges.
401+
// If the schema is incompatible, this will fail validation.
402+
403+
liveWithNewSchema, err := typed.AsTyped(state.Live.AsValue(), &cs.Parser.Schema, state.Live.TypeRef())
404+
if err != nil {
405+
return err
406+
}
407+
state.Live = liveWithNewSchema
408+
return nil
409+
}
410+
411+
func (cs ChangeParser) preprocess(_ Parser) (Operation, error) {
412+
return cs, nil
413+
}
414+
388415
// TestCase is the list of operations that need to be run, as well as
389416
// the object/managedfields as they are supposed to look like after all
390417
// the operations have been successfully performed. If Object/Managed is

‎merge/schema_change_test.go‎

Lines changed: 252 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,252 @@
1+
/*
2+
Copyright 2018 The Kubernetes Authors.
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
package merge_test
18+
19+
import (
20+
"testing"
21+
22+
"sigs.k8s.io/structured-merge-diff/v4/fieldpath"
23+
. "sigs.k8s.io/structured-merge-diff/v4/internal/fixture"
24+
"sigs.k8s.io/structured-merge-diff/v4/merge"
25+
"sigs.k8s.io/structured-merge-diff/v4/typed"
26+
)
27+
28+
var structParser = func() *typed.Parser {
29+
oldParser, err := typed.NewParser(`types:
30+
- name: v1
31+
map:
32+
fields:
33+
- name: struct
34+
type:
35+
namedType: struct
36+
- name: struct
37+
map:
38+
fields:
39+
- name: numeric
40+
type:
41+
scalar: numeric
42+
- name: string
43+
type:
44+
scalar: string`)
45+
if err != nil {
46+
panic(err)
47+
}
48+
return oldParser
49+
}()
50+
51+
var structWithAtomicParser = func() *typed.Parser {
52+
newParser, err := typed.NewParser(`types:
53+
- name: v1
54+
map:
55+
fields:
56+
- name: struct
57+
type:
58+
namedType: struct
59+
- name: struct
60+
map:
61+
fields:
62+
- name: numeric
63+
type:
64+
scalar: numeric
65+
- name: string
66+
type:
67+
scalar: string
68+
elementRelationship: atomic`)
69+
if err != nil {
70+
panic(err)
71+
}
72+
return newParser
73+
}()
74+
75+
func TestGranularToAtomicSchemaChanges(t *testing.T) {
76+
tests := map[string]TestCase{
77+
"to-atomic": {
78+
Ops: []Operation{
79+
Apply{
80+
Manager: "one",
81+
Object: `
82+
struct:
83+
numeric: 1
84+
`,
85+
APIVersion: "v1",
86+
},
87+
ChangeParser{Parser: structWithAtomicParser},
88+
Apply{
89+
Manager: "two",
90+
Object: `
91+
struct:
92+
string: "string"
93+
`,
94+
APIVersion: "v1",
95+
Conflicts: merge.Conflicts{
96+
merge.Conflict{Manager: "one", Path: _P("struct")},
97+
},
98+
},
99+
ForceApply{
100+
Manager: "two",
101+
Object: `
102+
struct:
103+
string: "string"
104+
`,
105+
APIVersion: "v1",
106+
},
107+
},
108+
Object: `
109+
struct:
110+
string: "string"
111+
`,
112+
APIVersion: "v1",
113+
Managed: fieldpath.ManagedFields{
114+
"two": fieldpath.NewVersionedSet(_NS(
115+
_P("struct"),
116+
), "v1", true),
117+
},
118+
},
119+
"to-atomic-owner-with-no-child-fields": {
120+
Ops: []Operation{
121+
Apply{
122+
Manager: "one",
123+
Object: `
124+
struct:
125+
numeric: 1
126+
`,
127+
APIVersion: "v1",
128+
},
129+
ForceApply{ // take the only child field from manager "one"
130+
Manager: "two",
131+
Object: `
132+
struct:
133+
numeric: 2
134+
`,
135+
APIVersion: "v1",
136+
},
137+
ChangeParser{Parser: structWithAtomicParser},
138+
Apply{
139+
Manager: "three",
140+
Object: `
141+
struct:
142+
string: "string"
143+
`,
144+
APIVersion: "v1",
145+
Conflicts: merge.Conflicts{
146+
// We expect no conflict with "one" because we do not allow a manager
147+
// to own a map without owning any of the children.
148+
merge.Conflict{Manager: "two", Path: _P("struct")},
149+
},
150+
},
151+
ForceApply{
152+
Manager: "two",
153+
Object: `
154+
struct:
155+
string: "string"
156+
`,
157+
APIVersion: "v1",
158+
},
159+
},
160+
Object: `
161+
struct:
162+
string: "string"
163+
`,
164+
APIVersion: "v1",
165+
Managed: fieldpath.ManagedFields{
166+
"two": fieldpath.NewVersionedSet(_NS(
167+
_P("struct"),
168+
), "v1", true),
169+
},
170+
},
171+
}
172+
173+
for name, test := range tests {
174+
t.Run(name, func(t *testing.T) {
175+
if err := test.Test(structParser); err != nil {
176+
t.Fatal(err)
177+
}
178+
})
179+
}
180+
}
181+
182+
func TestAtomicToGranularSchemaChanges(t *testing.T) {
183+
tests := map[string]TestCase{
184+
"to-granular": {
185+
Ops: []Operation{
186+
Apply{
187+
Manager: "one",
188+
Object: `
189+
struct:
190+
numeric: 1
191+
string: "a"
192+
`,
193+
APIVersion: "v1",
194+
},
195+
Apply{
196+
Manager: "two",
197+
Object: `
198+
struct:
199+
string: "b"
200+
`,
201+
APIVersion: "v1",
202+
Conflicts: merge.Conflicts{
203+
merge.Conflict{Manager: "one", Path: _P("struct")},
204+
},
205+
},
206+
ChangeParser{Parser: structParser},
207+
Apply{
208+
Manager: "two",
209+
Object: `
210+
struct:
211+
string: "b"
212+
`,
213+
APIVersion: "v1",
214+
Conflicts: merge.Conflicts{
215+
merge.Conflict{Manager: "one", Path: _P("struct", "string")},
216+
},
217+
},
218+
ForceApply{
219+
Manager: "two",
220+
Object: `
221+
struct:
222+
string: "b"
223+
`,
224+
APIVersion: "v1",
225+
},
226+
},
227+
Object: `
228+
struct:
229+
numeric: 1
230+
string: "b"
231+
`,
232+
APIVersion: "v1",
233+
Managed: fieldpath.ManagedFields{
234+
"one": fieldpath.NewVersionedSet(_NS(
235+
_P("struct"),
236+
_P("struct", "numeric"),
237+
), "v1", true),
238+
"two": fieldpath.NewVersionedSet(_NS(
239+
_P("struct", "string"),
240+
), "v1", true),
241+
},
242+
},
243+
}
244+
245+
for name, test := range tests {
246+
t.Run(name, func(t *testing.T) {
247+
if err := test.Test(structWithAtomicParser); err != nil {
248+
t.Fatal(err)
249+
}
250+
})
251+
}
252+
}

‎merge/update.go‎

Lines changed: 37 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -122,13 +122,16 @@ func (s *Updater) update(oldObject, newObject *typed.TypedValue, version fieldpa
122122
// this is a CREATE call).
123123
func (s *Updater) Update(liveObject, newObject *typed.TypedValue, version fieldpath.APIVersion, managers fieldpath.ManagedFields, manager string) (*typed.TypedValue, fieldpath.ManagedFields, error) {
124124
var err error
125+
managers, err = s.reconcileManagedFieldsWithSchemaChanges(liveObject, managers)
126+
if err != nil {
127+
return nil, fieldpath.ManagedFields{}, err
128+
}
125129
if s.enableUnions {
126130
newObject, err = liveObject.NormalizeUnions(newObject)
127131
if err != nil {
128132
return nil, fieldpath.ManagedFields{}, err
129133
}
130134
}
131-
managers = shallowCopyManagers(managers)
132135
managers, compare, err := s.update(liveObject, newObject, version, managers, manager, true)
133136
if err != nil {
134137
return nil, fieldpath.ManagedFields{}, err
@@ -157,8 +160,11 @@ func (s *Updater) Update(liveObject, newObject *typed.TypedValue, version fieldp
157160
// and return it. If the object hasn't changed, nil is returned (the
158161
// managers can still have changed though).
159162
func (s *Updater) Apply(liveObject, configObject *typed.TypedValue, version fieldpath.APIVersion, managers fieldpath.ManagedFields, manager string, force bool) (*typed.TypedValue, fieldpath.ManagedFields, error) {
160-
managers = shallowCopyManagers(managers)
161163
var err error
164+
managers, err = s.reconcileManagedFieldsWithSchemaChanges(liveObject, managers)
165+
if err != nil {
166+
return nil, fieldpath.ManagedFields{}, err
167+
}
162168
if s.enableUnions {
163169
configObject, err = configObject.NormalizeUnionsApply(configObject)
164170
if err != nil {
@@ -204,14 +210,6 @@ func (s *Updater) Apply(liveObject, configObject *typed.TypedValue, version fiel
204210
return newObject, managers, nil
205211
}
206212

207-
func shallowCopyManagers(managers fieldpath.ManagedFields) fieldpath.ManagedFields {
208-
newManagers := fieldpath.ManagedFields{}
209-
for manager, set := range managers {
210-
newManagers[manager] = set
211-
}
212-
return newManagers
213-
}
214-
215213
// prune will remove a field, list or map item, iff:
216214
// * applyingManager applied it last time
217215
// * applyingManager didn't apply it this time
@@ -300,3 +298,32 @@ func (s *Updater) addBackDanglingItems(merged, pruned *typed.TypedValue, lastSet
300298
}
301299
return merged.RemoveItems(mergedSet.Difference(prunedSet).Intersection(lastSet.Set())), nil
302300
}
301+
302+
// reconcileManagedFieldsWithSchemaChanges reconciles the managed fields with any changes to the
303+
// object's schema since the managed fields were written.
304+
//
305+
// Supports:
306+
// - changing types from atomic to granular
307+
// - changing types from granular to atomic
308+
func (s *Updater) reconcileManagedFieldsWithSchemaChanges(liveObject *typed.TypedValue, managers fieldpath.ManagedFields) (fieldpath.ManagedFields, error) {
309+
result := fieldpath.ManagedFields{}
310+
for manager, versionedSet := range managers {
311+
tv, err := s.Converter.Convert(liveObject, versionedSet.APIVersion())
312+
if s.Converter.IsMissingVersionError(err) { // okay to skip, obsolete versions will be deleted automatically anyway
313+
continue
314+
}
315+
if err != nil {
316+
return nil, err
317+
}
318+
reconciled, err := typed.ReconcileFieldSetWithSchema(versionedSet.Set(), tv)
319+
if err != nil {
320+
return nil, err
321+
}
322+
if reconciled != nil {
323+
result[manager] = fieldpath.NewVersionedSet(reconciled, versionedSet.APIVersion(), versionedSet.Applied())
324+
} else {
325+
result[manager] = versionedSet
326+
}
327+
}
328+
return result, nil
329+
}

0 commit comments

Comments
 (0)