[Beta provider] Add support for new kubelet and sysctl option groups for GKE nodes

[gplasky]

Community Note

• Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request
• Please do not leave "+1" or "me too" comments, they generate extra noise for issue followers and do not help prioritize the request
• If you are interested in working on this issue or have submitted a pull request, please leave a comment. If the issue is assigned to the "modular-magician" user, it is either in the process of being autogenerated, or is planned to be autogenerated soon. If the issue is assigned to a user, that user is claiming responsibility for the issue. If the issue is assigned to "hashibot", a community member has claimed the issue already.

Description

Node Configs for controlling select Kubelet and sysctl values (NodeKubeletConfig and LinuxNodeConfig, respectively) is now available in the beta Kubernetes Engine API as a part of the NodeConfig object.

New or Affected Resource(s)

Affected:

• google_container_cluster
• google_container_node_pool

Potential Terraform Configuration

resource "google_container_cluster" "primary" {

  ...

  node_config {
     ...

    linux_node_config {
      sysctls {
        net-core-netdev_max_backlog  = integer
        net-core-rmem_max            = integer
        ...
        # Remaining supported LinuxNodeConfig values
    }

    kubelet_config {
      cpu_manager_policy   = string
      cpu_cfs_quota        = boolean
      cpu_cfs_quota_period = string
    }
  }
}

References

https://cloud.google.com/sdk/gcloud/reference/beta/container/clusters/create#--system-config-from-file

https://cloud.google.com/kubernetes-engine/docs/reference/rest/v1beta1/NodeConfig#LinuxNodeConfig
https://cloud.google.com/kubernetes-engine/docs/reference/rest/v1beta1/NodeConfig#nodekubeletconfig

[rmanyari]

Hey - happy to work on this one. I can submit an PR in the next few (2-3) days.

[rmanyari]

@gplasky LinuxNodeConfig takes a Sysctls map[string]string where the keys are the kernel configs in the original format.

Your proposed configuration replaces dots with dashes. Any thoughts on that? I started to do something like:

+ 542         sysCfg := sysCfgList[0].(map[string]interface{})                                                                                                                                                     
+ 543         sysctls := make(map[string]string)                                                                                                                                                                   
+ 544         for k, v := range sysCfg {                                                                                                                                                                           
+ 545                 var casted string                                                                                                                                                                            
+ 546                 if i, ok := v.(int); ok {                                                                                                                                                                    
+ 547                         casted = strconv.Itoa(i)                                                                                                                                                             
+ 548                 }                                                                                                                                                                                            
+ 549                 if s, ok := v.(string); ok {                                                                                                                                                                 
+ 550                         casted = s                                                                                                                                                                           
+ 551                 }                                                                                                                                                                                            
+ 552                 if casted == nil {                                                                                                                                                                           
+ 553                         continue                                                                                                                                                                             
+ 554                 }                                                                                                                                                                                            
+ 555                 sysctls[strings.ReplaceAll(k, "-", ".")] = casted                                                                                                                                            
+ 556         }                                                                                                                                                                                                    
+ 557         return &containerBeta.LinuxNodeConfig{                                                                                                                                                               
+ 558                 Sysctls: sysctls,                                                                                                                                                                            
+ 559         }   

But it feels a bit forced... Not sure if there's precedence in other places in the providers where we avoid having dots in the attribute names.

I'd like some feedback on this before I get too far with unit tests and all that good stuff.

[gplasky]

I have no strong preference on the format, and in fact waffled a bit when deciding which formatting to use in my example. I'm not actually sure if TF supports dot-based naming but I'm definitely in favor of going down that route if so. That said, I believe only underscores and dashes are supported:

https://www.terraform.io/docs/configuration/syntax.html#identifiers

If that's the case I think that leaves us with either all underscores (loses the distinction of the separate sections e.g. net.foo.bar) or use dashes to differentiate as in the above. No strong opinion on either.

Edit: After a quick scan through the codebase, the all _ way seems to be more idiomatic.

[rmanyari]

Sounds good. If we use only _ then I'd probably need to keep a map of TF parameter string -> sysctl key. No big deal, I'll drop some comments to keep track of this decision. Thanks for the input!

[rmanyari]

Opened a PR, it's still WIP (needs more tests and one of the acceptance tests doesn't pass right now) but any early feedback is appreaciated.

[ghost]

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. If you feel I made an error 🤖 🙉 , please reach out to my human friends 👉 hashibot-feedback@hashicorp.com. Thanks!