feat: Linux node config

[m10ev]

Not sure if this is the right approach but thought I'd give it a try.

References:

• https://registry.terraform.io/providers/hashicorp/google/latest/docs/resources/container_cluster#linux_node_config
• https://cloud.google.com/sdk/gcloud/reference/beta/container/node-pools/create#--system-config-from-file
• support for linux_node_config #715

[comment-bot-dev]

Thanks for the PR! 🚀
✅ Lint checks have passed.

[bharathkkb]

Thanks for the PR @m10ev
Would be great if we could add a test, maybe here

[bharathkkb]

for a config like

all = {
      sysctls = {
        "foo"  = 1
      }
}
pool-01 = {
      sysctls = {
       "bar" = 2
      }
}

Wouldn't merge of the second map override the key sysctls from the first unless we do

sysctls = merge(lookup(local.node_pools_linux_node_configs["all"],"sysctls",{}),lookup(local.node_pools_linux_node_configs[each.value["name"]],"sysctls",{}))

[m10ev]

Yes, you are right. Are you in favour of keeping this structure and doing this more complex "deep" merge or do you prefer to flatten the structure one level because sysctls is currently the only key in this map?

[bharathkkb]

@m10ev I think it makes sense to flatten as only sysctls is available.

[m10ev]

I'm just guessing about the test assertion right now because I don't have an existing cluster in order to see the output format of the describe command. If you have access to view the output from the integration tests, could you help me with that?

[bharathkkb]

The tests LGTM, there was a small typo.

[bharathkkb]

Suggested change

	"net.core.netdev_max_backlog" => "10000"
	"net.core.netdev_max_backlog" => "10000",

[bharathkkb]

We can probably assert the whole object instead of including because there should not be any other keys in sysctls.

"linuxNodeConfig" => including(
                  "sysctls" => {
                    "net.core.netdev_max_backlog" => "10000",
                    "net.core.rmem_max" => "10000"
                  }
                )

[morgante]

Actually, it's preferable to use including as asserting the whole object is more brittle to API changes.

[bharathkkb]

@morgante In this particular case, wouldn't the API including additional configs within sysctls be an error? My reasoning for suggesting whole object for sysctls was in case of any bug being introduced in the logic where we merge all and nodepool specific config.

[morgante]

It could be an API error, but it could also be a new default that we haven't included. In either case we couldn't fix it in the module so I'd prefer to avoid brittle logic.

[tenevdev]

Hello, I'm keen to use this feature, too. Is it blocked waiting for one more approval from the owners or it needs extra work?

[bharathkkb]

@tenevdev you can reference the main branch or this will be available with the 0.13 release- #779