Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[WIP] Document patch strategy markers in api-conventions #4218

Closed
wants to merge 7 commits into from
Closed

[WIP] Document patch strategy markers in api-conventions #4218

Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
26fb1e4
Start documenting patch strategy markers
mariantalla Nov 5, 2019
ab2dd84
Document list/map/structType, specify listType=map (not associative)
mariantalla Dec 16, 2019
56a601f
(minor) Clarify explanation of patch strategies
mariantalla Dec 30, 2019
2473333
Add a section on topology update considerations
mariantalla Dec 30, 2019
09c31c1
Split directives in separate lines
mariantalla Jan 21, 2020
957240d
Correct marker references, clarify types
mariantalla Jan 23, 2020
df4289b
Fix typo touse -> to use
mariantalla Feb 3, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions contributors/devel/sig-architecture/api-conventions.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -591,6 +591,23 @@ saved. For more details on how to use Merge Patch, see the RFC.
detailed explanation of how it works and why it needed to be introduced, see
[here](/contributors/devel/sig-api-machinery/strategic-merge-patch.md).

#### Declaring Patch Strategy and Object Topology
In the context of [strategic merge patch](/contributors/devel/sig-api-machinery/strategic-merge-patch.md), a set of go markers can be used by API authors to declare the patch strategy of lists, maps and structs (`//+patchStrategy` and `//+patchMergeKey`). Work to move the `apply` functionality to the control plane ([KEP](https://github.com/kubernetes/enhancements/blob/master/keps/sig-api-machinery/0006-apply.md)) has also introduced markers to describe the topology of lists, maps and structs (`//+listType`, `//+listMapKeys`, `//+mapType`, `//+structType`).

| Marker | Definition |
|--------|------------|
| `//+patchStrategy=merge` | Defines that the merge strategy will be to merge elements of the two lists into one. |
| `//+patchStrategy=replace` (default)| Defines that the merge strategy will be to use the list provided in the patch literally rather than merging. |
| `// +patchMergeKey=<key-name>` | Applicable in the context where lists are used as maps/dictionaries, i.e. where each list entry is a key-value pair. `patchMergeKey` defines which field should be seen as the key. Combined with `patchStrategy`, this describes what changes should take place on specific elements of a list as part of a patch operation. |
| `//+listType=atomic` | Applicable to objects that are of type list. A single actor owns and can replace the entire list, but cannot change individual elements of it. |
| `//+listType=set` | Applicable to objects that are of type list, and the elements are unique and scalar. Different actors can update individual elements in the list. |
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the restriction for set is that elements have to be unique and atomic (scalar or atomic list/struct/map).

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I agree with that restriction. But, I think currently we only correctly handle the case of scalar set elements, and have a TODO in the structured-merge-diff repo to support any atomic type as an element.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://github.com/kubernetes-sigs/structured-merge-diff/blob/9818ffe7e8e1c6765e6484b9ce087c8656cc60cc/typed/helpers.go#L210

| `//+listType=map` | Applicable to objects that are of type list, and elements are map-like, i.e. lists that are used as dictionaries. Different actors can update individual elements in the list. |
| `//+listMapKeys=<list of keys>` | Applicable in the context where `listType=map`. `listMapKeys` is a list of strings, that defines which combination of keys should be used as the identifier of each element. Combinations of `listMapKeys` must be unique. Values of keys may be of any scalar type. |
| `//+mapType=atomic` | A single actor can only replace the map entirely. |
| `//+mapType=granular` (default) | Multiple actors can update individual map entries separately. |
| `//+structType=atomic` | A single actor can only replace the struct entirely. |
| `//+structType=granular` (default) | Multiple actors can update individual struct fields separately. |

## Idempotency

All compatible Kubernetes APIs MUST support "name idempotency" and respond with
Expand Down