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.

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

fix #4516: adding a blocking delete operation #4578

Merged
merged 3 commits into from Dec 7, 2022

Conversation

shawkins
Copy link
Contributor

@shawkins shawkins commented Nov 15, 2022

Description

Fixes #4516 with the ability to make delete a blocking operation.

The context changes are due to elevating timeout to the base context. To avoid multiple inheritance issues with Timeoutable it was converted to non-generic.

This option turns out to be a lot simpler than adding the force option to serverSideApply and replace - as all of the delete options (timeout, grace period, propagation) are needed for those contexts as well. And we just introduced a NonDeletingOperation interface which further complicates what could be common api point for specifying delete options.

The blocking delete is similar to what exists in the kubectl client when --wait is used as a parameter (delete, apply, replace). The returned statusdetails are used to determine the set of uids that you're tracking. Once none of those exist, you'll consider things deleted. A corner case is the possibility of incomplete statusdetails - that was noted in an old issue, but I'm not sure that it is still relevant and kubectl doesn't consider that case either.

This simplifies the workaround to just two calls:
op.withTimeout(10, TimeUnit.SECONDS).delete();
op.create();

This is also applicable to #3246 and similar issues.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • Feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change
  • Chore (non-breaking change which doesn't affect codebase;
    test, version modification, documentation, etc.)

Checklist

  • Code contributed by me aligns with current project license: Apache 2.0
  • I Added CHANGELOG entry regarding this change
  • I have implemented unit tests to cover my changes
  • I have added/updated the javadocs and other documentation accordingly
  • No new bugs, code smells, etc. in SonarCloud report
  • I tested my code in Kubernetes
  • I tested my code in OpenShift

@shawkins shawkins marked this pull request as draft November 15, 2022 18:25
@shawkins shawkins force-pushed the delete_existing branch 2 times, most recently from 7a88644 to ddbac28 Compare November 16, 2022 12:16
@shawkins
Copy link
Contributor Author

Updated with a changelog and rebased to master.

One change from the previous commit is that the blocking method was generalized so that it would work for list contexts to be applied in parallel, rather than serially.

@shawkins shawkins marked this pull request as ready for review November 28, 2022 20:46
@sonarcloud
Copy link

sonarcloud bot commented Nov 29, 2022

SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 1 Code Smell

19.4% 19.4% Coverage
9.7% 9.7% Duplication

@manusa manusa added this to the 6.3.0 milestone Dec 7, 2022
@manusa manusa merged commit 6bcb118 into fabric8io:master Dec 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Replacement for deleteExisting behavior.
2 participants