Skip to content

fix: resolve rolling deployment not working correctly#139

Open
PierrickLP wants to merge 1 commit into
claffin:mainfrom
PierrickLP:fix-rolling-config
Open

fix: resolve rolling deployment not working correctly#139
PierrickLP wants to merge 1 commit into
claffin:mainfrom
PierrickLP:fix-rolling-config

Conversation

@PierrickLP
Copy link
Copy Markdown

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_healthy to check if we have enough healthy instances), but they are not considered healthy in the providers check_alive method (we add them to ip_ready only if they are not expected to be recycled).

This means that if every instances are currently expired, we will have a negative available_after_recycle value (-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 :

  • Setup your env : AWS_MIN_SCALING=2, AWS_MAX_SCALING=2, ROLLING_DEPLOYMENT=True, ROLLING_MIN_AVAILABLE=1, ROLLING_BATCH_SIZE=1, AGE_LIMIT=360
  • Wait for the 2 instances to be deployed and to reach the age limit.
  • Both instances should disappear from the ui and from the available proxies, but no other proxies will replace them.
  • You should see in the logs that they cannot be recycled : Would reduce available proxies below minimum (-1 < 1)

Type of Change

  • Bug fix
  • New feature
  • Enhancement to existing functionality
  • Documentation update

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

  • I've run pytest locally and all tests pass
  • I've added tests for new functionality (if applicable)
  • My code follows the project's style
  • I've updated documentation if needed

Important Note

All PRs must pass the automated test suite before they can be merged. The GitHub Actions workflow will automatically run pytest on your changes using the python-app-testing.yml workflow.

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.

1 participant