-
Notifications
You must be signed in to change notification settings - Fork 218
feat: using SSA for finalizer and primary patch #2305
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.
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
Merged
Merged
Changes from all commits
Commits
Show all changes
28 commits
Select commit
Hold shift + click to select a range
2a70c0d
feat: using SSA for finalizer and primary patch
csviri 051aedc
test fix
csviri 19da907
format
csviri 3c5646c
wip
csviri 4b2bb22
wip
csviri da598ab
wip
csviri e221291
wip
csviri d22e2d0
builds now
csviri 7146d9b
unit tests pass
csviri dec7c8f
format
csviri 5e28d7d
finalizer removal with patch without SSA
csviri 6719270
format
csviri a201d1e
patch related changes + IT fixes
csviri d79eeb1
format
csviri b0f1447
test wip
csviri e970d46
unit tests
csviri 04f1bad
Integration test
csviri 1253093
ITs
csviri 03f0010
IT
csviri 922925f
web page sample update
csviri 500c18d
refactor
csviri ff9c699
docs updatestarted
csviri df454eb
docs
csviri d8ac961
docs
csviri 4f1054b
docs: improve
metacosm 52f4da0
refactor: simplify finalizer handling
metacosm 7b29dc0
fix: format
metacosm 89f5e0e
better expection message
csviri File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
114 changes: 22 additions & 92 deletions
114
...ramework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/UpdateControl.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,150 +1,80 @@ | ||
package io.javaoperatorsdk.operator.api.reconciler; | ||
|
||
import java.util.Optional; | ||
|
||
import io.fabric8.kubernetes.api.model.HasMetadata; | ||
import io.fabric8.kubernetes.client.CustomResource; | ||
|
||
public class UpdateControl<P extends HasMetadata> extends BaseControl<UpdateControl<P>> { | ||
|
||
private final P resource; | ||
private final boolean updateStatus; | ||
private final boolean updateResource; | ||
private final boolean patchResource; | ||
private final boolean patchStatus; | ||
|
||
private UpdateControl( | ||
P resource, boolean updateStatus, boolean updateResource, boolean patchStatus) { | ||
if ((updateResource || updateStatus) && resource == null) { | ||
P resource, boolean patchResource, boolean patchStatus) { | ||
if ((patchResource || patchStatus) && resource == null) { | ||
throw new IllegalArgumentException("CustomResource cannot be null in case of update"); | ||
} | ||
this.resource = resource; | ||
this.updateStatus = updateStatus; | ||
this.updateResource = updateResource; | ||
this.patchResource = patchResource; | ||
this.patchStatus = patchStatus; | ||
} | ||
|
||
/** | ||
* Creates an update control instance that instructs the framework to do an update on resource | ||
* itself, not on the status. Note that usually as a results of a reconciliation should be a | ||
* status update not an update to the resource itself. | ||
* | ||
* Using this update makes sure that the resource in the next reconciliation is the updated one - | ||
* this is not guaranteed by default if you do an update on a resource by the Kubernetes client. | ||
* | ||
* @param <T> custom resource type | ||
* @param customResource customResource to use for update | ||
* @return initialized update control | ||
*/ | ||
public static <T extends HasMetadata> UpdateControl<T> updateResource(T customResource) { | ||
return new UpdateControl<>(customResource, false, true, false); | ||
} | ||
|
||
/** | ||
* Preferred way to update the status. It does not do optimistic locking. Uses JSON Patch to patch | ||
* the resource. | ||
* <p> | ||
* Note that this does not work, if the {@link CustomResource#initStatus() initStatus} is | ||
* implemented, since it breaks the diffing process. Don't implement it if using this method. | ||
* Note that this does not work, if the {@link CustomResource#initStatus()} is implemented, since | ||
* it breaks the diffing process. Don't implement it if using this method. | ||
* </p> | ||
* There is also an issue with setting value to null with older Kubernetes versions (1.19 and | ||
* below). See: <a href= | ||
* There is also an issue with setting value to {@code null} with older Kubernetes versions (1.19 | ||
* and below). See: <a href= | ||
* "https://github.com/fabric8io/kubernetes-client/issues/4158">https://github.com/fabric8io/kubernetes-client/issues/4158</a> | ||
* | ||
* @param <T> resource type | ||
* @param customResource the custom resource with target status | ||
* @return UpdateControl instance | ||
*/ | ||
public static <T extends HasMetadata> UpdateControl<T> patchStatus(T customResource) { | ||
return new UpdateControl<>(customResource, true, false, true); | ||
} | ||
|
||
/** | ||
* Note that usually "patchStatus" is advised to be used instead of this method. | ||
* <p> | ||
* Updates the status with optimistic locking regarding current resource version reconciled. Note | ||
* that this also ensures that on next reconciliation is the most up-to-date custom resource is | ||
* used. | ||
* </p> | ||
* | ||
* @param <T> resource type | ||
* @param customResource the custom resource with target status | ||
* @return UpdateControl instance | ||
*/ | ||
public static <T extends HasMetadata> UpdateControl<T> updateStatus(T customResource) { | ||
return new UpdateControl<>(customResource, true, false, false); | ||
} | ||
|
||
/** | ||
* As a results of this there will be two call to K8S API. First the custom resource will be | ||
* updates then the status sub-resource. | ||
* | ||
* Using this update makes sure that the resource in the next reconciliation is the updated one - | ||
* this is not guaranteed by default if you do an update on a resource by the Kubernetes client. | ||
* | ||
* @param <T> resource type | ||
* @param customResource - custom resource to use in both API calls | ||
* @return UpdateControl instance | ||
*/ | ||
public static <T extends HasMetadata> UpdateControl<T> updateResourceAndStatus( | ||
T customResource) { | ||
return new UpdateControl<>(customResource, true, true, false); | ||
return new UpdateControl<>(customResource, false, true); | ||
} | ||
|
||
/** | ||
* Updates the resource - with optimistic locking - and patches the status without optimistic | ||
* locking in place. | ||
* | ||
* Note that using this method, it is not guaranteed that the most recent updated resource will be | ||
* in case for next reconciliation. | ||
* | ||
* @param customResource to update | ||
* @return UpdateControl instance | ||
* @param <T> resource type | ||
*/ | ||
public static <T extends HasMetadata> UpdateControl<T> updateResourceAndPatchStatus( | ||
T customResource) { | ||
return new UpdateControl<>(customResource, true, true, true); | ||
public static <T extends HasMetadata> UpdateControl<T> patchResource(T customResource) { | ||
return new UpdateControl<>(customResource, true, false); | ||
} | ||
|
||
/** | ||
* Marked for removal, because of confusing name. It does not patch the resource but rather | ||
* updates it. | ||
* | ||
* @deprecated use {@link UpdateControl#updateResourceAndPatchStatus(HasMetadata)} | ||
* | ||
* @param customResource to update | ||
* @return UpdateControl instance | ||
* @param <T> resource type | ||
*/ | ||
@Deprecated(forRemoval = true) | ||
public static <T extends HasMetadata> UpdateControl<T> patchResourceAndStatus(T customResource) { | ||
return updateResourceAndStatus(customResource); | ||
return new UpdateControl<>(customResource, true, true); | ||
} | ||
|
||
public static <T extends HasMetadata> UpdateControl<T> noUpdate() { | ||
return new UpdateControl<>(null, false, false, false); | ||
} | ||
|
||
public P getResource() { | ||
return resource; | ||
return new UpdateControl<>(null, false, false); | ||
} | ||
|
||
public boolean isUpdateStatus() { | ||
return updateStatus; | ||
public Optional<P> getResource() { | ||
return Optional.ofNullable(resource); | ||
} | ||
|
||
public boolean isUpdateResource() { | ||
return updateResource; | ||
public boolean isPatchResource() { | ||
return patchResource; | ||
} | ||
|
||
public boolean isPatchStatus() { | ||
return patchStatus; | ||
} | ||
|
||
public boolean isNoUpdate() { | ||
return !updateResource && !updateStatus; | ||
return !patchResource && !patchStatus; | ||
} | ||
|
||
public boolean isUpdateResourceAndStatus() { | ||
return updateResource && updateStatus; | ||
public boolean isPatchResourceAndStatus() { | ||
return patchResource && patchStatus; | ||
} | ||
|
||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why was this removed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This works only if the patch is with optimistic locking, but now neither of the patches do. Mainly for default SSA it definitely should not be, but we should not press it probably for non-SSA either.