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.

Sign up for GitHub

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

Jump to bottom

routine: fix that acl stops replicating after regaining leadership (#12295) #12565

Merged
merged 3 commits into from
Apr 5, 2022
Merged

routine: fix that acl stops replicating after regaining leadership (#12295) #12565

merged 3 commits into from
Apr 5, 2022

Conversation

weichuliu
Copy link
Contributor

Fix the bug that caused acl replication to stop after a node lose-regain leadership.

The investigation is at #12295 (comment)

@hashicorp-cla
Copy link

hashicorp-cla commented Mar 16, 2022 •
edited
Loading

CLA assistant check
All committers have signed the CLA.

mkeeler
mkeeler reviewed Mar 22, 2022
View reviewed changes
Copy link
Member

@mkeeler mkeeler left a comment

Choose a reason for hiding this comment

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

Thanks for the PR @weichuliu . The code looks great and makes total sense how the original issue occurs and why it fixes it.

Before merging this it would be great to have a test added for the routine Manager to verify the cancleCh behavior and that it allows restarting a task that was already cancelled. I am very confident that this works as described we just want to ensure that we do not break that behavior in the future.

@mkeeler mkeeler added pr/needs-tests PR needs tests before being accepted pr/no-docs PR does not include docs and should not trigger reminder for cherrypicking them. labels Mar 22, 2022
@weichuliu
Copy link
Contributor Author

@mkeeler test updated. It's appreciated if you can help to review.

@weichuliu
Copy link
Contributor Author

@mkeeler I have fiexed the review feedbacks. I believe this one is good to go.

I'd appreciate if you can follow up with this PR

@Amier3 Amier3 linked an issue Apr 5, 2022 that may be closed by this pull request
ACL replication error #12295
Closed
mkeeler
mkeeler approved these changes Apr 5, 2022
View reviewed changes
Copy link
Member

@mkeeler mkeeler left a comment

Choose a reason for hiding this comment

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

Looks wonderful. Thanks for all your work and addressing my feedback.

@mkeeler mkeeler added backport/1.9 pr/no-changelog PR does not need a corresponding .changelog entry pr/no-metrics-test and removed pr/needs-tests PR needs tests before being accepted labels Apr 5, 2022
@mkeeler mkeeler merged commit 24245ed into hashicorp:main Apr 5, 2022
@hc-github-team-consul-core
Copy link
Contributor

🍒 If backport labels were added before merging, cherry-picking will start automatically.

To retroactively trigger a backport after merging, add backport labels and re-run https://circleci.com/gh/hashicorp/consul/625431.

@hc-github-team-consul-core
Copy link
Contributor

🍒✅ Cherry pick of commit 24245ed onto release/1.12.x succeeded!

hc-github-team-consul-core pushed a commit that referenced this pull request Apr 5, 2022
@weichuliu @hc-github-team-consul-core
routine: fix that acl stops replicating after regaining leadership (#…
15e750c 
…12295) (#12565)

* routine: fix that acl stops replicating after regaining leadership (#12295)

* routine: add TestManager_StopBlocking (#12295)

* routine: update TestManager_StopBlocking (#12295)
@hc-github-team-consul-core
Copy link
Contributor

🍒✅ Cherry pick of commit 24245ed onto release/1.11.x succeeded!

hc-github-team-consul-core pushed a commit that referenced this pull request Apr 5, 2022
@weichuliu @hc-github-team-consul-core
routine: fix that acl stops replicating after regaining leadership (#…
01d7793 
…12295) (#12565)

* routine: fix that acl stops replicating after regaining leadership (#12295)

* routine: add TestManager_StopBlocking (#12295)

* routine: update TestManager_StopBlocking (#12295)
@hc-github-team-consul-core
Copy link
Contributor

🍒✅ Cherry pick of commit 24245ed onto release/1.10.x succeeded!

hc-github-team-consul-core pushed a commit that referenced this pull request Apr 5, 2022
@weichuliu @hc-github-team-consul-core
routine: fix that acl stops replicating after regaining leadership (#…
31c35f8 
…12295) (#12565)

* routine: fix that acl stops replicating after regaining leadership (#12295)

* routine: add TestManager_StopBlocking (#12295)

* routine: update TestManager_StopBlocking (#12295)
@hc-github-team-consul-core
Copy link
Contributor

🍒❌ Cherry pick of commit 24245ed onto release/1.9.x failed! Build Log

mkeeler added a commit that referenced this pull request Apr 5, 2022
@mkeeler
Creating a changelog entry for #12565
07d82cd
mkeeler pushed a commit that referenced this pull request Apr 5, 2022
@weichuliu @mkeeler
routine: fix that acl stops replicating after regaining leadership (#…
a09f1f4 
…12295) (#12565)

* routine: fix that acl stops replicating after regaining leadership (#12295)

* routine: add TestManager_StopBlocking (#12295)

* routine: update TestManager_StopBlocking (#12295)
@mkeeler mkeeler mentioned this pull request Apr 5, 2022
Merged
mkeeler added a commit that referenced this pull request Apr 5, 2022
@mkeeler @weichuliu
routine: fix that acl stops replicating after regaining leadership (#…
eb59ac3 
…12295) (#12565) (#12700)

* routine: fix that acl stops replicating after regaining leadership (#12295)

* routine: add TestManager_StopBlocking (#12295)

* routine: update TestManager_StopBlocking (#12295)

Co-authored-by: weichuliu <typhoonsword@gmail.com>
mkeeler added a commit that referenced this pull request Apr 5, 2022
@mkeeler
Creating a changelog entry for #12565 (#12699)
e941d50
hc-github-team-consul-core pushed a commit that referenced this pull request Apr 5, 2022
@mkeeler @hc-github-team-consul-core
Creating a changelog entry for #12565 (#12699)
20d86b6
hc-github-team-consul-core pushed a commit that referenced this pull request Apr 5, 2022
@mkeeler @hc-github-team-consul-core
Creating a changelog entry for #12565 (#12699)
38a9b75
hc-github-team-consul-core pushed a commit that referenced this pull request Apr 5, 2022
@mkeeler @hc-github-team-consul-core
Creating a changelog entry for #12565 (#12699)
49ea27f
hc-github-team-consul-core pushed a commit that referenced this pull request Apr 5, 2022
@mkeeler @hc-github-team-consul-core
Creating a changelog entry for #12565 (#12699)
0313766
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mkeeler mkeeler mkeeler approved these changes

Assignees

@mkeeler mkeeler

Labels
pr/no-changelog PR does not need a corresponding .changelog entry pr/no-docs PR does not include docs and should not trigger reminder for cherrypicking them. pr/no-metrics-test
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

ACL replication error
4 participants
@weichuliu @hashicorp-cla @hc-github-team-consul-core @mkeeler