Improve traffic handling #497
Conversation
| @@ -592,10 +593,13 @@ def health(region, stack_ref, all, output, field, w, watch): | |||
| help='Ignore failing validation checks') | |||
| @click.option('--update-if-exists', is_flag=True, | |||
| help='Updates the the stack if it exists') | |||
| @click.option('--ensure-no-traffic', is_flag=True, | |||
| help="Don't create a stack if it would automatically receive " | |||
There was a problem hiding this comment.
I think this phrase fails to deliver the idea that it's happening because of 0 traffic weight - maybe something like "Don't create a stack if it has no traffic and would automatically receive it after the change". What do you think?
There was a problem hiding this comment.
New stacks never have traffic so "if it has no traffic" is always true.
There was a problem hiding this comment.
Is this going to fail when you create the very first version of a stack?
There was a problem hiding this comment.
Yes, but the flag is off by default, so you can just omit the flag the first time. I made the behaviour like this to prevent any unexcepted results, specially for Continuous Delivery pipelines.
There was a problem hiding this comment.
@jmcs I'm not sure if this buys us a lot if the flag is not enabled by default. It's easy to forget and if you don't already know about the issue, you have no chance.
Can we do this w/o a flag? Like always avoid creating a new stack version if there are some existing versions and all of them have "0%" traffic assigned?
There was a problem hiding this comment.
@a1exsh do you want to collect some other users upvoting your idea? It would definitely break compatibility, but maybe that's what most Senza users want, so I would be fine with it.
There was a problem hiding this comment.
@hjacobs @jmcs Well, I was reluctant to suggest this in the first place, but maybe the better response is to make sure senza delete doesn't mess up the traffic weights?
Then you can keep semantics of senza create as they are currently and no incompatible change is needed.
Of course one may intentionally set the traffic of the only stack version to 0 and then deploy a second stack, but there is no much sense trying to prevent that. This will then work as documented by AWS: https://docs.aws.amazon.com/Route53/latest/DeveloperGuide/resource-record-sets-values-weighted.html#rrsets-values-weighted-weight
If you set Weight to 0 for all of the records in the group, traffic is routed to all resources with equal probability. This ensures that you don't accidentally disable routing for a group of weighted records.
There was a problem hiding this comment.
I like the "safe by default" approach way better as well. Having an optional flag will require quite some knowledge.
From my experience so far, use cases where the current behavior is desired of the new behavior are rather few actually. Therefore I would be really in favor of a solution which optimises for the 80% even though it would break compatibility. Especially as the broken functionality should be quite obvious (compared to the "broken" behaviour)
senza/traffic.py
Outdated
| @@ -26,14 +26,10 @@ | |||
| DNS_ZONE_CACHE = {} | |||
|
|
|||
|
|
|||
| def get_weights(dns_names: list, identifier: str, all_identifiers) -> ({str: int}, int, int): | |||
| def get_weights_for_dns(dns_names): | |||
1 similar comment
|
👍 |
|
@jmcs is this a "won't fix" now? :) |
Fixes #490