[autoscaler] Translate to/from AWS 'Name' tag#2219
Conversation
|
Looks good to me. This makes me wonder if it might be, after all, better to use provider specific names for the tags. Having one condition here is not a problem, but who knows, maybe in future we end up having multiple similar cases where we have to do something provider specific configurations. |
|
Test FAILed. |
|
It's a fair point, if I knew about this edge case that would definitely have been the preferable option before... |
| for tag in node.tags: | ||
| tags[tag["Key"]] = tag["Value"] | ||
| return tags | ||
| return from_aws_format(tags) |
There was a problem hiding this comment.
why do we need this for this method in particular? ie, why not have this for all methods where you call to_aws_format?
tags is a dict and is passed by ref; so every time you call to_aws_format you end up modifying the original...
There was a problem hiding this comment.
This is the only one that returns tags?
And yes, the mutation is fine.
|
Tests look unrelated. |
What do these changes do?
GCP support moved tags to have cross-cloud names (#2061), however we should still use "Name" for the node name. GCP doesn't support upper-case tags, so implement this translation in the AWS node provider.
fyi @hartikainen
Related issue number
#2209