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

adding a special instance-types string 'not-specified' #833

Merged
merged 1 commit into from
Dec 3, 2024

Conversation

myaser
Copy link
Member

@myaser myaser commented Dec 2, 2024

empty instance-types is not supported by cluster-registery. using not-specified will render karpenter node-pools without instance-type requirements

this way we can create node-pools like this

  - config_items:
      taints: nvidia.com/gpu=present:NoSchedule
      requirements: |
        - key: karpenter.k8s.aws/instance-gpu-manufacturer
          operator: In
          values: 
            - nvidia
    discount_strategy: null
    instance_types:
      - not-specified
    max_size: null
    min_size: null
    name: karpenter-gpu
    profile: worker-karpenter
  - config_items:
    discount_strategy: null
    instance_types:
      - not-specified
    max_size: null
    min_size: null
    name: karpenter-catch-all
    profile: worker-karpenter

@myaser myaser added the minor Minor changes, e.g. low risk config updates, changes that do not introduce a new API call. label Dec 2, 2024
@myaser myaser force-pushed the karpenter-relax-instance-types-requirements branch 2 times, most recently from 0353100 to ac84c64 Compare December 2, 2024 19:43
@myaser
Copy link
Member Author

myaser commented Dec 2, 2024

👍🏻

@myaser myaser force-pushed the karpenter-relax-instance-types-requirements branch from ac84c64 to 6497dd9 Compare December 2, 2024 20:03
@demonCoder95
Copy link
Member

👍

1 similar comment
@myaser
Copy link
Member Author

myaser commented Dec 2, 2024

👍🏻

@myaser myaser merged commit 9231688 into master Dec 3, 2024
8 of 10 checks passed
@myaser myaser deleted the karpenter-relax-instance-types-requirements branch December 3, 2024 10:21
Comment on lines +196 to +197
minSize := int64(math.Min(float64(aws.Int64Value(asg.DesiredCapacity)), float64(aws.Int64Value(asg.MinSize))))
maxSize := int64(math.Max(float64(aws.Int64Value(asg.DesiredCapacity)), float64(aws.Int64Value(asg.MaxSize))))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are min/max builtins in go since 1.21 https://pkg.go.dev/builtin#min
so something like this

minSize := min(aws.Int64Value(asg.DesiredCapacity), aws.Int64Value(asg.MinSize))

should work

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
minor Minor changes, e.g. low risk config updates, changes that do not introduce a new API call.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants