Support EC2 instance tags per node role

[mumoshu]

@c-knowles Would this be what you had needed?

This feature will be handy when e.g. your monitoring tools discovers EC2 instances and then groups resource metrics with EC2 instance tags.

Changes:

• Add support for stackTags to SpotFleet-based node pool
• Add a new configuration key instanceTags to cluster.yaml per worker.nodePools[], controller and etcd.

I recomment to use stackTags for adding cluster-wide metadata to all the resources managed by Cfn, whereas instanceTags are by definition used for node-group-wide metadata to EC2 instances(node pool name, node role, and anything configurable per node pool for example).

A typical cluster.yaml using both stackTags and instanceTags would look like:

worker:
  nodePools:
  - name: pool1
     instanceTags:
       myrole: worker
       type: ondemand
     # Propagated to (hopefully, it's up to cfn) all the stack resources for the pool1 stack
     stackTags:
       env: prod
  - name: pool2
     spotFleet:
       targetCapacity: 2
     # Propagated to EC2 instances managed by this spot fleet (via tag-spot-instance.service)
     instanceTags:
       myrole: worker
       type: spot
     # Propagated to (hopefully, it's up to cfn) all the stack resources for the pool2 stack
     stackTags:
       env: prod

controller:
  instanceTags:
    myrole: controller
    type: ondemand
  # invalid as controller nodes don't have a dedicated cfn stack
  # stackTags:

etcd:
  instanceTags:
    myrole: etcd
    type: ondemand
  # invalid as etcd nodes don't have a dedicated cfn stack
  # stackTags:

stackTags:
  env: prod

Resolves #1026
Depends on #1024 in order to test in combination with spot fleet support

[mumoshu]

An example capture of spot instance tags:

worker:
  nodePools:
  - name: spot
     spotFleet:
       targetCapacity: ...
     stackTags:
       Environment: Production
     instanceTags:
       myrole: worker
       mytype: spot

[codecov-io]

Codecov Report

Merging #1027 into master will not change coverage.
The diff coverage is 0%.

@@           Coverage Diff           @@
##           master    #1027   +/-   ##
=======================================
  Coverage   34.91%   34.91%           
=======================================
  Files          59       59           
  Lines        4159     4159           
=======================================
  Hits         1452     1452           
  Misses       2545     2545           
  Partials      162      162

Impacted Files	Coverage Δ
model/spot_fleet.go	0% <0%> (ø)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0d7cec8...12e30ec. Read the comment docs.

[cknowles]

@mumoshu wow, this is great! Let's merge it asap. Maybe it's more than most people will need but the flexibility will be good.

What's the use case for worker[*].stackTags when there is also a root stackTags applicable to everything plus worker[*].instanceTags applicable to each node pool?

I was looking through the code to see why we hadn't added much about stack tags here and I found some Tags methods here and here. Could those be reused or maybe they are now deprecated?

[cknowles]

Seems like part of #1024 slipped in here?

[mumoshu]

Yes, you are correct. Sorry but I had no time to test this in isolation, as noted in Depends on #1024 in the PR description!

[mumoshu]

@c-knowles Thanks for reviewing!

What's the use case for worker[*].stackTags when there is also a root stackTags applicable to everything plus worker[*].instanceTags applicable to each node pool?

Just wanted to sync up with you but have you verified this behavior?
When I've tested it while developing this feature, the root stackTags had not propagated to workers nodes, at least.

[cknowles]

@mumoshu nope, I've not run that case. OK then, I expected it to work like that as we've described stackTags as recommended for cluster-wide metadata.

[cknowles]

Perhaps we could simplify and remove stackTags from node pools?

[mumoshu]

@c-knowles Thx for the suggestion!
For what it's worth, I don't have specific use-cases, but in case you'd like to tag the cfn stack created per node pool, nodePools[].stackTags are the only way to go right now.

[mumoshu]

@c-knowles I've opened #1038 for discussing that

[cknowles]

@mumoshu I've just been trying this, I could not get instanceTags to work across any of controller/etcd/worker. Just trying to debug now.

After updating an existing cluster, I also have a root stack has a tag that does not exist any more in the root stackTags. I think this is an AWS problem I saw before though - the tags are not updated properly using the SDK.