Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Support for cluster_autoscaling #93

Closed
lawliet89 opened this issue Mar 8, 2019 · 16 comments · Fixed by #328
Closed

Add Support for cluster_autoscaling #93

lawliet89 opened this issue Mar 8, 2019 · 16 comments · Fixed by #328
Assignees
Labels
enhancement New feature or request triaged Scoped and ready for work

Comments

@lawliet89
Copy link
Contributor

This is the node auto-provisioning feature that is supported by the cluster_autoscaling field.

@aaron-lane aaron-lane added the enhancement New feature or request label May 6, 2019
@thecodejunkie
Copy link

I was just looking at this module for this feature as we need to create clusters that have node auto-provisioning enabled. Is there any planned ETA (or workaround) for this? Thanks!

@morgante
Copy link
Contributor

morgante commented Oct 30, 2019

It's not currently supported, but we should add it!

Expected interface:

cluster_autoscaling = {
  enabled = true
  max_cpu_cores = 20
  min_cpu_cores = 5
  max_memory_gb = 30
  min_memory_gb = 10
}

@morgante morgante added the triaged Scoped and ready for work label Oct 30, 2019
@thecodejunkie
Copy link

@morgante I know I'm asking a lot here, but any chance this could be added very soon? It's literally the only thing we're missing, in the module, to be able to migrate our current solution over to Terraform (this module looks too good to do it using the normal provider 😄)

@thecodejunkie
Copy link

By the way, we need to be able to enable auto-scaling on a per-node pool level

@morgante
Copy link
Contributor

morgante commented Oct 31, 2019

