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

modify on_error:destroy behavior for deployments #317

Merged
merged 6 commits into from
Sep 8, 2021
Merged

Conversation

leokalev
Copy link

@leokalev leokalev commented Aug 23, 2021

connectors/kubernetes.py: a deployment handled by the connector is
'foreign' (not created by the connector), the connector cannot
auto-restore it if it has been deleted. Therefore, it should not delete
its deployment-under-control when the on_error setting is configured to
'destroy' (which is actually intended as 'destroy and re-create on the next
adjust').

This patch modifies the connector to set replicas=0, instead -
consistent with the behavior of the servoX predecessor (servo-k8s).

(Linear task: https://linear.app/opsani/issue/ENG-257/servoxkubernetes-modify-on-failure-destroy-behavior-for-deployments)

connectors/kubernetes.py: a deployment handled by the connector is
'foreign' (not created by the connector), the connector cannot
auto-restore it if it has been deleted. Therefore, it should not delete
its deployment-under-control when the on_error setting is configured to
'destroy' (which is actually intended as 'destroy and re-create on the next
adjust').

This patch modifies the connector to set replicas=0, instead -
consistent with the behavior of the servoX predecessor (servo-k8s).
@linear
Copy link

linear bot commented Aug 23, 2021

ENG-257 ServoX/kubernetes: modify on_failure: destroy behavior for deployments

when working with a deployment (that isn't created by ServoX/kubernetes), the 'on_failure: destroy' behavior should be modified to match the old servo-k8s, namely: scale the deployment to 0 instances (thus destroying all pods).

This proposed change should not affect the 'canary mode' behavior - in this mode, the kubernetes connector itself creates the canary pod and it can (and should) destroy it when a n adjust failure occurs.

@leokalev leokalev requested review from linkous8 and pnickolov August 26, 2021 14:02
- rename 'destroy' config option to 'shutdown'
- modify tests to match
- fix method comment
Copy link
Contributor

@linkous8 linkous8 left a comment

Choose a reason for hiding this comment

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

Changes look good overall

@leokalev leokalev requested a review from blakewatters August 31, 2021 16:24
@linkous8 linkous8 self-requested a review August 31, 2021 17:00
Copy link
Contributor

@linkous8 linkous8 left a comment

Choose a reason for hiding this comment

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

Please add requested unit test.

Also probably worth resolving the the open items from the last review although I will leave those to your discretion

https://github.com/opsani/servox/pull/317/files#r698604812
https://github.com/opsani/servox/pull/317/files#r698605379

@linkous8 linkous8 self-requested a review August 31, 2021 17:57
Copy link
Contributor

@linkous8 linkous8 left a comment

Choose a reason for hiding this comment

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

LGTM

@leokalev leokalev merged commit 5e94627 into main Sep 8, 2021
@leokalev leokalev deleted the lk-eng-257 branch September 8, 2021 16:50
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.

3 participants