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

fix: use node affinity objects instead of their keys when mapping #88

Merged
merged 1 commit into from
Aug 21, 2024

Conversation

vladklokun
Copy link
Contributor

@vladklokun vladklokun commented Aug 20, 2024

Previously, when a user specified node affinities, for every provided node affinity we would try to map every key to an "affinity" block. This produced invalid Terraform resources, because the values of the "key", "name" and "operator" keys on their own are not enough to create a valid "affinity" block.

This commit uses entire affinity objects and maps them to appropriate Terraform representations, fixing the problem.

UPD:

Steps to Reproduce

  1. Set up the example manifest from the Cast AI GKE Cluster repo:
git clone [email protected]/castai/terraform-provider-castai.git
cd terraform-provider-castai/examples/gke/gke_cluster_autoscaler_policies
# Follow README.md#Usage to set up the example
  1. Apply the following patch:
diff --git a/examples/gke/gke_cluster_autoscaler_policies/castai.tf b/examples/gke/gke_cluster_autoscaler_policies/castai.tf
index 268712d..ed895e8 100644
--- a/examples/gke/gke_cluster_autoscaler_policies/castai.tf
+++ b/examples/gke/gke_cluster_autoscaler_policies/castai.tf
@@ -102,6 +102,24 @@ module "castai-gke-cluster" {
       ]
 
       constraints = {
+        dedicated_node_affinity = {
+          name    = "test_affinity_foo"
+          az_name = "us-central1-c"
+          instance_types = ["e2-standard-2"]
+          affinity = [
+            {
+              key      = "gke.io/gcp-nodepool"
+              operator = "In"
+              values = ["foo"]
+            },
+            {
+              key      = "gke.io/gcp-nodepool-02"
+              operator = "In"
+              values = ["bar"]
+            }
+          ]
+        }
+
         fallback_restore_rate_seconds = 1800
         spot                          = true
         use_spot_fallbacks            = true
  1. Plan:
tf plan -var-file tf.vars
  1. Observe the output.

Expected output

A plan to apply with dedicated node affinity included:

Terraform will perform the following actions:

# Note: Snipped for brevity

Plan: 31 to add, 0 to change, 0 to destroy.

Actual output

The following error at the end of the plan:

Terraform planned the following actions, but then encountered a problem:

# Note: Snipped for brevity

Plan: 30 to add, 0 to change, 0 to destroy.

Error: Missing required argument

  with module.castai-gke-cluster.castai_node_template.this["spot_tmpl"],
  on .terraform/modules/castai-gke-cluster/main.tf line 37, in resource "castai_node_template" "this":
  37: resource "castai_node_template" "this" {

The argument "constraints.0.dedicated_node_affinity.0.affinity.0.operator" is
required, but no definition was found.

Error: Missing required argument

  with module.castai-gke-cluster.castai_node_template.this["spot_tmpl"],
  on .terraform/modules/castai-gke-cluster/main.tf line 37, in resource "castai_node_template" "this":
  37: resource "castai_node_template" "this" {

The argument "constraints.0.dedicated_node_affinity.0.affinity.1.operator" is
required, but no definition was found.

Previously, when a user specified node affinities,
for every provided node affinity
we would try to map every _key_ to an "affinity" block.
This produced invalid Terraform resources,
because the values of the "key", "name" and "operator" keys on their own
are not enough to create a valid "affinity" block.

This commit uses entire affinity objects
and maps them to appropriate Terraform representations, fixing the problem.
@vladklokun vladklokun self-assigned this Aug 20, 2024
@vladklokun vladklokun requested a review from a team as a code owner August 20, 2024 11:18
@vladklokun vladklokun merged commit d718c04 into main Aug 21, 2024
1 check passed
@vladklokun vladklokun deleted the fix-dynamic-dedicated-node-affinity branch August 21, 2024 09:38
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.

4 participants