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 duplicate finalizers race condition #965

Merged
merged 3 commits into from Jul 28, 2022

Conversation

alex-hunt-materialize
Copy link
Contributor

@alex-hunt-materialize alex-hunt-materialize commented Jul 27, 2022

Potential fix for duplicate finalizers race condition, by testing that the finalizers haven't changed since we pulled the object.

I have confirmed that this doesn't break anything, but I have no idea how to test that it actually solves the issue.

Resolves #964

Motivation

If multiple controllers attempt to modify the finalizers for an object, they previously could make modifications that conflicted with each other. Finalizers are modified by list index, so we must know that the entire set of finalizers is the same to safely make any modifications.

Solution

Test that finalizers haven't changed since we pulled the object.

kube-rs#964

Signed-off-by: Alex Hunt <alex.hunt@materialize.com>
Copy link
Member

@nightkr nightkr left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

I'd just like to add a comment for the future about why we do the otherwise-seemingly-pointless test.

kube-runtime/src/finalizer.rs Outdated Show resolved Hide resolved
@clux clux added the changelog-fix changelog fix category for prs label Jul 28, 2022
@clux clux added this to the 0.75.0 milestone Jul 28, 2022
Co-authored-by: Teo Klestrup Röijezon <teo@nullable.se>
Signed-off-by: alex-hunt-materialize <86254996+alex-hunt-materialize@users.noreply.github.com>
@codecov-commenter
Copy link

Codecov Report

Merging #965 (0c95615) into master (b6cc8f9) will decrease coverage by 0.04%.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##           master     #965      +/-   ##
==========================================
- Coverage   72.48%   72.43%   -0.05%     
==========================================
  Files          64       64              
  Lines        4426     4430       +4     
==========================================
+ Hits         3208     3209       +1     
- Misses       1218     1221       +3     
Impacted Files Coverage Δ
kube-runtime/src/finalizer.rs 0.00% <0.00%> (ø)
kube-runtime/src/wait.rs 71.69% <0.00%> (+1.88%) ⬆️

Signed-off-by: Alex Hunt <alex.hunt@materialize.com>
@clux clux merged commit 3cab178 into kube-rs:master Jul 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog-fix changelog fix category for prs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Duplicate finalizers when multiple controllers attempt to reconcile a pod
4 participants