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

update terraform guide to demonstrate deploying the provisioner #1551

Closed
wants to merge 1 commit into from

Conversation

tzneal
Copy link
Contributor

@tzneal tzneal commented Mar 21, 2022

1. Issue, if available:
N/A

2. Description of changes:

If someone is using terraform, they likely want to use it to deploy
the provisioner as well. Due to the defaulting of values, this is
not quite as straightforward as it should be.

3. How was this change tested?

Applied the demonstrated config locally using terraform successfully.

4. Does this change impact docs?

  • Yes, PR includes docs updates
  • Yes, issue opened: link to issue
  • No

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

If someone is using terraform, they likely want to use it to deploy
the provisioner as well.  Due to the defaulting of values, this is
not quite as straightforward as it should be.
@netlify
Copy link

netlify bot commented Mar 21, 2022

✅ Deploy Preview for karpenter-docs-prod ready!

🔨 Explore the source changes: fb4508f

🔍 Inspect the deploy log: https://app.netlify.com/sites/karpenter-docs-prod/deploys/623881bc2f226e0008710cd3

😎 Browse the preview: https://deploy-preview-1551--karpenter-docs-prod.netlify.app

@tzneal
Copy link
Contributor Author

tzneal commented Mar 21, 2022

I tried just using computed_fields to eliminate the need to specify the provider apiVersion/kind, and I may be doing something wrong but it doesn't seem to work. If I removed the provider apiVersion/kind and add those as computed fields, it fails to apply:

  + resource "kubernetes_manifest" "provisioner_default" {
      + computed_fields = [
          + "spec.provider.apiVersion",
          + "spec.provider.kind",
        ]
....
ubernetes_manifest.provisioner_default: Creating...
╷
│ Error: Provider produced inconsistent result after apply
│
│ When applying changes to kubernetes_manifest.provisioner_default, provider "provider[\"registry.terraform.io/hashicorp/kubernetes\"]" produced an unexpected new value: .object: wrong
│ final value type: incorrect object attributes.
│
│ This is a bug in the provider, which should be reported in the provider's own issue tracker.
╵

@stevehipwell
Copy link
Contributor

@tzneal I'm not sure a PR is the correct place to discuss this, but why don't you show exactly what you're doing as I'd like to see the actual manifest you're adding and which fields you've set as computed.

@tzneal
Copy link
Contributor Author

tzneal commented Mar 21, 2022

Sure, this fails to apply for me. Specifying the provider apiVersion and kind does work.

resource "kubernetes_manifest" "provisioner_default" {
  manifest = {
    "apiVersion" = "karpenter.sh/v1alpha5"
    "kind" = "Provisioner"
    "metadata" = {
      "name" = "default"
    }
    "spec" = {
      "limits" = {
        "resources" = {
          "cpu" = "1k"
        }
      }
      "provider" = {
        "securityGroupSelector" = {
          "karpenter.sh/discovery" = var.cluster_name
        }
        "subnetSelector" = {
          "karpenter.sh/discovery" = var.cluster_name
        }
      }
      "requirements" = [
        {
          "key" = "karpenter.sh/capacity-type"
          "operator" = "In"
          "values" = [
            "spot",
          ]
        },
        {
          "key" = "kubernetes.io/arch"
          "operator" = "In"
          "values"   = [
              "amd64",
          ]
        },
      ]
      "ttlSecondsAfterEmpty" = 30
    }
  }
  computed_fields = ["spec.provider.apiVersion", "spec.provider.kind"]
 
}

@stevehipwell
Copy link
Contributor

@tzneal what is the error you get?

@tzneal
Copy link
Contributor Author

tzneal commented Mar 21, 2022

@tzneal what is the error you get?


kubernetes_manifest.provisioner_default: Creating...
╷
│ Error: Provider produced inconsistent result after apply
│
│ When applying changes to kubernetes_manifest.provisioner_default, provider "provider[\"registry.terraform.io/hashicorp/kubernetes\"]" produced an unexpected new value: .object: wrong
│ final value type: incorrect object attributes.
│
│ This is a bug in the provider, which should be reported in the provider's own issue tracker.

@stevehipwell
Copy link
Contributor

@tzneal what is the resource state if you manually add it with kubectl and server side apply? There may well be other computed fields that are missing.

@tzneal
Copy link
Contributor Author

tzneal commented Mar 21, 2022

@tzneal what is the resource state if you manually add it with kubectl and server side apply? There may well be other computed fields that are missing.

Specifying kind & apiVersion is all that's required to make it work with TF, so I don't believe anything else is missing.

If I create it with kubectl and then try to apply with TF it fails and there are some metadata differences and the apiVersion/kind:

      ~ spec       = {
              ~ kubeletConfiguration   = {
                  ~ clusterDNS = null -> (known after apply)
                }
              ~ labels                 = null -> (known after apply)
              ~ provider               = {
                  - apiVersion            = "extensions.karpenter.sh/v1alpha1" -> null
                  - kind                  = "AWS" -> null
                    # (2 unchanged elements hidden)
                }
              ~ taints                 = null -> (known after apply)
              ~ ttlSecondsUntilExpired = null -> (known after apply)
                # (3 unchanged elements hidden)
            }

@stevehipwell
Copy link
Contributor

To test you need to create a brand new provisioner with kubectl and compare the created resource yaml to the input yaml. You also need to make sure you're using kubectl apply --server-side.

@tzneal
Copy link
Contributor Author

tzneal commented Mar 21, 2022

To test you need to create a brand new provisioner with kubectl and compare the created resource yaml to the input yaml. You also need to make sure you're using kubectl apply --server-side.

I deleted the old provisioner created a new one with this spec:

apiVersion: karpenter.sh/v1alpha5
kind: Provisioner
metadata:
  name: default
spec:
  limits:
    resources:
      cpu: 1k
  provider:
    securityGroupSelector:
      karpenter.sh/discovery: terraform-tnealt-karpenter-demo
    subnetSelector:
      karpenter.sh/discovery: terraform-tnealt-karpenter-demo
  requirements:
  - key: karpenter.sh/capacity-type
    operator: In
    values:
    - spot
  - key: kubernetes.io/arch
    operator: In
    values:
    - amd64
  ttlSecondsAfterEmpty: 30

The diff between it and what was on the server is:

--- created.yaml	2022-03-21 14:17:17.000000000 -0500
+++ test.yaml	2022-03-21 14:17:20.000000000 -0500
@@ -1,18 +1,12 @@
 apiVersion: karpenter.sh/v1alpha5
 kind: Provisioner
 metadata:
-  creationTimestamp: "2022-03-21T19:17:06Z"
-  generation: 1
   name: default
-  resourceVersion: "120194"
-  uid: 2af55a9f-d9bd-4031-9dfb-6eb7e98b3216
 spec:
   limits:
     resources:
       cpu: 1k
   provider:
-    apiVersion: extensions.karpenter.sh/v1alpha1
-    kind: AWS
     securityGroupSelector:
       karpenter.sh/discovery: terraform-tnealt-karpenter-demo
     subnetSelector:
@@ -27,7 +21,3 @@
     values:
     - amd64
   ttlSecondsAfterEmpty: 30
-status:
-  resources:
-    cpu: "0"
-    memory: "0"

The only non-status and non-metadata differences are the spec.provider.apiVersion and spec.provider.kind which are listed in my computed_fields, and there is still an error thrown. Do you have an example of using kubernetes_manifest with Karpenter and computed_fields that works?

@stevehipwell
Copy link
Contributor

@tzneal I think you're going to either specify the API version and kind or debug Terraform. Personally I always want to specify these fields as a change in default here would be breaking and I don't use the TF resource so can't help with an example. I am interested as to why it's failing though.

@jcogilvie
Copy link

Just confirming that creating a kubernetes_manifest as you added (although, without some of the quotes on key names) is indeed working for me.

@snorlaX-sleeps
Copy link
Contributor

snorlaX-sleeps commented Mar 28, 2022

@tzneal - I was also unable to get the Terraform kubernetes_manifest to work with computed fields (including adding every possible field returned from the K8s API I didn't want to set to computed_fields)
Glad to see it does work once limits / provider spec are added to the manifest

@dewjam
Copy link
Contributor

dewjam commented Mar 29, 2022

FYI @tzneal . The Terraform Getting Started Guide is being revamped in #1332 as well. The author is planning to include the Provisioner manifest as part of his work. Take a look when you all get a second.

@tzneal
Copy link
Contributor Author

tzneal commented Mar 30, 2022

FYI @tzneal . The Terraform Getting Started Guide is being revamped in #1332 as well. The author is planning to include the Provisioner manifest as part of his work. Take a look when you all get a second.

SGTM, happy to close this one once that is in.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants