Support for Kubelet and Linux Node configurations in GKE node pools #2279

rmanyari · 2020-07-20T18:01:09Z

Per issue 6773, this PR adds support for KubeletConfig and LinuxNodeConfig attributes at GKE cluster and node creation. Updates are also supported at the node pool level.

Signed-off-by: rodrigo@twosigma.com

container: added support for `kubelet_config` and `linux_node_config` to GKE node pools (beta)

rmanyari · 2020-07-20T18:01:54Z

This is still WIP, the acceptance test for Linux node configs doesn't pass yet but still wanted to get this out to get some early feedback.

rileykarson

I shared a few early thoughts! I haven't run the test yet, do you mind sharing what kind of issue you're encountering?

rileykarson · 2020-07-20T18:05:49Z

google-beta/resource_container_node_pool_test.go

+		Steps: []resource.TestStep{
+			{
+				Config: testAccContainerNodePool_withKubeletConfig(cluster, np, "static"),
+				Check: resource.ComposeTestCheckFunc(


Given that you're importing the resource, you can actually just omit all the Check stuff. It's superseded by some default checks done when importing.

Got it, will remove.

google-beta/node_config.go

rmanyari · 2020-07-21T04:15:15Z

I was running into some de-serialization problem. Fixed that in the process of changing from TypeList to TypeMap.

Just finished running the new acceptance tests:

$ GOOGLE_CREDENTIALS=$(cat ../key.json) GOOGLE_PROJECT=<redacted> GOOGLE_REGION=us-centr
al1 GOOGLE_ZONE=us-central1-a TF_ACC=true go test -v ./... -run withLinuxNodeConfig -timeout 45m
=== RUN   TestAccContainerNodePool_withLinuxNodeConfig
=== PAUSE TestAccContainerNodePool_withLinuxNodeConfig
=== CONT  TestAccContainerNodePool_withLinuxNodeConfig
--- PASS: TestAccContainerNodePool_withLinuxNodeConfig (706.08s)
PASS
ok      github.com/terraform-providers/terraform-provider-google-beta/google-beta       706.121s

$ GOOGLE_CREDENTIALS=$(cat ../key.json) GOOGLE_PROJECT=<redacted> GOOGLE_REGION=us-centr
al1 GOOGLE_ZONE=us-central1-a TF_ACC=true go test -v ./... -run withKubeletConfig -timeout 45m                                                                                                              
=== RUN   TestAccContainerNodePool_withKubeletConfig
=== PAUSE TestAccContainerNodePool_withKubeletConfig
=== CONT  TestAccContainerNodePool_withKubeletConfig
--- PASS: TestAccContainerNodePool_withKubeletConfig (577.83s)
PASS
ok      github.com/terraform-providers/terraform-provider-google-beta/google-beta       577.855s

Also updated a bit the documentation while I'm at it.

It's pretty much ready to go.

rileykarson

Glad to hear that helped fix them! I hoped as much 🙂

LGTM- thanks for your contribution @rmanyari!

Prior to merging this PR I've got to upstream the changes to our code generator repo and merge it there. Once that's done, I'll merge it here. It shouldn't be long- probably just a couple hours.

rileykarson · 2020-07-24T17:59:31Z

Dropping a brief update given this has taken days and not hours like I expected. I ran into some issues due to a bug introduced in the GKE resource by another PR initially, and resolved those. Now, I'm trying to get a successful test run in order to merge this. Terraform is sending the correct messages to the API, but the API isn't working how we'd expect. I've filed a bug internally against GKE, so I'll see if that gets traction.

Specifically, I've been unsuccessful testing TestAccContainerNodePool_withKubeletConfig. Upstream I added a second stage that performs an update, and that second stage fails when testing in CI. Actually, for my local test project, the first stage doesn't even pass (I suspect due to a network rule or something). I'd be interested in your experience with the same test, if you have a few minutes. If you run off [the auto-pr-3760 branch in the modular-magician fork], I'd expect it to fail when the update hangs, ultimately running for ~50 minutes.

rmanyari · 2020-07-27T15:35:59Z

Welp, sorry for the late reply. Let me try that out.

rmanyari · 2020-08-03T15:10:46Z

Yep - I think I'm running into the same prob. The test hangs because the node pool gets stuck in "repairing" I think.

Looking at the output of a gcloud command, I can see these values for kubeletConfig:

$ gcloud beta --project cloud-latency-167422 container node-pools --zone us-central1-a describe tf-test-np-fy3ipqmpfh --cluster tf-test-cluster-w2cqunuw94 | grep -A5 kubelet
  kubeletConfig:
    cpuCfsQuotaPeriod: 100us
    cpuManagerPolicy: static
  machineType: n1-standard-1
  metadata:
    disable-legacy-endpoints: 'true'

But the GCE instance shows:

$ gcloud --project cloud-latency-167422 compute instances describe gke-tf-test-cluster--tf-test-np-fy3ip-e3411f53-ndcg --zone us-central1-a | grep -A10 CFS
      cpuCFSQuota: true
      cpuCFSQuotaPeriod: 200us
      cpuManagerPolicy: default
      enableDebuggingHandlers: true
      evictionHard:
        memory.available: 100Mi
        nodefs.available: 10%
        nodefs.inodesFree: 5%
        pid.available: 10%
      featureGates:
        DynamicKubeletConfig: false
        RotateKubeletServerCertificate: true

Which don't really match... So maybe we wait until the backend fixes are pushed before trying again?

rileykarson · 2020-08-14T17:10:19Z

Sorry again for the delay @rmanyari! I've been following up upstream based on the results of the update test. It turns out that while the GKE API accepts [static, default], the actual configuration options that are expected are [static, none]. It passed the option directly on to the nodes & they're failing to register because they're configured with an invalid option.

The GKE team is now aware and working on fixing it. In the meantime, I'll check with my team early next week whether we can merge prior to the API working correctly. The static option works & we'll be able to ensure that none is the correct option available in Terraform even if the underlying API doesn't know it.

rmanyari · 2020-08-18T16:01:13Z

That would be great. We are really eager to start using these features in our clusters. Thanks again for shepherding this!

rileykarson · 2020-08-18T16:53:16Z

Spoke with my team- we'll validate for the correct options for kubelet_config.cpu_manager_policy and warn in our documentation that one of them doesn't currently work.

@rmanyari: Do you mind rebasing your change on top of the master branch to resolve the merge conflict that's since cropped up?

It's possible you'll get some weirdness from your Git upstream because we've moved from terraform-providers/terraform-provider-google-beta to hashicorp/terraform-provider-google-beta but I believe the move should be transparent.

rmanyari · 2020-08-19T14:32:07Z

It went pretty well :)

rileykarson · 2020-08-19T16:46:26Z

Glad to hear!

Thanks again for your contribution, and my apologies for the extended holding pattern. Merging now.

ghost added the size/xl label Jul 20, 2020

ghost requested review from rileykarson July 20, 2020 18:01

rileykarson reviewed Jul 20, 2020

View reviewed changes

rmanyari force-pushed the issue-6773 branch from 812a880 to 594fd80 Compare July 21, 2020 03:59

ghost added the documentation label Jul 21, 2020

rmanyari changed the title ~~[WIP] Support for Kubelet and Linux Node configurations in GKE node pools~~ Support for Kubelet and Linux Node configurations in GKE node pools Jul 21, 2020

rileykarson reviewed Jul 21, 2020

View reviewed changes

rileykarson approved these changes Jul 21, 2020

View reviewed changes

rileykarson mentioned this pull request Jul 21, 2020

Upstream support for Kubelet and Linux Node configurations in GKE GoogleCloudPlatform/magic-modules#3760

Merged

rmanyari added 3 commits August 19, 2020 10:16

schema changes to support Kernel and Kubelet configs

e78d6ce

wrap up initial kubelet/linux config support. Add some tests

b9b9aa2

simplify sysctls handling, fix tests, add documentation

3f2f236

rmanyari force-pushed the issue-6773 branch from 594fd80 to af4ac65 Compare August 19, 2020 14:20

fix missing import after rebase

ea72e96

rmanyari force-pushed the issue-6773 branch from af4ac65 to ea72e96 Compare August 19, 2020 14:22

rileykarson merged commit 6a74ea6 into hashicorp:master Aug 19, 2020

This was referenced Aug 19, 2020

Upstream support for Kubelet and Linux Node configurations in GKE hashicorp/terraform-provider-google#7060

Merged

Upstream support for Kubelet and Linux Node configurations in GKE #2403

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support for Kubelet and Linux Node configurations in GKE node pools #2279

Support for Kubelet and Linux Node configurations in GKE node pools #2279

rmanyari commented Jul 20, 2020 •

edited by rileykarson

Loading

rmanyari commented Jul 20, 2020

rileykarson left a comment

rileykarson Jul 20, 2020

rmanyari Jul 20, 2020

rmanyari Jul 21, 2020

rmanyari commented Jul 21, 2020

rileykarson left a comment

rileykarson commented Jul 24, 2020

rmanyari commented Jul 27, 2020

rmanyari commented Aug 3, 2020

rileykarson commented Aug 14, 2020

rmanyari commented Aug 18, 2020

rileykarson commented Aug 18, 2020

rmanyari commented Aug 19, 2020

rileykarson commented Aug 19, 2020

Support for Kubelet and Linux Node configurations in GKE node pools #2279

Support for Kubelet and Linux Node configurations in GKE node pools #2279

Conversation

rmanyari commented Jul 20, 2020 • edited by rileykarson Loading

rmanyari commented Jul 20, 2020

rileykarson left a comment

Choose a reason for hiding this comment

rileykarson Jul 20, 2020

Choose a reason for hiding this comment

rmanyari Jul 20, 2020

Choose a reason for hiding this comment

rmanyari Jul 21, 2020

Choose a reason for hiding this comment

rmanyari commented Jul 21, 2020

rileykarson left a comment

Choose a reason for hiding this comment

rileykarson commented Jul 24, 2020

rmanyari commented Jul 27, 2020

rmanyari commented Aug 3, 2020

rileykarson commented Aug 14, 2020

rmanyari commented Aug 18, 2020

rileykarson commented Aug 18, 2020

rmanyari commented Aug 19, 2020

rileykarson commented Aug 19, 2020

rmanyari commented Jul 20, 2020 •

edited by rileykarson

Loading