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 issue-314 by checking zookeeperCluster resource version before updating sts #326

Merged
merged 5 commits into from
Jun 7, 2021

Conversation

srteam2020
Copy link
Contributor

Change log description

The PR makes the following changes:

  • label the current RV of the zookeeperCluster to the sts when creation
  • check whether the current RV of the zookeeperCluster is larger than the RV labeled to the sts before updating the sts

Purpose of the change

Fixes #314

@codecov-commenter
Copy link

Codecov Report

Merging #326 (78718d8) into master (a99c93b) will decrease coverage by 0.77%.
The diff coverage is 27.77%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #326      +/-   ##
==========================================
- Coverage   84.67%   83.90%   -0.78%     
==========================================
  Files          11       11              
  Lines        1312     1330      +18     
==========================================
+ Hits         1111     1116       +5     
- Misses        133      145      +12     
- Partials       68       69       +1     
Impacted Files Coverage Δ
...er/zookeepercluster/zookeepercluster_controller.go 59.39% <27.77%> (-1.28%) ⬇️

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 a99c93b...78718d8. Read the comment docs.

@anishakj
Copy link
Contributor

@srteam2020 Thanks for raising PR, Could you please add unit tests to increase the test coverage for the new function added?

@srteam2020
Copy link
Contributor Author

@anishakj Thanks for the reply! I will add an unit test for the newly added function and push again later.

@anishakj
Copy link
Contributor

anishakj commented Jun 3, 2021

@anishakj Thanks for the reply! I will add an unit test for the newly added function and push again later.

@srteam2020 Any updates on adding unit tests?

@srteam2020
Copy link
Contributor Author

Hi @anishakj I was busy on some ddls these two weeks. I will work on it this weekend.

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 anishakj merged commit b087a7f into pravega:master Jun 7, 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.

[BUG] Reading stale ZookeeperCluster spec/status can lead to undesired pod and PVC deletion
3 participants