Skip to content
This repository has been archived by the owner on May 6, 2020. It is now read-only.

aws: add tags on AWS resources #176

Merged
merged 1 commit into from
Feb 13, 2020
Merged

aws: add tags on AWS resources #176

merged 1 commit into from
Feb 13, 2020

Conversation

alban
Copy link
Contributor

@alban alban commented Feb 12, 2020

Ideas of possible use cases:

  • when several people share the same AWS account, they can add add a tag "CreatedBy"="Tom" to know who created which cluster.
  • when the terraform state is deleted and the user wants to manually delete AWS resources from the web console.

@alban
Copy link
Contributor Author

alban commented Feb 13, 2020

The CI failed with:

Error: l8eci1581527822-ch-worker terraform-20200212171726670100000001: invalid tag attributes: value missing

  on ../../aws/flatcar-linux/kubernetes/workers/workers.tf line 2, in resource "aws_autoscaling_group" "workers":
   2: resource "aws_autoscaling_group" "workers" {

It seems that aws_autoscaling_group does not accept tags with empty string values:
hashicorp/terraform-provider-aws#9049

I will change the default value to not include tags with empty string values.

@alban
Copy link
Contributor Author

alban commented Feb 13, 2020

The CI is happy now.

invidian
invidian approved these changes Feb 13, 2020
View reviewed changes
Copy link
Contributor

@invidian invidian left a comment

Choose a reason for hiding this comment

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

LGTM

rata
rata approved these changes Feb 13, 2020
View reviewed changes
Copy link
Contributor

@rata rata left a comment

Choose a reason for hiding this comment

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

LGTM.

Seems weird the not useful default "CreatedBy = "Unspecified" but I guess it encourages people to set it... So, I'm okay with that too. We can change it later if needed :)

johananl
johananl reviewed Feb 13, 2020
View reviewed changes
docs/flatcar-linux/aws.md
@@ -223,6 +223,7 @@ Reference the DNS zone id with `"${aws_route53_zone.zone-for-clusters.zone_id}"`
| service_cidr | CIDR IPv4 range to assign to Kubernetes services | "10.3.0.0/16" | "10.3.0.0/24" |
| cluster_domain_suffix | FQDN suffix for Kubernetes services answered by coredns. | "cluster.local" | "k8s.example.com" |
| certs_validity_period_hours | Validity of all the certificates in hours | 8760 | 17520 |
| tags | Optional details to tag on AWS resources | `{}` | `{"CreatedBy" = "Devops team"}` |
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
| tags | Optional details to tag on AWS resources | `{}` | `{"CreatedBy" = "Devops team"}` |
| tags | Optional details to tag on AWS resources | `{}` | `{"CreatedBy" = "DevOps team"}` |

@alban alban merged commit e872a5d into master Feb 13, 2020
@invidian invidian deleted the alban/tags branch February 13, 2020 15:04
@alban alban mentioned this pull request Feb 13, 2020
Merged
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@johananl johananl johananl left review comments

@invidian invidian invidian approved these changes

@rata rata rata approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@alban @rata @johananl @invidian