fix: resolve rolling deployment not working correctly#139
Open
PierrickLP wants to merge 1 commit into
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Pull Request
Description
Currently, the rolling deployment doesn't work as expected. Instances to be recycled are considered healthy by the RollingDeploymentManager (see rolling.py:114, we remove 1 (the current instance) from
total_healthyto check if we have enough healthy instances), but they are not considered healthy in the providerscheck_alivemethod (we add them toip_readyonly if they are not expected to be recycled).This means that if every instances are currently expired, we will have a negative
available_after_recyclevalue (-1) and no proxy will be recycled.I have reproduced the error only on AWS but it should be the same on each provider.
Steps to reproduce :
Would reduce available proxies below minimum (-1 < 1)Type of Change
Testing
I added tests to each providers to make sure one, and only one, expired instance is correctly recycled if we start with 2 instances and have rolling deployment enabled with a min_available of 1. Each tests fail without the changes and pass with them.
I tested in production with AWS provider only, and whereas the rolling deployment didn't work before, it now works as expected.
Checklist
pytestlocally and all tests passImportant Note
All PRs must pass the automated test suite before they can be merged. The GitHub Actions workflow will automatically run
pyteston your changes using thepython-app-testing.ymlworkflow.