pravega / zookeeper-operator

Kubernetes Operator for Zookeeper
Apache License 2.0
366 stars 203 forks source link

Issue #410: fix reconcile if foreground cascading deletion is used #530

Open lipov3cz3k opened 1 year ago

lipov3cz3k commented 1 year ago

Change log description

Fix unexpected resource creation when the ZookeeperCluster is already deleted

Purpose of the change

Fixes #410

What the code does

Exit reconcile when resource has finalizer and is marked for delete. That's prevent to apply other reconcile functions to create already deleted resources.

How to verify it

Delete zookeepercluster resource in foreground mode -> without this patch it goes to loop (you can fix it by deleting zookeeper in default background mode)

kubectl delete zookeeperclusters.zookeeper.pravega.io zookeeper  --cascade=foreground
codecov[bot] commented 1 year ago

Codecov Report

Attention: 4 lines in your changes are missing coverage. Please review.

Comparison is base (2c8bfec) 85.91% compared to head (d383afb) 85.81%.

Files Patch % Lines
controllers/zookeepercluster_controller.go 69.23% 3 Missing and 1 partial :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #530 +/- ## ========================================== - Coverage 85.91% 85.81% -0.10% ========================================== Files 12 12 Lines 1633 1636 +3 ========================================== + Hits 1403 1404 +1 Misses 145 145 - Partials 85 87 +2 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

janhoy commented 1 year ago

I also experienced this multiple times. Have to explicitly use non-cascading delete for it to work. Can someone please review and approve this?

janhoy commented 1 year ago

@lipov3cz3k can you perhaps fix the two failed tests "DCO" and "codecov/patch", so it is more likely that the maintainers can approve and merge this?

anishakj commented 1 year ago

@lipov3cz3k Could you please increase the code coverage?

anishakj commented 9 months ago

@lipov3cz3k Is it possible to increase the coverage?