coreos / terraform-aws-kubernetes

Install a Kubernetes cluster the CoreOS Tectonic Way: HA, self-hosted, RBAC, etcd Operator, and more
Apache License 2.0
116 stars 67 forks source link

Terraform plan errors if `tectonic_base_domain` is a variable #7

Closed bodgit closed 6 years ago

bodgit commented 6 years ago

I'm trying to test this out in an empty AWS account and given I had to create and supply the route53 zone name I figured I could do something like this:

resource "aws_route53_zone" "tectonic" {
  name = "tectonic.example.com"
}

module "kubernetes" {
  source = "coreos/kubernetes/aws"

  tectonic_base_domain         = "${aws_route53_zone.tectonic.name}"
  tectonic_cluster_name        = "..."
  tectonic_admin_email         = "..."
  tectonic_admin_password_hash = "..."
  tectonic_aws_ssh_key         = "..."
  tectonic_license_path        = "${path.module}/tectonic-license.txt"
  tectonic_pull_secret_path    = "${path.module}/config.json"
}

However this results in the following error:

$ terraform plan

Error: module.kubernetes.module.etcd_certs.tls_cert_request.etcd_peer: dns_names: should be a list

Error: module.kubernetes.module.etcd_certs.tls_cert_request.etcd_server: dns_names: should be a list

If I change my manifest to the below, then it works but there's now no guarantee that the zone would exist before the module needs it:

resource "aws_route53_zone" "tectonic" {
  name = "tectonic.example.com"
}

module "kubernetes" {
  source = "coreos/kubernetes/aws"

  tectonic_base_domain         = "tectonic.example.com"
  tectonic_cluster_name        = "..."
  tectonic_admin_email         = "..."
  tectonic_admin_password_hash = "..."
  tectonic_aws_ssh_key         = "..."
  tectonic_license_path        = "${path.module}/tectonic-license.txt"
  tectonic_pull_secret_path    = "${path.module}/config.json"
}

If I add an output that just returns the value of ${aws_route53_zone.tectonic.name} then it returns a flat string exactly as it should so I'm not sure why it's exploding the way it is.

This is with Terraform 0.10.8 and version 1.7.5-tectonic.1 of the module.

squat commented 6 years ago

This seems like a Terraform issue rather than an issue with this module :/ . Like you said, if you explicitly supply a string rather than a variable, the cluster builds.

bodgit commented 6 years ago

I've pulled down master of this module instead of the version from the module registry and the plan seems to pass. The changes in https://github.com/coreos/terraform-aws-kubernetes/commit/e50ed698445430db6006d893dafe33f9b83a263a#diff-230559a0d099ca913894ba9ea207acb3 to tectonic.tf seem to be what has fixed it.

Incidentally, it takes a very long time (about 15 minutes) to run a terraform get as I end up pulling down 16 copies of the https://github.com/coreos/tectonic-installer repository to get the various modules in it and I end up with a .terraform/ directory that is about 2 GB in size.

squat commented 6 years ago

@bodgit thanks for pointing this out. We will be releasing a new version within a week's time; that release will contain this change as well. As for the large download size, this is primarily a sub-optimal implementation issue in terraform, which should instead perform some type of sparse-checkout of git modules. That said, I am working on refactoring this module to eliminate this issue on our side. Could you update this issue, or open a new issue, to reflect this download size problem?

bodgit commented 6 years ago

We will be releasing a new version within a week's time; that release will contain this change as well.

Ok, I can probably run from master for now, not a problem.

As for the large download size, this is primarily a sub-optimal implementation issue in terraform, which should instead perform some type of sparse-checkout of git modules.

Yeah, I figured Terraform could be a bit more intelligent with how it handles git downloads, especially if it sees multiple references to the same repository.

I'll open another issue for the download size.