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 graceful shutdown #1611

Merged
merged 1 commit into from
Apr 12, 2024
Merged

Fix graceful shutdown #1611

merged 1 commit into from
Apr 12, 2024

Conversation

ViliusS
Copy link
Contributor

@ViliusS ViliusS commented Apr 8, 2024

This closes #1595

Summary Of Changes

Previously all actions in preStop hook were performed independently of each other. For example, drain would kick in even if queue sinchronization returned an error. This resulted in cases where graceful shutdown was assumed just because last action in the script, i.e. rabbitmq-upgrade drain, is a success.

Now all actions in the hook actually waits for the previous action to complete successfully. The only exception is checking for the deletion marker file. Script will still exit with 0 (and shutdown will continue) if the marker is present without executing other actions.

Local Testing

I'm not familiar with Go and I don't have development environment to test these changes, sorry. This is just a friendly PR from a fellow sys admin. I'm also not sure if ampersands in Go strings need to be escaped to actually achieve && in the final preStop hook. I did test the final script by manually modifying preStop hook on my local RabbitMQ environment, though.

Previously all actions in preStop hook were performed independently of
each other. For example, drain would kick in even if queue
sinchronization returned an error. This resulted in cases where graceful
shutdown was assumed just because last action in the script, i.e.
`rabbitmq-upgrade drain`, is a success.

Now all actions in the hook actually waits for the previous action to
complete successfully. The only exception is checking for the deletion
marker file. Script will still exit with 0 (and shutdown will continue)
if the marker is present without executing other actions.
@lukebakken lukebakken self-assigned this Apr 9, 2024
@lukebakken lukebakken self-requested a review April 9, 2024 12:57
Copy link
Contributor

@lukebakken lukebakken left a comment

Choose a reason for hiding this comment

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

Thanks @ViliusS, this appears correct to me, though I don't know the best way to test it. @Zerpet if you have suggestions, let me know and I will test.

Copy link
Collaborator

@Zerpet Zerpet left a comment

Choose a reason for hiding this comment

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

LGTM

@Zerpet Zerpet merged commit 4cd2198 into rabbitmq:main Apr 12, 2024
15 checks passed
@Zerpet Zerpet modified the milestones: 2.8.0, 2.8.1 Apr 12, 2024
@ViliusS ViliusS deleted the fix-prestop-hook branch April 15, 2024 05:15
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.

Operator doesn't wait for successful queue synchronization on shutdown
3 participants