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

Issue-180: Add disable finalizer config #181

Merged
merged 5 commits into from
Oct 19, 2021

Conversation

CraneShiEMC
Copy link
Contributor

Change log description

add DisableFinalizer config and skip adding zkFinalziers when set it to true.

Purpose of the change

Fixes #180

What the code does

skip adding zkFinalziers when DisableFinalizer set to true.

How to verify it

Set disableFinalizer to true in values and deploy Bookkeeper operator, the deployed Bookkeeper clusters should have the finalizers as empty.

Signed-off-by: Shi, Crane <crane.shi@emc.com>
Signed-off-by: Shi, Crane <crane.shi@emc.com>
@codecov-commenter
Copy link

codecov-commenter commented Oct 18, 2021

Codecov Report

Merging #181 (bc698fc) into master (7f5b39f) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #181   +/-   ##
=======================================
  Coverage   80.36%   80.36%           
=======================================
  Files          10       10           
  Lines        1839     1839           
=======================================
  Hits         1478     1478           
  Misses        281      281           
  Partials       80       80           
Impacted Files Coverage Δ
.../bookkeepercluster/bookkeepercluster_controller.go 53.19% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7f5b39f...bc698fc. Read the comment docs.

Signed-off-by: Aaron <aaron.wu@dell.com>
Signed-off-by: Shi, Crane <crane.shi@emc.com>
cmd/manager/main.go Outdated Show resolved Hide resolved
@@ -72,6 +73,10 @@ func main() {
log.Warn("----- Running in test mode. Make sure you are NOT in production -----")
}

if controllerconfig.DisableFinalizer {
log.Warn("----- Running with finalizer disabled. -----")
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea. We probably should do that on the other operators to avoid confusion for anybody debugging issues in future.

Co-authored-by: David Maddison <maddisondavid@gmail.com>
Signed-off-by: Shi, Crane <crane.shi@emc.com>
Copy link
Contributor

@anishakj anishakj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@anishakj
Copy link
Contributor

@CraneShiEMC looks like PR is forked from a wrong repo, Could you please correct it

@anishakj anishakj merged commit 0f43cf8 into pravega:master Oct 19, 2021
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.

Add customized config to skip finalizers.
5 participants