@thecodejunkie Can you clarify which kind of autoscaling you're looking for?

  • Cluster autoscaler automatically scales existing node pools up and down (adding new nodes): this is already supported in the module. (In fact, it's our default behavior.)
  • Node autoprovisioning will automatically provision entire new node pools as necessary: this is what we need to add in this issue

@thecodejunkie
Copy link

@morgante oh sorry, yes it's cluster autoscaling I was looking for. I've been unable to find any docs for the node_pools inputs. The only docs I've been able to find are those mentioned in this PR https://github.com/terraform-google-modules/terraform-google-kubernetes-engine/pull/297/files but the autoscaling property is not mentioned? Thanks!

@morgante
Copy link
Contributor

morgante commented Oct 31, 2019

You can do it like this:

  node_pools = [
    {
      ...
      autoscaling = true
      min_count = 10
      max_count = 100
    },
  ]

@thecodejunkie
Copy link

@morgante 👍 then perhaps the autoscaling property should also be documented in #297 ?

@morgante
Copy link
Contributor

Absolutely, @neelesh9795 please make sure to document all autoscaling properties.

@kopachevsky
Copy link
Contributor

kopachevsky commented Nov 15, 2019

it this format is fine?

variable "cluster_autoscaling" {
  type = object({
    enabled = bool
    resource_limits = map(number)
  })
  default = {
    enabled = false
    resource_limits = {}
  }
}

and example of value

  cluster_autoscaling = {
    enabled = true
    resource_limits = {
      max_cpu_cores = 20
      min_cpu_cores = 5
      max_memory_gb = 30
      min_memory_gb = 10
    }
  }

Can't set "enabled" field to same map as long has different type

@morgante
Copy link
Contributor

Yes, that works.

kopachevsky added a commit to kopachevsky/terraform-google-kubernetes-engine that referenced this issue Nov 15, 2019
kopachevsky added a commit to kopachevsky/terraform-google-kubernetes-engine that referenced this issue Nov 18, 2019
kopachevsky added a commit to kopachevsky/terraform-google-kubernetes-engine that referenced this issue Nov 18, 2019
Added cluster auto scaling
kopachevsky added a commit to kopachevsky/terraform-google-kubernetes-engine that referenced this issue Nov 18, 2019
Fixed autogen cluster config
Updated docs
Updated simple-regional-beta example
kopachevsky added a commit to kopachevsky/terraform-google-kubernetes-engine that referenced this issue Nov 18, 2019
Fixed cluster auto scaling conditions
kopachevsky added a commit to kopachevsky/terraform-google-kubernetes-engine that referenced this issue Nov 18, 2019
Added tests for cluster auto scaling
kopachevsky added a commit to kopachevsky/terraform-google-kubernetes-engine that referenced this issue Nov 18, 2019
kopachevsky added a commit to kopachevsky/terraform-google-kubernetes-engine that referenced this issue Nov 18, 2019
Added tests for cluster auto scaling
kopachevsky added a commit to kopachevsky/terraform-google-kubernetes-engine that referenced this issue Nov 19, 2019
Moved cluster autos scaling config to node_pool fixture to fix project networks quota
kopachevsky added a commit to kopachevsky/terraform-google-kubernetes-engine that referenced this issue Nov 19, 2019
Added cluster auto scaling
Updated docs
Added tests for cluster auto scaling in node_pool fixture
kopachevsky added a commit to kopachevsky/terraform-google-kubernetes-engine that referenced this issue Nov 20, 2019
Added cluster auto scaling
Updated docs
Added tests for cluster auto scaling in node_pool fixture
kopachevsky added a commit to kopachevsky/terraform-google-kubernetes-engine that referenced this issue Nov 20, 2019
Added cluster auto scaling
Updated docs
Added tests for cluster auto scaling in node_pool fixture
kopachevsky added a commit to kopachevsky/terraform-google-kubernetes-engine that referenced this issue Nov 20, 2019
Added cluster auto scaling
Updated docs
Added tests for cluster auto scaling in node_pool fixture
kopachevsky added a commit to kopachevsky/terraform-google-kubernetes-engine that referenced this issue Nov 21, 2019
Updated docs

Added tests for cluster autoscaling in node_pool fixture

* Fix terraform-google-modules#93
kopachevsky added a commit to kopachevsky/terraform-google-kubernetes-engine that referenced this issue Nov 21, 2019
Updated docs

Added tests for cluster autoscaling in node_pool fixture

* Fix terraform-google-modules#93
kopachevsky added a commit to kopachevsky/terraform-google-kubernetes-engine that referenced this issue Nov 21, 2019
Simplified cluster autoscaling config
kopachevsky added a commit to kopachevsky/terraform-google-kubernetes-engine that referenced this issue Nov 21, 2019
Simplified cluster autoscaling config
kopachevsky added a commit to kopachevsky/terraform-google-kubernetes-engine that referenced this issue Nov 21, 2019
kopachevsky added a commit to kopachevsky/terraform-google-kubernetes-engine that referenced this issue Nov 21, 2019
@kopachevsky
Copy link
Contributor

@morgante it seems both resource types and both limits (min and max) required on API level, on provider they are optional, for example this one will fail:

  cluster_autoscaling {
    enabled = true
    resource_limits {
      resource_type = "cpu"
      maximum = 30
    }
}

And more insetting this one as well:

  cluster_autoscaling {
    enabled = true
    resource_limits {
      resource_type = "cpu"
      minimum = 10
      maximum = 30
    }
}

The error output is:

Error: googleapi: Error 400: Resource limits for memory must be specified when enabling node autoprovisioning., badRequest
  on ../../../modules/beta-public-cluster/cluster.tf line 22, in resource "google_container_cluster" "primary":
  22: resource "google_container_cluster" "primary"

So my question here, should we switch from map to object for variable as long all fields seems to be mandatory?

kopachevsky added a commit to kopachevsky/terraform-google-kubernetes-engine that referenced this issue Nov 21, 2019
@morgante
Copy link
Contributor

@kopachevsky What happens if you only specify memory? From the response provided, it only calls out memory as required.

@kopachevsky
Copy link
Contributor

kopachevsky commented Nov 22, 2019

@morgante Will send output later today, in a middle of other task now, but if specify only memory, without cpu - it also fails, only full set of args works

@kopachevsky
Copy link
Contributor

Error: googleapi: Error 400: Resource limits for cpu must be specified when enabling node autoprovisioning., badRequest

@morgante
Copy link
Contributor

@kopachevsky In that case, we should indeed make it an object since all params are apparently required.

kopachevsky added a commit to kopachevsky/terraform-google-kubernetes-engine that referenced this issue Nov 25, 2019
kopachevsky added a commit to kopachevsky/terraform-google-kubernetes-engine that referenced this issue Nov 25, 2019
kopachevsky added a commit to kopachevsky/terraform-google-kubernetes-engine that referenced this issue Nov 25, 2019
kopachevsky added a commit to kopachevsky/terraform-google-kubernetes-engine that referenced this issue Nov 25, 2019
kopachevsky added a commit to kopachevsky/terraform-google-kubernetes-engine that referenced this issue Nov 25, 2019
kopachevsky added a commit to kopachevsky/terraform-google-kubernetes-engine that referenced this issue Nov 25, 2019
aaron-lane pushed a commit that referenced this issue Nov 26, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request triaged Scoped and ready for work
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants