jenkins-x / terraform-aws-eks-jx

A Terraform module for creating Jenkins X infrastructure on AWS
Apache License 2.0
63 stars 43 forks source link

Managed node groups not being set to specified node_machine_type #248

Closed marsdalesa closed 3 years ago

marsdalesa commented 3 years ago

Summary

Managed nodes do not follow the instance type as set by node_machine_type, the terraform-aws-eks module expects the provided variable to be instance_types, not instance_type and further, expects it to be of type list(string) and not string.

Due to this, the node_machine_type setting is ignored and instances are created as m4.large

Steps to reproduce the behavior

Create cluster using this module, supplying node_machine_type = "m5.large" (this is actually the default for the variable and is ignored)

Expected behavior

Cluster to install with managed node group containing EC2 instances matching the specified node_machine_type

Actual behavior

Instances default to m4.large type as specified in terraform-aws-eks module

Terraform version

The output of terraform version is:

Terraform v0.14.6

Module version

1.15.10

Operating system

Ubuntu 20.10

ankitm123 commented 3 years ago

/assign

ankitm123 commented 3 years ago

the terraform-aws-eks module expects the provided variable to be instance_types, not instance_type and further, expects it to be of type list(string) and not string

yeah, need to use a separate variable for the node group type, cannot re-use the one used for self managed worker groups, hmm.

cpsburca commented 3 years ago

Any updates on this

marsdalesa commented 3 years ago

I haven't spoken to Ankit for a bit, I'll check in with him later and possibly make a PR using a separate variable for this as per his comment. If you are running terraform locally and want to hack this to get up and running, after running terraform init you could edit .terraform/modules/eks-jx/modules/cluster/main.tf and just set this line to instance_types = [var.node_machine_type]

cpsburca commented 3 years ago

I think it's not the only one problem, as the variable is string but it should be a list.

marsdalesa commented 3 years ago

I think it's not the only one problem, as the variable is string but it should be a list.

Yep, that's why the hack I gave uses []. I do believe the intention of it being a list was for spot instances - which weren't applicable to managed node groups (until December 2020) but that would require further work upstream if it hasn't been done and yeah - would have to be factored into any PR here.

cpsburca commented 3 years ago

Even after fixing the module and all related variables the node_instance_type is the same. Is even not the default one from variables from the module (m5.large is defined but it's creating m4.large). From what I see this variable is used only for self-managed nodes. Output from terraform plan:

 # module.eks-jx.module.cluster.module.eks.module.node_groups.aws_eks_node_group.workers["eks-jx-node-group"] will be created
  + resource "aws_eks_node_group" "workers" {
      + ami_type        = "AL2_x86_64"
      + arn             = (known after apply)
      + capacity_type   = (known after apply)
      + cluster_name    = "XXXXX"
      + disk_size       = 10
      + id              = (known after apply)
      + instance_types  = [
          + "m4.large",
        ]