search

zalando-stups / senza

Deploy immutable application stacks and create and execute AWS CloudFormation templates in a sane way
https://pypi.python.org/pypi/stups-senza
Other
96 stars 72 forks source link

Extend spotinst elastigroup #514

Closed lmineiro closed 6 years ago

lmineiro commented 6 years ago

This change tries to simplify the definition of Elastgroups by assuming some defaults like the native Auto Scaling Group counterpart.

It also adds support for the automatic subnet mapping from Senza::StupsAutoConfiguration

It assumes, for now, that the cloud accounts provisioned in Spotinst will be named "aws:". This is used for Spotinst cloud account resolution and it's needed to use a specific account instead of the default one.

/cc @danieldop @jmcs

coveralls commented 6 years ago

Coverage Status

Coverage remained the same at 89.546% when pulling 9a7ebaf4ab9bd52a7bbedb8ddb2e87c054ac3a0c on lmineiro:extend-spotinst-elastigroup into 686167d05034f910556a59c196810366f4ecb9f8 on zalando-stups:master.

jmcs commented 6 years ago

:+1:

lmineiro commented 6 years ago

:+1:

lmineiro commented 6 years ago

Thanks for the review @talzur.

spotinst/components/elastigroup.py:112 - why are the tags only added when none exist?

This is for consistency. Across pretty much every Elastigroup settings, whenever they're defined already, those are taken as the desired state and left untouched. For the particular case of the tags, the expectation is that if someone defined custom tags, any of the 3 "automatic" tags can also be defined there only once, which seems like a reasonable effort.

spotinst/components/elastigroup.py:123 - is this the only place you set the group name?

This was actually changed/fixed in https://github.com/zalando-stups/senza/pull/516/files#diff-3392db1a8e49c9b06e285754efc51c62L112

spotinst/components/elastigroup.py:21 - ELASTIGROUP_DEFAULT_PRODUCT

Thanks for the thorough explanation. To the best of my knowledge, the entire STUPS platform, which Senza is part of, was created for VPC setups. This should be fine. I know it will be for our usage, at least.