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

[Feature Request / Bug]: autoscaler_nodepools example labels is wrong #1074

Closed
dstockhammer opened this issue Oct 31, 2023 · 2 comments
Closed

Comments

@dstockhammer
Copy link

Description

The latest example values for labels and taints in autoscaler_nodepools in kube.tf.example seem wrong:

autoscaler_nodepools = [
  {
    # ...
    labels      = {
      "node.kubernetes.io/role=peak-workloads"
    }
    taints      = 
    [{
       key: "node.kubernetes.io/role"
       value: "peak-workloads"
       effect: "NoExecute"
    }]  
  }
]

From looking at the code I believe that it should be the following:

autoscaler_nodepools = [
  {
    #...

    labels = [{
      "node.kubernetes.io/role": "peak-workloads"
    }]

    taints = [{
        key: "node.kubernetes.io/role"
        value: "specific-workloads"  # not `peak-workloads`
        effect: "NoExecute"
    }]
  }
]

However, now the labels and taints schemas are different for autoscaler_nodepools and agent_nodepools. I'm not convinced that this makes a lot of sense...

Is the plan to change all labels/taints to the new more verbose schema? Obviously as maintainers you're free to choose whichever schema you wish (although I personally prefer the original more concise string over the object), but in either case I think it should be consistent.

Thanks :)

@Silvest89
Copy link
Contributor

The new schema allows for easier implementation in the cluster-autoscaler codebase. (I had a pr merged for that as well)
Kubernetes labels are maps, and Taints are objects. So it was easier to make them more in line with Kubernetes.

But yeah I will open a PR to fix the wrong example :P

@mysticaltech mysticaltech closed this as not planned Won't fix, can't repro, duplicate, stale Nov 1, 2023
@mysticaltech mysticaltech reopened this Nov 1, 2023
@mysticaltech
Copy link
Collaborator

@dstockhammer The fix by @Silvest89 was just released in v2.9.1. Enjoy!

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

No branches or pull requests

3 participants