Skip to content

Commit 38a5adb

Browse files
committed
adding create function proposal and modifying reasoning
Signed-off-by: James Fuller <james.fuller@mettle.co.uk>
1 parent 6f7f11c commit 38a5adb

File tree

1 file changed

+20
-9
lines changed

1 file changed

+20
-9
lines changed

052-k8s-server-side-apply.md

Lines changed: 20 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -21,33 +21,44 @@ This proposal makes Kubernetes keep track of each field's "owner" and any change
2121

2222
This would let the Kubernetes apiserver handle the three-way diff, updates etc, and allow other controllers, webhooks etc to touch fields, labels, annotations etc the operator doesn't care about.
2323

24+
Conflicts can happen with Server Side Apply as described [here](https://github.com/fabric8io/kubernetes-client/blob/v6.5.1/doc/CHEATSHEET.md#server-side-apply), it is open to choose the method that Strimzi would use for this.
25+
26+
[Kubernetes suggest](https://kubernetes.io/docs/reference/using-api/server-side-apply/#conflicts:~:text=It%20is%20strongly%20recommended%20for%20controllers%20to%20always%20%22force%22%20conflicts%2C%20since%20they%20might%20not%20be%20able%20to%20resolve%20or%20act%20on%20these%20conflicts.) using the above _force_ method to make sure Strimzi have control over the parts they want. As described in the Kubernetes docs _"This forces the operation to succeed, changes the value of the field, and removes the field from all other managers' entries in managedFields"_, this obviously means if another party is _forcing_ the same field that they will create a loop of ownership but this would be rare as operators should only care about their own fields.
27+
2428
It appears as simple as modifying [this line](https://github.com/strimzi/strimzi-kafka-operator/blob/18d76bfabcfb9e91c71f9afda60b9dd880797f02/operator-common/src/main/java/io/strimzi/operator/common/operator/resource/AbstractNamespacedResourceOperator.java#LL263C88-L263C102).
2529
From:
2630

2731
```
28-
....patch(PatchContext.of(PatchType.JSON)...
32+
....patch(PatchContext.of(PatchType.JSON), desired);
2933
```
3034

3135
To:
3236

3337
```
34-
....patch(PatchContext.of(PatchType.SERVER_SIDE_APPLY)...
38+
....patch(new PatchContext.Builder().withPatchType(PatchType.SERVER_SIDE_APPLY).withForce(true).build(), desired);
3539
```
3640

37-
According to the [package being used](https://github.com/fabric8io/kubernetes-client/blob/v6.5.1/doc/CHEATSHEET.md#server-side-apply).
41+
client.services().withName("name").patch(service);
3842

39-
NOTE: The version of fabric8io being used is [6.5.1](https://github.com/strimzi/strimzi-kafka-operator/blob/18d76bfabcfb9e91c71f9afda60b9dd880797f02/pom.xml#L106) which [contains Server Side Apply](https://github.com/fabric8io/kubernetes-client/blob/v6.5.1/doc/CHEATSHEET.md#server-side-apply).
43+
And Kubernetes recommend it is used on CREATE also:
44+
[From](https://github.com/strimzi/strimzi-kafka-operator/blob/18d76bfabcfb9e91c71f9afda60b9dd880797f02/operator-common/src/main/java/io/strimzi/operator/common/operator/resource/AbstractNamespacedResourceOperator.java#L272):
4045

41-
It is likely that other parts of the codebase will need modifying in terms of label/annotation discovery so that they only check labels/annotations that are owned by themselves.
42-
Unfortunately, as the person raising this proposal I am unaware of Java and precisely how the codebase works and seek input from others.
46+
```
47+
ReconcileResult<T> result = ReconcileResult.created(operation().inNamespace(namespace).resource(desired).create());
48+
```
4349

44-
Conflicts can happen with Server Side Apply as described [here](https://github.com/fabric8io/kubernetes-client/blob/v6.5.1/doc/CHEATSHEET.md#server-side-apply), it is open to discussion the method that Strimzi would use for this, if _force_ is chosen then it is implemented slightly differently, [like so](https://github.com/fabric8io/kubernetes-client/blob/v6.5.1/doc/CHEATSHEET.md#server-side-apply:~:text=If%20the%20resources,true).build()%2C%20service)%3B):
50+
To:
4551

4652
```
47-
....patch(new PatchContext.Builder().withPatchType(PatchType.SERVER_SIDE_APPLY).withForce(true).build(), service);
53+
ReconcileResult<T> result = ReconcileResult.patched(operation().inNamespace(namespace).withName(name).patch(new PatchContext.Builder().withPatchType(PatchType.SERVER_SIDE_APPLY).withForce(true).build(), desired);
4854
```
4955

50-
I suggest using the above _force_ method to make sure Strimzi have control over the parts they want. As described in the Kubernetes docs _"This forces the operation to succeed, changes the value of the field, and removes the field from all other managers' entries in managedFields"_, this obviously means if another party is _forcing_ the same field that they will create a loop of ownership but this would be rare as operators should only care about their own fields.
56+
According to the [package being used](https://github.com/fabric8io/kubernetes-client/blob/v6.5.1/doc/CHEATSHEET.md#server-side-apply).
57+
58+
NOTE: The version of fabric8io being used is [6.5.1](https://github.com/strimzi/strimzi-kafka-operator/blob/18d76bfabcfb9e91c71f9afda60b9dd880797f02/pom.xml#L106) which [contains Server Side Apply](https://github.com/fabric8io/kubernetes-client/blob/v6.5.1/doc/CHEATSHEET.md#server-side-apply).
59+
60+
It is likely that other parts of the codebase will need modifying in terms of label/annotation discovery so that they only check labels/annotations that are owned by themselves.
61+
Unfortunately, as the person raising this proposal I am unaware of Java and precisely how the codebase works and seek input from others.
5162

5263
### Three-way diff
5364

0 commit comments

Comments
 (0)