From 26fb1e4c228d8bd7f245aca9bf340c9e88898d95 Mon Sep 17 00:00:00 2001 From: Maria Ntalla Date: Tue, 5 Nov 2019 17:57:57 +0000 Subject: [PATCH 1/7] Start documenting patch strategy markers --- .../devel/sig-architecture/api-conventions.md | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/contributors/devel/sig-architecture/api-conventions.md b/contributors/devel/sig-architecture/api-conventions.md index 57fbfaade06..1c0eff45cbb 100644 --- a/contributors/devel/sig-architecture/api-conventions.md +++ b/contributors/devel/sig-architecture/api-conventions.md @@ -591,6 +591,19 @@ 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). +#### Defining Patch Strategy +In the context of strategic merge patch, a set of go markers can be used by API authors to define the merge strategy of collections of fields (lists, maps and structs). + +| Marker | Definition | +|--------|------------| +| `// +patchStrategy=merge/replace` | Defines that the merge strategy will be to merge elements of the two lists into one (merge), use the list provided in the patch literally rather than merging (replace) | +| `// +patchMergeKey=` | This is applicable in the context of lists being used as maps/dictionaries, i.e. with each entry in the list seen as a key-value pair, with a specific field considered the key. `patchMergeKey` defines what 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/set/associative` | | +| `// +mapType=separate/atomic` | | +| `// +structType=separate/atomic` | | + + + ## Idempotency All compatible Kubernetes APIs MUST support "name idempotency" and respond with From ab2dd842e46c2b7645c11f4d065c243e3e56c90e Mon Sep 17 00:00:00 2001 From: Maria Ntalla Date: Mon, 16 Dec 2019 13:39:55 +0000 Subject: [PATCH 2/7] Document list/map/structType, specify listType=map (not associative) --- .../devel/sig-architecture/api-conventions.md | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/contributors/devel/sig-architecture/api-conventions.md b/contributors/devel/sig-architecture/api-conventions.md index 1c0eff45cbb..5b319de774b 100644 --- a/contributors/devel/sig-architecture/api-conventions.md +++ b/contributors/devel/sig-architecture/api-conventions.md @@ -596,11 +596,12 @@ In the context of strategic merge patch, a set of go markers can be used by API | Marker | Definition | |--------|------------| -| `// +patchStrategy=merge/replace` | Defines that the merge strategy will be to merge elements of the two lists into one (merge), use the list provided in the patch literally rather than merging (replace) | -| `// +patchMergeKey=` | This is applicable in the context of lists being used as maps/dictionaries, i.e. with each entry in the list seen as a key-value pair, with a specific field considered the key. `patchMergeKey` defines what 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/set/associative` | | -| `// +mapType=separate/atomic` | | -| `// +structType=separate/atomic` | | +| `//+patchStrategy=merge/replace` | Defines that the merge strategy will be to: merge elements of the two lists into one (if set to merge), or use the list provided in the patch literally rather than merging (if set to replace) | +| `// +patchMergeKey=` | Applicable in the context of lists being used as maps/dictionaries, i.e. with each entry in the list seen as a key-value pair, with a specific field considered the key. `patchMergeKey` defines what 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/set/map` | Applicable to objects that are of type list. If set to `atomic`, it defines that a single actor can replace the entire list, but cannot change individual elements of it. If not set to `atomic`, different actors can update individual elements in the list. In that case, the listType can either be `set` (contains scalar elements) or `map` (contains map-like elements). | +| `//+listMapKeys=` | Applicable in the context of lists being used as maps/dictionaries. A list of strings, that defines which combination of keys should be used as the identifier of each element. | +| `//+mapType=atomic/granular` | If set to `atomic`, a single actor can replace the map entirely. If set to `granular`, multiple actors can update individual map entries seprately. | +| `//+structType=atomic/granular` | If set to `atomic`, a single actor can replace the struct entirely. If set to `granular`, multiple actors can update individual struct fields seprately. | From 56a601f2061993782807050e52ef203f6536c0d5 Mon Sep 17 00:00:00 2001 From: Maria Ntalla Date: Mon, 30 Dec 2019 11:20:40 +0000 Subject: [PATCH 3/7] (minor) Clarify explanation of patch strategies --- .../devel/sig-architecture/api-conventions.md | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/contributors/devel/sig-architecture/api-conventions.md b/contributors/devel/sig-architecture/api-conventions.md index 5b319de774b..9cd68f1f25e 100644 --- a/contributors/devel/sig-architecture/api-conventions.md +++ b/contributors/devel/sig-architecture/api-conventions.md @@ -591,17 +591,17 @@ 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). -#### Defining Patch Strategy -In the context of strategic merge patch, a set of go markers can be used by API authors to define the merge strategy of collections of fields (lists, maps and structs). +#### Declaring Patch Strategy +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 merge strategy of collections of fields (lists, maps and structs). | Marker | Definition | |--------|------------| -| `//+patchStrategy=merge/replace` | Defines that the merge strategy will be to: merge elements of the two lists into one (if set to merge), or use the list provided in the patch literally rather than merging (if set to replace) | -| `// +patchMergeKey=` | Applicable in the context of lists being used as maps/dictionaries, i.e. with each entry in the list seen as a key-value pair, with a specific field considered the key. `patchMergeKey` defines what 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/set/map` | Applicable to objects that are of type list. If set to `atomic`, it defines that a single actor can replace the entire list, but cannot change individual elements of it. If not set to `atomic`, different actors can update individual elements in the list. In that case, the listType can either be `set` (contains scalar elements) or `map` (contains map-like elements). | -| `//+listMapKeys=` | Applicable in the context of lists being used as maps/dictionaries. A list of strings, that defines which combination of keys should be used as the identifier of each element. | -| `//+mapType=atomic/granular` | If set to `atomic`, a single actor can replace the map entirely. If set to `granular`, multiple actors can update individual map entries seprately. | -| `//+structType=atomic/granular` | If set to `atomic`, a single actor can replace the struct entirely. If set to `granular`, multiple actors can update individual struct fields seprately. | +| `//+patchStrategy=merge/replace` | Defines that the merge strategy will be to: merge elements of the two lists into one (if set to merge), or use the list provided in the patch literally rather than merging (if set to replace). Default is `replace`. | +| `// +patchMergeKey=` | 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/set/map` | Applicable to objects that are of type list. If set to `atomic`, it defines that a single actor can replace the entire list, but cannot change individual elements of it. Unless set to `atomic`, different actors can update individual elements in the list. In that case, the listType can either be `set` (contains scalar elements only) or `map` (contains map-like elements). | +| `//+listMapKeys=` | Applicable in the context of lists being used as maps/dictionaries. It's a list of strings, that defines which combination of keys should be used as the identifier of each element. | +| `//+mapType=atomic/granular` | If set to `atomic`, a single actor can replace the map entirely. If set to `granular`, multiple actors can update individual map entries separately. | +| `//+structType=atomic/granular` | If set to `atomic`, a single actor can replace the struct entirely. If set to `granular`, multiple actors can update individual struct fields separately. | From 2473333d7becb779dbbefdabab52d3c5fe72d5c8 Mon Sep 17 00:00:00 2001 From: Maria Ntalla Date: Mon, 30 Dec 2019 16:27:11 +0000 Subject: [PATCH 4/7] Add a section on topology update considerations It mixes up a couple of different concepts though...needs some more work. --- .../devel/sig-architecture/api-conventions.md | 23 +++++++++++++++---- 1 file changed, 18 insertions(+), 5 deletions(-) diff --git a/contributors/devel/sig-architecture/api-conventions.md b/contributors/devel/sig-architecture/api-conventions.md index 9cd68f1f25e..a50387d7bd5 100644 --- a/contributors/devel/sig-architecture/api-conventions.md +++ b/contributors/devel/sig-architecture/api-conventions.md @@ -592,7 +592,7 @@ 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 -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 merge strategy of collections of fields (lists, maps and structs). +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 collections of fields (lists, maps and structs). | Marker | Definition | |--------|------------| @@ -600,10 +600,23 @@ In the context of [strategic merge patch](/contributors/devel/sig-api-machinery/ | `// +patchMergeKey=` | 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/set/map` | Applicable to objects that are of type list. If set to `atomic`, it defines that a single actor can replace the entire list, but cannot change individual elements of it. Unless set to `atomic`, different actors can update individual elements in the list. In that case, the listType can either be `set` (contains scalar elements only) or `map` (contains map-like elements). | | `//+listMapKeys=` | Applicable in the context of lists being used as maps/dictionaries. It's a list of strings, that defines which combination of keys should be used as the identifier of each element. | -| `//+mapType=atomic/granular` | If set to `atomic`, a single actor can replace the map entirely. If set to `granular`, multiple actors can update individual map entries separately. | -| `//+structType=atomic/granular` | If set to `atomic`, a single actor can replace the struct entirely. If set to `granular`, multiple actors can update individual struct fields separately. | - - +| `//+mapType=atomic/granular` | If set to `atomic`, a single actor can only replace the map entirely. If set to `granular`, multiple actors can update individual map entries separately. | +| `//+structType=atomic/granular` | If set to `atomic`, a single actor can only replace the struct entirely. If set to `granular`, multiple actors can update individual struct fields separately. | + +##### Considerations on updates to patch strategies/topologies, and server-side apply + +A newer version of an API may decide to redefine the patch strategy or topology for a list, map, or struct. This section lists some scenarios and expected behaviours. + +* A `listType` can only be updated from `atomic` to `set` if the list elements are scalars. Similarly, `listType` updates from `atomic` to `map` will only succeed if the list items are key-value pairs, and a `listMapKeys` marker is added to define the list of keys that uniquely identify a list entry. +* When updating `listType` from `set` to `atomic`, the next actor (e.g. `kubectl`) to apply the list will entirely replace it. The operation will not give out conflicts, even if the actor has't previously issued requests against this field. +* When using a Kubernetes API version that does not support setting list, map and struct topologies (1.15 and before): lists, maps and structs are considered atomic. When upgrading to an API version that supports declaring topology for these fields (starting at 1.16): + * if `listType` is set to `set` or `map` + * if `listType` is set to `atomic`, `apply` operations using server-side apply will result in conflicts, unless they leave the field values unchanged. +* Updates from specifying topologies to _not_ specifying them, will be equivalent to the topologies being set to `atomic`. +* Updates between scalar and nested types are allowed, but are risky. If such a path must be followed, it's best to test outside a production system, and configure to `atomic` as a middle step. + * From `set` to `map`: while new objects can be created without problems following the updated topology, existing objects can no longer be changed. + * From `map` to `set`: there's high risk of losing or corrupting data of existing objects. Just reconfiguring `listType` without reapplying objects will empty the contents of existing objects. +* Updating `listMapKeys` for a `listType=map` will succeed as long as the spec of the existing objects is valid by both key validations; the original and the update. For example, with an original configuration of `listMapKeys=port,protocol` and an updated configuration of `listMapKeys=name,version`, list entries of an existing object must have unique, not omitted combinations of both `port,protocol` and `name,version`; otherwise the object can't be updated. ## Idempotency From 09c31c1616c51f75b64de4a1db523c8e7121a12e Mon Sep 17 00:00:00 2001 From: Maria Ntalla Date: Tue, 21 Jan 2020 17:29:47 +0000 Subject: [PATCH 5/7] Split directives in separate lines for readability --- .../devel/sig-architecture/api-conventions.md | 34 +++++++------------ 1 file changed, 12 insertions(+), 22 deletions(-) diff --git a/contributors/devel/sig-architecture/api-conventions.md b/contributors/devel/sig-architecture/api-conventions.md index a50387d7bd5..5b89462f5c2 100644 --- a/contributors/devel/sig-architecture/api-conventions.md +++ b/contributors/devel/sig-architecture/api-conventions.md @@ -591,32 +591,22 @@ 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 -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 collections of fields (lists, maps and structs). +#### 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/replace` | Defines that the merge strategy will be to: merge elements of the two lists into one (if set to merge), or use the list provided in the patch literally rather than merging (if set to replace). Default is `replace`. | +| `//+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 touse the list provided in the patch literally rather than merging. | | `// +patchMergeKey=` | 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/set/map` | Applicable to objects that are of type list. If set to `atomic`, it defines that a single actor can replace the entire list, but cannot change individual elements of it. Unless set to `atomic`, different actors can update individual elements in the list. In that case, the listType can either be `set` (contains scalar elements only) or `map` (contains map-like elements). | -| `//+listMapKeys=` | Applicable in the context of lists being used as maps/dictionaries. It's a list of strings, that defines which combination of keys should be used as the identifier of each element. | -| `//+mapType=atomic/granular` | If set to `atomic`, a single actor can only replace the map entirely. If set to `granular`, multiple actors can update individual map entries separately. | -| `//+structType=atomic/granular` | If set to `atomic`, a single actor can only replace the struct entirely. If set to `granular`, multiple actors can update individual struct fields separately. | - -##### Considerations on updates to patch strategies/topologies, and server-side apply - -A newer version of an API may decide to redefine the patch strategy or topology for a list, map, or struct. This section lists some scenarios and expected behaviours. - -* A `listType` can only be updated from `atomic` to `set` if the list elements are scalars. Similarly, `listType` updates from `atomic` to `map` will only succeed if the list items are key-value pairs, and a `listMapKeys` marker is added to define the list of keys that uniquely identify a list entry. -* When updating `listType` from `set` to `atomic`, the next actor (e.g. `kubectl`) to apply the list will entirely replace it. The operation will not give out conflicts, even if the actor has't previously issued requests against this field. -* When using a Kubernetes API version that does not support setting list, map and struct topologies (1.15 and before): lists, maps and structs are considered atomic. When upgrading to an API version that supports declaring topology for these fields (starting at 1.16): - * if `listType` is set to `set` or `map` - * if `listType` is set to `atomic`, `apply` operations using server-side apply will result in conflicts, unless they leave the field values unchanged. -* Updates from specifying topologies to _not_ specifying them, will be equivalent to the topologies being set to `atomic`. -* Updates between scalar and nested types are allowed, but are risky. If such a path must be followed, it's best to test outside a production system, and configure to `atomic` as a middle step. - * From `set` to `map`: while new objects can be created without problems following the updated topology, existing objects can no longer be changed. - * From `map` to `set`: there's high risk of losing or corrupting data of existing objects. Just reconfiguring `listType` without reapplying objects will empty the contents of existing objects. -* Updating `listMapKeys` for a `listType=map` will succeed as long as the spec of the existing objects is valid by both key validations; the original and the update. For example, with an original configuration of `listMapKeys=port,protocol` and an updated configuration of `listMapKeys=name,version`, list entries of an existing object must have unique, not omitted combinations of both `port,protocol` and `name,version`; otherwise the object can't be updated. +| `//+listType=atomic/set/map` | Applicable to objects that are of type list. A single actor can replace the entire list, but cannot change individual elements of it. Unless set to `atomic`, different actors can update individual elements in the list. In that case, the listType can either be `set` (contains scalar elements only) or `map` (contains map-like elements). | +| `//+listType=set` | Applicable to objects that are of type list, and the elements are scalar. Different actors can update individual elements in the list. | +| `//+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=` | Applicable in the context where `listType=map`. `listMapKeys` a list of strings, that defines which combination of keys should be used as the identifier of each element. | +| `//+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 From 957240d2df62066093d221d9e7195def6aeadc2a Mon Sep 17 00:00:00 2001 From: Maria Ntalla Date: Thu, 23 Jan 2020 16:41:14 +0000 Subject: [PATCH 6/7] Correct marker references, clarify types --- contributors/devel/sig-architecture/api-conventions.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/contributors/devel/sig-architecture/api-conventions.md b/contributors/devel/sig-architecture/api-conventions.md index 5b89462f5c2..e8a1bc92d7c 100644 --- a/contributors/devel/sig-architecture/api-conventions.md +++ b/contributors/devel/sig-architecture/api-conventions.md @@ -599,10 +599,10 @@ In the context of [strategic merge patch](/contributors/devel/sig-api-machinery/ | `//+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 touse the list provided in the patch literally rather than merging. | | `// +patchMergeKey=` | 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/set/map` | Applicable to objects that are of type list. A single actor can replace the entire list, but cannot change individual elements of it. Unless set to `atomic`, different actors can update individual elements in the list. In that case, the listType can either be `set` (contains scalar elements only) or `map` (contains map-like elements). | -| `//+listType=set` | Applicable to objects that are of type list, and the elements are scalar. Different actors can update individual elements in the list. | +| `//+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. | | `//+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=` | Applicable in the context where `listType=map`. `listMapKeys` a list of strings, that defines which combination of keys should be used as the identifier of each element. | +| `//+listMapKeys=` | 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. | From df4289b7a057d264cc32adf7d982936221c65371 Mon Sep 17 00:00:00 2001 From: Maria Ntalla Date: Mon, 3 Feb 2020 10:54:14 +0000 Subject: [PATCH 7/7] Fix typo touse -> to use --- contributors/devel/sig-architecture/api-conventions.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contributors/devel/sig-architecture/api-conventions.md b/contributors/devel/sig-architecture/api-conventions.md index e8a1bc92d7c..cfbd3620cc3 100644 --- a/contributors/devel/sig-architecture/api-conventions.md +++ b/contributors/devel/sig-architecture/api-conventions.md @@ -597,7 +597,7 @@ In the context of [strategic merge patch](/contributors/devel/sig-api-machinery/ | 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 touse the list provided in the patch literally rather than merging. | +| `//+patchStrategy=replace` (default)| Defines that the merge strategy will be to use the list provided in the patch literally rather than merging. | | `// +patchMergeKey=` | 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. |