Skip to content

Commit 0861047

Browse files
author
ymqytw
committed
add a section and one alternitive
1 parent 23721e4 commit 0861047

File tree

1 file changed

+115
-5
lines changed

1 file changed

+115
-5
lines changed

contributors/design-proposals/preserve-order-in-strategic-merge-patch.md

+115-5
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,49 @@ mergingList:
5555
- otherItemN /
5656
```
5757
58+
### When $setElementOrder is not present and patching a list
59+
60+
The new directive $setElementOrder is optional.
61+
The new server applying a patch request without the directive will be
62+
a best-effort operation to keep the order of elements that are already in the list:
63+
64+
For changes and deletions, the server will just merge the change without sorting them.
65+
So the items in the list will retain the order.
66+
67+
For additions, the items will be appended to the end of the list.
68+
69+
So it changes a little from previous releases, but is still fully backward compatible with old clients.
70+
71+
Examples where A and C have been changed, B has been deleted and D has been added.
72+
73+
Patch:
74+
75+
```yaml
76+
list:
77+
- A'
78+
- $patch: delete
79+
B
80+
- D
81+
```
82+
83+
Live:
84+
85+
```yaml
86+
list:
87+
- C
88+
- B
89+
- A
90+
```
91+
92+
Result:
93+
94+
```yaml
95+
list:
96+
- C
97+
- A
98+
- D
99+
```
100+
58101
### `$setElementOrder` may contain elements not present in the patch list
59102

60103
The $setElementOrder value may contain elements that are not present in the patch
@@ -158,11 +201,10 @@ list:
158201
The new version patch is always a superset of the old version patch.
159202
The new patch has one additional parallel list which will be dropped by the old server.
160203

161-
The new directive is optional.
162-
Patch requests without the directive will retain the behavior from previous releases.
163-
This will ensure backward compatibility with old clients.
164-
165-
This change is fully backward compatible.
204+
As mentioned [above](#when-setelementorder-is-not-present-and-patching-a-list),
205+
the new directive is optional.
206+
Patch requests without the directive will change a little,
207+
but still be fully backward compatible.
166208

167209
### kubectl
168210
If an old kubectl sends a old patch to a new server, the server will not preserve the order which behave as an old server.
@@ -329,6 +371,74 @@ finalizers:
329371

330372
# Alternative Considered
331373

374+
# 1. Use the patch list to set order
375+
376+
## Proposed Change
377+
378+
This approach can considered as merging the parallel list and patch list into one single list.
379+
380+
For list of maps, the patch list will have all entries that are
381+
either a map that contains the mergeKey and other changes
382+
or a map that contains the mergeKey only.
383+
384+
For list of primitives, the patch list will be the same as the list in users' local config.
385+
386+
## Reason of Rejection
387+
388+
It cannot work correctly in the following concurrent writers case,
389+
because PATCH in k8s doesn't use optimistic locking, so the following may happen.
390+
391+
Live config is:
392+
393+
```yaml
394+
list:
395+
- mergeKey: a
396+
other: A
397+
- mergeKey: b
398+
other: B
399+
- mergeKey: c
400+
other: C
401+
```
402+
403+
Writer foo first GET the object from the server.
404+
It wants to delete B, so it calculate the patch and is about to send it to the server:
405+
406+
```yaml
407+
list:
408+
- mergeKey: a
409+
- mergeKey: b
410+
$patch: delete
411+
- mergeKey: c
412+
```
413+
414+
Before foo sending the patch to the server,
415+
writer bar GET the object and it want to update A.
416+
417+
Patch from bar is:
418+
419+
```yaml
420+
list:
421+
- mergeKey: a
422+
other: A'
423+
- mergeKey: b
424+
- mergeKey: c
425+
```
426+
427+
After the server first applying foo's patch and then bar's patch,
428+
the final result will be wrong.
429+
Because entry b has been recreated which is not desired.
430+
431+
```yaml
432+
list:
433+
- mergeKey: a
434+
other: A
435+
- mergeKey: b
436+
- mergeKey: c
437+
other: C
438+
```
439+
440+
# 2. Use $position Directive
441+
332442
## Proposed Change
333443

334444
Use an approach similar to [MongoDB](https://docs.mongodb.com/manual/reference/operator/update/position/).

0 commit comments

Comments
 (0)