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: rolling deployments race #2790

Merged
merged 1 commit into from
Sep 23, 2024
Merged

fix: rolling deployments race #2790

merged 1 commit into from
Sep 23, 2024

Conversation

stuartwdouglas
Copy link
Collaborator

@stuartwdouglas stuartwdouglas commented Sep 23, 2024

If we delete the deployments/runners straight away it may still be in some controllers route tables. By adding a small delay we make sure that all the controllers will have updated their table.

This is a pretty nasty hack, but will likely be temporary.

fixes: #2789

@stuartwdouglas stuartwdouglas requested review from a team and deniseli and removed request for a team September 23, 2024 08:56
If we delete the deployments/runners straight away it may still be in some
controllers route tables. By adding a small delay we make sure that all
the controllers will have updated their table.

This is a pretty nasty hack, but will likely be temporary.

fixes: #2789
@ftl-robot ftl-robot mentioned this pull request Sep 23, 2024
Copy link
Contributor

@deniseli deniseli left a comment

Choose a reason for hiding this comment

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

Should we open an issue to track undoing of this, or no need since you already have a longer running plan to fix?

@stuartwdouglas
Copy link
Collaborator Author

I will open an issue, I realized that it is bigger than just this case, it also affects things like subscriptions where one controller can decide the deployment is ready and activate message delivery, and another controller can then start processing messages before it's route table is updated.

@stuartwdouglas stuartwdouglas merged commit 2580e66 into main Sep 23, 2024
91 checks passed
@stuartwdouglas stuartwdouglas deleted the stuartwdouglas/2789 branch September 23, 2024 20:33
@alecthomas
Copy link
Collaborator

I think this will all get resolved by the provisioner. eg. the provisioner won't provision subscriptions until routing tables are provisioned

@stuartwdouglas
Copy link
Collaborator Author

Currently I don't start subscriptions until route tables are provisioned, however it is local to a single controller, so we don't have a way of knowing that all route tables in all controllers are updated.

@alecthomas
Copy link
Collaborator

Right, but I think as part of this provisioning change we'll need to ensure that routing tables are actually propagated.

@alecthomas
Copy link
Collaborator

What I mean is, each thing created by the provisioner will need to guarantee that it's fully propagated before it's counted as provisioned.

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.

Race in kube rolling deployments
3 participants